On Mon, 17 Jun 2019 07:38:54 -0600
David Ahern <dsah...@gmail.com> wrote:
> On 6/16/19 2:04 PM, Stefano Brivio wrote:
> > We could do this:
> >
> > - strict checking enabled (iproute2 >= 5.0.0):
> > - in inet{,6}_dump_fib(): if NLM_F_MATCH is set, set
> > filter->filter_set in any case
> >
> > - in fn_trie_dump_leaf() and rt6_dump_route(): use filter->filter_set
> > to decide if we want to filter depending on RTM_F_CLONED being
> > set/unset. If other filters (rt_type, dev, protocol) are not set,
> > they are still wildcards (existing implementation)
> >
> > - no strict checking (iproute2 < 5.0.0):
> > - we can't filter consistently, so apply no filters at all: dump all
> > the routes (filter->filter_set not set), cached and uncached. That
> > means more netlink messages, but no spam as iproute2 filters them
> > anyway, and list/flush cache commands work again.
> >
> > I would drop 1/8, turn 2/8 and 6/8 into a straightforward:
> >
> > if (cb->strict_check) {
> > err = ip_valid_fib_dump_req(net, nlh, &filter, cb);
> > if (err < 0)
> > return err;
> > + if (nlh->nlmsg_flags & NLM_F_MATCH)
> > + filter.filter_set = 1;
> > } else if (nlmsg_len(nlh) >= sizeof(struct rtmsg)) {
> > struct rtmsg *rtm = nlmsg_data(nlh);
> >
> > and other patches remain the same.
> >
> > What do you think?
> >
>
> With strict checking (5.0 and forward):
> - RTM_F_CLONED NOT set means dump only FIB entries
> - RTM_F_CLONED set means dump only exceptions
Okay. Should we really ignore the RFC and NLM_F_MATCH though? If we add
field(s) to the filter, it comes almost for free, something like:
if (nlh->nlmsg_flags & NLM_F_MATCH)
filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED;
instead of:
filter->dump_exceptions = rtm->rtm_flags & RTM_F_CLONED;
> Without strict checking (old iproute2 on any kernel):
> - dump all, userspace has to sort
>
> Kernel side this can be handled with new field, dump_exceptions, in the
> filter that defaults to true and then is reset in the strict path if the
> flag is not set.
I guess we need to add two fields, we'll need a 'dump_routes' too.
Otherwise, the dump functions can't distinguish between the three cases
('no strict checking', 'strict checking and RTM_F_CLONED', 'strict
checking and no RTM_F_CLONED'). How would you do this with a single
additional field?
--
Stefano
