Re: [PATCH v3 1/2] io_uring/zcrx: add a read limit to recvzc requests

[David Wei <dw@davidwei.uk>]

On 2025-02-24 05:11, Pavel Begunkov wrote:
> On 2/24/25 04:13, David Wei wrote:
>> Currently multishot recvzc requests have no read limit and will remain
>> active so as long as the socket remains open. But, there are sometimes a
>> need to do a fixed length read e.g. peeking at some data in the socket.
>>
>> Add a length limit to recvzc requests `len`. A value of 0 means no limit
>> which is the previous behaviour. A positive value N specifies how many
>> bytes to read from the socket.
>>
>> Data will still be posted in aux completions, as before. This could be
>> split across multiple frags. But the primary recvzc request will now
>> complete once N bytes have been read. The completion of the recvzc
>> request will have res and cflags both set to 0.
> 
> Looks fine, can be improved later.
> 
> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> 
> 
>> Signed-off-by: David Wei <dw@davidwei.uk>
>> ---
>>   io_uring/net.c  | 16 +++++++++++++---
>>   io_uring/zcrx.c | 13 +++++++++----
>>   io_uring/zcrx.h |  2 +-
>>   3 files changed, 23 insertions(+), 8 deletions(-)
> ...
>> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
>> index f2d326e18e67..9c95b5b6ec4e 100644
>> --- a/io_uring/zcrx.c
>> +++ b/io_uring/zcrx.c
>> @@ -817,6 +817,7 @@ io_zcrx_recv_skb(read_descriptor_t *desc, struct sk_buff *skb,
> ...
>>   static int io_zcrx_tcp_recvmsg(struct io_kiocb *req, struct io_zcrx_ifq *ifq,
>>                   struct sock *sk, int flags,
>> -                unsigned issue_flags)
>> +                unsigned issue_flags, unsigned int *outlen)
>>   {
>> +    unsigned int len = *outlen;
>>       struct io_zcrx_args args = {
>>           .req = req,
>>           .ifq = ifq,
>>           .sock = sk->sk_socket,
>>       };
>>       read_descriptor_t rd_desc = {
>> -        .count = 1,
>> +        .count = len ? len : UINT_MAX,
> 
> typedef struct {
>     ...
>     size_t count;
> } read_descriptor_t;
> 
> Should be SIZE_MAX, but it's not worth of respinning.

I'll send a follow up.

>