From: Jens Axboe <axboe@kernel.dk>
To: Gabriel Krisman Bertazi <krisman@suse.de>
Cc: io-uring@vger.kernel.org
Subject: Re: [PATCH 2/5] io_uring/net: add provided buffer support for IORING_OP_SEND
Date: Thu, 25 Apr 2024 09:11:22 -0600 [thread overview]
Message-ID: <20557585-cd75-4202-b0e5-eabf774ece8e@kernel.dk> (raw)
In-Reply-To: <878r11zmdj.fsf@mailhost.krisman.be>
On 4/25/24 5:56 AM, Gabriel Krisman Bertazi wrote:
> Jens Axboe <axboe@kernel.dk> writes:
>
>> It's pretty trivial to wire up provided buffer support for the send
>> side, just like how it's done the receive side. This enables setting up
>> a buffer ring that an application can use to push pending sends to,
>> and then have a send pick a buffer from that ring.
>>
>> One of the challenges with async IO and networking sends is that you
>> can get into reordering conditions if you have more than one inflight
>> at the same time. Consider the following scenario where everything is
>> fine:
>>
>> 1) App queues sendA for socket1
>> 2) App queues sendB for socket1
>> 3) App does io_uring_submit()
>> 4) sendA is issued, completes successfully, posts CQE
>> 5) sendB is issued, completes successfully, posts CQE
>>
>> All is fine. Requests are always issued in-order, and both complete
>> inline as most sends do.
>
>
>
>
>
>
>>
>> However, if we're flooding socket1 with sends, the following could
>> also result from the same sequence:
>>
>> 1) App queues sendA for socket1
>> 2) App queues sendB for socket1
>> 3) App does io_uring_submit()
>> 4) sendA is issued, socket1 is full, poll is armed for retry
>> 5) Space frees up in socket1, this triggers sendA retry via task_work
>> 6) sendB is issued, completes successfully, posts CQE
>> 7) sendA is retried, completes successfully, posts CQE
>>
>> Now we've sent sendB before sendA, which can make things unhappy. If
>> both sendA and sendB had been using provided buffers, then it would look
>> as follows instead:
>>
>> 1) App queues dataA for sendA, queues sendA for socket1
>> 2) App queues dataB for sendB queues sendB for socket1
>> 3) App does io_uring_submit()
>> 4) sendA is issued, socket1 is full, poll is armed for retry
>> 5) Space frees up in socket1, this triggers sendA retry via task_work
>> 6) sendB is issued, picks first buffer (dataA), completes successfully,
>> posts CQE (which says "I sent dataA")
>> 7) sendA is retried, picks first buffer (dataB), completes successfully,
>> posts CQE (which says "I sent dataB")
>
> Hi Jens,
>
> If I understand correctly, when sending a buffer, we set sr->len to be
> the smallest between the buffer size and what was requested in sqe->len.
> But, when we disconnect the buffer from the request, we can get in a
> situation where the buffers and requests mismatch, and only one buffer
> gets sent.
>
> Say we are sending two buffers through non-bundle sends with different
> sizes to the same socket in this order:
>
> buff[1]->len = 128
> buff[2]->len = 256
>
> And SQEs like this:
>
> sqe[1]->len = 128
> sqe[2]->len = 256
>
> If sqe1 picks buff1 it is all good. But, if sqe[2] runs first, then
> sqe[1] picks buff2, and it will only send the first 128, won't it?
> Looking at the patch I don't see how you avoid this condition, but
> perhaps I'm missing something?
>
> One suggestion would be requiring sqe->len to be 0 when using send with
> provided buffers, so we simply use the entire buffer in
> the ring. wdyt?
It might not hurt to just enforce it to be 0, in fact I think any sane
use case would do that and I don't think the above use case is a very
valid one. It's a bit of "you get to keep both pieces when it breaks".
Do you want to send a patch that just enforces it to be 0? We do have
that requirement in other spots for provided buffers and multishot, so I
think it'll make sense to do here too regardless of the sanity of the
use case.
--
Jens Axboe
next prev parent reply other threads:[~2024-04-25 15:11 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-20 13:29 [PATCHSET v2 0/5] Send and receive bundles Jens Axboe
2024-04-20 13:29 ` [PATCH 1/5] io_uring/net: add generic multishot retry helper Jens Axboe
2024-04-20 13:29 ` [PATCH 2/5] io_uring/net: add provided buffer support for IORING_OP_SEND Jens Axboe
2024-04-25 11:56 ` Gabriel Krisman Bertazi
2024-04-25 12:19 ` Gabriel Krisman Bertazi
2024-04-25 15:11 ` Jens Axboe [this message]
2024-04-29 18:15 ` [PATCH] io_uring: Require zeroed sqe->len on provided-buffers send Gabriel Krisman Bertazi
2024-04-30 13:02 ` Jens Axboe
2024-05-01 20:47 ` Gabriel Krisman Bertazi
2024-05-01 20:55 ` Jens Axboe
2024-04-20 13:29 ` [PATCH 3/5] io_uring/kbuf: add helpers for getting/peeking multiple buffers Jens Axboe
2024-04-20 13:29 ` [PATCH 4/5] io_uring/net: support bundles for send Jens Axboe
2024-04-20 13:29 ` [PATCH 5/5] io_uring/net: support bundles for recv Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20557585-cd75-4202-b0e5-eabf774ece8e@kernel.dk \
--to=axboe@kernel.dk \
--cc=io-uring@vger.kernel.org \
--cc=krisman@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.