Hi Phil,
On Wed, May 21, 2025 at 05:32:57PM +0200, Phil Sutter wrote:
> Hi Pablo,
>
> On Wed, May 21, 2025 at 12:28:01AM +0200, Pablo Neira Ayuso wrote:
[...]
> > 2) I wonder if nft_hook_find_ops() will need a hashtable sooner or
> > later. With the wildcard, the number of devices could be significantly
> > large in this list lookup.
>
> Maybe, yes. Is it useful to have a single flowtable for all virtual
> functions (e.g.) on a hypervisor? Or would one rather have distinct
> flowtables for each VM/container?
[...]
> Callers are:
>
> - nft_{flowtable,netdev}_event(): Interface is added or removed
> - nft_flow_offload_eval(): New flow being offloaded
> - nft_offload_netdev_event(): Interface is removed
>
> All these are "slow path" at least. I could try building a test case to
> see how performance scales, but since we hit the function just once for
> each new connection, I guess it's hard to get significant data out of
> it.
This can be added later, not a deal breaker.
This is event path which might slow down adding new entries via
rtnetlink maybe, but I would need to have a closer look.
[...]
> > == netfilter: nf_tables: Add "notications" <-- typo: "notifications"
> >
> > I suggest you add a new NFNLGRP_NFT_DEV group for these notifications,
> > so NFNLGRP_NFTABLES is only used for control plane updates via
> > nfnetlink API. In this case, these events are triggered by rtnetlink
> > when a new device is registered and it matches the existing an
> > existing device if I understood the rationale.
>
> Yes, MSG_NEWDEV and MSG_DELDEV are triggered if a new device matches a
> hook or if a hooked device is removed (or renamed, so the hook won't
> match anymore).
>
> Having a distinct NFNLGRP for them requires a new 'nft monitor' mode,
> right? So we can't have a single monitor process for ruleset changes and
> these device events. Should not be a problem, though.
Having a different group allows to filter out events you might not
care about, it is a simple netlink event filtering facility.
I think this feature is for debugging purpose only, correct? So a
separated group should be fine. IIUC, this event does not modify the
ruleset, it only tells us a hook for a matching device is being
registered.
Thanks.
