[PATCH net-next 0/2] net: rtnl: introduce rcu_replace_pointer_rtnl

[PATCH net-next 0/2] net: rtnl: introduce rcu_replace_pointer_rtnl

[Pedro Tammela]

Introduce the rcu_replace_pointer_rtnl helper to lockdep check rtnl lock
rcu replacements, alongside the already existing helpers.

Patch 2 uses the new helper in the rtnl_unregister_* functions.

Originally this change was part of the P4TC series, as it's a recurrent
pattern there, but since it has a use case in mainline we are pushing it
separately.

Jamal Hadi Salim (1):
  net: rtnl: introduce rcu_replace_pointer_rtnl

Pedro Tammela (1):
  net: rtnl: use rcu_replace_pointer_rtnl in rtnl_unregister_*

 include/linux/rtnetlink.h | 12 ++++++++++++
 net/core/rtnetlink.c      | 12 +++---------
 2 files changed, 15 insertions(+), 9 deletions(-)

-- 
2.40.1

[PATCH net-next 1/2] net: rtnl: introduce rcu_replace_pointer_rtnl

[Pedro Tammela]

From: Jamal Hadi Salim <jhs@mojatatu.com>

Introduce the rcu_replace_pointer_rtnl helper to lockdep check rtnl lock
rcu replacements, alongside the already existing helpers.

This is a quality of life helper so instead of using:
   rcu_replace_pointer(rp, p, lockdep_rtnl_is_held())
   .. or the open coded..
   rtnl_dereference() / rcu_assign_pointer()
   .. or the lazy check version ..
   rcu_replace_pointer(rp, p, 1)
Use:
   rcu_replace_pointer_rtnl(rp, p)

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 include/linux/rtnetlink.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 6a8543b34e2c..410529fca18b 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -79,6 +79,18 @@ static inline bool lockdep_rtnl_is_held(void)
 #define rtnl_dereference(p)					\
 	rcu_dereference_protected(p, lockdep_rtnl_is_held())
 
+/**
+ * rcu_replace_pointer_rtnl - replace an RCU pointer under rtnl_lock, returning
+ * its old value
+ * @rp: RCU pointer, whose value is returned
+ * @p: regular pointer
+ *
+ * Perform a replacement under rtnl_lock, where @rp is an RCU-annotated
+ * pointer. The old value of @rp is returned, and @rp is set to @p
+ */
+#define rcu_replace_pointer_rtnl(rp, p)			\
+	rcu_replace_pointer(rp, p, lockdep_rtnl_is_held())
+
 static inline struct netdev_queue *dev_ingress_queue(struct net_device *dev)
 {
 	return rtnl_dereference(dev->ingress_queue);
-- 
2.40.1

[PATCH net-next 2/2] net: rtnl: use rcu_replace_pointer_rtnl in rtnl_unregister_*

[Pedro Tammela]

With the introduction of the rcu_replace_pointer_rtnl helper,
cleanup the rtnl_unregister_* functions to use the helper instead
of open coding it.

Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 net/core/rtnetlink.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5e0ab4c08f72..94c4572512b8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -342,8 +342,7 @@ int rtnl_unregister(int protocol, int msgtype)
 		return -ENOENT;
 	}
 
-	link = rtnl_dereference(tab[msgindex]);
-	RCU_INIT_POINTER(tab[msgindex], NULL);
+	link = rcu_replace_pointer_rtnl(tab[msgindex], NULL);
 	rtnl_unlock();
 
 	kfree_rcu(link, rcu);
@@ -368,18 +367,13 @@ void rtnl_unregister_all(int protocol)
 	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
 
 	rtnl_lock();
-	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
+	tab = rcu_replace_pointer_rtnl(rtnl_msg_handlers[protocol], NULL);
 	if (!tab) {
 		rtnl_unlock();
 		return;
 	}
-	RCU_INIT_POINTER(rtnl_msg_handlers[protocol], NULL);
 	for (msgindex = 0; msgindex < RTM_NR_MSGTYPES; msgindex++) {
-		link = rtnl_dereference(tab[msgindex]);
-		if (!link)
-			continue;
-
-		RCU_INIT_POINTER(tab[msgindex], NULL);
+		link = rcu_replace_pointer_rtnl(tab[msgindex], NULL);
 		kfree_rcu(link, rcu);
 	}
 	rtnl_unlock();
-- 
2.40.1

Re: [PATCH net-next 1/2] net: rtnl: introduce rcu_replace_pointer_rtnl

[Ido Schimmel]

On Fri, Dec 15, 2023 at 02:57:10PM -0300, Pedro Tammela wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> Introduce the rcu_replace_pointer_rtnl helper to lockdep check rtnl lock
> rcu replacements, alongside the already existing helpers.
> 
> This is a quality of life helper so instead of using:
>    rcu_replace_pointer(rp, p, lockdep_rtnl_is_held())
>    .. or the open coded..
>    rtnl_dereference() / rcu_assign_pointer()
>    .. or the lazy check version ..
>    rcu_replace_pointer(rp, p, 1)
> Use:
>    rcu_replace_pointer_rtnl(rp, p)
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH net-next 2/2] net: rtnl: use rcu_replace_pointer_rtnl in rtnl_unregister_*

