[PATCH v1 net-next 0/2] dev: Hold per-netns RTNL in register_netdev().

[PATCH v1 net-next 0/2] dev: Hold per-netns RTNL in register_netdev().

[Kuniyuki Iwashima]

Patch 1 adds rtnl_net_lock_killable() and Patch 2 uses it in
register_netdev() and converts it and unregister_netdev() to
per-netns RTNL.

With this and the netdev notifier series [0], ASSERT_RTNL_NET()
for NETDEV_REGISTER [1] wasn't fired on a simplest QEMU setup
like e1000 + x86_64_defconfig + CONFIG_DEBUG_NET_SMALL_RTNL.

[0]: https://lore.kernel.org/netdev/20250104063735.36945-1-kuniyu@amazon.com/

[1]:
---8<---
diff --git a/net/core/rtnl_net_debug.c b/net/core/rtnl_net_debug.c
index f406045cbd0e..c0c30929002e 100644
--- a/net/core/rtnl_net_debug.c
+++ b/net/core/rtnl_net_debug.c
@@ -21,7 +21,6 @@ static int rtnl_net_debug_event(struct notifier_block *nb,
 	case NETDEV_DOWN:
 	case NETDEV_REBOOT:
 	case NETDEV_CHANGE:
-	case NETDEV_REGISTER:
 	case NETDEV_UNREGISTER:
 	case NETDEV_CHANGEMTU:
 	case NETDEV_CHANGEADDR:
@@ -60,19 +59,10 @@ static int rtnl_net_debug_event(struct notifier_block *nb,
 		ASSERT_RTNL();
 		break;
 
-	/* Once an event fully supports RTNL_NET, move it here
-	 * and remove "if (0)" below.
-	 *
-	 * case NETDEV_XXX:
-	 *	ASSERT_RTNL_NET(net);
-	 *	break;
-	 */
-	}
-
-	/* Just to avoid unused-variable error for dev and net. */
-	if (0)
+	case NETDEV_REGISTER:
 		ASSERT_RTNL_NET(net);
+		break;
+	}
 
 	return NOTIFY_DONE;
 }
---8<---


Kuniyuki Iwashima (2):
  rtnetlink: Add rtnl_net_lock_killable().
  dev: Hold per-netns RTNL in (un)?register_netdev().

 include/linux/rtnetlink.h |  6 ++++++
 net/core/dev.c            | 14 ++++++++++----
 net/core/rtnetlink.c      | 11 ++++++++++-
 3 files changed, 26 insertions(+), 5 deletions(-)

-- 
2.39.5 (Apple Git-154)

[PATCH v1 net-next 1/2] rtnetlink: Add rtnl_net_lock_killable().

[Kuniyuki Iwashima]

rtnl_lock_killable() is used only in register_netdev()
and will be converted to per-netns RTNL.

Let's unexport it and add the corresponding helper.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/linux/rtnetlink.h |  6 ++++++
 net/core/rtnetlink.c      | 11 ++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 3b9d132cbc9e..4bc2ee0b10b0 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -102,6 +102,7 @@ void __rtnl_net_unlock(struct net *net);
 void rtnl_net_lock(struct net *net);
 void rtnl_net_unlock(struct net *net);
 int rtnl_net_trylock(struct net *net);
+int rtnl_net_lock_killable(struct net *net);
 int rtnl_net_lock_cmp_fn(const struct lockdep_map *a, const struct lockdep_map *b);
 
 bool rtnl_net_is_locked(struct net *net);
@@ -138,6 +139,11 @@ static inline int rtnl_net_trylock(struct net *net)
 	return rtnl_trylock();
 }
 
+static inline int rtnl_net_lock_killable(struct net *net)
+{
+	return rtnl_lock_killable();
+}
+
 static inline void ASSERT_RTNL_NET(struct net *net)
 {
 	ASSERT_RTNL();
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 6b745096809d..1f4d4b5570ab 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -84,7 +84,6 @@ int rtnl_lock_killable(void)
 {
 	return mutex_lock_killable(&rtnl_mutex);
 }
-EXPORT_SYMBOL(rtnl_lock_killable);
 
 static struct sk_buff *defer_kfree_skb_list;
 void rtnl_kfree_skbs(struct sk_buff *head, struct sk_buff *tail)
@@ -221,6 +220,16 @@ int rtnl_net_trylock(struct net *net)
 }
 EXPORT_SYMBOL(rtnl_net_trylock);
 
