Re: [PATCH nf] netfilter: nf_tables: unbind callbacks from flowtable destroy path

[wenxu <wenxu@xxxxxxxxx>]

Acked-by: wenxu <wenxu@xxxxxxxxx>

在 2020/1/4 1:04, Pablo Neira Ayuso 写道:
> Callback unbinding needs to be done after nf_flow_table_free(),
> otherwise entries are not removed from the hardware.
>
> Update nft_unregister_flowtable_net_hooks() to call
> nf_unregister_net_hook() instead since the commit/abort paths do not
> deal with the callback unbinding anymore.
>
> Add a comment to nft_flowtable_event() to clarify that
> flow_offload_netdev_event() already removes the entries before the
> callback unbinding.
>
> Fixes: 8bb69f3b2918 ("netfilter: nf_tables: add flowtable offload control plane")
> Fixes ff4bf2f42a40 ("netfilter: nf_tables: add nft_unregister_flowtable_hook()")
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> Follows up after:
> https://patchwork.ozlabs.org/patch/1213936/
> https://patchwork.ozlabs.org/patch/1213406/
>
>   net/netfilter/nf_tables_api.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 273f3838318b..43f05b3acd60 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -5984,6 +5984,7 @@ nft_flowtable_type_get(struct net *net, u8 family)
>   	return ERR_PTR(-ENOENT);
>   }
>   
> +/* Only called from error and netdev event paths. */
>   static void nft_unregister_flowtable_hook(struct net *net,
>   					  struct nft_flowtable *flowtable,
>   					  struct nft_hook *hook)
> @@ -5999,7 +6000,7 @@ static void nft_unregister_flowtable_net_hooks(struct net *net,
>   	struct nft_hook *hook;
>   
>   	list_for_each_entry(hook, &flowtable->hook_list, list)
> -		nft_unregister_flowtable_hook(net, flowtable, hook);
> +		nf_unregister_net_hook(net, &hook->ops);
>   }
>   
>   static int nft_register_flowtable_net_hooks(struct net *net,
> @@ -6448,12 +6449,14 @@ static void nf_tables_flowtable_destroy(struct nft_flowtable *flowtable)
>   {
>   	struct nft_hook *hook, *next;
>   
> +	flowtable->data.type->free(&flowtable->data);
>   	list_for_each_entry_safe(hook, next, &flowtable->hook_list, list) {
> +		flowtable->data.type->setup(&flowtable->data, hook->ops.dev,
> +					    FLOW_BLOCK_UNBIND);
>   		list_del_rcu(&hook->list);
>   		kfree(hook);
>   	}
>   	kfree(flowtable->name);
> -	flowtable->data.type->free(&flowtable->data);
>   	module_put(flowtable->data.type->owner);
>   	kfree(flowtable);
>   }
> @@ -6497,6 +6500,7 @@ static void nft_flowtable_event(unsigned long event, struct net_device *dev,
>   		if (hook->ops.dev != dev)
>   			continue;
>   
> +		/* flow_offload_netdev_event() cleans up entries for us. */
>   		nft_unregister_flowtable_hook(dev_net(dev), flowtable, hook);
>   		list_del_rcu(&hook->list);
>   		kfree_rcu(hook, rcu);