[Ido Schimmel]

On Fri, Dec 15, 2023 at 02:57:11PM -0300, Pedro Tammela wrote:
> With the introduction of the rcu_replace_pointer_rtnl helper,
> cleanup the rtnl_unregister_* functions to use the helper instead
> of open coding it.
> 
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH net-next 1/2] net: rtnl: introduce rcu_replace_pointer_rtnl

[Nikolay Aleksandrov]

On 15/12/2023 19:57, Pedro Tammela wrote:
> From: Jamal Hadi Salim <jhs@mojatatu.com>
> 
> Introduce the rcu_replace_pointer_rtnl helper to lockdep check rtnl lock
> rcu replacements, alongside the already existing helpers.
> 
> This is a quality of life helper so instead of using:
>    rcu_replace_pointer(rp, p, lockdep_rtnl_is_held())
>    .. or the open coded..
>    rtnl_dereference() / rcu_assign_pointer()
>    .. or the lazy check version ..
>    rcu_replace_pointer(rp, p, 1)
> Use:
>    rcu_replace_pointer_rtnl(rp, p)
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> ---
>  include/linux/rtnetlink.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index 6a8543b34e2c..410529fca18b 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -79,6 +79,18 @@ static inline bool lockdep_rtnl_is_held(void)
>  #define rtnl_dereference(p)					\
>  	rcu_dereference_protected(p, lockdep_rtnl_is_held())
>  
> +/**
> + * rcu_replace_pointer_rtnl - replace an RCU pointer under rtnl_lock, returning
> + * its old value
> + * @rp: RCU pointer, whose value is returned
> + * @p: regular pointer
> + *
> + * Perform a replacement under rtnl_lock, where @rp is an RCU-annotated
> + * pointer. The old value of @rp is returned, and @rp is set to @p
> + */
> +#define rcu_replace_pointer_rtnl(rp, p)			\
> +	rcu_replace_pointer(rp, p, lockdep_rtnl_is_held())
> +
>  static inline struct netdev_queue *dev_ingress_queue(struct net_device *dev)
>  {
>  	return rtnl_dereference(dev->ingress_queue);

Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 2/2] net: rtnl: use rcu_replace_pointer_rtnl in rtnl_unregister_*

[Nikolay Aleksandrov]

On 15/12/2023 19:57, Pedro Tammela wrote:
> With the introduction of the rcu_replace_pointer_rtnl helper,
> cleanup the rtnl_unregister_* functions to use the helper instead
> of open coding it.
> 
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
> ---
>  net/core/rtnetlink.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 5e0ab4c08f72..94c4572512b8 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -342,8 +342,7 @@ int rtnl_unregister(int protocol, int msgtype)
>  		return -ENOENT;
>  	}
>  
> -	link = rtnl_dereference(tab[msgindex]);
> -	RCU_INIT_POINTER(tab[msgindex], NULL);
> +	link = rcu_replace_pointer_rtnl(tab[msgindex], NULL);
>  	rtnl_unlock();
>  
>  	kfree_rcu(link, rcu);
> @@ -368,18 +367,13 @@ void rtnl_unregister_all(int protocol)
>  	BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
>  
>  	rtnl_lock();
> -	tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
> +	tab = rcu_replace_pointer_rtnl(rtnl_msg_handlers[protocol], NULL);
>  	if (!tab) {
>  		rtnl_unlock();
>  		return;
>  	}
> -	RCU_INIT_POINTER(rtnl_msg_handlers[protocol], NULL);
>  	for (msgindex = 0; msgindex < RTM_NR_MSGTYPES; msgindex++) {
> -		link = rtnl_dereference(tab[msgindex]);
> -		if (!link)
> -			continue;
> -
> -		RCU_INIT_POINTER(tab[msgindex], NULL);
> +		link = rcu_replace_pointer_rtnl(tab[msgindex], NULL);
>  		kfree_rcu(link, rcu);
>  	}
>  	rtnl_unlock();

Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>

Re: [PATCH net-next 0/2] net: rtnl: introduce rcu_replace_pointer_rtnl

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 15 Dec 2023 14:57:09 -0300 you wrote:
> Introduce the rcu_replace_pointer_rtnl helper to lockdep check rtnl lock
> rcu replacements, alongside the already existing helpers.
> 
> Patch 2 uses the new helper in the rtnl_unregister_* functions.
> 
> Originally this change was part of the P4TC series, as it's a recurrent
> pattern there, but since it has a use case in mainline we are pushing it
> separately.
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] net: rtnl: introduce rcu_replace_pointer_rtnl
    https://git.kernel.org/netdev/net-next/c/32da0f00ddcb
  - [net-next,2/2] net: rtnl: use rcu_replace_pointer_rtnl in rtnl_unregister_*
    https://git.kernel.org/netdev/net-next/c/174523479aae

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html