[PATCH net-next v2 0/3] net: remove rtnl_lock from the callers of queue APIs

[PATCH net-next v2 0/3] net: remove rtnl_lock from the callers of queue APIs

[Stanislav Fomichev]

All drivers that use queue management APIs already depend on the netdev
lock. Ultimately, we want to have most of the paths that work with
specific netdev to be rtnl_lock-free (ethtool mostly in particular).
Queue API currently has a much smaller API surface, so start with
rtnl_lock from it:

- add mutex to each dmabuf binding (to replace rtnl_lock)
- move netdev lock management to the callers of netdev_rx_queue_restart
  and drop rtnl_lock

v2:
- drop "net: protect net_devmem_dmabuf_bindings by new
  net_devmem_bindings_mutex" (Jakub)
- add missing mutex_unlock (Jakub)
- undo rtnl_lock removal from netdev_nl_queue_get_doit and
  netdev_nl_queue_get_dumpit, needs more care to grab
  either or but no both rtnl_lock/ops_lock (Jakub)

Cc: Mina Almasry <almasrymina@google.com>

Stanislav Fomichev (3):
  net: create netdev_nl_sock to wrap bindings list
  net: add granular lock for the netdev netlink socket
  net: drop rtnl_lock for queue_mgmt operations

 Documentation/netlink/specs/netdev.yaml   |  4 +--
 drivers/net/ethernet/broadcom/bnxt/bnxt.c |  4 +--
 drivers/net/netdevsim/netdev.c            |  4 +--
 include/net/netdev_netlink.h              | 12 +++++++
 net/core/devmem.c                         |  4 +--
 net/core/netdev-genl-gen.c                |  4 +--
 net/core/netdev-genl-gen.h                |  6 ++--
 net/core/netdev-genl.c                    | 38 +++++++++++++----------
 net/core/netdev_rx_queue.c                | 16 ++++------
 9 files changed, 52 insertions(+), 40 deletions(-)
 create mode 100644 include/net/netdev_netlink.h

-- 
2.48.1

[PATCH net-next v2 1/3] net: create netdev_nl_sock to wrap bindings list

[Stanislav Fomichev]

No functional changes. Next patches will add more granular locking
to netdev_nl_sock.

Cc: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 Documentation/netlink/specs/netdev.yaml |  4 ++--
 include/net/netdev_netlink.h            | 11 +++++++++++
 net/core/netdev-genl-gen.c              |  4 ++--
 net/core/netdev-genl-gen.h              |  6 +++---
 net/core/netdev-genl.c                  | 19 +++++++++----------
 5 files changed, 27 insertions(+), 17 deletions(-)
 create mode 100644 include/net/netdev_netlink.h

diff --git a/Documentation/netlink/specs/netdev.yaml b/Documentation/netlink/specs/netdev.yaml
index 36f1152bfac3..f5e0750ab71d 100644
--- a/Documentation/netlink/specs/netdev.yaml
+++ b/Documentation/netlink/specs/netdev.yaml
@@ -745,8 +745,8 @@ name: netdev
             - irq-suspend-timeout
 
 kernel-family:
-  headers: [ "linux/list.h"]
-  sock-priv: struct list_head
+  headers: [ "net/netdev_netlink.h"]
+  sock-priv: struct netdev_nl_sock
 
 mcast-groups:
   list:
diff --git a/include/net/netdev_netlink.h b/include/net/netdev_netlink.h
new file mode 100644
index 000000000000..1599573d35c9
--- /dev/null
+++ b/include/net/netdev_netlink.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __NET_NETDEV_NETLINK_H
+#define __NET_NETDEV_NETLINK_H
+
+#include <linux/list.h>
+
+struct netdev_nl_sock {
+	struct list_head bindings;
+};
+
+#endif	/* __NET_NETDEV_NETLINK_H */
diff --git a/net/core/netdev-genl-gen.c b/net/core/netdev-genl-gen.c
index 996ac6a449eb..739f7b6506a6 100644
--- a/net/core/netdev-genl-gen.c
+++ b/net/core/netdev-genl-gen.c
@@ -9,7 +9,7 @@
 #include "netdev-genl-gen.h"
 
 #include <uapi/linux/netdev.h>
