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 02/10] rtnetlink: Protect link_ops by mutex.
Date: Wed, 6 Nov 2024 18:28:52 -0800	[thread overview]
Message-ID: <20241107022900.70287-3-kuniyu@amazon.com> (raw)
In-Reply-To: <20241107022900.70287-1-kuniyu@amazon.com>

rtnl_link_unregister() holds RTNL and calls synchronize_srcu(),
but rtnl_newlink() will acquire SRCU frist and then RTNL.

Then, we need to unlink ops and call synchronize_srcu() outside
of RTNL to avoid the deadlock.

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

Let's move as such and add a mutex to protect link_ops.

Now, link_ops is protected by its dedicated mutex and
rtnl_link_register() no longer needs to hold RTNL.

While at it, we move the initialisation of ops->dellink and
ops->srcu out of the mutex scope.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/rtnetlink.h |  2 +-
 net/core/rtnetlink.c    | 33 ++++++++++++++++++++-------------
 2 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 3ebfcc6e56fd..7559020f760c 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -71,7 +71,7 @@ static inline int rtnl_msg_family(const struct nlmsghdr *nlh)
 /**
  *	struct rtnl_link_ops - rtnetlink link operations
  *
- *	@list: Used internally, protected by RTNL and SRCU
+ *	@list: Used internally, protected by link_ops_mutex and SRCU
  *	@srcu: Used internally
  *	@kind: Identifier
  *	@netns_refund: Physical device, move to init_net on netns exit
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 634732fe4c64..fcccb916e468 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -466,6 +466,7 @@ void __rtnl_unregister_many(const struct rtnl_msg_handler *handlers, int n)
 }
 EXPORT_SYMBOL_GPL(__rtnl_unregister_many);
 
+static DEFINE_MUTEX(link_ops_mutex);
 static LIST_HEAD(link_ops);
 
 static struct rtnl_link_ops *rtnl_link_ops_get(const char *kind, int *srcu_index)
@@ -508,14 +509,6 @@ int __rtnl_link_register(struct rtnl_link_ops *ops)
 	struct rtnl_link_ops *tmp;
 	int err;
 
-	/* When RTNL is removed, add lock for link_ops. */
-	ASSERT_RTNL();
-
-	list_for_each_entry(tmp, &link_ops, list) {
-		if (!strcmp(ops->kind, tmp->kind))
-			return -EEXIST;
-	}
-
 	/* The check for alloc/setup is here because if ops
 	 * does not have that filled up, it is not possible
 	 * to use the ops for creating device. So do not
@@ -528,9 +521,20 @@ int __rtnl_link_register(struct rtnl_link_ops *ops)
 	if (err)
 		return err;
 
+	mutex_lock(&link_ops_mutex);
+
+	list_for_each_entry(tmp, &link_ops, list) {
+		if (!strcmp(ops->kind, tmp->kind)) {
+			err = -EEXIST;
+			goto unlock;
+		}
+	}
+
 	list_add_tail_rcu(&ops->list, &link_ops);
+unlock:
+	mutex_unlock(&link_ops_mutex);
 
-	return 0;
+	return err;
 }
 EXPORT_SYMBOL_GPL(__rtnl_link_register);
 
@@ -598,14 +602,17 @@ 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();
-
+	mutex_lock(&link_ops_mutex);
 	list_del_rcu(&ops->list);
+	mutex_unlock(&link_ops_mutex);
+
 	synchronize_srcu(&ops->srcu);
 	cleanup_srcu_struct(&ops->srcu);
 
+	/* Close the race with setup_net() and cleanup_net() */
+	down_write(&pernet_ops_rwsem);
+	rtnl_lock_unregistering_all();
+
 	for_each_net(net)
 		__rtnl_kill_links(net, ops);
 
-- 
2.39.5 (Apple Git-154)


  parent 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 ` [PATCH v3 net-next 01/10] rtnetlink: Remove __rtnl_link_unregister() Kuniyuki Iwashima
2024-11-07  2:28 ` Kuniyuki Iwashima [this message]
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-3-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.