2022-02-01 02:06:24

by Ammar Faizi

[permalink] [raw]
Subject: [PATCH for-5.18 v1 0/3] Add `sendto(2)` and `recvfrom(2)` support

Hello,

This patchset adds sendto(2) and recvfrom(2) support for io_uring. It
also addresses an issue in the liburing GitHub repository [1].

## Motivations:

1) By using `sendto()` and `recvfrom()` we can make the submission
simpler compared to always using `sendmsg()` and `recvmsg()` from
the userspace. Especially for UDP socket.

2) There is a historical patch that tried to add the same
functionality, but did not end up being applied. [2]

On Tue, 7 Jul 2020 12:29:18 -0600, Jens Axboe <[email protected]> wrote:
> In a private conversation with the author, a good point was brought
> up that the sendto/recvfrom do not require an allocation of an async
> context, if we need to defer or go async with the request. I think
> that's a major win, to be honest. There are other benefits as well
> (like shorter path), but to me, the async less part is nice and will
> reduce overhead

## Changes summary:

1) Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
_____________________________________________________
The following call

send(sockfd, buf, len, flags);

is equivalent to

sendto(sockfd, buf, len, flags, NULL, 0);
_____________________________________________________
The following call

recv(sockfd, buf, len, flags);

is equivalent to

recvfrom(sockfd, buf, len, flags, NULL, NULL);
_____________________________________________________

Currently, io_uring supports send() and recv() operation. Now, we are
going to add sendto() and recvfrom() support. Since the latter is the
superset of the former, change the function name to the latter.

2) Make `move_addr_to_user()` be a non static function. This is required for
adding recvfrom() support for io_uring.

3) Add `sendto(2)` and `recvfrom(2)` support.

New opcodes:
- IORING_OP_SENDTO
- IORING_OP_RECVFROM

## How to apply

This work is based on Jens' tree, branch "io-uring-5.17".

The following changes since commit f6133fbd373811066c8441737e65f384c8f31974:

io_uring: remove unused argument from io_rsrc_node_alloc (2022-01-27 10:18:53 -0700)

are available in the Git repository at:

git://github.com/ammarfaizi2/linux-block.git tags/io_uring-sendto-recvfrom.v1

for you to fetch changes up to 68d110c39241b887ec388cd3316dbedb85b0cbcf:

io_uring: Add `sendto(2)` and `recvfrom(2)` support (2022-01-29 13:08:13 +0700)
```
----------------------------------------------------------------
io_uring-sendto-recvfrom.v1

----------------------------------------------------------------
```
## liburing support and test program:

git://github.com/ammarfaizi2/liburing.git tags/sendto-recvfrom.v1-2022-01-29

Run the test program from liburing:
```
./configure;
make -j$(nproc);
test/sendto_recvfrom;
echo $?;
```
## Changelog:

v1:
- Rebase the work (sync with "io_uring-5.17" branch in Jens' tree).
- Add BUILD_BUG_SQE_ELEM(48, __u64, addr3); for compile time
assertion.
- Reword the commit messages.
- Add Alviro Iskandar Setiawan to CC list (tester).

RFC v4:
- Rebase the work (sync with "for-next" branch in Jens' tree).
- Remove Tested-by tag from Nugra as this patch changes.
- (Address Praveen's comment) Zero `sendto_addr_len` and
`recvfrom_addr_len` on prep when the `req->opcode` is not
`IORING_OP_{SENDTO,RECVFROM}`.

RFC v3:
- Fix build error when CONFIG_NET is undefined for PATCH 1/3. I
tried to fix it in PATCH 3/3, but it should be fixed in PATCH 1/3,
otherwise it breaks the build in PATCH 1/3.
- Add `io_uring_prep_{sendto,recvfrom}` docs to the liburing.

RFC v2:
- Rebase the work, now this patchset is based on commit
bb3294e22482db4b7ec ("Merge branch 'for-5.17/drivers' into
for-next").
- In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
call as unlikely.
- Fix build error when CONFIG_NET is undefined.
- Update liburing test (the branch is still the same, just force
pushed).
- Add Nugra to CC list (tester).

## Refs
[1]: https://github.com/axboe/liburing/issues/397
[2]: https://lore.kernel.org/io-uring/[email protected]/

---
RFC v4: https://lore.kernel.org/io-uring/[email protected]/
RFC v3: https://lore.kernel.org/io-uring/[email protected]/
RFC v2: https://lore.kernel.org/io-uring/[email protected]/
RFC v1: https://lore.kernel.org/io-uring/[email protected]/
Signed-off-by: Ammar Faizi <[email protected]>
---
Ammar Faizi (3):
io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
net: Make `move_addr_to_user()` be a non static function
io_uring: Add `sendto(2)` and `recvfrom(2)` support

fs/io_uring.c | 95 +++++++++++++++++++++++++++++++----
include/linux/socket.h | 2 +
include/uapi/linux/io_uring.h | 5 +-
net/socket.c | 4 +-
4 files changed, 93 insertions(+), 13 deletions(-)


base-commit: f6133fbd373811066c8441737e65f384c8f31974
--
Ammar Faizi


2022-02-01 07:52:05

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH for-5.18 v1 0/3] Add `sendto(2)` and `recvfrom(2)` support

On 1/29/22 5:50 AM, Ammar Faizi wrote:
> Hello,
>
> This patchset adds sendto(2) and recvfrom(2) support for io_uring. It
> also addresses an issue in the liburing GitHub repository [1].
>
> ## Motivations:
>
> 1) By using `sendto()` and `recvfrom()` we can make the submission
> simpler compared to always using `sendmsg()` and `recvmsg()` from
> the userspace. Especially for UDP socket.
>
> 2) There is a historical patch that tried to add the same
> functionality, but did not end up being applied. [2]

As far as I can tell, the only win from sendto/recvfrom is that we can
handle async offload a bit cheaper compared to sendmsg/recvmsg. Is this
enough to warrant adding them separately? I don't know, which is why
this has been somewhat stalled for a while.

Maybe you have done some testing and have numbers (or other reasons) to
back up the submission? There's not a whole lot of justification in this
patchset.

--
Jens Axboe

2022-02-01 08:58:35

by Ammar Faizi

[permalink] [raw]
Subject: Re: [PATCH for-5.18 v1 0/3] Add `sendto(2)` and `recvfrom(2)` support

On 1/30/22 1:32 AM, Jens Axboe wrote:
> On 1/29/22 5:50 AM, Ammar Faizi wrote:
>> Hello,
>>
>> This patchset adds sendto(2) and recvfrom(2) support for io_uring. It
>> also addresses an issue in the liburing GitHub repository [1].
>>
>> ## Motivations:
>>
>> 1) By using `sendto()` and `recvfrom()` we can make the submission
>> simpler compared to always using `sendmsg()` and `recvmsg()` from
>> the userspace. Especially for UDP socket.
>>
>> 2) There is a historical patch that tried to add the same
>> functionality, but did not end up being applied. [2]
>
> As far as I can tell, the only win from sendto/recvfrom is that we can
> handle async offload a bit cheaper compared to sendmsg/recvmsg. Is this
> enough to warrant adding them separately? I don't know, which is why
> this has been somewhat stalled for a while.
>
> Maybe you have done some testing and have numbers (or other reasons) to
> back up the submission? There's not a whole lot of justification in this
> patchset.
>

So far, I haven't done it. I only created a test that ensures the
functionality is working properly.

I will play with this further. If I win, I will submit the v2 of
this series for review. Thanks, Jens!

--
Ammar Faizi