+int rtnl_net_lock_killable(struct net *net)
+{
+	int ret = rtnl_lock_killable();
+
+	if (!ret)
+		__rtnl_net_lock(net);
+
+	return ret;
+}
+
 static int rtnl_net_cmp_locks(const struct net *net_a, const struct net *net_b)
 {
 	if (net_eq(net_a, net_b))
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v1 net-next 1/2] rtnetlink: Add rtnl_net_lock_killable().

[Eric Dumazet]

On Sat, Jan 4, 2025 at 9:22 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> rtnl_lock_killable() is used only in register_netdev()
> and will be converted to per-netns RTNL.
>
> Let's unexport it and add the corresponding helper.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH v1 net-next 1/2] rtnetlink: Add rtnl_net_lock_killable().

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sat, 4 Jan 2025 17:21:48 +0900 you wrote:
> rtnl_lock_killable() is used only in register_netdev()
> and will be converted to per-netns RTNL.
> 
> Let's unexport it and add the corresponding helper.
> 
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> 
> [...]

Here is the summary with links:
  - [v1,net-next,1/2] rtnetlink: Add rtnl_net_lock_killable().
    https://git.kernel.org/netdev/net-next/c/7bd72a4aa226
  - [v1,net-next,2/2] dev: Hold per-netns RTNL in (un)?register_netdev().
    https://git.kernel.org/netdev/net-next/c/00fb9823939e

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

[PATCH v1 net-next 2/2] dev: Hold per-netns RTNL in (un)?register_netdev().

[Kuniyuki Iwashima]

Let's hold per-netns RTNL of dev_net(dev) in register_netdev()
and unregister_netdev().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/core/dev.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index e7223972b9aa..073f682a9653 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -10731,12 +10731,16 @@ EXPORT_SYMBOL_GPL(init_dummy_netdev);
  */
 int register_netdev(struct net_device *dev)
 {
+	struct net *net = dev_net(dev);
 	int err;
 
-	if (rtnl_lock_killable())
+	if (rtnl_net_lock_killable(net))
 		return -EINTR;
+
 	err = register_netdevice(dev);
-	rtnl_unlock();
+
+	rtnl_net_unlock(net);
+
 	return err;
 }
 EXPORT_SYMBOL(register_netdev);
@@ -11606,9 +11610,11 @@ EXPORT_SYMBOL(unregister_netdevice_many);
  */
 void unregister_netdev(struct net_device *dev)
 {
-	rtnl_lock();
+	struct net *net = dev_net(dev);
+
+	rtnl_net_lock(net);
 	unregister_netdevice(dev);
-	rtnl_unlock();
+	rtnl_net_unlock(net);
 }
 EXPORT_SYMBOL(unregister_netdev);
 
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v1 net-next 2/2] dev: Hold per-netns RTNL in (un)?register_netdev().

[Eric Dumazet]

On Sat, Jan 4, 2025 at 9:23 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> Let's hold per-netns RTNL of dev_net(dev) in register_netdev()
> and unregister_netdev().
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH v1 net-next 0/2] dev: Hold per-netns RTNL in register_netdev().

[Paolo Abeni]

