All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kuniyuki Iwashima <kuniyu@amazon.com>
To: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	Vincent Mailhol <mailhol.vincent@wanadoo.fr>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	Nikolay Aleksandrov <razor@blackwall.org>,
	Kuniyuki Iwashima <kuniyu@amazon.com>,
	Kuniyuki Iwashima <kuni1840@gmail.com>, <netdev@vger.kernel.org>
Subject: [PATCH v3 net-next 01/10] rtnetlink: Remove __rtnl_link_unregister().
Date: Wed, 6 Nov 2024 18:28:51 -0800	[thread overview]
Message-ID: <20241107022900.70287-2-kuniyu@amazon.com> (raw)
In-Reply-To: <20241107022900.70287-1-kuniyu@amazon.com>

rtnl_link_unregister() holds RTNL and calls __rtnl_link_unregister(),
where we call synchronize_srcu() to wait inflight RTM_NEWLINK requests
for per-netns RTNL.

We put synchronize_srcu() in __rtnl_link_unregister() due to ifb.ko
and dummy.ko.

However, rtnl_newlink() will acquire SRCU before RTNL later in this
series.  Then, lockdep will detect the deadlock:

   rtnl_link_unregister()       rtnl_newlink()
   ----                         ----
   lock(rtnl_mutex);
                                lock(&ops->srcu);
                                lock(rtnl_mutex);
   sync(&ops->srcu);

To avoid the problem, we must call synchronize_srcu() before RTNL in
rtnl_link_unregister().

As a preparation, let's remove __rtnl_link_unregister().

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 drivers/net/dummy.c     |  6 +++++-
 drivers/net/ifb.c       |  6 +++++-
 include/net/rtnetlink.h |  1 -
 net/core/rtnetlink.c    | 32 ++++++++++----------------------
 4 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index e9c5e1e11fa0..72618b6af44e 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -166,6 +166,7 @@ static int __init dummy_init_one(void)
 
 static int __init dummy_init_module(void)
 {
+	bool need_unregister = false;
 	int i, err = 0;
 
 	down_write(&pernet_ops_rwsem);
@@ -179,12 +180,15 @@ static int __init dummy_init_module(void)
 		cond_resched();
 	}
 	if (err < 0)
-		__rtnl_link_unregister(&dummy_link_ops);
+		need_unregister = true;
 
 out:
 	rtnl_unlock();
 	up_write(&pernet_ops_rwsem);
 
+	if (need_unregister)
+		rtnl_link_unregister(&dummy_link_ops);
+
 	return err;
 }
 
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 2c1b5def4a0b..a4b9ec4e8f30 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -424,6 +424,7 @@ static int __init ifb_init_one(int index)
 
 static int __init ifb_init_module(void)
 {
+	bool need_unregister = false;
 	int i, err;
 
 	down_write(&pernet_ops_rwsem);
@@ -437,12 +438,15 @@ static int __init ifb_init_module(void)
 		cond_resched();
 	}
 	if (err)
-		__rtnl_link_unregister(&ifb_link_ops);
+		need_unregister = true;
 
 out:
 	rtnl_unlock();
 	up_write(&pernet_ops_rwsem);
 
+	if (need_unregister)
+		rtnl_link_unregister(&ifb_link_ops);
+
 	return err;
 }
 
diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index b260c0cc9671..3ebfcc6e56fd 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -165,7 +165,6 @@ struct rtnl_link_ops {
 };
 
 int __rtnl_link_register(struct rtnl_link_ops *ops);
-void __rtnl_link_unregister(struct rtnl_link_ops *ops);
 
 int rtnl_link_register(struct rtnl_link_ops *ops);
 void rtnl_link_unregister(struct rtnl_link_ops *ops);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 3b33810d92a8..634732fe4c64 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -568,27 +568,6 @@ static void __rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops)
 	unregister_netdevice_many(&list_kill);
 }
 
