Thread (34 messages) flat view 34 messages, 4 authors, 2025-01-14

Re: [PATCH net-next 02/11] net: add helpers for lookup and walking netdevs under netdev_lock()

From: Eric Dumazet <edumazet@google.com>
Date: 2025-01-14 13:03:49

On Tue, Jan 14, 2025 at 4:51 AM Jakub Kicinski [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Add helpers for accessing netdevs under netdev_lock().
There's some careful handling needed to find the device and lock it
safely, without it getting unregistered, and without taking rtnl_lock
(the latter being the whole point of the new locking, after all).

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 net/core/dev.h |  16 +++++++
 net/core/dev.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 126 insertions(+)
diff --git a/net/core/dev.h b/net/core/dev.h
index d8966847794c..25ae732c0775 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -2,6 +2,7 @@
 #ifndef _NET_CORE_DEV_H
 #define _NET_CORE_DEV_H

+#include <linux/cleanup.h>
 #include <linux/types.h>
 #include <linux/rwsem.h>
 #include <linux/netdevice.h>
@@ -23,8 +24,23 @@ struct sd_flow_limit {
 extern int netdev_flow_limit_table_len;

 struct napi_struct *netdev_napi_by_id(struct net *net, unsigned int napi_id);
+struct napi_struct *
+netdev_napi_by_id_lock(struct net *net, unsigned int napi_id);
 struct net_device *dev_get_by_napi_id(unsigned int napi_id);

+struct net_device *netdev_get_by_index_lock(struct net *net, int ifindex);
+struct net_device *__netdev_put_lock(struct net_device *dev);
+struct net_device *
+netdev_xa_find_lock(struct net *net, struct net_device *dev,
+                   unsigned long *index);
+
+DEFINE_FREE(netdev_unlock, struct net_device *, if (_T) netdev_unlock(_T));
+
+#define for_each_netdev_lock_scoped(net, var_name, ifindex)            \
+       for (struct net_device *var_name __free(netdev_unlock) = NULL;  \
+            (var_name = netdev_xa_find_lock(net, var_name, &ifindex)); \
+            ifindex++)
+
 #ifdef CONFIG_PROC_FS
 int __init dev_proc_init(void);
 #else
diff --git a/net/core/dev.c b/net/core/dev.c
index fda4e1039bf0..5c1e71afbe1c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -783,6 +783,49 @@ struct napi_struct *netdev_napi_by_id(struct net *net, unsigned int napi_id)
        return napi;
 }

+/**
+ *     netdev_napi_by_id_lock() - find a device by NAPI ID and lock it
+ *     @net: the applicable net namespace
+ *     @napi_id: ID of a NAPI of a target device
+ *
+ *     Find a NAPI instance with @napi_id. Lock its device.
+ *     The device must be in %NETREG_REGISTERED state for lookup to succeed.
+ *     netdev_unlock() must be called to release it.
+ *
+ *     Return: pointer to NAPI, its device with lock held, NULL if not found.
+ */
+struct napi_struct *
+netdev_napi_by_id_lock(struct net *net, unsigned int napi_id)
+{
+       struct napi_struct *napi;
+       struct net_device *dev;
+
+       rcu_read_lock();
+       napi = netdev_napi_by_id(net, napi_id);
+       if (!napi || napi->dev->reg_state != NETREG_REGISTERED) {
nit: this should be READ_ONCE(napi->dev->reg_state) != NETREG_REGISTERED
quoted hunk ↗ jump to hunk
+               rcu_read_unlock();
+               return NULL;
+       }
+
+       dev = napi->dev;
+       dev_hold(dev);
+       rcu_read_unlock();
+
+       dev = __netdev_put_lock(dev);
+       if (!dev)
+               return NULL;
+
+       rcu_read_lock();
+       napi = netdev_napi_by_id(net, napi_id);
+       if (napi && napi->dev != dev)
+               napi = NULL;
+       rcu_read_unlock();
+
+       if (!napi)
+               netdev_unlock(dev);
+       return napi;
+}
+
 /**
  *     __dev_get_by_name       - find a device by its name
  *     @net: the applicable net namespace
@@ -971,6 +1014,73 @@ struct net_device *dev_get_by_napi_id(unsigned int napi_id)
        return napi ? napi->dev : NULL;
 }

+/* Release the held reference on the net_device, and if the net_device
+ * is still registered try to lock the instance lock. If device is being
+ * unregistered NULL will be returned (but the reference has been released,
+ * either way!)
+ *
+ * This helper is intended for locking net_device after it has been looked up
+ * using a lockless lookup helper. Lock prevents the instance from going away.
+ */
+struct net_device *__netdev_put_lock(struct net_device *dev)
+{
+       netdev_lock(dev);
+       if (dev->reg_state > NETREG_REGISTERED) {
I presume the netdev lock will be held at some point in
netdev_run_todo and/or unregister_netdevice_many_notify
so no need for a READ_ONCE() here.

Reviewed-by: Eric Dumazet <edumazet@google.com>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help