-#include <linux/list.h>
+#include <net/netdev_netlink.h>
 
 /* Integer value ranges */
 static const struct netlink_range_validation netdev_a_page_pool_id_range = {
@@ -217,7 +217,7 @@ struct genl_family netdev_nl_family __ro_after_init = {
 	.n_split_ops	= ARRAY_SIZE(netdev_nl_ops),
 	.mcgrps		= netdev_nl_mcgrps,
 	.n_mcgrps	= ARRAY_SIZE(netdev_nl_mcgrps),
-	.sock_priv_size	= sizeof(struct list_head),
+	.sock_priv_size	= sizeof(struct netdev_nl_sock),
 	.sock_priv_init	= __netdev_nl_sock_priv_init,
 	.sock_priv_destroy = __netdev_nl_sock_priv_destroy,
 };
diff --git a/net/core/netdev-genl-gen.h b/net/core/netdev-genl-gen.h
index e09dd7539ff2..17d39fd64c94 100644
--- a/net/core/netdev-genl-gen.h
+++ b/net/core/netdev-genl-gen.h
@@ -10,7 +10,7 @@
 #include <net/genetlink.h>
 
 #include <uapi/linux/netdev.h>
-#include <linux/list.h>
+#include <net/netdev_netlink.h>
 
 /* Common nested types */
 extern const struct nla_policy netdev_page_pool_info_nl_policy[NETDEV_A_PAGE_POOL_IFINDEX + 1];
@@ -42,7 +42,7 @@ enum {
 
 extern struct genl_family netdev_nl_family;
 
-void netdev_nl_sock_priv_init(struct list_head *priv);
-void netdev_nl_sock_priv_destroy(struct list_head *priv);
+void netdev_nl_sock_priv_init(struct netdev_nl_sock *priv);
+void netdev_nl_sock_priv_destroy(struct netdev_nl_sock *priv);
 
 #endif /* _LINUX_NETDEV_GEN_H */
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 2b774183d31c..a219be90c739 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -829,8 +829,8 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct nlattr *tb[ARRAY_SIZE(netdev_queue_id_nl_policy)];
 	struct net_devmem_dmabuf_binding *binding;
-	struct list_head *sock_binding_list;
 	u32 ifindex, dmabuf_fd, rxq_idx;
+	struct netdev_nl_sock *priv;
 	struct net_device *netdev;
 	struct sk_buff *rsp;
 	struct nlattr *attr;
@@ -845,10 +845,9 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 	ifindex = nla_get_u32(info->attrs[NETDEV_A_DEV_IFINDEX]);
 	dmabuf_fd = nla_get_u32(info->attrs[NETDEV_A_DMABUF_FD]);
 
-	sock_binding_list = genl_sk_priv_get(&netdev_nl_family,
-					     NETLINK_CB(skb).sk);
-	if (IS_ERR(sock_binding_list))
-		return PTR_ERR(sock_binding_list);
+	priv = genl_sk_priv_get(&netdev_nl_family, NETLINK_CB(skb).sk);
+	if (IS_ERR(priv))
+		return PTR_ERR(priv);
 
 	rsp = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!rsp)
@@ -909,7 +908,7 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 			goto err_unbind;
 	}
 
-	list_add(&binding->list, sock_binding_list);
+	list_add(&binding->list, &priv->bindings);
 
 	nla_put_u32(rsp, NETDEV_A_DMABUF_ID, binding->id);
 	genlmsg_end(rsp, hdr);
@@ -931,17 +930,17 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
-void netdev_nl_sock_priv_init(struct list_head *priv)
+void netdev_nl_sock_priv_init(struct netdev_nl_sock *priv)
 {
-	INIT_LIST_HEAD(priv);
+	INIT_LIST_HEAD(&priv->bindings);
 }
 
