public inbox for io-uring@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
@ 2025-11-01  2:24 David Wei
  2025-11-01  2:24 ` [PATCH v3 1/2] net: export netdev_get_by_index_lock() David Wei
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: David Wei @ 2025-11-01  2:24 UTC (permalink / raw)
  To: io-uring, netdev
  Cc: Jens Axboe, Pavel Begunkov, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman

netdev ops must be called under instance lock or rtnl_lock, but
io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
Fix this by taking the instance lock using netdev_get_by_index_lock().

netdev_get_by_index_lock() isn't available outside net/ by default, so
the first patch is a prep patch to export this under linux/netdevice.h.

Extended the instance lock section to include attaching a memory
provider. Could not move io_zcrx_create_area() outside, since the dmabuf
codepath IORING_ZCRX_AREA_DMABUF requires ifq->dev.

The netdev instance lock is taken first, followed by holding a ref. On
err, this is freed in LIFO order: put ref then unlock. After
successfully opening the mp on an rxq, the instance lock is unlocked and
ifq->if_rxq is set to a valid value. If there are future errs,
io_zcrx_ifq_free() will put the ref.

v3:
 - do not export netdev_get_by_index_lock()
 - fix netdev lock/ref cleanup

v2:
 - add Fixes tag
 - export netdev_get_by_index_lock()
 - use netdev_get_by_index_lock() + netdev_hold()
 - extend lock section to include net_mp_open_rxq()

David Wei (2):
  net: export netdev_get_by_index_lock()
  net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance
    lock

 include/linux/netdevice.h |  1 +
 io_uring/zcrx.c           | 16 ++++++++++------
 net/core/dev.h            |  1 -
 3 files changed, 11 insertions(+), 7 deletions(-)

-- 
2.47.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/2] net: export netdev_get_by_index_lock()
  2025-11-01  2:24 [PATCH v3 0/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
@ 2025-11-01  2:24 ` David Wei
  2025-11-03 23:47   ` Jakub Kicinski
  2025-11-01  2:24 ` [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
  2025-11-06 23:33 ` [PATCH v3 0/2] " Jens Axboe
  2 siblings, 1 reply; 11+ messages in thread
From: David Wei @ 2025-11-01  2:24 UTC (permalink / raw)
  To: io-uring, netdev
  Cc: Jens Axboe, Pavel Begunkov, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman

Need to call netdev_get_by_index_lock() from io_uring/zcrx.c, but it is
currently private to net. Export the function in linux/netdevice.h.

Signed-off-by: David Wei <dw@davidwei.uk>
---
 include/linux/netdevice.h | 1 +
 net/core/dev.h            | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d1a687444b27..77c46a2823ec 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3401,6 +3401,7 @@ struct net_device *dev_get_by_index(struct net *net, int ifindex);
 struct net_device *__dev_get_by_index(struct net *net, int ifindex);
 struct net_device *netdev_get_by_index(struct net *net, int ifindex,
 				       netdevice_tracker *tracker, gfp_t gfp);
+struct net_device *netdev_get_by_index_lock(struct net *net, int ifindex);
 struct net_device *netdev_get_by_name(struct net *net, const char *name,
 				      netdevice_tracker *tracker, gfp_t gfp);
 struct net_device *netdev_get_by_flags_rcu(struct net *net, netdevice_tracker *tracker,
diff --git a/net/core/dev.h b/net/core/dev.h
index 900880e8b5b4..df8a90fe89f8 100644
--- a/net/core/dev.h
+++ b/net/core/dev.h
@@ -29,7 +29,6 @@ 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 *net);
 struct net_device *
 netdev_xa_find_lock(struct net *net, struct net_device *dev,
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
  2025-11-01  2:24 [PATCH v3 0/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
  2025-11-01  2:24 ` [PATCH v3 1/2] net: export netdev_get_by_index_lock() David Wei
@ 2025-11-01  2:24 ` David Wei
  2025-11-03 13:51   ` Pavel Begunkov
                     ` (2 more replies)
  2025-11-06 23:33 ` [PATCH v3 0/2] " Jens Axboe
  2 siblings, 3 replies; 11+ messages in thread
From: David Wei @ 2025-11-01  2:24 UTC (permalink / raw)
  To: io-uring, netdev
  Cc: Jens Axboe, Pavel Begunkov, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman

netdev ops must be called under instance lock or rtnl_lock, but
io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
Fix this by taking the instance lock using netdev_get_by_index_lock().

Extended the instance lock section to include attaching a memory
provider. Could not move io_zcrx_create_area() outside, since the dmabuf
codepath IORING_ZCRX_AREA_DMABUF requires ifq->dev.

Fixes: 59b8b32ac8d4 ("io_uring/zcrx: add support for custom DMA devices")
Signed-off-by: David Wei <dw@davidwei.uk>
---
 io_uring/zcrx.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
index a816f5902091..4ffa336d677c 100644
--- a/io_uring/zcrx.c
+++ b/io_uring/zcrx.c
@@ -599,29 +599,30 @@ int io_register_zcrx_ifq(struct io_ring_ctx *ctx,
 	if (ret)
 		goto err;
 
-	ifq->netdev = netdev_get_by_index(current->nsproxy->net_ns, reg.if_idx,
-					  &ifq->netdev_tracker, GFP_KERNEL);
+	ifq->netdev = netdev_get_by_index_lock(current->nsproxy->net_ns, reg.if_idx);
 	if (!ifq->netdev) {
 		ret = -ENODEV;
 		goto err;
 	}
+	netdev_hold(ifq->netdev, &ifq->netdev_tracker, GFP_KERNEL);
 
 	ifq->dev = netdev_queue_get_dma_dev(ifq->netdev, reg.if_rxq);
 	if (!ifq->dev) {
 		ret = -EOPNOTSUPP;
-		goto err;
+		goto netdev_put_unlock;
 	}
 	get_device(ifq->dev);
 
 	ret = io_zcrx_create_area(ifq, &area);
 	if (ret)
-		goto err;
+		goto netdev_put_unlock;
 
 	mp_param.mp_ops = &io_uring_pp_zc_ops;
 	mp_param.mp_priv = ifq;
-	ret = net_mp_open_rxq(ifq->netdev, reg.if_rxq, &mp_param);
+	ret = __net_mp_open_rxq(ifq->netdev, reg.if_rxq, &mp_param, NULL);
 	if (ret)
-		goto err;
+		goto netdev_put_unlock;
+	netdev_unlock(ifq->netdev);
 	ifq->if_rxq = reg.if_rxq;
 
 	reg.zcrx_id = id;
@@ -640,6 +641,9 @@ int io_register_zcrx_ifq(struct io_ring_ctx *ctx,
 		goto err;
 	}
 	return 0;
+netdev_put_unlock:
+	netdev_put(ifq->netdev, &ifq->netdev_tracker);
+	netdev_unlock(ifq->netdev);
 err:
 	scoped_guard(mutex, &ctx->mmap_lock)
 		xa_erase(&ctx->zcrx_ctxs, id);
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
  2025-11-01  2:24 ` [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
@ 2025-11-03 13:51   ` Pavel Begunkov
  2025-11-03 17:47     ` David Wei
  2025-11-03 23:50   ` Jakub Kicinski
  2025-11-06 11:26   ` Pavel Begunkov
  2 siblings, 1 reply; 11+ messages in thread
From: Pavel Begunkov @ 2025-11-03 13:51 UTC (permalink / raw)
  To: David Wei, io-uring, netdev
  Cc: Jens Axboe, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

On 11/1/25 02:24, David Wei wrote:
> netdev ops must be called under instance lock or rtnl_lock, but
> io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
> Fix this by taking the instance lock using netdev_get_by_index_lock().
> 
> Extended the instance lock section to include attaching a memory
> provider. Could not move io_zcrx_create_area() outside, since the dmabuf
> codepath IORING_ZCRX_AREA_DMABUF requires ifq->dev.

It's probably fine for now, but this nested waiting feels
uncomfortable considering that it could be waiting for other
devices to finish IO via dmabuf fences.

-- 
Pavel Begunkov


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
  2025-11-03 13:51   ` Pavel Begunkov
@ 2025-11-03 17:47     ` David Wei
  2025-11-03 18:21       ` Pavel Begunkov
  0 siblings, 1 reply; 11+ messages in thread
From: David Wei @ 2025-11-03 17:47 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring, netdev
  Cc: Jens Axboe, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

On 2025-11-03 05:51, Pavel Begunkov wrote:
> On 11/1/25 02:24, David Wei wrote:
>> netdev ops must be called under instance lock or rtnl_lock, but
>> io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
>> Fix this by taking the instance lock using netdev_get_by_index_lock().
>>
>> Extended the instance lock section to include attaching a memory
>> provider. Could not move io_zcrx_create_area() outside, since the dmabuf
>> codepath IORING_ZCRX_AREA_DMABUF requires ifq->dev.
> 
> It's probably fine for now, but this nested waiting feels
> uncomfortable considering that it could be waiting for other
> devices to finish IO via dmabuf fences.
> 

Only the dmabuf path requires ifq->dev in io_zcrx_create_area(); I could
split this into two and then unlock netdev instance lock between holding
a ref and calling net_mp_open_rxq().

So the new ordering would be:

   1. io_zcrx_create_area() for !IORING_ZCRX_AREA_DMABUF
   2. netdev_get_by_index_lock(), hold netdev ref, unlock netdev
   3. io_zcrx_create_area() for IORING_ZCRX_AREA_DMABUF
   4. net_mp_open_rxq()

Jakub, do you see any problems in relocking?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
  2025-11-03 17:47     ` David Wei
@ 2025-11-03 18:21       ` Pavel Begunkov
  2025-11-03 18:29         ` David Wei
  0 siblings, 1 reply; 11+ messages in thread
From: Pavel Begunkov @ 2025-11-03 18:21 UTC (permalink / raw)
  To: David Wei, io-uring, netdev
  Cc: Jens Axboe, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

On 11/3/25 17:47, David Wei wrote:
> On 2025-11-03 05:51, Pavel Begunkov wrote:
>> On 11/1/25 02:24, David Wei wrote:
>>> netdev ops must be called under instance lock or rtnl_lock, but
>>> io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
>>> Fix this by taking the instance lock using netdev_get_by_index_lock().
>>>
>>> Extended the instance lock section to include attaching a memory
>>> provider. Could not move io_zcrx_create_area() outside, since the dmabuf
>>> codepath IORING_ZCRX_AREA_DMABUF requires ifq->dev.
>>
>> It's probably fine for now, but this nested waiting feels
>> uncomfortable considering that it could be waiting for other
>> devices to finish IO via dmabuf fences.
>>
> 
> Only the dmabuf path requires ifq->dev in io_zcrx_create_area(); I could
> split this into two and then unlock netdev instance lock between holding
> a ref and calling net_mp_open_rxq().
> 
> So the new ordering would be:
> 
>    1. io_zcrx_create_area() for !IORING_ZCRX_AREA_DMABUF
>    2. netdev_get_by_index_lock(), hold netdev ref, unlock netdev
>    3. io_zcrx_create_area() for IORING_ZCRX_AREA_DMABUF
>    4. net_mp_open_rxq()

To avoid dragging it on, can you do it as a follow up please? And
it's better to avoid splitting on IORING_ZCRX_AREA_DMABUF, either it
works for both or it doesn't at all.

-- 
Pavel Begunkov


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
  2025-11-03 18:21       ` Pavel Begunkov
@ 2025-11-03 18:29         ` David Wei
  0 siblings, 0 replies; 11+ messages in thread
From: David Wei @ 2025-11-03 18:29 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring, netdev
  Cc: Jens Axboe, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

On 2025-11-03 10:21, Pavel Begunkov wrote:
> On 11/3/25 17:47, David Wei wrote:
>> On 2025-11-03 05:51, Pavel Begunkov wrote:
>>> On 11/1/25 02:24, David Wei wrote:
>>>> netdev ops must be called under instance lock or rtnl_lock, but
>>>> io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
>>>> Fix this by taking the instance lock using netdev_get_by_index_lock().
>>>>
>>>> Extended the instance lock section to include attaching a memory
>>>> provider. Could not move io_zcrx_create_area() outside, since the dmabuf
>>>> codepath IORING_ZCRX_AREA_DMABUF requires ifq->dev.
>>>
>>> It's probably fine for now, but this nested waiting feels
>>> uncomfortable considering that it could be waiting for other
>>> devices to finish IO via dmabuf fences.
>>>
>>
>> Only the dmabuf path requires ifq->dev in io_zcrx_create_area(); I could
>> split this into two and then unlock netdev instance lock between holding
>> a ref and calling net_mp_open_rxq().
>>
>> So the new ordering would be:
>>
>>    1. io_zcrx_create_area() for !IORING_ZCRX_AREA_DMABUF
>>    2. netdev_get_by_index_lock(), hold netdev ref, unlock netdev
>>    3. io_zcrx_create_area() for IORING_ZCRX_AREA_DMABUF
>>    4. net_mp_open_rxq()
> 
> To avoid dragging it on, can you do it as a follow up please? And
> it's better to avoid splitting on IORING_ZCRX_AREA_DMABUF, either it
> works for both or it doesn't at all.
> 

Of course, follow ups are always my preference.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/2] net: export netdev_get_by_index_lock()
  2025-11-01  2:24 ` [PATCH v3 1/2] net: export netdev_get_by_index_lock() David Wei
@ 2025-11-03 23:47   ` Jakub Kicinski
  0 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2025-11-03 23:47 UTC (permalink / raw)
  To: David Wei
  Cc: io-uring, netdev, Jens Axboe, Pavel Begunkov, David S. Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman

On Fri, 31 Oct 2025 19:24:48 -0700 David Wei wrote:
> Need to call netdev_get_by_index_lock() from io_uring/zcrx.c, but it is
> currently private to net. Export the function in linux/netdevice.h.
> 
> Signed-off-by: David Wei <dw@davidwei.uk>

Acked-by: Jakub Kicinski <kuba@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
  2025-11-01  2:24 ` [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
  2025-11-03 13:51   ` Pavel Begunkov
@ 2025-11-03 23:50   ` Jakub Kicinski
  2025-11-06 11:26   ` Pavel Begunkov
  2 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2025-11-03 23:50 UTC (permalink / raw)
  To: David Wei
  Cc: io-uring, netdev, Jens Axboe, Pavel Begunkov, David S. Miller,
	Eric Dumazet, Paolo Abeni, Simon Horman

On Fri, 31 Oct 2025 19:24:49 -0700 David Wei wrote:
> netdev ops must be called under instance lock or rtnl_lock, but
> io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
> Fix this by taking the instance lock using netdev_get_by_index_lock().
> 
> Extended the instance lock section to include attaching a memory
> provider. Could not move io_zcrx_create_area() outside, since the dmabuf
> codepath IORING_ZCRX_AREA_DMABUF requires ifq->dev.
> 
> Fixes: 59b8b32ac8d4 ("io_uring/zcrx: add support for custom DMA devices")
> Signed-off-by: David Wei <dw@davidwei.uk>

Reviewed-by: Jakub Kicinski <kuba@kernel.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
  2025-11-01  2:24 ` [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
  2025-11-03 13:51   ` Pavel Begunkov
  2025-11-03 23:50   ` Jakub Kicinski
@ 2025-11-06 11:26   ` Pavel Begunkov
  2 siblings, 0 replies; 11+ messages in thread
From: Pavel Begunkov @ 2025-11-06 11:26 UTC (permalink / raw)
  To: David Wei, io-uring, netdev
  Cc: Jens Axboe, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman

On 11/1/25 02:24, David Wei wrote:
> netdev ops must be called under instance lock or rtnl_lock, but
> io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
> Fix this by taking the instance lock using netdev_get_by_index_lock().
> 
> Extended the instance lock section to include attaching a memory
> provider. Could not move io_zcrx_create_area() outside, since the dmabuf
> codepath IORING_ZCRX_AREA_DMABUF requires ifq->dev.

Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

-- 
Pavel Begunkov


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 0/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
  2025-11-01  2:24 [PATCH v3 0/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
  2025-11-01  2:24 ` [PATCH v3 1/2] net: export netdev_get_by_index_lock() David Wei
  2025-11-01  2:24 ` [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
@ 2025-11-06 23:33 ` Jens Axboe
  2 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2025-11-06 23:33 UTC (permalink / raw)
  To: io-uring, netdev, David Wei
  Cc: Pavel Begunkov, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman


On Fri, 31 Oct 2025 19:24:47 -0700, David Wei wrote:
> netdev ops must be called under instance lock or rtnl_lock, but
> io_register_zcrx_ifq() isn't doing this for netdev_queue_get_dma_dev().
> Fix this by taking the instance lock using netdev_get_by_index_lock().
> 
> netdev_get_by_index_lock() isn't available outside net/ by default, so
> the first patch is a prep patch to export this under linux/netdevice.h.
> 
> [...]

Applied, thanks!

[1/2] net: export netdev_get_by_index_lock()
      commit: 0da5d94bbc6af079f105264849dc3afd01b78aaa
[2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock
      (no commit info)

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-11-06 23:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-01  2:24 [PATCH v3 0/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
2025-11-01  2:24 ` [PATCH v3 1/2] net: export netdev_get_by_index_lock() David Wei
2025-11-03 23:47   ` Jakub Kicinski
2025-11-01  2:24 ` [PATCH v3 2/2] net: io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock David Wei
2025-11-03 13:51   ` Pavel Begunkov
2025-11-03 17:47     ` David Wei
2025-11-03 18:21       ` Pavel Begunkov
2025-11-03 18:29         ` David Wei
2025-11-03 23:50   ` Jakub Kicinski
2025-11-06 11:26   ` Pavel Begunkov
2025-11-06 23:33 ` [PATCH v3 0/2] " Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox