netdev - Re: [PATCH net-next v8 09/17] io_uring/zcrx: add interface queue and refill queue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5a72c16f-311e-46d9-baed-f0a25d2a3dff@davidwei.uk>
Date: Mon, 9 Dec 2024 15:50:27 -0800
From: David Wei <dw@...idwei.uk>
To: Simon Horman <horms@...nel.org>
Cc: io-uring@...r.kernel.org, netdev@...r.kernel.org,
 Jens Axboe <axboe@...nel.dk>, Pavel Begunkov <asml.silence@...il.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jesper Dangaard Brouer <hawk@...nel.org>, David Ahern <dsahern@...nel.org>,
 Mina Almasry <almasrymina@...gle.com>,
 Stanislav Fomichev <stfomichev@...il.com>, Joe Damato <jdamato@...tly.com>,
 Pedro Tammela <pctammela@...atatu.com>
Subject: Re: [PATCH net-next v8 09/17] io_uring/zcrx: add interface queue and
 refill queue

On 2024-12-06 08:05, Simon Horman wrote:
> On Wed, Dec 04, 2024 at 09:21:48AM -0800, David Wei wrote:
>> From: David Wei <davidhwei@...a.com>
>>
>> Add a new object called an interface queue (ifq) that represents a net
>> rx queue that has been configured for zero copy. Each ifq is registered
>> using a new registration opcode IORING_REGISTER_ZCRX_IFQ.
>>
>> The refill queue is allocated by the kernel and mapped by userspace
>> using a new offset IORING_OFF_RQ_RING, in a similar fashion to the main
>> SQ/CQ. It is used by userspace to return buffers that it is done with,
>> which will then be re-used by the netdev again.
>>
>> The main CQ ring is used to notify userspace of received data by using
>> the upper 16 bytes of a big CQE as a new struct io_uring_zcrx_cqe. Each
>> entry contains the offset + len to the data.
>>
>> For now, each io_uring instance only has a single ifq.
>>
>> Signed-off-by: David Wei <dw@...idwei.uk>
> 
> ...
> 
>> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
> 
> ...
> 
>> +int io_register_zcrx_ifq(struct io_ring_ctx *ctx,
>> +			  struct io_uring_zcrx_ifq_reg __user *arg)
>> +{
>> +	struct io_uring_zcrx_ifq_reg reg;
>> +	struct io_uring_region_desc rd;
>> +	struct io_zcrx_ifq *ifq;
>> +	size_t ring_sz, rqes_sz;
>> +	int ret;
>> +
>> +	/*
>> +	 * 1. Interface queue allocation.
>> +	 * 2. It can observe data destined for sockets of other tasks.
>> +	 */
>> +	if (!capable(CAP_NET_ADMIN))
>> +		return -EPERM;
>> +
>> +	/* mandatory io_uring features for zc rx */
>> +	if (!(ctx->flags & IORING_SETUP_DEFER_TASKRUN &&
>> +	      ctx->flags & IORING_SETUP_CQE32))
>> +		return -EINVAL;
>> +	if (ctx->ifq)
>> +		return -EBUSY;
>> +	if (copy_from_user(&reg, arg, sizeof(reg)))
>> +		return -EFAULT;
>> +	if (copy_from_user(&rd, u64_to_user_ptr(reg.region_ptr), sizeof(rd)))
>> +		return -EFAULT;
>> +	if (memchr_inv(&reg.__resv, 0, sizeof(reg.__resv)))
>> +		return -EINVAL;
>> +	if (reg.if_rxq == -1 || !reg.rq_entries || reg.flags)
>> +		return -EINVAL;
>> +	if (reg.rq_entries > IO_RQ_MAX_ENTRIES) {
>> +		if (!(ctx->flags & IORING_SETUP_CLAMP))
>> +			return -EINVAL;
>> +		reg.rq_entries = IO_RQ_MAX_ENTRIES;
>> +	}
>> +	reg.rq_entries = roundup_pow_of_two(reg.rq_entries);
>> +
>> +	if (!reg.area_ptr)
>> +		return -EFAULT;
>> +
>> +	ifq = io_zcrx_ifq_alloc(ctx);
>> +	if (!ifq)
>> +		return -ENOMEM;
>> +
>> +	ret = io_allocate_rbuf_ring(ifq, &reg, &rd);
>> +	if (ret)
>> +		goto err;
>> +
>> +	ifq->rq_entries = reg.rq_entries;
>> +	ifq->if_rxq = reg.if_rxq;
>> +
>> +	ring_sz = sizeof(struct io_uring);
>> +	rqes_sz = sizeof(struct io_uring_zcrx_rqe) * ifq->rq_entries;
> 
> Hi David,
> 
> A minor nit from my side: rqes_sz is set but otherwise unused in this
> function. Perhaps it can be removed?
> 
> Flagged by W=1 builds.

Hi Simon, thanks for flagging this, I'll remove it in the next version.