On Sat, Sep 21, 2024 at 11:10:34AM +0200, Florian Westphal wrote:
> Phil Sutter <phil@xxxxxx> wrote:
> > Notify user space if netdev hooks are updated due to netdev add/remove
> > events. Send minimal notification messages by introducing
> > NFT_MSG_NEWDEV/DELDEV message types describing a single device only.
> >
> > Signed-off-by: Phil Sutter <phil@xxxxxx>
> > ---
> > include/net/netfilter/nf_tables.h | 2 +
> > include/uapi/linux/netfilter/nf_tables.h | 5 +++
> > net/netfilter/nf_tables_api.c | 56 ++++++++++++++++++++++++
> > net/netfilter/nft_chain_filter.c | 1 +
> > 4 files changed, 64 insertions(+)
> >
> > diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> > index eaf2f5184bdf..f8da38e45277 100644
> > --- a/include/net/netfilter/nf_tables.h
> > +++ b/include/net/netfilter/nf_tables.h
> > @@ -1132,6 +1132,8 @@ int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
> > int nft_set_catchall_validate(const struct nft_ctx *ctx, struct nft_set *set);
> > int nf_tables_bind_chain(const struct nft_ctx *ctx, struct nft_chain *chain);
> > void nf_tables_unbind_chain(const struct nft_ctx *ctx, struct nft_chain *chain);
> > +void nf_tables_chain_device_notify(const struct nft_chain *chain,
> > + const struct net_device *dev, int event);
> >
> > enum nft_chain_types {
> > NFT_CHAIN_T_DEFAULT = 0,
> > diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> > index d6476ca5d7a6..3a874febf1ac 100644
> > --- a/include/uapi/linux/netfilter/nf_tables.h
> > +++ b/include/uapi/linux/netfilter/nf_tables.h
> > @@ -142,6 +142,8 @@ enum nf_tables_msg_types {
> > NFT_MSG_DESTROYOBJ,
> > NFT_MSG_DESTROYFLOWTABLE,
> > NFT_MSG_GETSETELEM_RESET,
> > + NFT_MSG_NEWDEV,
> > + NFT_MSG_DELDEV,
>
> This relies on implicit NFNL_CB_UNSPEC == 0 and nfnetlink
> bailing out whe NFT_MSG_NEWDEV appears in a netlink message
> coming from userspace.
I guess with 'implicit NFNL_CB_UNSPEC == 0' you mean the extra
nf_tables_cb array fields' 'type' value being 0 (nfnetlink.h explicitly
defines NFNL_CB_UNSPEC value as 0). I don't see the connection here
though, probably I miss nfnetlink_rcv_msg() relying on that field value
or so.
I do see implicit dependency on attr_count field being 0 via
nla_parse_deprecated().
> Is there precedence for this?
> If not, maybe better to add explicit entries to the
> nf_tables_cb[] array?
>
> AFAICS its fine as-is, nfnetlink won't blindly invoke
> NULL ->call() pointer, but I'm not sure this was designed
> to be this way or if this is a coincidence.
I see at least NFNL_MSG_ACCT_OVERQUOTA missing from nfnl_acct_cb. The
former was introduced in 2014. May I claim grandfathering? ;)
Cheers, Phil
