netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed
@ 2019-06-15  1:33 Stefano Brivio
  2019-06-24 18:20 ` Stefano Brivio
  2019-06-24 21:55 ` David Ahern
  0 siblings, 2 replies; 4+ messages in thread
From: Stefano Brivio @ 2019-06-15  1:33 UTC (permalink / raw)
  To: Stephen Hemminger, David Ahern
  Cc: David Miller, Jianlin Shi, Wei Wang, Martin KaFai Lau,
	Eric Dumazet, Matti Vaittinen, netdev

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(-)

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;
 		}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed
  2019-06-15  1:33 [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed Stefano Brivio
@ 2019-06-24 18:20 ` Stefano Brivio
  2019-06-24 21:55 ` David Ahern
  1 sibling, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2019-06-24 18:20 UTC (permalink / raw)
  To: Stephen Hemminger, David Ahern
  Cc: David Miller, Jianlin Shi, Wei Wang, Martin KaFai Lau,
	Eric Dumazet, Matti Vaittinen, netdev

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.

-- 
Stefano

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed
  2019-06-15  1:33 [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed Stefano Brivio
  2019-06-24 18:20 ` Stefano Brivio
@ 2019-06-24 21:55 ` David Ahern
  2019-06-25 11:40   ` Stefano Brivio
  1 sibling, 1 reply; 4+ messages in thread
From: David Ahern @ 2019-06-24 21:55 UTC (permalink / raw)
  To: Stefano Brivio, Stephen Hemminger
  Cc: David Miller, Jianlin Shi, Wei Wang, Martin KaFai Lau,
	Eric Dumazet, Matti Vaittinen, netdev

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;
>  		}
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed
  2019-06-24 21:55 ` David Ahern
@ 2019-06-25 11:40   ` Stefano Brivio
  0 siblings, 0 replies; 4+ messages in thread
From: Stefano Brivio @ 2019-06-25 11:40 UTC (permalink / raw)
  To: David Ahern
  Cc: Stephen Hemminger, David Miller, Jianlin Shi, Wei Wang,
	Martin KaFai Lau, Eric Dumazet, Matti Vaittinen, netdev

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.

-- 
Stefano

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-06-25 11:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-15  1:33 [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed Stefano Brivio
2019-06-24 18:20 ` Stefano Brivio
2019-06-24 21:55 ` David Ahern
2019-06-25 11:40   ` Stefano Brivio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).