Re: [nf-next PATCH v4 13/16] netfilter: nf_tables: Handle NETDEV_CHANGENAME events

[Florian Westphal <fw@strlen.de>]

Phil Sutter <phil@nwl.cc> 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@nwl.cc>
> ---
> 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?