Re: [nf-next RFC] netfilter: nf_tables: Feature ifname-based hook registration

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Oh, right. So a decision whether this is feasible and if, how it should
> > behave in detail, is urgent.
> 
> Downside is that this flag adds more complexity, since there will be
> two paths to test (flag on/off).

Right.

> Another concern is having a lot of devices, this is now interating
> linearly performing strcmp() to find matches from the control plane
> (ie. maybe this slow down time to load ruleset?), IIRC you mentioned
> this should not be an issue.

Can you suggest an alternative?

I see the following:

- revert to old behaviour (no search,
  lookup-by-name), and introduce a new netlink attribute for the 'wildcard
  name / full-search'.
  Since thats a big change this requires a revert in nf.git and then a
  followup change in nf-next to amend this.

- Only search if we have a wildcard.
  It should be enough to check, from nft_netdev_hook_alloc, if hook->ifname
  is null-terminated or not. If it is, lookup-by-name, else
  for_each_netdev search.

  Thats assuming that the netlink attributes are encoded as
  'eth\0' (4 bytes, no wildcard), vs 'eth' (3 bytes, wildcard).

> > Yes, that was the premise upon which I wrote the patch. I didn't intend
> > to make the flag toggle between the old interface hooks and the new
> > interface name hooks.
> 
> Mistyped name is another scenario this flag could help.

Regardless of this flag patch one could update nftables userspace to
display hints like we do for sets with the new '# count 42' comment
annotation.

Something that tells that the hook is subscribed for eth42 but currently
not active.

Same with flowtables, something that tells which devices are configured
(subscribed) and which devices are used (should likely still display
 ppp* and not list 4000k ppp1234 :-) ).

Phil, whats your take here?

From a quick glance there is currently no way for a user to tell if the
hook is active or not (except by adding a dummy counter rule).

> If this flag is added, I won't allow for updates until such
> possibility is carefully review, having all possible tricky scenarios
> in mind.

Makes sense to me.

> I think it boils down to the extra complexity that this flag adds is
> worth having or documenting the new behaviour is sufficient, assuming
> the two issues that have been mentioned are not problematic.

Yes.