-/**
- * __rtnl_link_unregister - Unregister rtnl_link_ops from rtnetlink.
- * @ops: struct rtnl_link_ops * to unregister
- *
- * The caller must hold the rtnl_mutex and guarantee net_namespace_list
- * integrity (hold pernet_ops_rwsem for writing to close the race
- * with setup_net() and cleanup_net()).
- */
-void __rtnl_link_unregister(struct rtnl_link_ops *ops)
-{
-	struct net *net;
-
-	list_del_rcu(&ops->list);
-	synchronize_srcu(&ops->srcu);
-	cleanup_srcu_struct(&ops->srcu);
-
-	for_each_net(net)
-		__rtnl_kill_links(net, ops);
-}
-EXPORT_SYMBOL_GPL(__rtnl_link_unregister);
-
 /* Return with the rtnl_lock held when there are no network
  * devices unregistering in any network namespace.
  */
@@ -617,10 +596,19 @@ static void rtnl_lock_unregistering_all(void)
  */
 void rtnl_link_unregister(struct rtnl_link_ops *ops)
 {
+	struct net *net;
+
 	/* Close the race with setup_net() and cleanup_net() */
 	down_write(&pernet_ops_rwsem);
 	rtnl_lock_unregistering_all();
-	__rtnl_link_unregister(ops);
+
+	list_del_rcu(&ops->list);
+	synchronize_srcu(&ops->srcu);
+	cleanup_srcu_struct(&ops->srcu);
+
+	for_each_net(net)
+		__rtnl_kill_links(net, ops);
+
 	rtnl_unlock();
 	up_write(&pernet_ops_rwsem);
 }
-- 
2.39.5 (Apple Git-154)


  reply	other threads:[~2024-11-07  2:29 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-07  2:28 [PATCH v3 net-next 00/10] rtnetlink: Convert rtnl_newlink() to per-netns RTNL Kuniyuki Iwashima
2024-11-07  2:28 ` Kuniyuki Iwashima [this message]
2024-11-07  2:28 ` [PATCH v3 net-next 02/10] rtnetlink: Protect link_ops by mutex Kuniyuki Iwashima
2024-11-07  2:28 ` [PATCH v3 net-next 03/10] rtnetlink: Remove __rtnl_link_register() Kuniyuki Iwashima
2024-11-07  2:28 ` [PATCH v3 net-next 04/10] rtnetlink: Introduce struct rtnl_nets and helpers Kuniyuki Iwashima
2024-11-07  2:28 ` [PATCH v3 net-next 06/10] veth: Set VETH_INFO_PEER to veth_link_ops.peer_type Kuniyuki Iwashima
2024-11-07  2:28 ` [PATCH v3 net-next 07/10] vxcan: Set VXCAN_INFO_PEER to vxcan_link_ops.peer_type Kuniyuki Iwashima
2024-11-07  2:28 ` [PATCH v3 net-next 08/10] netkit: Set IFLA_NETKIT_PEER_INFO to netkit_link_ops.peer_type Kuniyuki Iwashima
2024-11-07  2:28 ` [PATCH v3 net-next 09/10] rtnetlink: Convert RTM_NEWLINK to per-netns RTNL Kuniyuki Iwashima
2024-11-07  2:29 ` [PATCH v3 net-next 10/10] rtnetlink: Register rtnl_dellink() and rtnl_setlink() with RTNL_FLAG_DOIT_PERNET_WIP Kuniyuki Iwashima
2024-11-07 23:53 ` [PATCH v3 net-next 00/10] rtnetlink: Convert rtnl_newlink() to per-netns RTNL Jakub Kicinski
2024-11-08  0:02   ` Kuniyuki Iwashima

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241107022900.70287-2-kuniyu@amazon.com \
    --to=kuniyu@amazon.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=kuni1840@gmail.com \
    --cc=mailhol.vincent@wanadoo.fr \
    --cc=mkl@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.