netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com,
	andrew+netdev@lunn.ch, horms@kernel.org, jdamato@fastly.com,
	Jakub Kicinski <kuba@kernel.org>,
	leitao@debian.org
Subject: [PATCH net-next v2 08/11] net: protect threaded status of NAPI with netdev_lock()
Date: Tue, 14 Jan 2025 19:53:16 -0800	[thread overview]
Message-ID: <20250115035319.559603-9-kuba@kernel.org> (raw)
In-Reply-To: <20250115035319.559603-1-kuba@kernel.org>

Now that NAPI instances can't come and go without holding
netdev->lock we can trivially switch from rtnl_lock() to
netdev_lock() for setting netdev->threaded via sysfs.

Note that since we do not lock netdev_lock around sysfs
calls in the core we don't have to "trylock" like we do
with rtnl_lock.

Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
v2:
 - update the comment on dev_isalive()
v1: https://lore.kernel.org/20250114035118.110297-9-kuba@kernel.org

CC: leitao@debian.org
---
 include/linux/netdevice.h | 13 +++++++++++--
 net/core/dev.c            |  2 ++
 net/core/net-sysfs.c      | 34 ++++++++++++++++++++++++++++++++--
 3 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4ab33fbadd9f..bf3da95c9350 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -384,7 +384,7 @@ struct napi_struct {
 	int			rx_count; /* length of rx_list */
 	unsigned int		napi_id; /* protected by netdev_lock */
 	struct hrtimer		timer;
-	struct task_struct	*thread;
+	struct task_struct	*thread; /* protected by netdev_lock */
 	unsigned long		gro_flush_timeout;
 	unsigned long		irq_suspend_timeout;
 	u32			defer_hard_irqs;
@@ -2451,11 +2451,13 @@ struct net_device {
 	 * Drivers are free to use it for other protection.
 	 *
 	 * Protects:
-	 *	@napi_list, @net_shaper_hierarchy, @reg_state
+	 *	@napi_list, @net_shaper_hierarchy, @reg_state, @threaded
 	 *
 	 * Partially protects (writers must hold both @lock and rtnl_lock):
 	 *	@up
 	 *
+	 * Also protects some fields in struct napi_struct.
+	 *
 	 * Ordering: take after rtnl_lock.
 	 */
 	struct mutex		lock;
@@ -2697,6 +2699,13 @@ static inline void netdev_assert_locked(struct net_device *dev)
 	lockdep_assert_held(&dev->lock);
 }
 
+static inline void netdev_assert_locked_or_invisible(struct net_device *dev)
+{
+	if (dev->reg_state == NETREG_REGISTERED ||
+	    dev->reg_state == NETREG_UNREGISTERING)
+		netdev_assert_locked(dev);
+}
+
 static inline void netif_napi_set_irq(struct napi_struct *napi, int irq)
 {
 	napi->irq = irq;
diff --git a/net/core/dev.c b/net/core/dev.c
index 9734c3f5b862..d90bb100285d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6784,6 +6784,8 @@ int dev_set_threaded(struct net_device *dev, bool threaded)
 	struct napi_struct *napi;
 	int err = 0;
 
+	netdev_assert_locked_or_invisible(dev);
+
 	if (dev->threaded == threaded)
 		return 0;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 2d9afc6e2161..9365a7185a1d 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -36,7 +36,7 @@ static const char fmt_uint[] = "%u\n";
 static const char fmt_ulong[] = "%lu\n";
 static const char fmt_u64[] = "%llu\n";
 
-/* Caller holds RTNL or RCU */
+/* Caller holds RTNL, netdev->lock or RCU */
 static inline int dev_isalive(const struct net_device *dev)
 {
 	return READ_ONCE(dev->reg_state) <= NETREG_REGISTERED;
@@ -108,6 +108,36 @@ static ssize_t netdev_store(struct device *dev, struct device_attribute *attr,
 	return ret;
 }
 
+/* Same as netdev_store() but takes netdev_lock() instead of rtnl_lock() */
+static ssize_t
+netdev_lock_store(struct device *dev, struct device_attribute *attr,
+		  const char *buf, size_t len,
+		  int (*set)(struct net_device *, unsigned long))
+{
+	struct net_device *netdev = to_net_dev(dev);
+	struct net *net = dev_net(netdev);
+	unsigned long new;
+	int ret;
+
+	if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
+		return -EPERM;
+
+	ret = kstrtoul(buf, 0, &new);
+	if (ret)
+		return ret;
+
+	netdev_lock(netdev);
+
+	if (dev_isalive(netdev)) {
+		ret = (*set)(netdev, new);
+		if (ret == 0)
+			ret = len;
+	}
+	netdev_unlock(netdev);
+
+	return ret;
+}
+
 NETDEVICE_SHOW_RO(dev_id, fmt_hex);
 NETDEVICE_SHOW_RO(dev_port, fmt_dec);
 NETDEVICE_SHOW_RO(addr_assign_type, fmt_dec);
@@ -638,7 +668,7 @@ static ssize_t threaded_store(struct device *dev,
 			      struct device_attribute *attr,
 			      const char *buf, size_t len)
 {
-	return netdev_store(dev, attr, buf, len, modify_napi_threaded);
+	return netdev_lock_store(dev, attr, buf, len, modify_napi_threaded);
 }
 static DEVICE_ATTR_RW(threaded);
 
-- 
2.48.0


  parent reply	other threads:[~2025-01-15  3:53 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-15  3:53 [PATCH net-next v2 00/11] net: use netdev->lock to protect NAPI Jakub Kicinski
2025-01-15  3:53 ` [PATCH net-next v2 01/11] net: add netdev_lock() / netdev_unlock() helpers Jakub Kicinski
2025-01-15  8:21   ` Kuniyuki Iwashima
2025-01-15  8:36   ` Przemek Kitszel
2025-01-15  9:24     ` Ido Schimmel
2025-01-15 14:08     ` Jakub Kicinski
2025-01-15 14:24       ` Przemek Kitszel
2025-01-15  3:53 ` [PATCH net-next v2 02/11] net: make netdev_lock() protect netdev->reg_state Jakub Kicinski
2025-01-15  8:30   ` Kuniyuki Iwashima
2025-01-15 14:18     ` Jakub Kicinski
2025-01-15  3:53 ` [PATCH net-next v2 03/11] net: add helpers for lookup and walking netdevs under netdev_lock() Jakub Kicinski
2025-01-15  8:41   ` Kuniyuki Iwashima
2025-01-15  3:53 ` [PATCH net-next v2 04/11] net: add netdev->up protected by netdev_lock() Jakub Kicinski
2025-01-15  8:45   ` Kuniyuki Iwashima
2025-01-15  3:53 ` [PATCH net-next v2 05/11] net: protect netdev->napi_list with netdev_lock() Jakub Kicinski
2025-01-15  8:57   ` Kuniyuki Iwashima
2025-01-17 22:21     ` Eric Dumazet
2025-01-17 22:52       ` Jakub Kicinski
2025-01-15  3:53 ` [PATCH net-next v2 06/11] net: protect NAPI enablement " Jakub Kicinski
2025-01-15  9:02   ` Kuniyuki Iwashima
2025-01-21  8:32   ` Dan Carpenter
2025-01-21  8:50     ` David Laight
2025-01-21 15:27     ` Dan Carpenter
2025-01-15  3:53 ` [PATCH net-next v2 07/11] net: make netdev netlink ops hold netdev_lock() Jakub Kicinski
2025-01-15  9:07   ` Kuniyuki Iwashima
2025-01-15  3:53 ` Jakub Kicinski [this message]
2025-01-15  9:09   ` [PATCH net-next v2 08/11] net: protect threaded status of NAPI with netdev_lock() Kuniyuki Iwashima
2025-01-15  3:53 ` [PATCH net-next v2 09/11] net: protect napi->irq " Jakub Kicinski
2025-01-15  9:12   ` Kuniyuki Iwashima
2025-01-15  3:53 ` [PATCH net-next v2 10/11] net: protect NAPI config fields " Jakub Kicinski
2025-01-15  9:15   ` Kuniyuki Iwashima
2025-01-15  3:53 ` [PATCH net-next v2 11/11] netdev-genl: remove rtnl_lock protection from NAPI ops Jakub Kicinski
2025-01-15  9:18   ` Kuniyuki Iwashima
2025-01-16  3:30 ` [PATCH net-next v2 00/11] net: use netdev->lock to protect NAPI patchwork-bot+netdevbpf

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=20250115035319.559603-9-kuba@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jdamato@fastly.com \
    --cc=leitao@debian.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).