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

[Martin Lau <kafai@fb.com>]

On Sat, Jun 08, 2019 at 07:59:11AM +0200, Stefano Brivio wrote:
> On Sat, 8 Jun 2019 05:40:06 +0000
> Martin Lau <kafai@fb.com> wrote:
> 
> > On Thu, Jun 06, 2019 at 04:47:00PM -0600, David Ahern wrote:
> > > On 6/6/19 3:18 PM, Stefano Brivio wrote:  
> > > > On Thu, 6 Jun 2019 14:57:33 -0600
> > > > David Ahern <dsahern@gmail.com> wrote:
> > > >   
> > > >>> This will cause a non-trivial conflict with commit cc5c073a693f
> > > >>> ("ipv6: Move exception bucket to fib6_nh") on net-next. I can submit
> > > >>> an equivalent patch against net-next, if it helps.
> > > >>>     
> > > >>
> > > >> Thanks for doing this. It is on my to-do list.
> > > >>
> > > >> Can you do the same for IPv4?  
> > > > 
> > > > You mean this same fix? On IPv4, for flushing, iproute2
> > > > uses /proc/sys/net/ipv4/route/flush in iproute_flush_cache(), and that
> > > > works.
> > > > 
> > > > Listing doesn't work instead, for some different reason I haven't
> > > > looked into yet. That doesn't look as critical as the situation on IPv6
> > > > where one can't even flush the cache: exceptions can also be fetched
> > > > with 'ip route get', and that works.
> > > > 
> > > > Still, it's bad, I can look into it within a few days.
> > > >   
> > > 
> > > I meant the ability to dump the exception cache.
> > > 
> > > Currently, we do not get the exceptions in a fib dump. There is a flag
> > > to only show cloned (cached) entries, but no way to say 'no cloned
> > > entries'. Maybe these should only be dumped if the cloned flag is set.
> > > That's the use case I was targeting:
> > > 1. fib dumps - RTM_F_CLONED not set  
> > I also think the fib dump should stay as is.
> > 
> > To be clear, I do not expect exception routes output from the
> > 'ip [-6] r l'.  Otherwise, I will get pages of exceptions
> > that I am not interested at.  This should apply for both
> > v4 and v6.
> 
> 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.

> If we agree on this, I'll go ahead and start changing this in my patch
> for IPv6.