From: Stefano Brivio <sbrivio@redhat.com>
To: Martin Lau <kafai@fb.com>
Cc: David Ahern <dsahern@gmail.com>,
David Miller <davem@davemloft.net>,
Jianlin Shi <jishi@redhat.com>, Wei Wang <weiwan@google.com>,
Eric Dumazet <edumazet@google.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
Subject: Re: [PATCH net 1/2] ipv6: Dump route exceptions too in rt6_dump_route()
Date: Sat, 8 Jun 2019 17:02:06 +0200 [thread overview]
Message-ID: <20190608170206.4fa108f5@redhat.com> (raw)
In-Reply-To: <20190608071920.rio4ldr4fhjm2ztv@kafai-mbp.dhcp.thefacebook.com>
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.
It could from 2007, iproute2 9ab4c85b9af1 ("Fix bug in display of ipv6
cloned/cached routes"), to 2017, kernel 2b760fcf5cfb ("ipv6: hook up
exception table to store dst cache").
Something tells me it's wrong to fix userspace, because userspace "is
always right". There has been by the way a similar discussion on this
list in 2011, see https://lists.openwall.net/netdev/2011/12/28/27.
I would proceed like this:
- stick to the original semantic of RTM_F_CLONED and fix the issue at
hand, which would be v2 with your suggested clean-up and without
check on RTM_F_CLONED. Exceptions are always dumped and iproute2 will
filter them as it always did. Result: kernel sends exceptions on
netlink even if not "requested" but iproute2 works again and won't
spam you anyway, and the issue is fixed for the users
- fix this on IPv4 (as I mentioned, I think it's less critical, because
at least flushing works, and listing with 'route get' is awkward but
possible)
- retry adding NLM_F_MATCH (for net-next and iproute-next) according
to RFC 3549. Things changed a bit from 2011: we now have
NLM_F_DUMP_FILTERED, iproute2 already uses it (ip neigh) and we
wouldn't need to make iproute2 more complicated by handling old/new
kernel cases. So I think this would be reasonable now.
--
Stefano
next prev parent reply other threads:[~2019-06-08 15:02 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-06 20:13 [PATCH net 0/2] ipv6: Fix listing and flushing of cached route exceptions Stefano Brivio
2019-06-06 20:13 ` [PATCH net 1/2] ipv6: Dump route exceptions too in rt6_dump_route() Stefano Brivio
2019-06-06 20:57 ` David Ahern
2019-06-06 21:18 ` Stefano Brivio
2019-06-06 22:47 ` David Ahern
2019-06-06 23:07 ` Stefano Brivio
2019-06-08 5:40 ` Martin Lau
2019-06-08 5:59 ` Stefano Brivio
2019-06-08 7:19 ` Martin Lau
2019-06-08 15:02 ` Stefano Brivio [this message]
2019-06-08 15:47 ` Stefano Brivio
2019-06-10 19:42 ` Martin Lau
2019-06-10 21:01 ` Stefano Brivio
2019-06-10 5:56 ` Vaittinen, Matti
2019-06-10 19:01 ` Stefano Brivio
2019-06-06 21:44 ` Martin Lau
2019-06-06 22:17 ` Stefano Brivio
2019-06-06 22:37 ` Martin Lau
2019-06-06 22:48 ` David Ahern
2019-06-07 1:54 ` Stefano Brivio
2019-06-06 22:58 ` Stefano Brivio
2019-06-06 23:15 ` Stefano Brivio
2019-06-06 23:19 ` David Ahern
2019-06-06 23:31 ` Martin Lau
2019-06-06 20:13 ` [PATCH net 2/2] ip6_fib: Don't discard nodes with valid routing information in fib6_locate_1() Stefano Brivio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190608170206.4fa108f5@redhat.com \
--to=sbrivio@redhat.com \
--cc=davem@davemloft.net \
--cc=dsahern@gmail.com \
--cc=edumazet@google.com \
--cc=jishi@redhat.com \
--cc=kafai@fb.com \
--cc=matti.vaittinen@fi.rohmeurope.com \
--cc=netdev@vger.kernel.org \
--cc=weiwan@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).