On 1/4/25 9:21 AM, Kuniyuki Iwashima wrote:
> Patch 1 adds rtnl_net_lock_killable() and Patch 2 uses it in
> register_netdev() and converts it and unregister_netdev() to
> per-netns RTNL.
> 
> With this and the netdev notifier series [0], ASSERT_RTNL_NET()
> for NETDEV_REGISTER [1] wasn't fired on a simplest QEMU setup
> like e1000 + x86_64_defconfig + CONFIG_DEBUG_NET_SMALL_RTNL.
> 
> [0]: https://lore.kernel.org/netdev/20250104063735.36945-1-kuniyu@amazon.com/
> 
> [1]:
> ---8<---
> diff --git a/net/core/rtnl_net_debug.c b/net/core/rtnl_net_debug.c
> index f406045cbd0e..c0c30929002e 100644
> --- a/net/core/rtnl_net_debug.c
> +++ b/net/core/rtnl_net_debug.c
> @@ -21,7 +21,6 @@ static int rtnl_net_debug_event(struct notifier_block *nb,
>  	case NETDEV_DOWN:
>  	case NETDEV_REBOOT:
>  	case NETDEV_CHANGE:
> -	case NETDEV_REGISTER:
>  	case NETDEV_UNREGISTER:
>  	case NETDEV_CHANGEMTU:
>  	case NETDEV_CHANGEADDR:
> @@ -60,19 +59,10 @@ static int rtnl_net_debug_event(struct notifier_block *nb,
>  		ASSERT_RTNL();
>  		break;
>  
> -	/* Once an event fully supports RTNL_NET, move it here
> -	 * and remove "if (0)" below.
> -	 *
> -	 * case NETDEV_XXX:
> -	 *	ASSERT_RTNL_NET(net);
> -	 *	break;
> -	 */
> -	}
> -
> -	/* Just to avoid unused-variable error for dev and net. */
> -	if (0)
> +	case NETDEV_REGISTER:
>  		ASSERT_RTNL_NET(net);
> +		break;
> +	}
>  
>  	return NOTIFY_DONE;
>  }
> ---8<---

FTR, the above fooled a bit both PW and our scripts: I had to manually
mangle the cover letter into the merge commit. I guess it would be good
to avoid patch snips in the cover-letter,

Thanks!

Paolo

Re: [PATCH v1 net-next 0/2] dev: Hold per-netns RTNL in register_netdev().

[Kuniyuki Iwashima]

From: Paolo Abeni <pabeni@redhat.com>
Date: Tue, 7 Jan 2025 13:55:51 +0100
> On 1/4/25 9:21 AM, Kuniyuki Iwashima wrote:
> > Patch 1 adds rtnl_net_lock_killable() and Patch 2 uses it in
> > register_netdev() and converts it and unregister_netdev() to
> > per-netns RTNL.
> > 
> > With this and the netdev notifier series [0], ASSERT_RTNL_NET()
> > for NETDEV_REGISTER [1] wasn't fired on a simplest QEMU setup
> > like e1000 + x86_64_defconfig + CONFIG_DEBUG_NET_SMALL_RTNL.
> > 
> > [0]: https://lore.kernel.org/netdev/20250104063735.36945-1-kuniyu@amazon.com/
> > 
> > [1]:
> > ---8<---
> > diff --git a/net/core/rtnl_net_debug.c b/net/core/rtnl_net_debug.c
> > index f406045cbd0e..c0c30929002e 100644
> > --- a/net/core/rtnl_net_debug.c
> > +++ b/net/core/rtnl_net_debug.c
> > @@ -21,7 +21,6 @@ static int rtnl_net_debug_event(struct notifier_block *nb,
> >  	case NETDEV_DOWN:
> >  	case NETDEV_REBOOT:
> >  	case NETDEV_CHANGE:
> > -	case NETDEV_REGISTER:
> >  	case NETDEV_UNREGISTER:
> >  	case NETDEV_CHANGEMTU:
> >  	case NETDEV_CHANGEADDR:
> > @@ -60,19 +59,10 @@ static int rtnl_net_debug_event(struct notifier_block *nb,
> >  		ASSERT_RTNL();
> >  		break;
> >  
> > -	/* Once an event fully supports RTNL_NET, move it here
> > -	 * and remove "if (0)" below.
> > -	 *
> > -	 * case NETDEV_XXX:
> > -	 *	ASSERT_RTNL_NET(net);
> > -	 *	break;
> > -	 */
> > -	}
> > -
> > -	/* Just to avoid unused-variable error for dev and net. */
> > -	if (0)
> > +	case NETDEV_REGISTER:
> >  		ASSERT_RTNL_NET(net);
> > +		break;
> > +	}
> >  
> >  	return NOTIFY_DONE;
> >  }
> > ---8<---
> 
> FTR, the above fooled a bit both PW and our scripts: I had to manually
> mangle the cover letter into the merge commit. I guess it would be good
> to avoid patch snips in the cover-letter,

Oh sorry, noted.

Thank you !