On Sat, 8 Jun 2019 17:02:06 +0200 Stefano Brivio <sbri...@redhat.com> wrote:
> On Sat, 8 Jun 2019 07:19:23 +0000 > Martin Lau <ka...@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