public inbox for [email protected]
 help / color / mirror / Atom feed
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.


  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