[PATCH net-next v1 0/4] net: devmem: support leased RX and TX queues on netdev unregister

[PATCH net-next v1 0/4] net: devmem: support leased RX and TX queues on netdev unregister

[Mina Almasry]

Support leased RX and TX devmem bindings when either the virtual device
(e.g. netkit) or the physical backing net_device is unregistered:

1. Pass struct net_device **dev_ptr to net_devmem_lock_dev() so callers
   can lock either binding->map_dev or binding->bind_dev.
2. Rename binding->dev and binding->vdev to binding->map_dev (physical
   DMA device) and binding->bind_dev (device passed to bind-rx/bind-tx).
3. Always populate both map_dev and bind_dev for direct and leased RX/TX
   bindings, and rely on dev_memory_provider_uninstall() and
   netif_rxq_cleanup_unlease() to uninstall RX queues and clear
   binding->bind_dev once bound_rxqs is empty.
4. Add kselftests in nl_netdev.py covering leased RX and TX devmem
   bindings across both virtual-first and physical-first netdev
   unregistration orders.

Mina Almasry (4):
  net: devmem: pass net_device pointer to net_devmem_lock_dev()
  net: devmem: rename binding dev and vdev to map_dev and bind_dev
  net: devmem: support leased queues on netdev unregister
  selftests: net: add leased devmem RX and TX netdev unregister tests

 net/core/dev.c                           |   2 +-
 net/core/devmem.c                        | 121 +++++++++++----------
 net/core/devmem.h                        |  25 +++--
 net/core/netdev-genl.c                   |  58 ++++++----
 net/core/netdev_rx_queue.c               |   5 +
 tools/testing/selftests/net/nl_netdev.py | 132 ++++++++++++++++++++++-
 6 files changed, 249 insertions(+), 94 deletions(-)


base-commit: 7b6ab4dbc156e2f96419715130b3d5738a306c93
-- 
2.56.0.385.gac671c819a-goog

[PATCH net-next v1 1/4] net: devmem: pass net_device pointer to net_devmem_lock_dev()

[Mina Almasry]

Upcoming changes for leased queue support will track the physical DMA
device and the bound device in separate net_device pointers on struct
net_devmem_dmabuf_binding, with different teardown paths locking
different pointers.

Generalize net_devmem_lock_dev() to take a struct net_device **dev_ptr
instead of struct net_devmem_dmabuf_binding *. No functional change.

Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 net/core/devmem.c      | 36 ++++++++++++++++++------------------
 net/core/devmem.h      |  4 ++--
 net/core/netdev-genl.c |  2 +-
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 5c7d9640e9d14..f164c90e90b3b 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -85,39 +85,39 @@ net_devmem_dmabuf_binding_unmap(struct net_devmem_dmabuf_binding *binding)
 }
 
 /**
- * net_devmem_lock_dev() - lock a binding's net_device if still alive
- * @binding:		devmem binding whose @dev pointer to lock
+ * net_devmem_lock_dev() - lock a binding's net_device pointer if still alive
+ * @dev_ptr:		pointer to the net_device field on a devmem binding
  * @dev_tracker:	reftracker for the held net_device reference
  *
- * Safely dereference @binding->dev, acquire netdev_lock() on the device, and
- * verify that @binding->dev has not been cleared while waiting for the lock.
+ * Safely dereference @dev_ptr, acquire netdev_lock() on the device, and
+ * verify that *@dev_ptr has not been cleared while waiting for the lock.
  *
- * Invariant: @binding->dev is set once at bind time and is only ever cleared to
- * NULL while holding netdev_lock(dev) (in mp_dmabuf_devmem_uninstall(),
- * net_devmem_unbind_dmabuf(), or net_devmem_dev_unregister() during
- * NETDEV_UNREGISTER before synchronize_net() and netdev_run_todo() free the
- * net_device).
+ * Invariant: *@dev_ptr is set once at bind time and is only ever cleared to
+ * NULL while holding netdev_lock() on that device (in
+ * mp_dmabuf_devmem_uninstall(), net_devmem_unbind_dmabuf(), or
+ * net_devmem_dev_unregister() during NETDEV_UNREGISTER before
+ * synchronize_net() and netdev_run_todo() free the net_device).
  *
- * Taking rcu_read_lock() guarantees that a non-NULL @dev read from
- * @binding->dev has not yet completed unregister_netdevice_many_notify()
+ * Taking rcu_read_lock() guarantees that a non-NULL device read from
+ * *@dev_ptr has not yet completed unregister_netdevice_many_notify()
  * (which waits for an RCU grace period via synchronize_net() after
  * NETDEV_UNREGISTER before freeing the net_device), so netdev_hold() can
  * safely acquire a reference before sleeping on netdev_lock(). Once
- * netdev_lock(dev) is held, re-checking that READ_ONCE(binding->dev) == dev
- * guarantees that the device has not yet been detached from @binding and
+ * netdev_lock() is held, re-checking that READ_ONCE(*dev_ptr) == dev
+ * guarantees that the device has not yet been detached from the binding and
  * cannot finish unregistering until net_devmem_unlock_dev().
  *
- * Return: The locked and held net_device if @binding->dev is still non-NULL,
+ * Return: The locked and held net_device if *@dev_ptr is still non-NULL,
  * or NULL if already cleared.
  */
 struct net_device *
