[PATCH] net/netfilter: use kvfree_rcu to simplify the code

[PATCH] net/netfilter: use kvfree_rcu to simplify the code

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

The callback function of call_rcu() just calls kvfree(), so we can
use kvfree_rcu() instead of call_rcu() + callback function.

and move struct rcu_head into struct nf_hook_entries, then struct
nf_hook_entries_rcu_head can be removed

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 include/linux/netfilter.h | 13 +------------
 net/netfilter/core.c      | 21 ++-------------------
 2 files changed, 3 insertions(+), 31 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index 2b8aac2..c751b0a 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -111,28 +111,17 @@ struct nf_hook_entry {
 	void				*priv;
 };
 
-struct nf_hook_entries_rcu_head {
-	struct rcu_head head;
-	void	*allocation;
-};
-
 struct nf_hook_entries {
 	u16				num_hook_entries;
+	struct rcu_head rcu;
 	/* padding */
 	struct nf_hook_entry		hooks[];
 
 	/* trailer: pointers to original orig_ops of each hook,
-	 * followed by rcu_head and scratch space used for freeing
-	 * the structure via call_rcu.
 	 *
 	 *   This is not part of struct nf_hook_entry since its only
 	 *   needed in slow path (hook register/unregister):
 	 * const struct nf_hook_ops     *orig_ops[]
-	 *
-	 *   For the same reason, we store this at end -- its
-	 *   only needed when a hook is deleted, not during
-	 *   packet path processing:
-	 * struct nf_hook_entries_rcu_head     head
 	 */
 };
 
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index b9f551f0..8889f09 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -52,8 +52,7 @@ static struct nf_hook_entries *allocate_hook_entries_size(u16 num)
 	struct nf_hook_entries *e;
 	size_t alloc = sizeof(*e) +
 		       sizeof(struct nf_hook_entry) * num +
-		       sizeof(struct nf_hook_ops *) * num +
-		       sizeof(struct nf_hook_entries_rcu_head);
+		       sizeof(struct nf_hook_ops *) * num;
 
 	if (num == 0)
 		return NULL;
@@ -64,28 +63,12 @@ static struct nf_hook_entries *allocate_hook_entries_size(u16 num)
 	return e;
 }
 
-static void __nf_hook_entries_free(struct rcu_head *h)
-{
-	struct nf_hook_entries_rcu_head *head;
-
-	head = container_of(h, struct nf_hook_entries_rcu_head, head);
-	kvfree(head->allocation);
-}
-
 static void nf_hook_entries_free(struct nf_hook_entries *e)
 {
-	struct nf_hook_entries_rcu_head *head;
-	struct nf_hook_ops **ops;
-	unsigned int num;
-
 	if (!e)
 		return;
 
-	num = e->num_hook_entries;
-	ops = nf_hook_entries_get_hook_ops(e);
-	head = (void *)&ops[num];
-	head->allocation = e;
-	call_rcu(&head->head, __nf_hook_entries_free);
+	kvfree_rcu(e, rcu);
 }
 
 static unsigned int accept_all(void *priv,
-- 
2.9.4

Re: [PATCH] net/netfilter: use kvfree_rcu to simplify the code

[Simon Horman]

On Wed, Jan 22, 2025 at 03:44:50PM +0800, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> The callback function of call_rcu() just calls kvfree(), so we can
> use kvfree_rcu() instead of call_rcu() + callback function.
> 
> and move struct rcu_head into struct nf_hook_entries, then struct
> nf_hook_entries_rcu_head can be removed
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>

Reviewed-by: Simon Horman <horms@kernel.org>

...

Re: [PATCH] net/netfilter: use kvfree_rcu to simplify the code

[Pablo Neira Ayuso]

> diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
> index 2b8aac2..c751b0a 100644
> --- a/include/linux/netfilter.h
> +++ b/include/linux/netfilter.h
> @@ -111,28 +111,17 @@ struct nf_hook_entry {
>  	void				*priv;
>  };
>  
> -struct nf_hook_entries_rcu_head {
> -	struct rcu_head head;
> -	void	*allocation;
> -};
> -
>  struct nf_hook_entries {
>  	u16				num_hook_entries;
> +	struct rcu_head rcu;

This structure is accessed from the packet path.

>  	/* padding */
>  	struct nf_hook_entry		hooks[];
>  
>  	/* trailer: pointers to original orig_ops of each hook,
> -	 * followed by rcu_head and scratch space used for freeing
> -	 * the structure via call_rcu.
>  	 *
>  	 *   This is not part of struct nf_hook_entry since its only
>  	 *   needed in slow path (hook register/unregister):
>  	 * const struct nf_hook_ops     *orig_ops[]
> -	 *
> -	 *   For the same reason, we store this at end -- its
> -	 *   only needed when a hook is deleted, not during
> -	 *   packet path processing:
> -	 * struct nf_hook_entries_rcu_head     head

I think it is convenient to keep struct rcu_head at the end of it.