-void netdev_nl_sock_priv_destroy(struct list_head *priv)
+void netdev_nl_sock_priv_destroy(struct netdev_nl_sock *priv)
 {
 	struct net_devmem_dmabuf_binding *binding;
 	struct net_devmem_dmabuf_binding *temp;
 
-	list_for_each_entry_safe(binding, temp, priv, list) {
+	list_for_each_entry_safe(binding, temp, &priv->bindings, list) {
 		rtnl_lock();
 		net_devmem_unbind_dmabuf(binding);
 		rtnl_unlock();
-- 
2.48.1

[PATCH net-next v2 2/3] net: add granular lock for the netdev netlink socket

[Stanislav Fomichev]

As we move away from rtnl_lock for queue ops, introduce
per-netdev_nl_sock lock.

Cc: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 include/net/netdev_netlink.h | 1 +
 net/core/netdev-genl.c       | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/net/netdev_netlink.h b/include/net/netdev_netlink.h
index 1599573d35c9..075962dbe743 100644
--- a/include/net/netdev_netlink.h
+++ b/include/net/netdev_netlink.h
@@ -5,6 +5,7 @@
 #include <linux/list.h>
 
 struct netdev_nl_sock {
+	struct mutex lock;
 	struct list_head bindings;
 };
 
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index a219be90c739..63e10717efc5 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -859,6 +859,7 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 		goto err_genlmsg_free;
 	}
 
+	mutex_lock(&priv->lock);
 	rtnl_lock();
 
 	netdev = __dev_get_by_index(genl_info_net(info), ifindex);
@@ -918,6 +919,7 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 		goto err_unbind;
 
 	rtnl_unlock();
+	mutex_unlock(&priv->lock);
 
 	return 0;
 
@@ -925,6 +927,7 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 	net_devmem_unbind_dmabuf(binding);
 err_unlock:
 	rtnl_unlock();
+	mutex_unlock(&priv->lock);
 err_genlmsg_free:
 	nlmsg_free(rsp);
 	return err;
@@ -933,6 +936,7 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 void netdev_nl_sock_priv_init(struct netdev_nl_sock *priv)
 {
 	INIT_LIST_HEAD(&priv->bindings);
+	mutex_init(&priv->lock);
 }
 
 void netdev_nl_sock_priv_destroy(struct netdev_nl_sock *priv)
@@ -940,11 +944,13 @@ void netdev_nl_sock_priv_destroy(struct netdev_nl_sock *priv)
 	struct net_devmem_dmabuf_binding *binding;
 	struct net_devmem_dmabuf_binding *temp;
 
+	mutex_lock(&priv->lock);
 	list_for_each_entry_safe(binding, temp, &priv->bindings, list) {
 		rtnl_lock();
 		net_devmem_unbind_dmabuf(binding);
 		rtnl_unlock();
 	}
+	mutex_unlock(&priv->lock);
 }
 
 static int netdev_genl_netdevice_event(struct notifier_block *nb,
-- 
2.48.1

[PATCH net-next v2 3/3] net: drop rtnl_lock for queue_mgmt operations

[Stanislav Fomichev]

All drivers that use queue API are already converted to use
netdev instance lock. Move netdev instance lock management to
the netlink layer and drop rtnl_lock.

Cc: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt.c |  4 ++--
 drivers/net/netdevsim/netdev.c            |  4 ++--
 net/core/devmem.c                         |  4 ++--
 net/core/netdev-genl.c                    | 13 ++++++-------
 net/core/netdev_rx_queue.c                | 16 ++++++----------
 5 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index b09171110ec4..fed08aaf68e4 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -11381,14 +11381,14 @@ static void bnxt_irq_affinity_notify(struct irq_affinity_notify *notify,
 	if (pcie_tph_set_st_entry(irq->bp->pdev, irq->msix_nr, tag))
 		return;
 
-	rtnl_lock();
+	netdev_lock(irq->bp->dev);
 	if (netif_running(irq->bp->dev)) {
 		err = netdev_rx_queue_restart(irq->bp->dev, irq->ring_nr);
 		if (err)
 			netdev_err(irq->bp->dev,
 				   "RX queue restart failed: err=%d\n", err);
 	}
-	rtnl_unlock();
+	netdev_unlock(irq->bp->dev);
 }
 
 static void bnxt_irq_affinity_release(struct kref *ref)
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index d71fd2907cc8..e3152ebe98a2 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -787,7 +787,7 @@ nsim_qreset_write(struct file *file, const char __user *data,
 	if (ret != 2)
 		return -EINVAL;
 
-	rtnl_lock();
+	netdev_lock(ns->netdev);
 	if (queue >= ns->netdev->real_num_rx_queues) {
 		ret = -EINVAL;
 		goto exit_unlock;
@@ -801,7 +801,7 @@ nsim_qreset_write(struct file *file, const char __user *data,
 
 	ret = count;
 exit_unlock:
-	rtnl_unlock();
+	netdev_unlock(ns->netdev);
 	return ret;
 }
 
diff --git a/net/core/devmem.c b/net/core/devmem.c
index 7c6e0b5b6acb..5c4d79a1bcd8 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -25,7 +25,6 @@
 
 /* Device memory support */
 
-/* Protected by rtnl_lock() */
 static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
 
 static const struct memory_provider_ops dmabuf_devmem_ops;
@@ -128,9 +127,10 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
 		rxq->mp_params.mp_priv = NULL;
 		rxq->mp_params.mp_ops = NULL;
 
+		netdev_lock(binding->dev);
 		rxq_idx = get_netdev_rx_queue_index(rxq);
-
 		WARN_ON(netdev_rx_queue_restart(binding->dev, rxq_idx));
+		netdev_unlock(binding->dev);
 	}
 
 	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 63e10717efc5..a186fea63c09 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -860,12 +860,11 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	mutex_lock(&priv->lock);
-	rtnl_lock();
 
-	netdev = __dev_get_by_index(genl_info_net(info), ifindex);
+	netdev = netdev_get_by_index_lock(genl_info_net(info), ifindex);
 	if (!netdev || !netif_device_present(netdev)) {
 		err = -ENODEV;
-		goto err_unlock;
+		goto err_unlock_sock;
 	}
 
 	if (dev_xdp_prog_count(netdev)) {
@@ -918,7 +917,8 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		goto err_unbind;
 
-	rtnl_unlock();
+	netdev_unlock(netdev);
+
 	mutex_unlock(&priv->lock);
 
 	return 0;
@@ -926,7 +926,8 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 err_unbind:
 	net_devmem_unbind_dmabuf(binding);
 err_unlock:
-	rtnl_unlock();
+	netdev_unlock(netdev);
+err_unlock_sock:
 	mutex_unlock(&priv->lock);
 err_genlmsg_free:
 	nlmsg_free(rsp);
@@ -946,9 +947,7 @@ void netdev_nl_sock_priv_destroy(struct netdev_nl_sock *priv)
 
 	mutex_lock(&priv->lock);
 	list_for_each_entry_safe(binding, temp, &priv->bindings, list) {
-		rtnl_lock();
 		net_devmem_unbind_dmabuf(binding);
-		rtnl_unlock();
 	}
 	mutex_unlock(&priv->lock);
 }
diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
index 7419c41fd3cb..a5b234b33cd5 100644
--- a/net/core/netdev_rx_queue.c
+++ b/net/core/netdev_rx_queue.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
 #include <linux/netdevice.h>
+#include <net/netdev_lock.h>
 #include <net/netdev_queues.h>
 #include <net/netdev_rx_queue.h>
 #include <net/page_pool/memory_provider.h>
@@ -18,7 +19,7 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
 	    !qops->ndo_queue_mem_alloc || !qops->ndo_queue_start)
 		return -EOPNOTSUPP;
 
-	ASSERT_RTNL();
+	netdev_assert_locked(dev);
 
 	new_mem = kvzalloc(qops->ndo_queue_mem_size, GFP_KERNEL);
 	if (!new_mem)
@@ -30,8 +31,6 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
 		goto err_free_new_mem;
 	}
 
-	netdev_lock(dev);
-
 	err = qops->ndo_queue_mem_alloc(dev, new_mem, rxq_idx);
 	if (err)
 		goto err_free_old_mem;
@@ -54,8 +53,6 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
 
 	qops->ndo_queue_mem_free(dev, old_mem);
 
-	netdev_unlock(dev);
-
 	kvfree(old_mem);
 	kvfree(new_mem);
 
@@ -80,7 +77,6 @@ int netdev_rx_queue_restart(struct net_device *dev, unsigned int rxq_idx)
 	qops->ndo_queue_mem_free(dev, new_mem);
 
 err_free_old_mem:
-	netdev_unlock(dev);
 	kvfree(old_mem);
 
 err_free_new_mem:
@@ -118,9 +114,9 @@ int net_mp_open_rxq(struct net_device *dev, unsigned ifq_idx,
 {
 	int ret;
 
-	rtnl_lock();
+	netdev_lock(dev);
 	ret = __net_mp_open_rxq(dev, ifq_idx, p);
-	rtnl_unlock();
+	netdev_unlock(dev);
 	return ret;
 }
 
@@ -153,7 +149,7 @@ static void __net_mp_close_rxq(struct net_device *dev, unsigned ifq_idx,
 void net_mp_close_rxq(struct net_device *dev, unsigned ifq_idx,
 		      struct pp_memory_provider_params *old_p)
 {
-	rtnl_lock();
+	netdev_lock(dev);
 	__net_mp_close_rxq(dev, ifq_idx, old_p);
-	rtnl_unlock();
+	netdev_unlock(dev);
 }
-- 
2.48.1

Re: [PATCH net-next v2 1/3] net: create netdev_nl_sock to wrap bindings list

[Mina Almasry]

On Tue, Mar 11, 2025 at 7:40 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> No functional changes. Next patches will add more granular locking
> to netdev_nl_sock.
>
> Cc: Mina Almasry <almasrymina@google.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>

Reviewed-by: Mina Almasry <almasrymina@google.com>

-- 
Thanks,
Mina

Re: [PATCH net-next v2 2/3] net: add granular lock for the netdev netlink socket

[Mina Almasry]

On Tue, Mar 11, 2025 at 7:40 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> As we move away from rtnl_lock for queue ops, introduce
> per-netdev_nl_sock lock.
>
> Cc: Mina Almasry <almasrymina@google.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
>  include/net/netdev_netlink.h | 1 +
>  net/core/netdev-genl.c       | 6 ++++++
>  2 files changed, 7 insertions(+)
>
> diff --git a/include/net/netdev_netlink.h b/include/net/netdev_netlink.h
> index 1599573d35c9..075962dbe743 100644
> --- a/include/net/netdev_netlink.h
> +++ b/include/net/netdev_netlink.h
> @@ -5,6 +5,7 @@
>  #include <linux/list.h>
>
>  struct netdev_nl_sock {
> +       struct mutex lock;
>         struct list_head bindings;
>  };
>
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index a219be90c739..63e10717efc5 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -859,6 +859,7 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
>                 goto err_genlmsg_free;
>         }
>
> +       mutex_lock(&priv->lock);

You do not need to acquire this lock so early, no? AFAICT you only
need to lock around:

list_add(&binding->list, sock_binding_list);

Or is this to establish a locking order (sock_binding_list lock before
the netdev lock)?

-- 
Thanks,
Mina

Re: [PATCH net-next v2 3/3] net: drop rtnl_lock for queue_mgmt operations

[Mina Almasry]

On Tue, Mar 11, 2025 at 7:40 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> All drivers that use queue API are already converted to use
> netdev instance lock. Move netdev instance lock management to
> the netlink layer and drop rtnl_lock.
>
> Cc: Mina Almasry <almasrymina@google.com>
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>

Reviewed-by: Mina Almasry. <almasrymina@google.com>

-- 
Thanks,
Mina

Re: [PATCH net-next v2 2/3] net: add granular lock for the netdev netlink socket

[Stanislav Fomichev]

On 03/11, Mina Almasry wrote:
> On Tue, Mar 11, 2025 at 7:40 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > As we move away from rtnl_lock for queue ops, introduce
> > per-netdev_nl_sock lock.
> >
> > Cc: Mina Almasry <almasrymina@google.com>
> > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> > ---
> >  include/net/netdev_netlink.h | 1 +
> >  net/core/netdev-genl.c       | 6 ++++++
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/include/net/netdev_netlink.h b/include/net/netdev_netlink.h
> > index 1599573d35c9..075962dbe743 100644
> > --- a/include/net/netdev_netlink.h
> > +++ b/include/net/netdev_netlink.h
> > @@ -5,6 +5,7 @@
> >  #include <linux/list.h>
> >
> >  struct netdev_nl_sock {
> > +       struct mutex lock;
> >         struct list_head bindings;
> >  };
> >
> > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > index a219be90c739..63e10717efc5 100644
> > --- a/net/core/netdev-genl.c
> > +++ b/net/core/netdev-genl.c
> > @@ -859,6 +859,7 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
> >                 goto err_genlmsg_free;
> >         }
> >
> > +       mutex_lock(&priv->lock);
> 
> You do not need to acquire this lock so early, no? AFAICT you only
> need to lock around:
> 
> list_add(&binding->list, sock_binding_list);
> 
> Or is this to establish a locking order (sock_binding_list lock before
> the netdev lock)?

Right, if I acquire it later, I'd have to do the same order
in netdev_nl_sock_priv_destroy and it seems to be a bit more complicated
to do (since we go over the list of bindings over there).

Re: [PATCH net-next v2 2/3] net: add granular lock for the netdev netlink socket

[Mina Almasry]

On Tue, Mar 11, 2025 at 1:03 PM Stanislav Fomichev <stfomichev@gmail.com> wrote:
>
> On 03/11, Mina Almasry wrote:
> > On Tue, Mar 11, 2025 at 7:40 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > >
> > > As we move away from rtnl_lock for queue ops, introduce
> > > per-netdev_nl_sock lock.
> > >
> > > Cc: Mina Almasry <almasrymina@google.com>
> > > Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> > > ---
> > >  include/net/netdev_netlink.h | 1 +
> > >  net/core/netdev-genl.c       | 6 ++++++
> > >  2 files changed, 7 insertions(+)
> > >
> > > diff --git a/include/net/netdev_netlink.h b/include/net/netdev_netlink.h
> > > index 1599573d35c9..075962dbe743 100644
> > > --- a/include/net/netdev_netlink.h
> > > +++ b/include/net/netdev_netlink.h
> > > @@ -5,6 +5,7 @@
> > >  #include <linux/list.h>
> > >
> > >  struct netdev_nl_sock {
> > > +       struct mutex lock;
> > >         struct list_head bindings;
> > >  };
> > >
> > > diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> > > index a219be90c739..63e10717efc5 100644
> > > --- a/net/core/netdev-genl.c
> > > +++ b/net/core/netdev-genl.c
> > > @@ -859,6 +859,7 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
> > >                 goto err_genlmsg_free;
> > >         }
> > >
> > > +       mutex_lock(&priv->lock);
> >
> > You do not need to acquire this lock so early, no? AFAICT you only
> > need to lock around:
> >
> > list_add(&binding->list, sock_binding_list);
> >
> > Or is this to establish a locking order (sock_binding_list lock before
> > the netdev lock)?
>
> Right, if I acquire it later, I'd have to do the same order
> in netdev_nl_sock_priv_destroy and it seems to be a bit more complicated
> to do (since we go over the list of bindings over there).

Thanks,

Reviewed-by: Mina Almasry <almasrymina@google.com>


-- 
Thanks,
Mina

Re: [PATCH net-next v2 0/3] net: remove rtnl_lock from the callers of queue APIs

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 11 Mar 2025 07:40:23 -0700 you wrote:
> All drivers that use queue management APIs already depend on the netdev
> lock. Ultimately, we want to have most of the paths that work with
> specific netdev to be rtnl_lock-free (ethtool mostly in particular).
> Queue API currently has a much smaller API surface, so start with
> rtnl_lock from it:
> 
> - add mutex to each dmabuf binding (to replace rtnl_lock)
> - move netdev lock management to the callers of netdev_rx_queue_restart
>   and drop rtnl_lock
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/3] net: create netdev_nl_sock to wrap bindings list
    https://git.kernel.org/netdev/net-next/c/b6b67141d6f1
  - [net-next,v2,2/3] net: add granular lock for the netdev netlink socket
    https://git.kernel.org/netdev/net-next/c/10eef096be25
  - [net-next,v2,3/3] net: drop rtnl_lock for queue_mgmt operations
    https://git.kernel.org/netdev/net-next/c/1d22d3060b9b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html