-net_devmem_lock_dev(struct net_devmem_dmabuf_binding *binding,
+net_devmem_lock_dev(struct net_device **dev_ptr,
 		    netdevice_tracker *dev_tracker)
 {
 	struct net_device *dev;
 
 	rcu_read_lock();
-	dev = READ_ONCE(binding->dev);
+	dev = READ_ONCE(*dev_ptr);
 	if (!dev) {
 		rcu_read_unlock();
 		return NULL;
@@ -126,7 +126,7 @@ net_devmem_lock_dev(struct net_devmem_dmabuf_binding *binding,
 	rcu_read_unlock();
 
 	netdev_lock(dev);
-	if (READ_ONCE(binding->dev) != dev) {
+	if (READ_ONCE(*dev_ptr) != dev) {
 		netdev_unlock(dev);
 		netdev_put(dev, dev_tracker);
 		return NULL;
@@ -157,7 +157,7 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 		     binding->area.num_niovs, binding->free_count);
 
 	kvfree(binding->area.niovs);
-	dev = net_devmem_lock_dev(binding, &dev_tracker);
+	dev = net_devmem_lock_dev(&binding->dev, &dev_tracker);
 	net_devmem_dmabuf_binding_unmap(binding);
 	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
 	net_devmem_unlock_dev(dev, &dev_tracker);
diff --git a/net/core/devmem.h b/net/core/devmem.h
index c4f476947dfaa..c0aaba0041ff6 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -100,7 +100,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		       struct netlink_ext_ack *extack);
 struct net_devmem_dmabuf_binding *net_devmem_lookup_dmabuf(u32 id);
 struct net_device *
-net_devmem_lock_dev(struct net_devmem_dmabuf_binding *binding,
+net_devmem_lock_dev(struct net_device **dev_ptr,
 		    netdevice_tracker *dev_tracker);
 void net_devmem_unlock_dev(struct net_device *dev,
 			   netdevice_tracker *dev_tracker);
@@ -189,7 +189,7 @@ static inline struct net_devmem_dmabuf_binding *net_devmem_lookup_dmabuf(u32 id)
 }
 
 static inline struct net_device *
-net_devmem_lock_dev(struct net_devmem_dmabuf_binding *binding,
+net_devmem_lock_dev(struct net_device **dev_ptr,
 		    netdevice_tracker *dev_tracker)
 {
 	return NULL;
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 9c5bf17b04e57..2826f7e328340 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -1476,7 +1476,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) {
-		dev = net_devmem_lock_dev(binding, &dev_tracker);
+		dev = net_devmem_lock_dev(&binding->dev, &dev_tracker);
 		net_devmem_unbind_dmabuf(binding);
 		net_devmem_unlock_dev(dev, &dev_tracker);
 	}
-- 
2.56.0.385.gac671c819a-goog

[PATCH net-next v1 2/4] net: devmem: rename binding dev and vdev to map_dev and bind_dev

[Mina Almasry]

struct net_devmem_dmabuf_binding currently tracks two device pointers
under the names 'dev' and 'vdev':

- 'dev' is the physical net_device whose dma_dev holds the DMA mapping.
- 'vdev' is the virtual net_device the user bound to when using leased
  queues.

Rename 'dev' to 'map_dev' and 'vdev' to 'bind_dev' (typed as struct
net_device *), and rename the matching local variable in
netdev_nl_bind_tx_doit() from 'bind_dev' to 'map_dev' to match. No
functional change.

Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 net/core/dev.c         |  2 +-
 net/core/devmem.c      | 38 ++++++++++++++++++++------------------
 net/core/devmem.h      | 17 +++++++++--------
 net/core/netdev-genl.c | 31 +++++++++++++++----------------
 4 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1aece2fe2429f..e61ab67fb7e17 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4186,7 +4186,7 @@ static struct sk_buff *validate_xmit_unreadable_skb(struct sk_buff *skb,
 		niov = netmem_to_net_iov(skb_frag_netmem(&shinfo->frags[0]));
 		if (net_is_devmem_iov(niov) &&
 		    (net_devmem_iov_binding(niov)->direction != DMA_TO_DEVICE ||
-		     READ_ONCE(net_devmem_iov_binding(niov)->dev) != dev))
+		     READ_ONCE(net_devmem_iov_binding(niov)->map_dev) != dev))
 			goto out_free;
 	}
 
diff --git a/net/core/devmem.c b/net/core/devmem.c
index f164c90e90b3b..6446355e787d4 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -157,7 +157,7 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 		     binding->area.num_niovs, binding->free_count);
 
 	kvfree(binding->area.niovs);
-	dev = net_devmem_lock_dev(&binding->dev, &dev_tracker);
+	dev = net_devmem_lock_dev(&binding->map_dev, &dev_tracker);
 	net_devmem_dmabuf_binding_unmap(binding);
 	xa_erase(&net_devmem_dmabuf_bindings, binding->id);
 	net_devmem_unlock_dev(dev, &dev_tracker);
@@ -227,13 +227,14 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
 
 		rxq_idx = get_netdev_rx_queue_index(rxq);
 
-		if (netif_mp_close_rxq(binding->dev, rxq_idx, &mp_params))
+		if (netif_mp_close_rxq(binding->map_dev, rxq_idx, &mp_params))
 			busy = true;
 	}
 
-	if (binding->direction == DMA_FROM_DEVICE && binding->dev && !busy) {
+	if (binding->direction == DMA_FROM_DEVICE && binding->map_dev &&
+	    !busy) {
 		net_devmem_dmabuf_binding_unmap(binding);
-		WRITE_ONCE(binding->dev, NULL);
+		WRITE_ONCE(binding->map_dev, NULL);
 	}
 
 	percpu_ref_kill(&binding->ref);
@@ -252,16 +253,16 @@ void net_devmem_dev_unregister(struct net_device *dev)
 	xa_lock(&net_devmem_dmabuf_bindings);
 	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
 		if (binding->direction == DMA_FROM_DEVICE) {
-			if (READ_ONCE(binding->dev) != dev ||
+			if (READ_ONCE(binding->map_dev) != dev ||
 			    xa_get_mark(&net_devmem_dmabuf_bindings, xa_idx,
 					NET_DEVMEM_BINDING_LIVE))
 				continue;
 		} else {
-			if (READ_ONCE(binding->vdev) == dev)
-				WRITE_ONCE(binding->vdev, NULL);
-			if (READ_ONCE(binding->dev) != dev)
+			if (READ_ONCE(binding->bind_dev) == dev)
+				WRITE_ONCE(binding->bind_dev, NULL);
+			if (READ_ONCE(binding->map_dev) != dev)
 				continue;
-			WRITE_ONCE(binding->vdev, NULL);
+			WRITE_ONCE(binding->bind_dev, NULL);
 			__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
 					NET_DEVMEM_BINDING_LIVE);
 		}
@@ -270,7 +271,7 @@ void net_devmem_dev_unregister(struct net_device *dev)
 		if (!netif_is_queue_leasee(dev))
 			net_devmem_dmabuf_binding_unmap(binding);
 		xa_lock(&net_devmem_dmabuf_bindings);
-		WRITE_ONCE(binding->dev, NULL);
+		WRITE_ONCE(binding->map_dev, NULL);
 	}
 	xa_unlock(&net_devmem_dmabuf_bindings);
 }
@@ -304,7 +305,8 @@ int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 }
 
 struct net_devmem_dmabuf_binding *
-net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
+net_devmem_bind_dmabuf(struct net_device *map_dev,
+		       struct net_device *bind_dev,
 		       struct device *dma_dev,
 		       enum dma_data_direction direction,
 		       unsigned int dmabuf_fd, unsigned int niov_shift,
@@ -332,14 +334,14 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		return ERR_CAST(dmabuf);
 
 	binding = kzalloc_node(sizeof(*binding), GFP_KERNEL,
-			       dev_to_node(&dev->dev));
+			       dev_to_node(&map_dev->dev));
 	if (!binding) {
 		err = -ENOMEM;
 		goto err_put_dmabuf;
 	}
 
-	binding->dev = dev;
-	binding->vdev = vdev;
+	binding->map_dev = map_dev;
+	binding->bind_dev = bind_dev;
 	write_pnet(&binding->net, sock_net(nl_sk));
 	binding->uid = sk_uid(nl_sk);
 	binding->niov_shift = niov_shift;
