netdev - Re: [PATCH net 1/2] ipv6: Dump route exceptions too in rt6_dump

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 8 Jun 2019 07:59:11 +0200
From:   Stefano Brivio <sbrivio@...hat.com>
To:     Martin Lau <kafai@...com>
Cc:     David Ahern <dsahern@...il.com>,
        David Miller <davem@...emloft.net>,
        Jianlin Shi <jishi@...hat.com>, Wei Wang <weiwan@...gle.com>,
        Eric Dumazet <edumazet@...gle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net 1/2] ipv6: Dump route exceptions too in
 rt6_dump_route()

On Sat, 8 Jun 2019 05:40:06 +0000
Martin Lau <kafai@...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@...il.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():

	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?

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

-- 
Stefano