Skip to content

Commit

Permalink
net: protect threaded status of NAPI with netdev_lock()
Browse files Browse the repository at this point in the history
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>
Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://patch.msgid.link/20250115035319.559603-9-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Jakub Kicinski committed Jan 16, 2025
1 parent eeeec1d commit 1bb86cf
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 4 deletions.
13 changes: 11 additions & 2 deletions include/linux/netdevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 2 additions & 0 deletions net/core/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -6785,6 +6785,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;

Expand Down
34 changes: 32 additions & 2 deletions net/core/net-sysfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down

0 comments on commit 1bb86cf

Please sign in to comment.