@@ -559,8 +561,8 @@ struct net_devmem_dmabuf_binding *net_devmem_get_binding(struct sock *sk,
 	 */
 	dst_dev = dst_dev_rcu(dst);
 	if (unlikely(!dst_dev) ||
-	    unlikely(dst_dev != READ_ONCE(binding->dev) &&
-		     dst_dev != READ_ONCE(binding->vdev))) {
+	    unlikely(dst_dev != READ_ONCE(binding->map_dev) &&
+		     dst_dev != READ_ONCE(binding->bind_dev))) {
 		err = -ENODEV;
 		goto out_unlock;
 	}
@@ -684,8 +686,8 @@ static void mp_dmabuf_devmem_uninstall(void *mp_priv,
 		if (xa_empty(&binding->bound_rxqs)) {
 			if (!rxq->lease)
 				net_devmem_dmabuf_binding_unmap(binding);
-			ASSERT_EXCLUSIVE_WRITER(binding->dev);
-			WRITE_ONCE(binding->dev, NULL);
+			ASSERT_EXCLUSIVE_WRITER(binding->map_dev);
+			WRITE_ONCE(binding->map_dev, NULL);
 		}
 		break;
 	}
diff --git a/net/core/devmem.h b/net/core/devmem.h
index c0aaba0041ff6..9dcb629e10bfd 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -22,14 +22,13 @@ struct net_devmem_dmabuf_binding {
 	struct dma_buf_attachment *attachment;
 	struct sg_table *sgt;
 	/* Physical NIC that does the actual DMA for this binding. Set at bind
-	 * time and cleared to NULL under netdev_lock(dev).
+	 * time and cleared to NULL under netdev_lock(map_dev).
 	 */
-	struct net_device *dev;
-	/* Opaque cookie identifying the virtual device (e.g. netkit) the user
-	 * called bind-tx on. Used only for pointer comparison via READ_ONCE()
-	 * and cleared with WRITE_ONCE() on unregistration. Never dereferenced.
+	struct net_device *map_dev;
+	/* Device the user called bind-rx or bind-tx on. Set at bind time and
+	 * cleared to NULL on unregistration.
 	 */
-	void *vdev;
+	struct net_device *bind_dev;
 	possible_net_t net;
 	kuid_t uid;
 	/* Serialize net_devmem_dmabuf_binding_unmap() on attachment and sgt */
@@ -91,7 +90,8 @@ struct net_devmem_dmabuf_binding {
 #if defined(CONFIG_NET_DEVMEM)
 void __net_devmem_dmabuf_binding_free(struct work_struct *wq);
 struct net_devmem_dmabuf_binding *
-net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
+net_devmem_bind_dmabuf(struct net_device *map_dev,
+		       struct net_device *bind_dev,
 		       struct device *dma_dev,
 		       enum dma_data_direction direction,
 		       unsigned int dmabuf_fd, unsigned int niov_shift,
@@ -171,7 +171,8 @@ static inline void net_devmem_net_iov_dec_ref(struct net_iov *niov)
 }
 
 static inline struct net_devmem_dmabuf_binding *
-net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
+net_devmem_bind_dmabuf(struct net_device *map_dev,
+		       struct net_device *bind_dev,
 		       struct device *dma_dev,
 		       enum dma_data_direction direction,
 		       unsigned int dmabuf_fd,
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 2826f7e328340..962d7a8c75e9a 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -1195,9 +1195,8 @@ netdev_find_netmem_tx_dev(struct net_device *dev)
 int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct net_devmem_dmabuf_binding *binding;
-	struct net_device *bind_dev;
+	struct net_device *netdev, *map_dev;
 	struct netdev_nl_sock *priv;
-	struct net_device *netdev;
 	struct device *dma_dev;
 	u32 ifindex, dmabuf_fd;
 	struct sk_buff *rsp;
@@ -1245,27 +1244,27 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
 		goto err_unlock_netdev;
 	}
 
-	bind_dev = netdev_find_netmem_tx_dev(netdev);
-	if (!bind_dev) {
+	map_dev = netdev_find_netmem_tx_dev(netdev);
+	if (!map_dev) {
 		err = -EOPNOTSUPP;
 		NL_SET_ERR_MSG(info->extack,
 			       "No DMA-capable device found for netmem TX");
 		goto err_unlock_netdev;
 	}
 
-	if (bind_dev != netdev)
-		netdev_lock(bind_dev);
+	if (map_dev != netdev)
+		netdev_lock(map_dev);
 
-	dma_dev = netdev_queue_get_dma_dev(bind_dev, 0, NETDEV_QUEUE_TYPE_TX);
+	dma_dev = netdev_queue_get_dma_dev(map_dev, 0, NETDEV_QUEUE_TYPE_TX);
 
-	binding = net_devmem_bind_dmabuf(bind_dev,
-					 bind_dev != netdev ? netdev : NULL,
+	binding = net_devmem_bind_dmabuf(map_dev,
+					 map_dev != netdev ? netdev : NULL,
 					 dma_dev, DMA_TO_DEVICE, dmabuf_fd,
 					 PAGE_SHIFT, priv, NETLINK_CB(skb).sk,
 					 info->extack);
 	if (IS_ERR(binding)) {
 		err = PTR_ERR(binding);
-		goto err_unlock_bind_dev;
+		goto err_unlock_map_dev;
 	}
 
 	/* rsp was allocated large enough */
@@ -1273,16 +1272,16 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
 
 	genlmsg_end(rsp, hdr);
 
-	if (bind_dev != netdev)
-		netdev_unlock(bind_dev);
+	if (map_dev != netdev)
+		netdev_unlock(map_dev);
 	netdev_unlock(netdev);
 	mutex_unlock(&priv->lock);
 
 	return genlmsg_reply(rsp, info);
 
-err_unlock_bind_dev:
-	if (bind_dev != netdev)
-		netdev_unlock(bind_dev);
+err_unlock_map_dev:
+	if (map_dev != netdev)
+		netdev_unlock(map_dev);
 err_unlock_netdev:
 	netdev_unlock(netdev);
 err_unlock_sock:
@@ -1476,7 +1475,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) {
-		dev = net_devmem_lock_dev(&binding->dev, &dev_tracker);
+		dev = net_devmem_lock_dev(&binding->map_dev, &dev_tracker);
 		net_devmem_unbind_dmabuf(binding);
 		net_devmem_unlock_dev(dev, &dev_tracker);
 	}
-- 
2.56.0.385.gac671c819a-goog

[PATCH net-next v1 3/4] net: devmem: support leased queues on netdev unregister

[Mina Almasry]

Previously, bind_dev was only populated when bind-tx was called on a
virtual netdev (where map_dev != netdev), and leased RX bindings stored
the virtual netdev in map_dev without tracking the physical backing
netdev at all. Unregistering either the virtual device or the physical
backing device while a leased RX or TX devmem binding was active failed
to detach the dma-buf cleanly.

Always populate both device pointers on struct
net_devmem_dmabuf_binding:
- map_dev: the physical net_device whose dma_dev holds the DMA mapping
  (resolved from leased RX queues in netdev_nl_get_dma_dev(), with its
  netdev_lock held across net_devmem_bind_dmabuf()).
- bind_dev: the net_device the user called bind-rx or bind-tx on (equal
  to map_dev for direct bindings, or the virtual device for leased
  bindings).

With both pointers tracked on all bindings, handle socket teardown,
queue uninstall, and netdev unregistration in either order (bind_dev
first or map_dev first):
- Lock &binding->bind_dev during socket teardown in
  netdev_nl_sock_priv_destroy(), close and erase bound_rxqs via
  binding->bind_dev in net_devmem_unbind_dmabuf(), and require
  binding->map_dev to remain non-NULL in net_devmem_get_binding().
- Clear binding->bind_dev in mp_dmabuf_devmem_uninstall() once
  bound_rxqs is empty, leaving unleased bindings mapped until the
  binding is freed or map_dev is unregistered.
- In net_devmem_dev_unregister(dev), if binding->bind_dev == dev, clear
  NET_DEVMEM_BINDING_LIVE and clear binding->bind_dev if bound_rxqs is
  empty or dev is not a queue leasee; if binding->map_dev == dev, clear
  NET_DEVMEM_BINDING_LIVE, unmap the dmabuf, and clear binding->map_dev.
- Reject new bindings and RX queue opens in net_devmem_bind_dmabuf() and
  __netif_mp_open_rxq() if bind_dev or map_dev is no longer
  NETREG_REGISTERED.

Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 net/core/devmem.c          | 85 +++++++++++++++++++++-----------------
 net/core/devmem.h          |  8 ++--
 net/core/netdev-genl.c     | 33 ++++++++++-----
 net/core/netdev_rx_queue.c |  5 +++
 4 files changed, 79 insertions(+), 52 deletions(-)

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 6446355e787d4..1d6b89f196e02 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -86,7 +86,7 @@ net_devmem_dmabuf_binding_unmap(struct net_devmem_dmabuf_binding *binding)
 
 /**
  * net_devmem_lock_dev() - lock a binding's net_device pointer if still alive
- * @dev_ptr:		pointer to the net_device field on a devmem binding
+ * @dev_ptr:		pointer to &binding->map_dev or &binding->bind_dev
  * @dev_tracker:	reftracker for the held net_device reference
  *
  * Safely dereference @dev_ptr, acquire netdev_lock() on the device, and
@@ -95,17 +95,18 @@ net_devmem_dmabuf_binding_unmap(struct net_devmem_dmabuf_binding *binding)
  * Invariant: *@dev_ptr is set once at bind time and is only ever cleared to
  * NULL while holding netdev_lock() on that device (in
  * mp_dmabuf_devmem_uninstall(), net_devmem_unbind_dmabuf(), or
- * net_devmem_dev_unregister() during NETDEV_UNREGISTER before
- * synchronize_net() and netdev_run_todo() free the net_device).
+ * net_devmem_dev_unregister(), all before synchronize_net() and
+ * netdev_run_todo() free the net_device).
  *
  * Taking rcu_read_lock() guarantees that a non-NULL device read from
  * *@dev_ptr has not yet completed unregister_netdevice_many_notify()
  * (which waits for an RCU grace period via synchronize_net() after
- * NETDEV_UNREGISTER before freeing the net_device), so netdev_hold() can
- * safely acquire a reference before sleeping on netdev_lock(). Once
- * netdev_lock() is held, re-checking that READ_ONCE(*dev_ptr) == dev
- * guarantees that the device has not yet been detached from the binding and
- * cannot finish unregistering until net_devmem_unlock_dev().
+ * NETDEV_UNREGISTER and ndo_uninit before freeing the net_device), so
+ * netdev_hold() can safely acquire a reference before sleeping on
+ * netdev_lock(). Once netdev_lock() is held, re-checking that
+ * READ_ONCE(*dev_ptr) == dev guarantees that the device has not yet been
+ * detached from the binding and cannot finish unregistering until
+ * net_devmem_unlock_dev().
  *
  * Return: The locked and held net_device if *@dev_ptr is still non-NULL,
  * or NULL if already cleared.
@@ -219,22 +220,24 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
 	if (binding->list.next)
 		list_del(&binding->list);
 
-	xa_for_each(&binding->bound_rxqs, xa_idx, rxq) {
-		const struct pp_memory_provider_params mp_params = {
-			.mp_priv	= binding,
-			.mp_ops		= &dmabuf_devmem_ops,
-		};
+	if (binding->bind_dev) {
+		xa_for_each(&binding->bound_rxqs, xa_idx, rxq) {
+			const struct pp_memory_provider_params mp_params = {
+				.mp_priv	= binding,
+				.mp_ops		= &dmabuf_devmem_ops,
+			};
 
-		rxq_idx = get_netdev_rx_queue_index(rxq);
+			xa_erase(&binding->bound_rxqs, xa_idx);
+			rxq_idx = get_netdev_rx_queue_index(rxq);
 
-		if (netif_mp_close_rxq(binding->map_dev, rxq_idx, &mp_params))
-			busy = true;
-	}
+			if (netif_mp_close_rxq(binding->bind_dev, rxq_idx,
+					       &mp_params))
+				busy = true;
+		}
 
-	if (binding->direction == DMA_FROM_DEVICE && binding->map_dev &&
-	    !busy) {
-		net_devmem_dmabuf_binding_unmap(binding);
-		WRITE_ONCE(binding->map_dev, NULL);
+		if (binding->direction == DMA_FROM_DEVICE && !busy)
+			net_devmem_dmabuf_binding_unmap(binding);
+		WRITE_ONCE(binding->bind_dev, NULL);
 	}
 
 	percpu_ref_kill(&binding->ref);
@@ -252,24 +255,20 @@ void net_devmem_dev_unregister(struct net_device *dev)
 
 	xa_lock(&net_devmem_dmabuf_bindings);
 	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
-		if (binding->direction == DMA_FROM_DEVICE) {
-			if (READ_ONCE(binding->map_dev) != dev ||
-			    xa_get_mark(&net_devmem_dmabuf_bindings, xa_idx,
-					NET_DEVMEM_BINDING_LIVE))
-				continue;
-		} else {
-			if (READ_ONCE(binding->bind_dev) == dev)
-				WRITE_ONCE(binding->bind_dev, NULL);
-			if (READ_ONCE(binding->map_dev) != dev)
-				continue;
-			WRITE_ONCE(binding->bind_dev, NULL);
+		if (READ_ONCE(binding->bind_dev) == dev) {
 			__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
 					NET_DEVMEM_BINDING_LIVE);
+			if (xa_empty(&binding->bound_rxqs) ||
+			    !netif_is_queue_leasee(dev))
+				WRITE_ONCE(binding->bind_dev, NULL);
 		}
+		if (READ_ONCE(binding->map_dev) != dev)
+			continue;
 
+		__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
+				NET_DEVMEM_BINDING_LIVE);
 		xa_unlock(&net_devmem_dmabuf_bindings);
-		if (!netif_is_queue_leasee(dev))
-			net_devmem_dmabuf_binding_unmap(binding);
+		net_devmem_dmabuf_binding_unmap(binding);
 		xa_lock(&net_devmem_dmabuf_bindings);
 		WRITE_ONCE(binding->map_dev, NULL);
 	}
@@ -324,6 +323,15 @@ net_devmem_bind_dmabuf(struct net_device *map_dev,
 	size_t i;
 	long err;
 
+	netdev_assert_locked(bind_dev);
+	netdev_assert_locked(map_dev);
+
+	if (bind_dev->reg_state != NETREG_REGISTERED ||
+	    map_dev->reg_state != NETREG_REGISTERED) {
+		NL_SET_ERR_MSG(extack, "Device is being unregistered");
+		return ERR_PTR(-ENODEV);
+	}
+
 	if (!dma_dev) {
 		NL_SET_ERR_MSG(extack, "Device doesn't support DMA");
 		return ERR_PTR(-EOPNOTSUPP);
@@ -560,9 +568,9 @@ struct net_devmem_dmabuf_binding *net_devmem_get_binding(struct sock *sk,
 	 * net_device.
 	 */
 	dst_dev = dst_dev_rcu(dst);
-	if (unlikely(!dst_dev) ||
-	    unlikely(dst_dev != READ_ONCE(binding->map_dev) &&
-		     dst_dev != READ_ONCE(binding->bind_dev))) {
+	if (unlikely(!dst_dev || !READ_ONCE(binding->map_dev) ||
+		     (dst_dev != READ_ONCE(binding->map_dev) &&
+		      dst_dev != READ_ONCE(binding->bind_dev)))) {
 		err = -ENODEV;
 		goto out_unlock;
 	}
@@ -686,8 +694,7 @@ static void mp_dmabuf_devmem_uninstall(void *mp_priv,
 		if (xa_empty(&binding->bound_rxqs)) {
 			if (!rxq->lease)
 				net_devmem_dmabuf_binding_unmap(binding);
-			ASSERT_EXCLUSIVE_WRITER(binding->map_dev);
-			WRITE_ONCE(binding->map_dev, NULL);
+			WRITE_ONCE(binding->bind_dev, NULL);
 		}
 		break;
 	}
diff --git a/net/core/devmem.h b/net/core/devmem.h
index 9dcb629e10bfd..ff431e5e398e0 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -25,8 +25,10 @@ struct net_devmem_dmabuf_binding {
 	 * time and cleared to NULL under netdev_lock(map_dev).
 	 */
 	struct net_device *map_dev;
-	/* Device the user called bind-rx or bind-tx on. Set at bind time and
-	 * cleared to NULL on unregistration.
+	/* Device the user called bind-rx or bind-tx on. Equal to map_dev for
+	 * direct bindings, or the virtual device (e.g. netkit) for leased
+	 * bindings. Set at bind time and cleared to NULL under
+	 * netdev_lock(bind_dev).
 	 */
 	struct net_device *bind_dev;
 	possible_net_t net;
@@ -44,7 +46,7 @@ struct net_devmem_dmabuf_binding {
 	 * those refs are dropped and the binding is no longer desired or in
 	 * use. RX bindings are unmapped earlier when their queues are closed or
 	 * uninstalled, unless a leased or busy queue may still use the binding.
-	 * TX bindings are also unmapped earlier if their device is
+	 * Both RX and TX bindings are also unmapped earlier if map_dev is
 	 * unregistered.
 	 *
 	 * net_devmem_net_iov_inc_ref() on dmabuf net_iovs will increment this
diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
index 962d7a8c75e9a..442362f23fe88 100644
--- a/net/core/netdev-genl.c
+++ b/net/core/netdev-genl.c
@@ -1005,12 +1005,15 @@ static int netdev_nl_read_rxq_bitmap(struct genl_info *info,
 
 static struct device *
 netdev_nl_get_dma_dev(struct net_device *netdev, unsigned long *rxq_bitmap,
+		      struct net_device **map_dev,
 		      struct netlink_ext_ack *extack)
 {
 	struct device *dma_dev = NULL;
 	u32 rxq_idx, prev_rxq_idx;
 
+	*map_dev = netdev;
 	for_each_set_bit(rxq_idx, rxq_bitmap, netdev->real_num_rx_queues) {
+		struct netdev_rx_queue *rxq;
 		struct device *rxq_dma_dev;
 
 		rxq_dma_dev = netdev_queue_get_dma_dev(netdev, rxq_idx,
@@ -1023,6 +1026,11 @@ netdev_nl_get_dma_dev(struct net_device *netdev, unsigned long *rxq_bitmap,
 
 		dma_dev = rxq_dma_dev;
 		prev_rxq_idx = rxq_idx;
+		if (netif_rxq_is_leased(netdev, rxq_idx) &&
+		    netif_is_queue_leasee(netdev)) {
+			rxq = __netif_get_rx_queue(netdev, rxq_idx);
+			*map_dev = rxq->lease->dev;
+		}
 	}
 
 	return dma_dev;
@@ -1032,9 +1040,9 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct net_devmem_dmabuf_binding *binding;
 	unsigned int niov_shift = PAGE_SHIFT;
+	struct net_device *netdev, *map_dev;
 	u32 ifindex, dmabuf_fd, rxq_idx;
 	struct netdev_nl_sock *priv;
-	struct net_device *netdev;
 	unsigned long *rxq_bitmap;
 	struct device *dma_dev;
 	struct sk_buff *rsp;
@@ -1104,15 +1112,21 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		goto err_rxq_bitmap;
 
-	dma_dev = netdev_nl_get_dma_dev(netdev, rxq_bitmap, info->extack);
+	dma_dev = netdev_nl_get_dma_dev(netdev, rxq_bitmap, &map_dev,
+					info->extack);
 	if (IS_ERR(dma_dev)) {
 		err = PTR_ERR(dma_dev);
 		goto err_rxq_bitmap;
 	}
 
-	binding = net_devmem_bind_dmabuf(netdev, NULL, dma_dev, DMA_FROM_DEVICE,
-					 dmabuf_fd, niov_shift, priv,
-					 NETLINK_CB(skb).sk, info->extack);
+	if (map_dev != netdev)
+		netdev_lock(map_dev);
+	binding = net_devmem_bind_dmabuf(map_dev, netdev, dma_dev,
+					 DMA_FROM_DEVICE, dmabuf_fd,
+					 niov_shift, priv, NETLINK_CB(skb).sk,
+					 info->extack);
+	if (map_dev != netdev)
+		netdev_unlock(map_dev);
 	if (IS_ERR(binding)) {
 		err = PTR_ERR(binding);
 		goto err_rxq_bitmap;
@@ -1257,10 +1271,9 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
 
 	dma_dev = netdev_queue_get_dma_dev(map_dev, 0, NETDEV_QUEUE_TYPE_TX);
 
-	binding = net_devmem_bind_dmabuf(map_dev,
-					 map_dev != netdev ? netdev : NULL,
-					 dma_dev, DMA_TO_DEVICE, dmabuf_fd,
-					 PAGE_SHIFT, priv, NETLINK_CB(skb).sk,
+	binding = net_devmem_bind_dmabuf(map_dev, netdev, dma_dev,
+					 DMA_TO_DEVICE, dmabuf_fd, PAGE_SHIFT,
+					 priv, NETLINK_CB(skb).sk,
 					 info->extack);
 	if (IS_ERR(binding)) {
 		err = PTR_ERR(binding);
@@ -1475,7 +1488,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) {
-		dev = net_devmem_lock_dev(&binding->map_dev, &dev_tracker);
+		dev = net_devmem_lock_dev(&binding->bind_dev, &dev_tracker);
 		net_devmem_unbind_dmabuf(binding);
 		net_devmem_unlock_dev(dev, &dev_tracker);
 	}
diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
index a7261fedd5e76..5f65b9bbfc369 100644
--- a/net/core/netdev_rx_queue.c
+++ b/net/core/netdev_rx_queue.c
@@ -193,6 +193,11 @@ static int __netif_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
 	struct netdev_rx_queue *rxq;
 	int ret;
 
+	if (dev->reg_state != NETREG_REGISTERED) {
+		NL_SET_ERR_MSG(extack, "Device is being unregistered");
+		return -ENODEV;
+	}
+
 	if (!qops)
 		return -EOPNOTSUPP;
 
-- 
2.56.0.385.gac671c819a-goog

[PATCH net-next v1 4/4] selftests: net: add leased devmem RX and TX netdev unregister tests

[Mina Almasry]

Add devmem_lease_bind_rx_unregister_check and
devmem_lease_bind_tx_unregister_check to nl_netdev.py to verify that
leased RX and TX devmem bindings on a netkit interface backed by a
netdevsim queue detach their dma-buf cleanly while the netlink socket
remains open, both when the virtual device (bind_dev) is unregistered
before the physical device (map_dev) and when the physical device is
unregistered directly (which uninstalls the physical queue memory
provider and cascades unregistration to the leasing netkit).

Signed-off-by: Mina Almasry <almasrymina@google.com>
---
 tools/testing/selftests/net/nl_netdev.py | 132 ++++++++++++++++++++++-
 1 file changed, 129 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/nl_netdev.py b/tools/testing/selftests/net/nl_netdev.py
index dd0321c0b463a..fa4b5be093257 100755
--- a/tools/testing/selftests/net/nl_netdev.py
+++ b/tools/testing/selftests/net/nl_netdev.py
@@ -12,8 +12,9 @@ import os
 import struct
 from os import system
 from lib.py import ksft_run, ksft_exit, KsftFailEx, KsftSkipEx
-from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_raises, ksft_busy_wait
-from lib.py import EthtoolFamily, NetdevFamily, NetdevSimDev, NlError, defer, ip
+from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_not_in, ksft_raises, ksft_busy_wait
+from lib.py import EthtoolFamily, NetdevFamily, NetdevSimDev, Netlink
+from lib.py import NlError, RtnlFamily, cmd, defer, ip
 
 
 def empty_check(nf) -> None:
@@ -483,6 +484,129 @@ def devmem_bind_tx_unregister_check(_nf) -> None:
     nf_priv.close()
 
 
+def _create_netkit_single(rxqueues=2):
+    """Create a single-device netkit interface for queue lease tests."""
+    ifname = f"nk{os.getpid()}"
+    try:
+        with RtnlFamily() as rtnl:
+            rtnl.newlink(
+                {
+                    "ifname": ifname,
+                    "linkinfo": {
+                        "kind": "netkit",
+                        "data": {
+                            "mode": "l2",
+                            "pairing": "single",
+                        },
+                    },
+                    "num-rx-queues": rxqueues,
+                },
+                flags=[Netlink.NLM_F_CREATE, Netlink.NLM_F_EXCL],
+            )
+    except NlError as e:
+        if e.error in (errno.EOPNOTSUPP, errno.ENOENT):
+            raise KsftSkipEx(f"netkit single mode not supported: {e}") from e
+        raise
+    defer(cmd, f"ip link del dev {ifname}", fail=False)
+
+    link = ip(f"-d link show dev {ifname}", json=True)[0]
+    return ifname, link["ifindex"]
+
+
+def devmem_lease_bind_rx_unregister_check(nf) -> None:
+    """Verify leased RX devmem bindings detach cleanly on virt/phys unregister."""
+    for remove_virt_first in (False, True):
+        dmabuf_fd = _create_udmabuf()
+        defer(os.close, dmabuf_fd)
+        nf_priv = NetdevFamily()
+        defer(nf_priv.close)
+        nsimdev = NetdevSimDev(queue_count=2)
+        defer(nsimdev.remove)
+        nsim = nsimdev.nsims[0]
+
+        ip(f"link set dev {nsim.ifname} up")
+        with EthtoolFamily() as ef:
+            ef.rings_set({"header": {"dev-index": nsim.ifindex},
+                          "tcp-data-split": "enabled"})
+
+        nk_ifname, nk_ifindex = _create_netkit_single(rxqueues=2)
+        ip(f"link set dev {nk_ifname} up")
+
+        res = nf.queue_create({
+            "ifindex": nk_ifindex,
+            "type": "rx",
+            "lease": {
+                "ifindex": nsim.ifindex,
+                "queue": {"id": 1, "type": "rx"},
+            },
+        })
+
+        _devmem_bind(nf_priv.bind_rx, {
+            "ifindex": nk_ifindex,
+            "fd": dmabuf_fd,
+            "queues": [{"id": res["id"], "type": "rx"}],
+        })
+        ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
+
+        if remove_virt_first:
+            ip(f"link del dev {nk_ifname}")
+            phys_q = nf.queue_get({"ifindex": nsim.ifindex, "id": 1,
+                                   "type": "rx"})
+            ksft_not_in("dmabuf", phys_q)
+            ksft_not_in("lease", phys_q)
+            ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
+            nsimdev.remove()
+        else:
+            nsimdev.remove()
+            ksft_ne(cmd(f"ip link show dev {nk_ifname}", fail=False).ret, 0)
+        ksft_eq(_dmabuf_attach_count(dmabuf_fd), 0)
+
+        nf_priv.close()
+
+
+def devmem_lease_bind_tx_unregister_check(nf) -> None:
+    """Verify leased TX devmem bindings detach cleanly on virt/phys unregister."""
+    for remove_virt_first in (False, True):
+        dmabuf_fd = _create_udmabuf()
+        defer(os.close, dmabuf_fd)
+        nf_priv = NetdevFamily()
+        defer(nf_priv.close)
+        nsimdev = NetdevSimDev(queue_count=2)
+        defer(nsimdev.remove)
+        nsim = nsimdev.nsims[0]
+
+        ip(f"link set dev {nsim.ifname} up")
+
+        nk_ifname, nk_ifindex = _create_netkit_single(rxqueues=2)
+        ip(f"link set dev {nk_ifname} up")
+
+        nf.queue_create({
+            "ifindex": nk_ifindex,
+            "type": "rx",
+            "lease": {
+                "ifindex": nsim.ifindex,
+                "queue": {"id": 1, "type": "rx"},
+            },
+        })
+
+        _devmem_bind(nf_priv.bind_tx, {
+            "ifindex": nk_ifindex,
+            "fd": dmabuf_fd,
+        })
+        ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
+
+        if remove_virt_first:
+            ip(f"link del dev {nk_ifname}")
+            ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
+            nsimdev.remove()
+        else:
+            nsimdev.remove()
+            ksft_ne(cmd(f"ip link show dev {nk_ifname}", fail=False).ret, 0)
+        ksft_eq(_dmabuf_attach_count(dmabuf_fd), 0)
+
+        nf_priv.close()
+
+
 def main() -> None:
     """ Ksft boiler plate main """
     nf = NetdevFamily()
@@ -498,7 +622,9 @@ def main() -> None:
               page_pool_ifindex_leak_check,
               page_pool_stats_ifindex_check,
               devmem_bind_rx_unregister_check,
-              devmem_bind_tx_unregister_check
+              devmem_bind_tx_unregister_check,
+              devmem_lease_bind_rx_unregister_check,
+              devmem_lease_bind_tx_unregister_check
               ],
              args=(nf, ))
     ksft_exit()
-- 
2.56.0.385.gac671c819a-goog

Re: [PATCH net-next v1 3/4] net: devmem: support leased queues on netdev unregister

[Dragos Tatulea]

On 04.11.26 00:26, Mina Almasry wrote:
> Previously, bind_dev was only populated when bind-tx was called on a
> virtual netdev (where map_dev != netdev), and leased RX bindings stored
> the virtual netdev in map_dev without tracking the physical backing
> netdev at all. Unregistering either the virtual device or the physical
> backing device while a leased RX or TX devmem binding was active failed
> to detach the dma-buf cleanly.
> 
> Always populate both device pointers on struct
> net_devmem_dmabuf_binding:
> - map_dev: the physical net_device whose dma_dev holds the DMA mapping
>   (resolved from leased RX queues in netdev_nl_get_dma_dev(), with its
>   netdev_lock held across net_devmem_bind_dmabuf()).
> - bind_dev: the net_device the user called bind-rx or bind-tx on (equal
>   to map_dev for direct bindings, or the virtual device for leased
>   bindings).
> 
> With both pointers tracked on all bindings, handle socket teardown,
> queue uninstall, and netdev unregistration in either order (bind_dev
> first or map_dev first):
> - Lock &binding->bind_dev during socket teardown in
>   netdev_nl_sock_priv_destroy(), close and erase bound_rxqs via
>   binding->bind_dev in net_devmem_unbind_dmabuf(), and require
>   binding->map_dev to remain non-NULL in net_devmem_get_binding().
> - Clear binding->bind_dev in mp_dmabuf_devmem_uninstall() once
>   bound_rxqs is empty, leaving unleased bindings mapped until the
>   binding is freed or map_dev is unregistered.
Did you mean "leaving leased bindings"?

> - In net_devmem_dev_unregister(dev), if binding->bind_dev == dev, clear
>   NET_DEVMEM_BINDING_LIVE and clear binding->bind_dev if bound_rxqs is
>   empty or dev is not a queue leasee; if binding->map_dev == dev, clear
>   NET_DEVMEM_BINDING_LIVE, unmap the dmabuf, and clear binding->map_dev.
> - Reject new bindings and RX queue opens in net_devmem_bind_dmabuf() and
>   __netif_mp_open_rxq() if bind_dev or map_dev is no longer
>   NETREG_REGISTERED.
>
Why is it needed? This applies to all items in this list.
 
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> ---
>  net/core/devmem.c          | 85 +++++++++++++++++++++-----------------
>  net/core/devmem.h          |  8 ++--
>  net/core/netdev-genl.c     | 33 ++++++++++-----
>  net/core/netdev_rx_queue.c |  5 +++
>  4 files changed, 79 insertions(+), 52 deletions(-)
> 
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 6446355e787d4..1d6b89f196e02 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -86,7 +86,7 @@ net_devmem_dmabuf_binding_unmap(struct net_devmem_dmabuf_binding *binding)
>  
>  /**
>   * net_devmem_lock_dev() - lock a binding's net_device pointer if still alive
> - * @dev_ptr:		pointer to the net_device field on a devmem binding
> + * @dev_ptr:		pointer to &binding->map_dev or &binding->bind_dev
>   * @dev_tracker:	reftracker for the held net_device reference
>   *
>   * Safely dereference @dev_ptr, acquire netdev_lock() on the device, and
> @@ -95,17 +95,18 @@ net_devmem_dmabuf_binding_unmap(struct net_devmem_dmabuf_binding *binding)
>   * Invariant: *@dev_ptr is set once at bind time and is only ever cleared to
>   * NULL while holding netdev_lock() on that device (in
>   * mp_dmabuf_devmem_uninstall(), net_devmem_unbind_dmabuf(), or
> - * net_devmem_dev_unregister() during NETDEV_UNREGISTER before
> - * synchronize_net() and netdev_run_todo() free the net_device).
> + * net_devmem_dev_unregister(), all before synchronize_net() and
> + * netdev_run_todo() free the net_device).
>   *
>   * Taking rcu_read_lock() guarantees that a non-NULL device read from
>   * *@dev_ptr has not yet completed unregister_netdevice_many_notify()
>   * (which waits for an RCU grace period via synchronize_net() after
> - * NETDEV_UNREGISTER before freeing the net_device), so netdev_hold() can
> - * safely acquire a reference before sleeping on netdev_lock(). Once
> - * netdev_lock() is held, re-checking that READ_ONCE(*dev_ptr) == dev
> - * guarantees that the device has not yet been detached from the binding and
> - * cannot finish unregistering until net_devmem_unlock_dev().
> + * NETDEV_UNREGISTER and ndo_uninit before freeing the net_device), so
> + * netdev_hold() can safely acquire a reference before sleeping on
> + * netdev_lock(). Once netdev_lock() is held, re-checking that
> + * READ_ONCE(*dev_ptr) == dev guarantees that the device has not yet been
> + * detached from the binding and cannot finish unregistering until
> + * net_devmem_unlock_dev().
>   *
>   * Return: The locked and held net_device if *@dev_ptr is still non-NULL,
>   * or NULL if already cleared.
> @@ -219,22 +220,24 @@ void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
>  	if (binding->list.next)
>  		list_del(&binding->list);
>  
> -	xa_for_each(&binding->bound_rxqs, xa_idx, rxq) {
> -		const struct pp_memory_provider_params mp_params = {
> -			.mp_priv	= binding,
> -			.mp_ops		= &dmabuf_devmem_ops,
> -		};
> +	if (binding->bind_dev) {
> +		xa_for_each(&binding->bound_rxqs, xa_idx, rxq) {
> +			const struct pp_memory_provider_params mp_params = {
> +				.mp_priv	= binding,
> +				.mp_ops		= &dmabuf_devmem_ops,
> +			};
>  
> -		rxq_idx = get_netdev_rx_queue_index(rxq);
> +			xa_erase(&binding->bound_rxqs, xa_idx);
> +			rxq_idx = get_netdev_rx_queue_index(rxq);
>  
> -		if (netif_mp_close_rxq(binding->map_dev, rxq_idx, &mp_params))
> -			busy = true;
> -	}
> +			if (netif_mp_close_rxq(binding->bind_dev, rxq_idx,
> +					       &mp_params))
> +				busy = true;
> +		}
>  
> -	if (binding->direction == DMA_FROM_DEVICE && binding->map_dev &&
> -	    !busy) {
> -		net_devmem_dmabuf_binding_unmap(binding);
> -		WRITE_ONCE(binding->map_dev, NULL);
> +		if (binding->direction == DMA_FROM_DEVICE && !busy)
> +			net_devmem_dmabuf_binding_unmap(binding);
> +		WRITE_ONCE(binding->bind_dev, NULL);
>  	}
>  
>  	percpu_ref_kill(&binding->ref);
> @@ -252,24 +255,20 @@ void net_devmem_dev_unregister(struct net_device *dev)
>  
>  	xa_lock(&net_devmem_dmabuf_bindings);
>  	xa_for_each(&net_devmem_dmabuf_bindings, xa_idx, binding) {
> -		if (binding->direction == DMA_FROM_DEVICE) {
> -			if (READ_ONCE(binding->map_dev) != dev ||
> -			    xa_get_mark(&net_devmem_dmabuf_bindings, xa_idx,
> -					NET_DEVMEM_BINDING_LIVE))
> -				continue;
> -		} else {
> -			if (READ_ONCE(binding->bind_dev) == dev)
> -				WRITE_ONCE(binding->bind_dev, NULL);
> -			if (READ_ONCE(binding->map_dev) != dev)
> -				continue;
> -			WRITE_ONCE(binding->bind_dev, NULL);
> +		if (READ_ONCE(binding->bind_dev) == dev) {
>  			__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
>  					NET_DEVMEM_BINDING_LIVE);
> +			if (xa_empty(&binding->bound_rxqs) ||
> +			    !netif_is_queue_leasee(dev))
> +				WRITE_ONCE(binding->bind_dev, NULL);
>  		}
> +		if (READ_ONCE(binding->map_dev) != dev)
> +			continue;
>  
> +		__xa_clear_mark(&net_devmem_dmabuf_bindings, xa_idx,
> +				NET_DEVMEM_BINDING_LIVE);
>  		xa_unlock(&net_devmem_dmabuf_bindings);
> -		if (!netif_is_queue_leasee(dev))
> -			net_devmem_dmabuf_binding_unmap(binding);
> +		net_devmem_dmabuf_binding_unmap(binding);
>  		xa_lock(&net_devmem_dmabuf_bindings);
>  		WRITE_ONCE(binding->map_dev, NULL);
>  	}
> @@ -324,6 +323,15 @@ net_devmem_bind_dmabuf(struct net_device *map_dev,
>  	size_t i;
>  	long err;
>  
> +	netdev_assert_locked(bind_dev);
> +	netdev_assert_locked(map_dev);
> +
> +	if (bind_dev->reg_state != NETREG_REGISTERED ||
> +	    map_dev->reg_state != NETREG_REGISTERED) {
> +		NL_SET_ERR_MSG(extack, "Device is being unregistered");
> +		return ERR_PTR(-ENODEV);
> +	}
> +
>  	if (!dma_dev) {
>  		NL_SET_ERR_MSG(extack, "Device doesn't support DMA");
>  		return ERR_PTR(-EOPNOTSUPP);
> @@ -560,9 +568,9 @@ struct net_devmem_dmabuf_binding *net_devmem_get_binding(struct sock *sk,
>  	 * net_device.
>  	 */
>  	dst_dev = dst_dev_rcu(dst);
> -	if (unlikely(!dst_dev) ||
> -	    unlikely(dst_dev != READ_ONCE(binding->map_dev) &&
> -		     dst_dev != READ_ONCE(binding->bind_dev))) {
> +	if (unlikely(!dst_dev || !READ_ONCE(binding->map_dev) ||
> +		     (dst_dev != READ_ONCE(binding->map_dev) &&
> +		      dst_dev != READ_ONCE(binding->bind_dev)))) {
>  		err = -ENODEV;
>  		goto out_unlock;
>  	}
> @@ -686,8 +694,7 @@ static void mp_dmabuf_devmem_uninstall(void *mp_priv,
>  		if (xa_empty(&binding->bound_rxqs)) {
>  			if (!rxq->lease)
>  				net_devmem_dmabuf_binding_unmap(binding);
> -			ASSERT_EXCLUSIVE_WRITER(binding->map_dev);
rm or switch to bind_dev?> -			WRITE_ONCE(binding->map_dev, NULL);
> +			WRITE_ONCE(binding->bind_dev, NULL);
>  		}
>  		break;
>  	}
> diff --git a/net/core/devmem.h b/net/core/devmem.h
> index 9dcb629e10bfd..ff431e5e398e0 100644
> --- a/net/core/devmem.h
> +++ b/net/core/devmem.h
> @@ -25,8 +25,10 @@ struct net_devmem_dmabuf_binding {
>  	 * time and cleared to NULL under netdev_lock(map_dev).
>  	 */
>  	struct net_device *map_dev;
> -	/* Device the user called bind-rx or bind-tx on. Set at bind time and
> -	 * cleared to NULL on unregistration.
> +	/* Device the user called bind-rx or bind-tx on. Equal to map_dev for
> +	 * direct bindings, or the virtual device (e.g. netkit) for leased
> +	 * bindings. Set at bind time and cleared to NULL under
> +	 * netdev_lock(bind_dev).
>  	 */
>  	struct net_device *bind_dev;
>  	possible_net_t net;
> @@ -44,7 +46,7 @@ struct net_devmem_dmabuf_binding {
>  	 * those refs are dropped and the binding is no longer desired or in
>  	 * use. RX bindings are unmapped earlier when their queues are closed or
>  	 * uninstalled, unless a leased or busy queue may still use the binding.
> -	 * TX bindings are also unmapped earlier if their device is
> +	 * Both RX and TX bindings are also unmapped earlier if map_dev is
>  	 * unregistered.
>  	 *
>  	 * net_devmem_net_iov_inc_ref() on dmabuf net_iovs will increment this
> diff --git a/net/core/netdev-genl.c b/net/core/netdev-genl.c
> index 962d7a8c75e9a..442362f23fe88 100644
> --- a/net/core/netdev-genl.c
> +++ b/net/core/netdev-genl.c
> @@ -1005,12 +1005,15 @@ static int netdev_nl_read_rxq_bitmap(struct genl_info *info,
>  
>  static struct device *
>  netdev_nl_get_dma_dev(struct net_device *netdev, unsigned long *rxq_bitmap,
> +		      struct net_device **map_dev,
>  		      struct netlink_ext_ack *extack)
>  {
>  	struct device *dma_dev = NULL;
>  	u32 rxq_idx, prev_rxq_idx;
>  
> +	*map_dev = netdev;
>  	for_each_set_bit(rxq_idx, rxq_bitmap, netdev->real_num_rx_queues) {
> +		struct netdev_rx_queue *rxq;
>  		struct device *rxq_dma_dev;
>  
>  		rxq_dma_dev = netdev_queue_get_dma_dev(netdev, rxq_idx,
> @@ -1023,6 +1026,11 @@ netdev_nl_get_dma_dev(struct net_device *netdev, unsigned long *rxq_bitmap,
>  
>  		dma_dev = rxq_dma_dev;
>  		prev_rxq_idx = rxq_idx;
> +		if (netif_rxq_is_leased(netdev, rxq_idx) &&
> +		    netif_is_queue_leasee(netdev)) {
> +			rxq = __netif_get_rx_queue(netdev, rxq_idx);
> +			*map_dev = rxq->lease->dev;
> +		}
>  	}
>  
>  	return dma_dev;
> @@ -1032,9 +1040,9 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
>  {
>  	struct net_devmem_dmabuf_binding *binding;
>  	unsigned int niov_shift = PAGE_SHIFT;
> +	struct net_device *netdev, *map_dev;
>  	u32 ifindex, dmabuf_fd, rxq_idx;
>  	struct netdev_nl_sock *priv;
> -	struct net_device *netdev;
>  	unsigned long *rxq_bitmap;
>  	struct device *dma_dev;
>  	struct sk_buff *rsp;
> @@ -1104,15 +1112,21 @@ int netdev_nl_bind_rx_doit(struct sk_buff *skb, struct genl_info *info)
>  	if (err)
>  		goto err_rxq_bitmap;
>  
> -	dma_dev = netdev_nl_get_dma_dev(netdev, rxq_bitmap, info->extack);
> +	dma_dev = netdev_nl_get_dma_dev(netdev, rxq_bitmap, &map_dev,
> +					info->extack);
>  	if (IS_ERR(dma_dev)) {
>  		err = PTR_ERR(dma_dev);
>  		goto err_rxq_bitmap;
>  	}
>  
> -	binding = net_devmem_bind_dmabuf(netdev, NULL, dma_dev, DMA_FROM_DEVICE,
> -					 dmabuf_fd, niov_shift, priv,
> -					 NETLINK_CB(skb).sk, info->extack);
> +	if (map_dev != netdev)
> +		netdev_lock(map_dev);
> +	binding = net_devmem_bind_dmabuf(map_dev, netdev, dma_dev,
> +					 DMA_FROM_DEVICE, dmabuf_fd,
> +					 niov_shift, priv, NETLINK_CB(skb).sk,
> +					 info->extack);
> +	if (map_dev != netdev)
> +		netdev_unlock(map_dev);
>  	if (IS_ERR(binding)) {
>  		err = PTR_ERR(binding);
>  		goto err_rxq_bitmap;
> @@ -1257,10 +1271,9 @@ int netdev_nl_bind_tx_doit(struct sk_buff *skb, struct genl_info *info)
>  
>  	dma_dev = netdev_queue_get_dma_dev(map_dev, 0, NETDEV_QUEUE_TYPE_TX);
>  
> -	binding = net_devmem_bind_dmabuf(map_dev,
> -					 map_dev != netdev ? netdev : NULL,
> -					 dma_dev, DMA_TO_DEVICE, dmabuf_fd,
> -					 PAGE_SHIFT, priv, NETLINK_CB(skb).sk,
> +	binding = net_devmem_bind_dmabuf(map_dev, netdev, dma_dev,
> +					 DMA_TO_DEVICE, dmabuf_fd, PAGE_SHIFT,
> +					 priv, NETLINK_CB(skb).sk,
>  					 info->extack);
>  	if (IS_ERR(binding)) {
>  		err = PTR_ERR(binding);
> @@ -1475,7 +1488,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) {
> -		dev = net_devmem_lock_dev(&binding->map_dev, &dev_tracker);
> +		dev = net_devmem_lock_dev(&binding->bind_dev, &dev_tracker);
>  		net_devmem_unbind_dmabuf(binding);
>  		net_devmem_unlock_dev(dev, &dev_tracker);
>  	}
> diff --git a/net/core/netdev_rx_queue.c b/net/core/netdev_rx_queue.c
> index a7261fedd5e76..5f65b9bbfc369 100644
> --- a/net/core/netdev_rx_queue.c
> +++ b/net/core/netdev_rx_queue.c
> @@ -193,6 +193,11 @@ static int __netif_mp_open_rxq(struct net_device *dev, unsigned int rxq_idx,
>  	struct netdev_rx_queue *rxq;
>  	int ret;
>  
> +	if (dev->reg_state != NETREG_REGISTERED) {
> +		NL_SET_ERR_MSG(extack, "Device is being unregistered");
> +		return -ENODEV;
> +	}
> +
>  	if (!qops)
>  		return -EOPNOTSUPP;
>  
Thanks,
Dragos

Re: [PATCH net-next v1 2/4] net: devmem: rename binding dev and vdev to map_dev and bind_dev

[Dragos Tatulea]

On 04.11.26 00:26, Mina Almasry wrote:
> struct net_devmem_dmabuf_binding currently tracks two device pointers
> under the names 'dev' and 'vdev':
> 
> - 'dev' is the physical net_device whose dma_dev holds the DMA mapping.
> - 'vdev' is the virtual net_device the user bound to when using leased
>   queues.
> 
> Rename 'dev' to 'map_dev' and 'vdev' to 'bind_dev' (typed as struct
> net_device *), and rename the matching local variable in
> netdev_nl_bind_tx_doit() from 'bind_dev' to 'map_dev' to match. No
> functional change.
> 
> Signed-off-by: Mina Almasry <almasrymina@google.com>

Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>

Thanks,
Dragos

Re: [PATCH net-next v1 1/4] net: devmem: pass net_device pointer to net_devmem_lock_dev()

[Dragos Tatulea]

On 04.11.26 00:26, Mina Almasry wrote:
> Upcoming changes for leased queue support will track the physical DMA
> device and the bound device in separate net_device pointers on struct
> net_devmem_dmabuf_binding, with different teardown paths locking
> different pointers.
> 
> Generalize net_devmem_lock_dev() to take a struct net_device **dev_ptr
> instead of struct net_devmem_dmabuf_binding *. No functional change.
> 
> Signed-off-by: Mina Almasry <almasrymina@google.com>

Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>

Thanks,
Dragos

Re: [PATCH net-next v1 4/4] selftests: net: add leased devmem RX and TX netdev unregister tests

[Dragos Tatulea]

On 04.11.26 00:26, Mina Almasry wrote:
> Add devmem_lease_bind_rx_unregister_check and
> devmem_lease_bind_tx_unregister_check to nl_netdev.py to verify that
> leased RX and TX devmem bindings on a netkit interface backed by a
> netdevsim queue detach their dma-buf cleanly while the netlink socket
> remains open, both when the virtual device (bind_dev) is unregistered
> before the physical device (map_dev) and when the physical device is
> unregistered directly (which uninstalls the physical queue memory
> provider and cascades unregistration to the leasing netkit).
> 
> Signed-off-by: Mina Almasry <almasrymina@google.com>
> ---
>  tools/testing/selftests/net/nl_netdev.py | 132 ++++++++++++++++++++++-
>  1 file changed, 129 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/nl_netdev.py b/tools/testing/selftests/net/nl_netdev.py
> index dd0321c0b463a..fa4b5be093257 100755
> --- a/tools/testing/selftests/net/nl_netdev.py
> +++ b/tools/testing/selftests/net/nl_netdev.py
> @@ -12,8 +12,9 @@ import os
>  import struct
>  from os import system
>  from lib.py import ksft_run, ksft_exit, KsftFailEx, KsftSkipEx
> -from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_raises, ksft_busy_wait
> -from lib.py import EthtoolFamily, NetdevFamily, NetdevSimDev, NlError, defer, ip
> +from lib.py import ksft_eq, ksft_ge, ksft_ne, ksft_not_in, ksft_raises, ksft_busy_wait
> +from lib.py import EthtoolFamily, NetdevFamily, NetdevSimDev, Netlink
> +from lib.py import NlError, RtnlFamily, cmd, defer, ip
>  
>  
>  def empty_check(nf) -> None:
> @@ -483,6 +484,129 @@ def devmem_bind_tx_unregister_check(_nf) -> None:
>      nf_priv.close()
>  
>  
> +def _create_netkit_single(rxqueues=2):
> +    """Create a single-device netkit interface for queue lease tests."""
> +    ifname = f"nk{os.getpid()}"
> +    try:
> +        with RtnlFamily() as rtnl:
> +            rtnl.newlink(
> +                {
> +                    "ifname": ifname,
> +                    "linkinfo": {
> +                        "kind": "netkit",
> +                        "data": {
> +                            "mode": "l2",
> +                            "pairing": "single",
> +                        },
> +                    },
> +                    "num-rx-queues": rxqueues,
> +                },
> +                flags=[Netlink.NLM_F_CREATE, Netlink.NLM_F_EXCL],
> +            )
> +    except NlError as e:
> +        if e.error in (errno.EOPNOTSUPP, errno.ENOENT):
> +            raise KsftSkipEx(f"netkit single mode not supported: {e}") from e
> +        raise
> +    defer(cmd, f"ip link del dev {ifname}", fail=False)
> +
> +    link = ip(f"-d link show dev {ifname}", json=True)[0]
> +    return ifname, link["ifindex"]
> +
> +
> +def devmem_lease_bind_rx_unregister_check(nf) -> None:
> +    """Verify leased RX devmem bindings detach cleanly on virt/phys unregister."""
> +    for remove_virt_first in (False, True):
> +        dmabuf_fd = _create_udmabuf()
> +        defer(os.close, dmabuf_fd)
> +        nf_priv = NetdevFamily()
> +        defer(nf_priv.close)
> +        nsimdev = NetdevSimDev(queue_count=2)
> +        defer(nsimdev.remove)
> +        nsim = nsimdev.nsims[0]
> +
> +        ip(f"link set dev {nsim.ifname} up")
> +        with EthtoolFamily() as ef:
> +            ef.rings_set({"header": {"dev-index": nsim.ifindex},
> +                          "tcp-data-split": "enabled"})
> +
> +        nk_ifname, nk_ifindex = _create_netkit_single(rxqueues=2)
> +        ip(f"link set dev {nk_ifname} up")
> +
> +        res = nf.queue_create({
> +            "ifindex": nk_ifindex,
> +            "type": "rx",
> +            "lease": {
> +                "ifindex": nsim.ifindex,
> +                "queue": {"id": 1, "type": "rx"},
> +            },
> +        })
> +
> +        _devmem_bind(nf_priv.bind_rx, {
> +            "ifindex": nk_ifindex,
> +            "fd": dmabuf_fd,
> +            "queues": [{"id": res["id"], "type": "rx"}],
> +        })
> +        ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
> +
> +        if remove_virt_first:
> +            ip(f"link del dev {nk_ifname}")
> +            phys_q = nf.queue_get({"ifindex": nsim.ifindex, "id": 1,
> +                                   "type": "rx"})
> +            ksft_not_in("dmabuf", phys_q)
> +            ksft_not_in("lease", phys_q)
> +            ksft_ge(_dmabuf_attach_count(dmabuf_fd), 1)
> +            nsimdev.remove()
> +        else:
> +            nsimdev.remove()
> +            ksft_ne(cmd(f"ip link show dev {nk_ifname}", fail=False).ret, 0)
> +        ksft_eq(_dmabuf_attach_count(dmabuf_fd), 0)
> +
> +        nf_priv.close()
Any value in leaving this here? defer() should handle it.

Other than that:
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>

Thanks,
Dragos