On Sun, Sep 22, 2024 at 09:32:24AM +0200, Florian Westphal wrote:
> Phil Sutter <phil@xxxxxx> wrote:
> > For the sake of simplicity, treat them like consecutive NETDEV_REGISTER
> > and NETDEV_UNREGISTER events. If the new name matches a hook spec and
> > registration fails, escalate the error and keep things as they are.
> >
> > Signed-off-by: Phil Sutter <phil@xxxxxx>
> > ---
> > Changes since v3:
> > - Register first and handle errors to avoid having unregistered the
> > device but registration fails.
> > ---
> > net/netfilter/nf_tables_api.c | 5 +++++
> > net/netfilter/nft_chain_filter.c | 5 +++++
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> > index 2684990dd3dc..4d40c1905735 100644
> > --- a/net/netfilter/nf_tables_api.c
> > +++ b/net/netfilter/nf_tables_api.c
> > @@ -9371,6 +9371,11 @@ static int nf_tables_flowtable_event(struct notifier_block *this,
> > struct nft_table *table;
> > struct net *net;
> >
> > + if (event == NETDEV_CHANGENAME) {
> > + if (nf_tables_flowtable_event(this, NETDEV_REGISTER, ptr))
> > + return NOTIFY_BAD;
> > + event = NETDEV_UNREGISTER;
> > + }
>
> Consider flowtable that should claim devices "pv*".
> You get CHANGENAME, device name is, say, pv5.
>
> Device name is registered in nf_tables_flowtable_event().
> Then, event is set to UNREGISTER.
>
> AFAICS this may unreg the device again immediately, as unreg part
> only compares device pointer and we can't be sure the device was
> part of any flowtable when CHANGENAME was triggered.
>
> So I think nf_tables_flowtable_event() must handle CHANGENAME
> directly, first check if any flowtable holds the device at this time,
> then check if we need to register it with a new name, and do unreg
> only if it was previously part of any flowtable.
>
> Same logic needed for netdev chains.
>
> Does that make sense?
Oh, you're right: Registering the device again (with new name) then
searching *all* flowtables for the device and unregistering it will
undo the previous registration, too! This obviously needs proper
testing, too.
Thanks, Phil
