Re: [PATCH net 1/2] ipv6: Dump route exceptions too in rt6_dump_route()

[Stefano Brivio <sbrivio@redhat.com>]

On Sat, 8 Jun 2019 17:02:06 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:

> On Sat, 8 Jun 2019 07:19:23 +0000
> Martin Lau <kafai@fb.com> wrote:
> 
> > On Sat, Jun 08, 2019 at 07:59:11AM +0200, Stefano Brivio wrote:  
> > > I also agree it makes more sense to filter routes this way.
> > > 
> > > But it wasn't like this before 2b760fcf5cfb, so this smells like
> > > breaking userspace expectations, even though iproute already filters
> > > routes this way: with 'cache' it only displays routes with
> > > RTM_F_CLONED, without, it won't display exceptions, see filter_nlmsg():    
> > Thanks for pointing it out.
> >   
> > > 	if (filter.cloned == !(r->rtm_flags & RTM_F_CLONED))
> > > 		return 0;
> > > 
> > > This, together with the fact it's been like that for almost two years
> > > now, makes it acceptable in my opinion. What do you think?    
> > With learning the above fact on iproute2,
> > it makes even less sense to dump exceptions from the kernel side
> > when RTM_F_CLONED is not set.  
> 
> I just hit a more fundamental problem though: iproute2 filters on the
> flag, but never sets it on a dump request. Flags will be NLM_F_DUMP |
> NLM_F_REQUEST, no matter what, see rtnl_routedump_req(). So the current
> iproute2 would have no way to dump cached routes.

Partially wrong: it actually sets it on 'list':

	if (rtnl_routedump_req(&rth, dump_family, iproute_dump_filter) < 0) {

[...]
static int iproute_dump_filter(struct nlmsghdr *nlh, int reqlen)
[...]
	if (filter.cloned)
		rtm->rtm_flags |= RTM_F_CLONED;

but not on 'flush':

		if (rtnl_routedump_req(&rth, family, NULL) < 0) {

but this doesn't change things much: it still has no way to flush the
cache, because the dump to get the routes to flush doesn't contain the
exceptions.

So I would stick to my latest plan.

-- 
Stefano