From: Willem de Bruijn <[email protected]>
To: Jens Axboe <[email protected]>,
Willem de Bruijn <[email protected]>,
David Ahern <[email protected]>,
Breno Leitao <[email protected]>
Cc: Willem de Bruijn <[email protected]>,
[email protected], [email protected],
[email protected], [email protected], [email protected],
[email protected], [email protected], [email protected],
[email protected], [email protected],
[email protected], [email protected],
[email protected]
Subject: Re: [PATCH 0/5] add initial io_uring_cmd support for sockets
Date: Tue, 11 Apr 2023 11:00:24 -0400 [thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
Jens Axboe wrote:
> On 4/11/23 8:51?AM, Willem de Bruijn wrote:
> > Jens Axboe wrote:
> >> On 4/11/23 8:36?AM, David Ahern wrote:
> >>> On 4/11/23 6:00 AM, Breno Leitao wrote:
> >>>> I am not sure if avoiding io_uring details in network code is possible.
> >>>>
> >>>> The "struct proto"->uring_cmd callback implementation (tcp_uring_cmd()
> >>>> in the TCP case) could be somewhere else, such as in the io_uring/
> >>>> directory, but, I think it might be cleaner if these implementations are
> >>>> closer to function assignment (in the network subsystem).
> >>>>
> >>>> And this function (tcp_uring_cmd() for instance) is the one that I am
> >>>> planning to map io_uring CMDs to ioctls. Such as SOCKET_URING_OP_SIOCINQ
> >>>> -> SIOCINQ.
> >>>>
> >>>> Please let me know if you have any other idea in mind.
> >>>
> >>> I am not convinced that this io_uring_cmd is needed. This is one
> >>> in-kernel subsystem calling into another, and there are APIs for that.
> >>> All of this set is ioctl based and as Willem noted a little refactoring
> >>> separates the get_user/put_user out so that in-kernel can call can be
> >>> made with existing ops.
> >>
> >> How do you want to wire it up then? We can't use fops->unlocked_ioctl()
> >> obviously, and we already have ->uring_cmd() for this purpose.
> >
> > Does this suggestion not work?
>
> Not sure I follow, what suggestion?
>
This quote from earlier in the thread:
I was thinking just having sock_uring_cmd call sock->ops->ioctl, like
sock_do_ioctl.
> >
> >> I do think the right thing to do is have a common helper that returns
> >> whatever value you want (or sets it), and split the ioctl parts into a
> >> wrapper around that that simply copies in/out as needed. Then
> >> ->uring_cmd() could call that, or you could some exported function that
> >> does supports that.
> >>
> >> This works for the basic cases, though I do suspect we'll want to go
> >> down the ->uring_cmd() at some point for more advanced cases or cases
> >> that cannot sanely be done in an ioctl fashion.
> >
> > Right now the two examples are ioctls that return an integer. Do you
> > already have other calls in mind? That would help estimate whether
> > ->uring_cmd() indeed will be needed and we might as well do it now.
>
> Right, it's a proof of concept. But we'd want to support anything that
> setsockopt/getsockopt would do. This is necessary so that direct
> descriptors (eg ones that describe a struct file that isn't in the
> process file table or have a regular fd) can be used for anything that a
> regular file can. Beyond that, perhaps various things necessary for
> efficient zero copy rx.
>
> I do think we can make the ->uring_cmd() hookup a bit more palatable in
> terms of API. It really should be just a sub-opcode and then arguments
> to support that. The grunt of the work is really refactoring the ioctl
> and set/getsockopt bits so that they can be called in-kernel rather than
> assuming copy in/out is needed. Once that is done, the actual uring_cmd
> hookup should be simple and trivial.
That sounds like what I proposed above. That suggestion was only for
the narrow case where ioctls return an integer. The general approach
has to handle any put_user.
Though my initial skim of TCP, UDP and RAW did not bring up any other
forms.
getsockopt indeed has plenty of examples, such as receive zerocopy.
next prev parent reply other threads:[~2023-04-11 15:00 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-06 14:43 [PATCH 0/5] add initial io_uring_cmd support for sockets Breno Leitao
2023-04-06 14:43 ` [RFC PATCH 1/4] net: wire up support for file_operations->uring_cmd() Breno Leitao
2023-04-06 14:43 ` [RFC PATCH 2/4] net: add uring_cmd callback to UDP Breno Leitao
2023-04-11 12:54 ` Pavel Begunkov
2023-04-06 14:43 ` [RFC PATCH 3/4] net: add uring_cmd callback to TCP Breno Leitao
2023-04-06 14:43 ` [RFC PATCH 4/4] net: add uring_cmd callback to raw "protocol" Breno Leitao
2023-04-06 15:34 ` [PATCH 0/5] add initial io_uring_cmd support for sockets Willem de Bruijn
2023-04-06 15:59 ` Breno Leitao
2023-04-06 18:16 ` Willem de Bruijn
2023-04-07 2:46 ` David Ahern
2023-04-11 12:00 ` Breno Leitao
2023-04-11 14:36 ` David Ahern
2023-04-11 14:41 ` Jens Axboe
2023-04-11 14:51 ` Willem de Bruijn
2023-04-11 14:54 ` Jens Axboe
2023-04-11 15:00 ` Willem de Bruijn [this message]
2023-04-11 15:06 ` Jens Axboe
2023-04-11 15:24 ` Willem de Bruijn
2023-04-11 15:28 ` Jens Axboe
2023-04-12 13:53 ` Breno Leitao
2023-04-12 14:28 ` Willem de Bruijn
2023-04-13 0:02 ` Breno Leitao
2023-04-13 14:24 ` Willem de Bruijn
2023-04-13 14:45 ` Jakub Kicinski
2023-04-13 14:57 ` David Laight
2023-04-18 13:23 ` Breno Leitao
2023-04-18 19:41 ` Willem de Bruijn
2023-04-20 14:43 ` Breno Leitao
2023-04-20 16:48 ` Willem de Bruijn
2023-04-11 15:10 ` David Ahern
2023-04-11 15:17 ` Jens Axboe
2023-04-11 15:27 ` David Ahern
2023-04-11 15:29 ` Jens Axboe
2023-04-12 7:39 ` David Laight
2023-04-06 16:41 ` Keith Busch
2023-04-06 16:49 ` Jens Axboe
2023-04-06 16:58 ` Breno Leitao
2023-04-06 16:57 ` [PATCH RFC] io_uring: Pass whole sqe to commands Breno Leitao
2023-04-07 18:51 ` Keith Busch
2023-04-11 12:22 ` Breno Leitao
2023-04-11 12:39 ` Pavel Begunkov
2023-04-13 2:56 ` Ming Lei
2023-04-13 16:47 ` Breno Leitao
2023-04-14 2:12 ` Ming Lei
2023-04-14 13:12 ` Pavel Begunkov
2023-04-14 13:59 ` Ming Lei
2023-04-14 14:56 ` Pavel Begunkov
2023-04-16 9:51 ` Ming Lei
-- strict thread matches above, loose matches on Subject: below --
2023-05-02 9:21 [PATCH 0/5] add initial io_uring_cmd support for sockets Adrien Delorme
2023-05-02 13:03 ` Pavel Begunkov
2023-05-03 13:11 ` Adrien Delorme
2023-05-03 13:27 ` David Laight
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=64357608c396d_113ebd294ba@willemb.c.googlers.com.notmuch \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
[email protected] \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox