diff mbox series

[iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed

Message ID 7ae318a8b632c216df95362524cd4bb5f4f1f537.1560561439.git.sbrivio@redhat.com
State Changes Requested
Delegated to: stephen hemminger
Headers show
Series [iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed | expand

Commit Message

Stefano Brivio June 15, 2019, 1:33 a.m. UTC
With a current (5.1) kernel version, IPv6 exception routes can't be listed
(ip -6 route list cache) or flushed (ip -6 route flush cache). I'm
re-introducing kernel support for this, but, to allow the kernel to filter
routes based on the RTM_F_CLONED flag, we need to make sure this flag is
always passed when we want cached routes to be dumped.

Right now, this is only the case for listing operation. When flushing,
IPv6 routes are first dumped, and then deleted one by one, but the
RTM_F_CLONED flag is not passed depending on the filter during the
dump, so we don't get the routes that we need to flush if requested with
the 'cache' parameter.

Define a filter that is passed to rtnl_routedump_req() on flush and sets
RTM_F_CLONED in rtm_flags on the dump to get routes to be flushed, if
we're dealing with cached routes.

Fixes: aba5acdfdb34 ("(Logical change 1.3)")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
 ip/iproute.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Stefano Brivio June 24, 2019, 6:20 p.m. UTC | #1
Stephen,

On Sat, 15 Jun 2019 03:33:50 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:

> With a current (5.1) kernel version, IPv6 exception routes can't be listed
> (ip -6 route list cache) or flushed (ip -6 route flush cache). I'm
> re-introducing kernel support for this, but, to allow the kernel to filter
> routes based on the RTM_F_CLONED flag, we need to make sure this flag is
> always passed when we want cached routes to be dumped.

Support for listing IPv6 route exceptions is now back on net-next,
relevant commits:

564c91f7e563 fib_frontend, ip6_fib: Select routes or exceptions dump from RTM_F_CLONED
ef11209d4219 Revert "net/ipv6: Bail early if user only wants cloned entries"
3401bfb1638e ipv6/route: Don't match on fc_nh_id if not set in ip6_route_del()
bf9a8a061ddc ipv6/route: Change return code of rt6_dump_route() for partial node dumps
1e47b4837f3b ipv6: Dump route exceptions if requested
40cb35d5dc04 ip6_fib: Don't discard nodes with valid routing information in fib6_locate_1()

and this iproute2 patch works together with that as it is. Do I need to
re-submit? Thanks.
David Ahern June 24, 2019, 9:55 p.m. UTC | #2
On 6/14/19 7:33 PM, Stefano Brivio wrote:
> diff --git a/ip/iproute.c b/ip/iproute.c
> index 2b3dcc5dbd53..192442b42062 100644
> --- a/ip/iproute.c
> +++ b/ip/iproute.c
> @@ -1602,6 +1602,16 @@ static int save_route_prep(void)
>  	return 0;
>  }
>  
> +static int iproute_flush_flags(struct nlmsghdr *nlh, int reqlen)

rename that to iproute_flush_filter to be consistent with
iproute_dump_filter.

Actually, why can't the flush code use iproute_dump_filter?

> +{
> +	struct rtmsg *rtm = NLMSG_DATA(nlh);
> +
> +	if (filter.cloned)
> +		rtm->rtm_flags |= RTM_F_CLONED;
> +
> +	return 0;
> +}
> +
>  static int iproute_flush(int family, rtnl_filter_t filter_fn)
>  {
>  	time_t start = time(0);
> @@ -1624,7 +1634,7 @@ static int iproute_flush(int family, rtnl_filter_t filter_fn)
>  	filter.flushe = sizeof(flushb);
>  
>  	for (;;) {
> -		if (rtnl_routedump_req(&rth, family, NULL) < 0) {
> +		if (rtnl_routedump_req(&rth, family, iproute_flush_flags) < 0) {
>  			perror("Cannot send dump request");
>  			return -2;
>  		}
>
Stefano Brivio June 25, 2019, 11:40 a.m. UTC | #3
On Mon, 24 Jun 2019 15:55:49 -0600
David Ahern <dsahern@gmail.com> wrote:

> On 6/14/19 7:33 PM, Stefano Brivio wrote:
> > diff --git a/ip/iproute.c b/ip/iproute.c
> > index 2b3dcc5dbd53..192442b42062 100644
> > --- a/ip/iproute.c
> > +++ b/ip/iproute.c
> > @@ -1602,6 +1602,16 @@ static int save_route_prep(void)
> >  	return 0;
> >  }
> >  
> > +static int iproute_flush_flags(struct nlmsghdr *nlh, int reqlen)  
> 
> rename that to iproute_flush_filter to be consistent with
> iproute_dump_filter.

I originally wanted to make it obvious that it's not an actual filter,
but:

> Actually, why can't the flush code use iproute_dump_filter?

...come on. That would be too simple.

No, my original understanding was that strict checking didn't imply
filtering. It does, and the current kernel implementation matches,
now also for RTM_F_CACHED. So yes, we can use it, and it doesn't cause
any unexpected behaviours with older kernels either. Sending v2. Thanks
for pointing this out.
diff mbox series

Patch

diff --git a/ip/iproute.c b/ip/iproute.c
index 2b3dcc5dbd53..192442b42062 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1602,6 +1602,16 @@  static int save_route_prep(void)
 	return 0;
 }
 
+static int iproute_flush_flags(struct nlmsghdr *nlh, int reqlen)
+{
+	struct rtmsg *rtm = NLMSG_DATA(nlh);
+
+	if (filter.cloned)
+		rtm->rtm_flags |= RTM_F_CLONED;
+
+	return 0;
+}
+
 static int iproute_flush(int family, rtnl_filter_t filter_fn)
 {
 	time_t start = time(0);
@@ -1624,7 +1634,7 @@  static int iproute_flush(int family, rtnl_filter_t filter_fn)
 	filter.flushe = sizeof(flushb);
 
 	for (;;) {
-		if (rtnl_routedump_req(&rth, family, NULL) < 0) {
+		if (rtnl_routedump_req(&rth, family, iproute_flush_flags) < 0) {
 			perror("Cannot send dump request");
 			return -2;
 		}