[PATCH 0/5] add initial io_uring_cmd support for sockets

[PATCH 0/5] add initial io_uring_cmd support for sockets

[Breno Leitao]

From: Breno Leitao <leit@fb.com>

This patchset creates the initial plumbing for a io_uring command for
sockets.

For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
heavily based on the ioctl operations.

In order to test this code, I created a liburing test, which is
currently located at [1], and I will create a pull request once we are
good with this patch.

I've also run test/io_uring_passthrough to make sure the first patch
didn't regressed the NVME passthrough path.

This patchset is a RFC for two different reasons:
  * It changes slighlty on how IO uring command operates. I.e, we are
    now passing the whole SQE to the io_uring_cmd callback (instead of
    an opaque buffer). This seems to be more palatable instead of
    creating some custom structure just to fit small parameters, as in
    SOCKET_URING_OP_SIOC{IN,OUT}Q. Is this OK?

  * Pavel has some ideas about the SQE->cmd_op field, so, we can start
    discussing it here.

This work is heavily inspired by Jens Axboe's initial implementation.

[1] https://github.com/leitao/liburing/blob/master/test/socket-io-cmd.c

Breno Leitao (4):
  net: wire up support for file_operations->uring_cmd()
  net: add uring_cmd callback to UDP
  net: add uring_cmd callback to TCP
  net: add uring_cmd callback to raw "protocol"

 include/linux/net.h      |  2 ++
 include/net/raw.h        |  3 +++
 include/net/sock.h       |  6 ++++++
 include/net/tcp.h        |  2 ++
 include/net/udp.h        |  2 ++
 include/uapi/linux/net.h |  5 +++++
 net/core/sock.c          | 17 +++++++++++++++--
 net/dccp/ipv4.c          |  1 +
 net/ipv4/af_inet.c       |  3 +++
 net/ipv4/raw.c           | 26 ++++++++++++++++++++++++++
 net/ipv4/tcp.c           | 34 ++++++++++++++++++++++++++++++++++
 net/ipv4/tcp_ipv4.c      |  1 +
 net/ipv4/udp.c           | 18 ++++++++++++++++++
 net/l2tp/l2tp_ip.c       |  1 +
 net/mptcp/protocol.c     |  1 +
 net/sctp/protocol.c      |  1 +
 net/socket.c             | 13 +++++++++++++
 17 files changed, 134 insertions(+), 2 deletions(-)

-- 
2.34.1

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

On Thu, Apr 6, 2023 at 10:45 AM Breno Leitao <leitao@debian.org> wrote:
>
> From: Breno Leitao <leit@fb.com>
>
> This patchset creates the initial plumbing for a io_uring command for
> sockets.
>
> For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
> and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
> SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
> heavily based on the ioctl operations.

This duplicates all the existing ioctl logic of each protocol.

Can this just call the existing proto_ops.ioctl internally and translate from/to
io_uring format as needed?

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Breno Leitao]

On Thu, Apr 06, 2023 at 11:34:28AM -0400, Willem de Bruijn wrote:
> On Thu, Apr 6, 2023 at 10:45 AM Breno Leitao <leitao@debian.org> wrote:
> >
> > From: Breno Leitao <leit@fb.com>
> >
> > This patchset creates the initial plumbing for a io_uring command for
> > sockets.
> >
> > For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
> > and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
> > SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
> > heavily based on the ioctl operations.
> 
> This duplicates all the existing ioctl logic of each protocol.
> 
> Can this just call the existing proto_ops.ioctl internally and translate from/to
> io_uring format as needed?

This is doable, and we have two options in this case:

1) Create a ioctl core function that does not call `put_user()`, and
call it from both the `udp_ioctl` and `udp_uring_cmd`, doing the proper
translations. Something as:

	int udp_ioctl_core(struct sock *sk, int cmd, unsigned long arg)
	{
		int amount;
		switch (cmd) {
		case SIOCOUTQ: {
			amount = sk_wmem_alloc_get(sk);
			break;
		}
		case SIOCINQ: {
			amount = max_t(int, 0, first_packet_length(sk));
			break;
		}
		default:
			return -ENOIOCTLCMD;
		}
		return amount;
	}

	int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
	{
		int amount = udp_ioctl_core(sk, cmd, arg);

		return put_user(amount, (int __user *)arg);
	}
	EXPORT_SYMBOL(udp_ioctl);


2) Create a function for each "case entry". This seems a bit silly for
UDP, but it makes more sense for other protocols. The code will look
something like:

	 int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
	 {
		switch (cmd) {
		case SIOCOUTQ:
		{
			int amount = udp_ioctl_siocoutq();
			return put_user(amount, (int __user *)arg);
		}
		...
	  }

What is the best approach?

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Keith Busch]

On Thu, Apr 06, 2023 at 07:43:26AM -0700, Breno Leitao wrote:
> This patchset creates the initial plumbing for a io_uring command for
> sockets.
> 
> For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
> and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
> SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
> heavily based on the ioctl operations.

Do you have asynchronous operations in mind for a future patch? The io_uring
command infrastructure makes more sense for operations that return EIOCBQUEUED,
otherwise it doesn't have much benefit over ioctl.
 
> In order to test this code, I created a liburing test, which is
> currently located at [1], and I will create a pull request once we are
> good with this patch.
> 
> I've also run test/io_uring_passthrough to make sure the first patch
> didn't regressed the NVME passthrough path.
> 
> This patchset is a RFC for two different reasons:
>   * It changes slighlty on how IO uring command operates. I.e, we are
>     now passing the whole SQE to the io_uring_cmd callback (instead of
>     an opaque buffer). This seems to be more palatable instead of
>     creating some custom structure just to fit small parameters, as in
>     SOCKET_URING_OP_SIOC{IN,OUT}Q. Is this OK?

I think I'm missing something from this series. Where is the io_uring_cmd
change to point to the sqe?

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Jens Axboe]

On 4/6/23 10:41?AM, Keith Busch wrote:
> On Thu, Apr 06, 2023 at 07:43:26AM -0700, Breno Leitao wrote:
>> This patchset creates the initial plumbing for a io_uring command for
>> sockets.
>>
>> For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
>> and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
>> SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
>> heavily based on the ioctl operations.
> 
> Do you have asynchronous operations in mind for a future patch? The
> io_uring command infrastructure makes more sense for operations that
> return EIOCBQUEUED, otherwise it doesn't have much benefit over ioctl.

Basically nothing returns EIOCBQUEUED, it's mostly sync/poll driven on
the networking side. The primary use case for this is with direct
descriptors, as you can't do get/setsockopt with those. And that means
you'd then need to instantiate a regular descriptor first and then
register it, rather than keep it all direct from the start.

-- 
Jens Axboe

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Breno Leitao]

Hello Keith,

On Thu, Apr 06, 2023 at 10:41:52AM -0600, Keith Busch wrote:
> On Thu, Apr 06, 2023 at 07:43:26AM -0700, Breno Leitao wrote:
> > This patchset creates the initial plumbing for a io_uring command for
> > sockets.
> > 
> > For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
> > and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
> > SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
> > heavily based on the ioctl operations.
> 
> Do you have asynchronous operations in mind for a future patch? The io_uring
> command infrastructure makes more sense for operations that return EIOCBQUEUED,
> otherwise it doesn't have much benefit over ioctl.

I think this brings value even for synchronos operations, such as, you
can just keep using io_uring operations on network operations, other
than, using some io_uring operations and then doing a regular ioctl(2).
So, it improves the user experience.

The other benefit is calling several operations at a single io_uring
submit. It means you can save several syscalls and getting the same work
done.

>  
> > In order to test this code, I created a liburing test, which is
> > currently located at [1], and I will create a pull request once we are
> > good with this patch.
> > 
> > I've also run test/io_uring_passthrough to make sure the first patch
> > didn't regressed the NVME passthrough path.
> > 
> > This patchset is a RFC for two different reasons:
> >   * It changes slighlty on how IO uring command operates. I.e, we are
> >     now passing the whole SQE to the io_uring_cmd callback (instead of
> >     an opaque buffer). This seems to be more palatable instead of
> >     creating some custom structure just to fit small parameters, as in
> >     SOCKET_URING_OP_SIOC{IN,OUT}Q. Is this OK?
> 
> I think I'm missing something from this series. Where is the io_uring_cmd
> change to point to the sqe?

My bad, the patch was not part of the patchset. I've just submitted it
under the same RFC cover letter now.

Here is the link, if it helps:

https://lkml.org/lkml/2023/4/6/990

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

On Thu, Apr 6, 2023 at 11:59 AM Breno Leitao <leitao@debian.org> wrote:
>
> On Thu, Apr 06, 2023 at 11:34:28AM -0400, Willem de Bruijn wrote:
> > On Thu, Apr 6, 2023 at 10:45 AM Breno Leitao <leitao@debian.org> wrote:
> > >
> > > From: Breno Leitao <leit@fb.com>
> > >
> > > This patchset creates the initial plumbing for a io_uring command for
> > > sockets.
> > >
> > > For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
> > > and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
> > > SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
> > > heavily based on the ioctl operations.
> >
> > This duplicates all the existing ioctl logic of each protocol.
> >
> > Can this just call the existing proto_ops.ioctl internally and translate from/to
> > io_uring format as needed?
>
> This is doable, and we have two options in this case:
>
> 1) Create a ioctl core function that does not call `put_user()`, and
> call it from both the `udp_ioctl` and `udp_uring_cmd`, doing the proper
> translations. Something as:
>
>         int udp_ioctl_core(struct sock *sk, int cmd, unsigned long arg)
>         {
>                 int amount;
>                 switch (cmd) {
>                 case SIOCOUTQ: {
>                         amount = sk_wmem_alloc_get(sk);
>                         break;
>                 }
>                 case SIOCINQ: {
>                         amount = max_t(int, 0, first_packet_length(sk));
>                         break;
>                 }
>                 default:
>                         return -ENOIOCTLCMD;
>                 }
>                 return amount;
>         }
>
>         int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
>         {
>                 int amount = udp_ioctl_core(sk, cmd, arg);
>
>                 return put_user(amount, (int __user *)arg);
>         }
>         EXPORT_SYMBOL(udp_ioctl);
>
>
> 2) Create a function for each "case entry". This seems a bit silly for
> UDP, but it makes more sense for other protocols. The code will look
> something like:
>
>          int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
>          {
>                 switch (cmd) {
>                 case SIOCOUTQ:
>                 {
>                         int amount = udp_ioctl_siocoutq();
>                         return put_user(amount, (int __user *)arg);
>                 }
>                 ...
>           }
>
> What is the best approach?

A, the issue is that sock->ops->ioctl directly call put_user.

I was thinking just having sock_uring_cmd call sock->ops->ioctl, like
sock_do_ioctl.

But that would require those callbacks to return a negative error or
positive integer, rather than calling put_user. And then move the
put_user to sock_do_ioctl. Such a change is at least as much code
change as your series. Though without the ending up with code
duplication. It also works only if all ioctls only put_user of integer
size. That's true for TCP, UDP and RAW, but not sure if true more
broadly.

Another approach may be to pass another argument to the ioctl
callbacks, whether to call put_user or return the integer and let the
caller take care of the output to user. This could possibly be
embedded in the a high-order bit of the cmd, so that it fails on ioctl
callbacks that do not support this mode.

Of the two approaches you suggest, I find the first preferable.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[David Ahern]

On 4/6/23 12:16 PM, Willem de Bruijn wrote:
> On Thu, Apr 6, 2023 at 11:59 AM Breno Leitao <leitao@debian.org> wrote:
>>
>> On Thu, Apr 06, 2023 at 11:34:28AM -0400, Willem de Bruijn wrote:
>>> On Thu, Apr 6, 2023 at 10:45 AM Breno Leitao <leitao@debian.org> wrote:
>>>>
>>>> From: Breno Leitao <leit@fb.com>
>>>>
>>>> This patchset creates the initial plumbing for a io_uring command for
>>>> sockets.
>>>>
>>>> For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
>>>> and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
>>>> SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
>>>> heavily based on the ioctl operations.
>>>
>>> This duplicates all the existing ioctl logic of each protocol.
>>>
>>> Can this just call the existing proto_ops.ioctl internally and translate from/to
>>> io_uring format as needed?
>>
>> This is doable, and we have two options in this case:
>>
>> 1) Create a ioctl core function that does not call `put_user()`, and
>> call it from both the `udp_ioctl` and `udp_uring_cmd`, doing the proper
>> translations. Something as:
>>
>>         int udp_ioctl_core(struct sock *sk, int cmd, unsigned long arg)
>>         {
>>                 int amount;
>>                 switch (cmd) {
>>                 case SIOCOUTQ: {
>>                         amount = sk_wmem_alloc_get(sk);
>>                         break;
>>                 }
>>                 case SIOCINQ: {
>>                         amount = max_t(int, 0, first_packet_length(sk));
>>                         break;
>>                 }
>>                 default:
>>                         return -ENOIOCTLCMD;
>>                 }
>>                 return amount;
>>         }
>>
>>         int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
>>         {
>>                 int amount = udp_ioctl_core(sk, cmd, arg);
>>
>>                 return put_user(amount, (int __user *)arg);
>>         }
>>         EXPORT_SYMBOL(udp_ioctl);
>>
>>
>> 2) Create a function for each "case entry". This seems a bit silly for
>> UDP, but it makes more sense for other protocols. The code will look
>> something like:
>>
>>          int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
>>          {
>>                 switch (cmd) {
>>                 case SIOCOUTQ:
>>                 {
>>                         int amount = udp_ioctl_siocoutq();
>>                         return put_user(amount, (int __user *)arg);
>>                 }
>>                 ...
>>           }
>>
>> What is the best approach?
> 
> A, the issue is that sock->ops->ioctl directly call put_user.
> 
> I was thinking just having sock_uring_cmd call sock->ops->ioctl, like
> sock_do_ioctl.
> 
> But that would require those callbacks to return a negative error or
> positive integer, rather than calling put_user. And then move the
> put_user to sock_do_ioctl. Such a change is at least as much code
> change as your series. Though without the ending up with code
> duplication. It also works only if all ioctls only put_user of integer
> size. That's true for TCP, UDP and RAW, but not sure if true more
> broadly.
> 
> Another approach may be to pass another argument to the ioctl
> callbacks, whether to call put_user or return the integer and let the
> caller take care of the output to user. This could possibly be
> embedded in the a high-order bit of the cmd, so that it fails on ioctl
> callbacks that do not support this mode.
> 
> Of the two approaches you suggest, I find the first preferable.

The first approach sounds better to me and it would be good to avoid
io_uring details in the networking code (ie., cmd->sqe->cmd_op).

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Breno Leitao]

On Thu, Apr 06, 2023 at 08:46:38PM -0600, David Ahern wrote:
> On 4/6/23 12:16 PM, Willem de Bruijn wrote:
> > On Thu, Apr 6, 2023 at 11:59 AM Breno Leitao <leitao@debian.org> wrote:
> >>
> >> On Thu, Apr 06, 2023 at 11:34:28AM -0400, Willem de Bruijn wrote:
> >>> On Thu, Apr 6, 2023 at 10:45 AM Breno Leitao <leitao@debian.org> wrote:
> >>>>
> >>>> From: Breno Leitao <leit@fb.com>
> >>>>
> >>>> This patchset creates the initial plumbing for a io_uring command for
> >>>> sockets.
> >>>>
> >>>> For now, create two uring commands for sockets, SOCKET_URING_OP_SIOCOUTQ
> >>>> and SOCKET_URING_OP_SIOCINQ. They are similar to ioctl operations
> >>>> SIOCOUTQ and SIOCINQ. In fact, the code on the protocol side itself is
> >>>> heavily based on the ioctl operations.
> >>>
> >>> This duplicates all the existing ioctl logic of each protocol.
> >>>
> >>> Can this just call the existing proto_ops.ioctl internally and translate from/to
> >>> io_uring format as needed?
> >>
> >> This is doable, and we have two options in this case:
> >>
> >> 1) Create a ioctl core function that does not call `put_user()`, and
> >> call it from both the `udp_ioctl` and `udp_uring_cmd`, doing the proper
> >> translations. Something as:
> >>
> >>         int udp_ioctl_core(struct sock *sk, int cmd, unsigned long arg)
> >>         {
> >>                 int amount;
> >>                 switch (cmd) {
> >>                 case SIOCOUTQ: {
> >>                         amount = sk_wmem_alloc_get(sk);
> >>                         break;
> >>                 }
> >>                 case SIOCINQ: {
> >>                         amount = max_t(int, 0, first_packet_length(sk));
> >>                         break;
> >>                 }
> >>                 default:
> >>                         return -ENOIOCTLCMD;
> >>                 }
> >>                 return amount;
> >>         }
> >>
> >>         int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
> >>         {
> >>                 int amount = udp_ioctl_core(sk, cmd, arg);
> >>
> >>                 return put_user(amount, (int __user *)arg);
> >>         }
> >>         EXPORT_SYMBOL(udp_ioctl);
> >>
> >>
> >> 2) Create a function for each "case entry". This seems a bit silly for
> >> UDP, but it makes more sense for other protocols. The code will look
> >> something like:
> >>
> >>          int udp_ioctl(struct sock *sk, int cmd, unsigned long arg)
> >>          {
> >>                 switch (cmd) {
> >>                 case SIOCOUTQ:
> >>                 {
> >>                         int amount = udp_ioctl_siocoutq();
> >>                         return put_user(amount, (int __user *)arg);
> >>                 }
> >>                 ...
> >>           }
> >>
> >> What is the best approach?
> > 
> > A, the issue is that sock->ops->ioctl directly call put_user.
> > 
> > I was thinking just having sock_uring_cmd call sock->ops->ioctl, like
> > sock_do_ioctl.
> > 
> > But that would require those callbacks to return a negative error or
> > positive integer, rather than calling put_user. And then move the
> > put_user to sock_do_ioctl. Such a change is at least as much code
> > change as your series. Though without the ending up with code
> > duplication. It also works only if all ioctls only put_user of integer
> > size. That's true for TCP, UDP and RAW, but not sure if true more
> > broadly.
> > 
> > Another approach may be to pass another argument to the ioctl
> > callbacks, whether to call put_user or return the integer and let the
> > caller take care of the output to user. This could possibly be
> > embedded in the a high-order bit of the cmd, so that it fails on ioctl
> > callbacks that do not support this mode.
> > 
> > Of the two approaches you suggest, I find the first preferable.
> 
> The first approach sounds better to me and it would be good to avoid
> io_uring details in the networking code (ie., cmd->sqe->cmd_op).

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.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[David Ahern]

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.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Jens Axboe]

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.

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.

-- 
Jens Axboe

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

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?

> > 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.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Jens Axboe]

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?

>>> 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.

-- 
Jens Axboe

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

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.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Jens Axboe]

On 4/11/23 9:00?AM, Willem de Bruijn wrote:
> 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.

But that doesn't work, because sock->ops->ioctl() assumes the arg is
memory in userspace. Or do you mean change all of the sock->ops->ioctl()
to pass in on-stack memory (or similar) and have it work with a kernel
address?

>>>> 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.

Right

> 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.

-- 
Jens Axboe

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[David Ahern]

On 4/11/23 8:41 AM, 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.
> 
> 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.
> 

My meta point is that there are uapis today to return this information
to applications (and I suspect this is just the start of more networking
changes - both data retrieval and adjusting settings). io_uring is
wanting to do this on behalf of the application without a syscall. That
makes io_uring yet another subsystem / component managing a socket. Any
change to the networking stack required by io_uring should be usable by
all other in-kernel socket owners or managers. ie., there is no reason
for io_uring specific code here.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Jens Axboe]

On 4/11/23 9:10?AM, David Ahern wrote:
> On 4/11/23 8:41 AM, 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.
>>
>> 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.
>>
> 
> My meta point is that there are uapis today to return this information
> to applications (and I suspect this is just the start of more networking
> changes - both data retrieval and adjusting settings). io_uring is
> wanting to do this on behalf of the application without a syscall. That
> makes io_uring yet another subsystem / component managing a socket. Any
> change to the networking stack required by io_uring should be usable by
> all other in-kernel socket owners or managers. ie., there is no reason
> for io_uring specific code here.

I think we are in violent agreement here, what I'm describing is exactly
that - it'd make ioctl/{set,get}sockopt call into the same helpers that
->uring_cmd() would, with the only difference being that the former
would need copy in/out and the latter would not.

But let me just stress that for direct descriptors, we cannot currently
call ioctl or set/getsockopt. This means we have to instantiate a
regular descriptor first, do those things, then register it to never use
the regular file descriptor again. That's wasteful, and this is what we
want to enable (direct use of ioctl set/getsockopt WITHOUT a normal file
descriptor). It's not just for "oh it'd be handy to also do this from
io_uring" even if that would be a worthwhile goal in itself.

-- 
Jens Axboe

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

Jens Axboe wrote:
> On 4/11/23 9:00?AM, Willem de Bruijn wrote:
> > 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.
> 
> But that doesn't work, because sock->ops->ioctl() assumes the arg is
> memory in userspace. Or do you mean change all of the sock->ops->ioctl()
> to pass in on-stack memory (or similar) and have it work with a kernel
> address?

That was what I suggested indeed.

It's about as much code change as this patch series. But it avoids
the code duplication.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[David Ahern]

On 4/11/23 9:17 AM, Jens Axboe wrote:
> On 4/11/23 9:10?AM, David Ahern wrote:
>> On 4/11/23 8:41 AM, 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.
>>>
>>> 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.
>>>
>>
>> My meta point is that there are uapis today to return this information
>> to applications (and I suspect this is just the start of more networking
>> changes - both data retrieval and adjusting settings). io_uring is
>> wanting to do this on behalf of the application without a syscall. That
>> makes io_uring yet another subsystem / component managing a socket. Any
>> change to the networking stack required by io_uring should be usable by
>> all other in-kernel socket owners or managers. ie., there is no reason
>> for io_uring specific code here.
> 
> I think we are in violent agreement here, what I'm describing is exactly
> that - it'd make ioctl/{set,get}sockopt call into the same helpers that
> ->uring_cmd() would, with the only difference being that the former
> would need copy in/out and the latter would not.
> 
> But let me just stress that for direct descriptors, we cannot currently
> call ioctl or set/getsockopt. This means we have to instantiate a
> regular descriptor first, do those things, then register it to never use
> the regular file descriptor again. That's wasteful, and this is what we
> want to enable (direct use of ioctl set/getsockopt WITHOUT a normal file
> descriptor). It's not just for "oh it'd be handy to also do this from
> io_uring" even if that would be a worthwhile goal in itself.
> 

Christoph's patch set a few years back that removed set_fs broke the
ability to do in-kernel ioctl and {s,g}setsockopt calls. I did not
follow that change; was it a deliberate intent to not allow these
in-kernel calls vs wanting to remove the set_fs? e.g., can we add a
kioctl variant for in-kernel use of the APIs?

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Jens Axboe]

On 4/11/23 9:24?AM, Willem de Bruijn wrote:
> Jens Axboe wrote:
>> On 4/11/23 9:00?AM, Willem de Bruijn wrote:
>>> 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.
>>
>> But that doesn't work, because sock->ops->ioctl() assumes the arg is
>> memory in userspace. Or do you mean change all of the sock->ops->ioctl()
>> to pass in on-stack memory (or similar) and have it work with a kernel
>> address?
> 
> That was what I suggested indeed.
> 
> It's about as much code change as this patch series. But it avoids
> the code duplication.

Breno, want to tackle that as a prep patch first? Should make the
functional changes afterwards much more straightforward, and will allow
support for anything really.

-- 
Jens Axboe

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Jens Axboe]

On 4/11/23 9:27?AM, David Ahern wrote:
> On 4/11/23 9:17 AM, Jens Axboe wrote:
>> On 4/11/23 9:10?AM, David Ahern wrote:
>>> On 4/11/23 8:41 AM, 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.
>>>>
>>>> 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.
>>>>
>>>
>>> My meta point is that there are uapis today to return this information
>>> to applications (and I suspect this is just the start of more networking
>>> changes - both data retrieval and adjusting settings). io_uring is
>>> wanting to do this on behalf of the application without a syscall. That
>>> makes io_uring yet another subsystem / component managing a socket. Any
>>> change to the networking stack required by io_uring should be usable by
>>> all other in-kernel socket owners or managers. ie., there is no reason
>>> for io_uring specific code here.
>>
>> I think we are in violent agreement here, what I'm describing is exactly
>> that - it'd make ioctl/{set,get}sockopt call into the same helpers that
>> ->uring_cmd() would, with the only difference being that the former
>> would need copy in/out and the latter would not.
>>
>> But let me just stress that for direct descriptors, we cannot currently
>> call ioctl or set/getsockopt. This means we have to instantiate a
>> regular descriptor first, do those things, then register it to never use
>> the regular file descriptor again. That's wasteful, and this is what we
>> want to enable (direct use of ioctl set/getsockopt WITHOUT a normal file
>> descriptor). It's not just for "oh it'd be handy to also do this from
>> io_uring" even if that would be a worthwhile goal in itself.
>>
> 
> Christoph's patch set a few years back that removed set_fs broke the
> ability to do in-kernel ioctl and {s,g}setsockopt calls. I did not
> follow that change; was it a deliberate intent to not allow these
> in-kernel calls vs wanting to remove the set_fs? e.g., can we add a
> kioctl variant for in-kernel use of the APIs?

I think it'd be much better to cleanly split it out rather than try and
hack around it.

-- 
Jens Axboe

RE: [PATCH 0/5] add initial io_uring_cmd support for sockets

[David Laight]

RnJvbTogRGF2aWQgQWhlcm4NCj4gU2VudDogMTEgQXByaWwgMjAyMyAxNjoyOA0KLi4uLg0KPiBD
aHJpc3RvcGgncyBwYXRjaCBzZXQgYSBmZXcgeWVhcnMgYmFjayB0aGF0IHJlbW92ZWQgc2V0X2Zz
IGJyb2tlIHRoZQ0KPiBhYmlsaXR5IHRvIGRvIGluLWtlcm5lbCBpb2N0bCBhbmQge3MsZ31zZXRz
b2Nrb3B0IGNhbGxzLiBJIGRpZCBub3QNCj4gZm9sbG93IHRoYXQgY2hhbmdlOyB3YXMgaXQgYSBk
ZWxpYmVyYXRlIGludGVudCB0byBub3QgYWxsb3cgdGhlc2UNCj4gaW4ta2VybmVsIGNhbGxzIHZz
IHdhbnRpbmcgdG8gcmVtb3ZlIHRoZSBzZXRfZnM/IGUuZy4sIGNhbiB3ZSBhZGQgYQ0KPiBraW9j
dGwgdmFyaWFudCBmb3IgaW4ta2VybmVsIHVzZSBvZiB0aGUgQVBJcz8NCg0KSSB0aGluayB0aGF0
IHdhcyBhIHNpZGUgZWZmZWN0LCBhbmQgd2l0aCBubyBpbi10cmVlIGluLWtlcm5lbA0KdXNlcnMg
KGFwYXJ0IGZyb20gbGltaXRlZCBjYWxscyBpbiBicGYpIGl0IHdhcyBkZWVtZWQgYWNjZXB0YWJs
ZS4NCihJdCBpcyBhIFBJVEEgZm9yIGFueSBjb2RlIHRyeWluZyB0byB1c2UgU0NUUCBpbiBrZXJu
ZWwuKQ0KDQpPbmUgcHJvYmxlbSBpcyB0aGF0IG5vdCBhbGwgc29ja29wdCBjYWxscyBwYXNzIHRo
ZSBjb3JyZWN0IGxlbmd0aC4NCkFuZCBzb21lIG9mIHRoZW0gY2FuIGhhdmUgdmVyeSBsb25nIGJ1
ZmZlcnMuDQpOb3QgdG8gbWVudGlvbiB0aGUgb25lcyB0aGF0IGFyZSByZWFkLW1vZGlmeS13cml0
ZS4NCg0KQSBwbGF1c2libGUgc29sdXRpb24gaXMgdG8gcGFzcyBhICdmYXQgcG9pbnRlcicgdGhh
dCBjb250YWlucw0Kc29tZSwgb3IgYWxsLCBvZjoNCgktIEEgdXNlcnNwYWNlIGJ1ZmZlciBwb2lu
dGVyLg0KCS0gQSBrZXJuZWwgYnVmZmVyIHBvaW50ZXIuDQoJLSBUaGUgbGVuZ3RoIHN1cHBsaWVk
IGJ5IHRoZSB1c2VyLg0KCS0gVGhlIGxlbmd0aCBvZiB0aGUga2VybmVsIGJ1ZmZlci4NCgk9IFRo
ZSBudW1iZXIgb2YgYnl0ZXMgdG8gY29weSBvbiBjb21wbGV0aW9uLg0KRm9yIHNpbXBsZSB1c2Vy
IHJlcXVlc3RzIHRoZSBzeXNjYWxsIGVudHJ5L2V4aXQgY29kZQ0Kd291bGQgY29weSB0aGUgZGF0
YSB0byBhIHNob3J0IG9uLXN0YWNrIGJ1ZmZlci4NCktlcm5lbCB1c2VycyBqdXN0IHBhc3MgdGhl
IGtlcm5lbCBhZGRyZXNzLg0KT2RkIHJlcXVlc3RzIGNhbiBqdXN0IHVzZSB0aGUgdXNlciBwb2lu
dGVyLg0KDQpQcm9iYWJseSBuZWVkcyBhY2Nlc3NvcnMgdGhhdCBhZGQgaW4gYW4gb2Zmc2V0Lg0K
DQpJdCBtaWdodCBhbHNvIGJlIHRoYXQgc29tZSBvZiB0aGUgcHJvYmxlbWF0aWMgc29ja29wdA0K
d2VyZSBpbiBkZWNuZXQgLSBub3cgcmVtb3ZlZC4NCg0KCURhdmlkDQoNCi0NClJlZ2lzdGVyZWQg
QWRkcmVzcyBMYWtlc2lkZSwgQnJhbWxleSBSb2FkLCBNb3VudCBGYXJtLCBNaWx0b24gS2V5bmVz
LCBNSzEgMVBULCBVSw0KUmVnaXN0cmF0aW9uIE5vOiAxMzk3Mzg2IChXYWxlcykNCg=

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Breno Leitao]

On Tue, Apr 11, 2023 at 09:28:29AM -0600, Jens Axboe wrote:
> On 4/11/23 9:24?AM, Willem de Bruijn wrote:
> > Jens Axboe wrote:
> >> On 4/11/23 9:00?AM, Willem de Bruijn wrote:
> >> But that doesn't work, because sock->ops->ioctl() assumes the arg is
> >> memory in userspace. Or do you mean change all of the sock->ops->ioctl()
> >> to pass in on-stack memory (or similar) and have it work with a kernel
> >> address?
> > 
> > That was what I suggested indeed.
> > 
> > It's about as much code change as this patch series. But it avoids
> > the code duplication.
> 
> Breno, want to tackle that as a prep patch first? Should make the
> functional changes afterwards much more straightforward, and will allow
> support for anything really.

Absolutely. I just want to make sure that I got the proper approach that
we agreed here.

Let me explain what I understood taking TCP as an example:

1) Rename tcp_ioctl() to something as _tcp_ioctl() where the 'arg'
argument is now just a kernel memory (located in the stack frame from the
callee).

2) Recreate "tcp_ioctl()" that will basically allocate a 'arg' in the
stack and call _tcp_ioctl() passing that 'arg' argument. At the bottom of
this (tcp_ioctl() function) function, call `put_user(in_kernel_arg, userspace_arg)

3) Repeat it for the 20 protocols that implement ioctl:

	ag  "struct proto .* = {" -A 20 net/ | grep \.ioctl
	net/dccp/ipv6.c 	.ioctl	= dccp_ioctl,
	net/dccp/ipv4.c		.ioctl	= dccp_ioctl,
	net/ieee802154/socket.c .ioctl	= dgram_ioctl,
	net/ipv4/udplite.c	.ioctl	= udp_ioctl,
	net/ipv4/raw.c 		.ioctl	= raw_ioctl,
	net/ipv4/udp.c		.ioctl	= udp_ioctl,
	net/ipv4/tcp_ipv4.c 	.ioctl	= tcp_ioctl,
	net/ipv6/raw.c		.ioctl	= rawv6_ioctl,
	net/ipv6/tcp_ipv6.c	.ioctl	= tcp_ioctl,
	net/ipv6/udp.c	 	.ioctl	= udp_ioctl,
	net/ipv6/udplite.c	.ioctl	= udp_ioctl,
	net/l2tp/l2tp_ip6.c	.ioctl	= l2tp_ioctl,
	net/l2tp/l2tp_ip.c	.ioctl	= l2tp_ioctl,
	net/phonet/datagram.:	.ioctl	= pn_ioctl,
	net/phonet/pep.c	.ioctl	= pep_ioctl,
	net/rds/af_rds.c	.ioctl	=	rds_ioctl,
	net/sctp/socket.c	.ioctl  =	sctp_ioctl,
	net/sctp/socket.c	.ioctl	= sctp_ioctl,
	net/xdp/xsk.c		.ioctl	= sock_no_ioctl,
	net/mptcp/protocol.c	.ioctl	= mptcp_ioctl,

Am I missing something?

Thanks!

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

Breno Leitao wrote:
> On Tue, Apr 11, 2023 at 09:28:29AM -0600, Jens Axboe wrote:
> > On 4/11/23 9:24?AM, Willem de Bruijn wrote:
> > > Jens Axboe wrote:
> > >> On 4/11/23 9:00?AM, Willem de Bruijn wrote:
> > >> But that doesn't work, because sock->ops->ioctl() assumes the arg is
> > >> memory in userspace. Or do you mean change all of the sock->ops->ioctl()
> > >> to pass in on-stack memory (or similar) and have it work with a kernel
> > >> address?
> > > 
> > > That was what I suggested indeed.
> > > 
> > > It's about as much code change as this patch series. But it avoids
> > > the code duplication.
> > 
> > Breno, want to tackle that as a prep patch first? Should make the
> > functional changes afterwards much more straightforward, and will allow
> > support for anything really.
> 
> Absolutely. I just want to make sure that I got the proper approach that
> we agreed here.
> 
> Let me explain what I understood taking TCP as an example:
> 
> 1) Rename tcp_ioctl() to something as _tcp_ioctl() where the 'arg'
> argument is now just a kernel memory (located in the stack frame from the
> callee).
> 
> 2) Recreate "tcp_ioctl()" that will basically allocate a 'arg' in the
> stack and call _tcp_ioctl() passing that 'arg' argument. At the bottom of
> this (tcp_ioctl() function) function, call `put_user(in_kernel_arg, userspace_arg)
> 
> 3) Repeat it for the 20 protocols that implement ioctl:
> 
> 	ag  "struct proto .* = {" -A 20 net/ | grep \.ioctl
> 	net/dccp/ipv6.c 	.ioctl	= dccp_ioctl,
> 	net/dccp/ipv4.c		.ioctl	= dccp_ioctl,
> 	net/ieee802154/socket.c .ioctl	= dgram_ioctl,
> 	net/ipv4/udplite.c	.ioctl	= udp_ioctl,
> 	net/ipv4/raw.c 		.ioctl	= raw_ioctl,
> 	net/ipv4/udp.c		.ioctl	= udp_ioctl,
> 	net/ipv4/tcp_ipv4.c 	.ioctl	= tcp_ioctl,
> 	net/ipv6/raw.c		.ioctl	= rawv6_ioctl,
> 	net/ipv6/tcp_ipv6.c	.ioctl	= tcp_ioctl,
> 	net/ipv6/udp.c	 	.ioctl	= udp_ioctl,
> 	net/ipv6/udplite.c	.ioctl	= udp_ioctl,
> 	net/l2tp/l2tp_ip6.c	.ioctl	= l2tp_ioctl,
> 	net/l2tp/l2tp_ip.c	.ioctl	= l2tp_ioctl,
> 	net/phonet/datagram.:	.ioctl	= pn_ioctl,
> 	net/phonet/pep.c	.ioctl	= pep_ioctl,
> 	net/rds/af_rds.c	.ioctl	=	rds_ioctl,
> 	net/sctp/socket.c	.ioctl  =	sctp_ioctl,
> 	net/sctp/socket.c	.ioctl	= sctp_ioctl,
> 	net/xdp/xsk.c		.ioctl	= sock_no_ioctl,
> 	net/mptcp/protocol.c	.ioctl	= mptcp_ioctl,
> 
> Am I missing something?

The suggestion is to convert all to take kernel memory and do the
put_cmsg in the caller of .ioctl. Rather than create a wrapper for
each individual instance and add a separate .iouring_cmd for each.

"change all of the sock->ops->ioctl() to pass in on-stack memory
(or similar) and have it work with a kernel address"

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Breno Leitao]

On Wed, Apr 12, 2023 at 10:28:41AM -0400, Willem de Bruijn wrote:
> Breno Leitao wrote:
> > On Tue, Apr 11, 2023 at 09:28:29AM -0600, Jens Axboe wrote:
> > > On 4/11/23 9:24?AM, Willem de Bruijn wrote:
> > > > Jens Axboe wrote:
> > > >> On 4/11/23 9:00?AM, Willem de Bruijn wrote:
> > > >> But that doesn't work, because sock->ops->ioctl() assumes the arg is
> > > >> memory in userspace. Or do you mean change all of the sock->ops->ioctl()
> > > >> to pass in on-stack memory (or similar) and have it work with a kernel
> > > >> address?
> > > > 
> > > > That was what I suggested indeed.
> > > > 
> > > > It's about as much code change as this patch series. But it avoids
> > > > the code duplication.
> > > 
> > > Breno, want to tackle that as a prep patch first? Should make the
> > > functional changes afterwards much more straightforward, and will allow
> > > support for anything really.
> > 
> > Absolutely. I just want to make sure that I got the proper approach that
> > we agreed here.
> > 
> > Let me explain what I understood taking TCP as an example:
> > 
> > 1) Rename tcp_ioctl() to something as _tcp_ioctl() where the 'arg'
> > argument is now just a kernel memory (located in the stack frame from the
> > callee).
> > 
> > 2) Recreate "tcp_ioctl()" that will basically allocate a 'arg' in the
> > stack and call _tcp_ioctl() passing that 'arg' argument. At the bottom of
> > this (tcp_ioctl() function) function, call `put_user(in_kernel_arg, userspace_arg)
> > 
> > 3) Repeat it for the 20 protocols that implement ioctl:
> > 
> > 	ag  "struct proto .* = {" -A 20 net/ | grep \.ioctl
> > 	net/dccp/ipv6.c 	.ioctl	= dccp_ioctl,
> > 	net/dccp/ipv4.c		.ioctl	= dccp_ioctl,
> > 	net/ieee802154/socket.c .ioctl	= dgram_ioctl,
> > 	net/ipv4/udplite.c	.ioctl	= udp_ioctl,
> > 	net/ipv4/raw.c 		.ioctl	= raw_ioctl,
> > 	net/ipv4/udp.c		.ioctl	= udp_ioctl,
> > 	net/ipv4/tcp_ipv4.c 	.ioctl	= tcp_ioctl,
> > 	net/ipv6/raw.c		.ioctl	= rawv6_ioctl,
> > 	net/ipv6/tcp_ipv6.c	.ioctl	= tcp_ioctl,
> > 	net/ipv6/udp.c	 	.ioctl	= udp_ioctl,
> > 	net/ipv6/udplite.c	.ioctl	= udp_ioctl,
> > 	net/l2tp/l2tp_ip6.c	.ioctl	= l2tp_ioctl,
> > 	net/l2tp/l2tp_ip.c	.ioctl	= l2tp_ioctl,
> > 	net/phonet/datagram.:	.ioctl	= pn_ioctl,
> > 	net/phonet/pep.c	.ioctl	= pep_ioctl,
> > 	net/rds/af_rds.c	.ioctl	=	rds_ioctl,
> > 	net/sctp/socket.c	.ioctl  =	sctp_ioctl,
> > 	net/sctp/socket.c	.ioctl	= sctp_ioctl,
> > 	net/xdp/xsk.c		.ioctl	= sock_no_ioctl,
> > 	net/mptcp/protocol.c	.ioctl	= mptcp_ioctl,
> > 
> > Am I missing something?
> 
> The suggestion is to convert all to take kernel memory and do the
> put_cmsg in the caller of .ioctl. Rather than create a wrapper for
> each individual instance and add a separate .iouring_cmd for each.
> 
> "change all of the sock->ops->ioctl() to pass in on-stack memory
> (or similar) and have it work with a kernel address"

is it possible to do it for cases where we don't know what is the size
of the buffer?

For instance the raw_ioctl()/rawv6_ioctl() case. The "arg" argument is
used in different ways (one for input and one for output):

  1) If cmd = SIOCOUTQ or SIOCINQ, then the return value will be
  returned to userspace:
  	put_user(amount, (int __user *)arg)

  2) For default cmd, ipmr_ioctl() is called, which reads from the `arg`
  parameter:
	copy_from_user(&vr, arg, sizeof(vr)

How to handle these contradictory behaviour ahead of time (at callee
time, where the buffers will be prepared)?

Thank you!

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

Breno Leitao wrote:
> On Wed, Apr 12, 2023 at 10:28:41AM -0400, Willem de Bruijn wrote:
> > Breno Leitao wrote:
> > > On Tue, Apr 11, 2023 at 09:28:29AM -0600, Jens Axboe wrote:
> > > > On 4/11/23 9:24?AM, Willem de Bruijn wrote:
> > > > > Jens Axboe wrote:
> > > > >> On 4/11/23 9:00?AM, Willem de Bruijn wrote:
> > > > >> But that doesn't work, because sock->ops->ioctl() assumes the arg is
> > > > >> memory in userspace. Or do you mean change all of the sock->ops->ioctl()
> > > > >> to pass in on-stack memory (or similar) and have it work with a kernel
> > > > >> address?
> > > > > 
> > > > > That was what I suggested indeed.
> > > > > 
> > > > > It's about as much code change as this patch series. But it avoids
> > > > > the code duplication.
> > > > 
> > > > Breno, want to tackle that as a prep patch first? Should make the
> > > > functional changes afterwards much more straightforward, and will allow
> > > > support for anything really.
> > > 
> > > Absolutely. I just want to make sure that I got the proper approach that
> > > we agreed here.
> > > 
> > > Let me explain what I understood taking TCP as an example:
> > > 
> > > 1) Rename tcp_ioctl() to something as _tcp_ioctl() where the 'arg'
> > > argument is now just a kernel memory (located in the stack frame from the
> > > callee).
> > > 
> > > 2) Recreate "tcp_ioctl()" that will basically allocate a 'arg' in the
> > > stack and call _tcp_ioctl() passing that 'arg' argument. At the bottom of
> > > this (tcp_ioctl() function) function, call `put_user(in_kernel_arg, userspace_arg)
> > > 
> > > 3) Repeat it for the 20 protocols that implement ioctl:
> > > 
> > > 	ag  "struct proto .* = {" -A 20 net/ | grep \.ioctl
> > > 	net/dccp/ipv6.c 	.ioctl	= dccp_ioctl,
> > > 	net/dccp/ipv4.c		.ioctl	= dccp_ioctl,
> > > 	net/ieee802154/socket.c .ioctl	= dgram_ioctl,
> > > 	net/ipv4/udplite.c	.ioctl	= udp_ioctl,
> > > 	net/ipv4/raw.c 		.ioctl	= raw_ioctl,
> > > 	net/ipv4/udp.c		.ioctl	= udp_ioctl,
> > > 	net/ipv4/tcp_ipv4.c 	.ioctl	= tcp_ioctl,
> > > 	net/ipv6/raw.c		.ioctl	= rawv6_ioctl,
> > > 	net/ipv6/tcp_ipv6.c	.ioctl	= tcp_ioctl,
> > > 	net/ipv6/udp.c	 	.ioctl	= udp_ioctl,
> > > 	net/ipv6/udplite.c	.ioctl	= udp_ioctl,
> > > 	net/l2tp/l2tp_ip6.c	.ioctl	= l2tp_ioctl,
> > > 	net/l2tp/l2tp_ip.c	.ioctl	= l2tp_ioctl,
> > > 	net/phonet/datagram.:	.ioctl	= pn_ioctl,
> > > 	net/phonet/pep.c	.ioctl	= pep_ioctl,
> > > 	net/rds/af_rds.c	.ioctl	=	rds_ioctl,
> > > 	net/sctp/socket.c	.ioctl  =	sctp_ioctl,
> > > 	net/sctp/socket.c	.ioctl	= sctp_ioctl,
> > > 	net/xdp/xsk.c		.ioctl	= sock_no_ioctl,
> > > 	net/mptcp/protocol.c	.ioctl	= mptcp_ioctl,
> > > 
> > > Am I missing something?
> > 
> > The suggestion is to convert all to take kernel memory and do the
> > put_cmsg in the caller of .ioctl. Rather than create a wrapper for
> > each individual instance and add a separate .iouring_cmd for each.
> > 
> > "change all of the sock->ops->ioctl() to pass in on-stack memory
> > (or similar) and have it work with a kernel address"
> 
> is it possible to do it for cases where we don't know what is the size
> of the buffer?
> 
> For instance the raw_ioctl()/rawv6_ioctl() case. The "arg" argument is
> used in different ways (one for input and one for output):
> 
>   1) If cmd = SIOCOUTQ or SIOCINQ, then the return value will be
>   returned to userspace:
>   	put_user(amount, (int __user *)arg)
> 
>   2) For default cmd, ipmr_ioctl() is called, which reads from the `arg`
>   parameter:
> 	copy_from_user(&vr, arg, sizeof(vr)
> 
> How to handle these contradictory behaviour ahead of time (at callee
> time, where the buffers will be prepared)?
> 
> Thank you!

Ah you found a counter-example to the simple pattern of put_user.

The answer perhaps depends on how many such counter-examples you
encounter in the list you gave. If this is the only one, exceptions
in the wrapper are reasonable. Not if there are many.

Is the intent for io_uring to support all cases eventually? The
current patch series only targeted more common fast path operations.

Probably also relevant is whether/how the approach can be extended
to [gs]etsockopt, as that was another example given, with the same
challenge.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Jakub Kicinski]

On Thu, 13 Apr 2023 10:24:31 -0400 Willem de Bruijn wrote:
> Probably also relevant is whether/how the approach can be extended
> to [gs]etsockopt, as that was another example given, with the same
> challenge.

I had the same thought, given BPF filtering/integration with *etsockopt
is repeatedly giving us grief.
The only lesson from that I can think of is that we should perhaps
suffer thru the one-by-one conversions for a while. Pulling the cases
we inspected out into common code, rather than hope we can cover
everything in one fell swoop.

RE: [PATCH 0/5] add initial io_uring_cmd support for sockets

[David Laight]

RnJvbTogV2lsbGVtIGRlIEJydWlqbg0KPiBTZW50OiAxMyBBcHJpbCAyMDIzIDE1OjI1DQouLi4N
Cj4gPiBGb3IgaW5zdGFuY2UgdGhlIHJhd19pb2N0bCgpL3Jhd3Y2X2lvY3RsKCkgY2FzZS4gVGhl
ICJhcmciIGFyZ3VtZW50IGlzDQo+ID4gdXNlZCBpbiBkaWZmZXJlbnQgd2F5cyAob25lIGZvciBp
bnB1dCBhbmQgb25lIGZvciBvdXRwdXQpOg0KPiA+DQo+ID4gICAxKSBJZiBjbWQgPT0gU0lPQ09V
VFEgb3IgU0lPQ0lOUSwgdGhlbiB0aGUgcmV0dXJuIHZhbHVlIHdpbGwgYmUNCj4gPiAgIHJldHVy
bmVkIHRvIHVzZXJzcGFjZToNCj4gPiAgIAlwdXRfdXNlcihhbW91bnQsIChpbnQgX191c2VyICop
YXJnKQ0KDQpUaGVyZSBpcyBhbHdheXMgdGhlIG9wdGlvbiBvZiBkZWZpbmluZyBhbHRlcm5hdGUg
aW9jdGwNCidjbWQnIGNvZGVzIHRoYXQgdXNlciBJT1IoKSBhbmQgSU9XKCkgYW5kIHJlcXVpcmlu
ZyB0aGF0DQppb191cmluZyBhcHBsaWNhdGlvbnMgdXNlIHRoZSBhbHRlcm5hdGUgZm9ybXMuDQoN
ClRoZW4gaGF2ZSB0d28gJ2lvY3RsJyBmdW5jdGlvbnMgd2l0aCBhIG5ldyBvbmUgZm9yIElPUigp
DQp0eXBlIGNvbW1hbmRzIGFuZCB0aGUgZXhpc3Rpbmcgb25lIGZvciBjb21wYXRpYmlsaXR5DQp0
aGF0IG1pZ2h0IGp1c3QgZG8gYSB0cmFuc2xhdGlvbiAob3IgcmV0dXJuIGEgdHJhbnNsYXRlZA0K
Y29tbWFuZCB0byBhdm9pZCBleHRyYSBzdGFjayB1c2UpLg0KDQpZb3UgbWF5IHN0aWxsIHdhbnQg
dG8gcGFzcyB0aHJvdWdoIGJvdGggdGhlIGtlcm5lbCBhbmQNCnVzZXIgKGlmIGEgdXNlciByZXF1
ZXN0KSBidWZmZXIgYWRkcmVzc2VzIHRvIGFsbG93IGZvcg0KdGhvc2UgYnJva2VuIHJlcXVlc3Rz
IHdoZXJlIHRoZSBidWZmZXIgZGlyZWN0aW9uIGJpdHMNCmFyZSB3cm9uZy4NCg0KCURhdmlkDQoN
Ci0NClJlZ2lzdGVyZWQgQWRkcmVzcyBMYWtlc2lkZSwgQnJhbWxleSBSb2FkLCBNb3VudCBGYXJt
LCBNaWx0b24gS2V5bmVzLCBNSzEgMVBULCBVSw0KUmVnaXN0cmF0aW9uIE5vOiAxMzk3Mzg2IChX
YWxlcykNCg=

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Breno Leitao]

On Thu, Apr 13, 2023 at 10:24:31AM -0400, Willem de Bruijn wrote:
> > How to handle these contradictory behaviour ahead of time (at callee
> > time, where the buffers will be prepared)?
> 
> Ah you found a counter-example to the simple pattern of put_user.
> 
> The answer perhaps depends on how many such counter-examples you
> encounter in the list you gave. If this is the only one, exceptions
> in the wrapper are reasonable. Not if there are many.


Hello Williem,

I spend sometime dealing with it, and the best way for me to figure out
how much work this is, was implementing a PoC. You can find a basic PoC
in the link below. It is not 100% complete (still need to convert 4
simple ioctls), but, it deals with the most complicated cases. The
missing parts are straighforward if we are OK with this approach.

	https://github.com/leitao/linux/commits/ioctl_refactor

Details
===
1)  Change the ioctl callback to use kernel memory arguments. This
changes a lot of files but most of them are trivial. This is the new
ioctl callback:

struct proto {

        int                     (*ioctl)(struct sock *sk, int cmd,
-                                        unsigned long arg);
+                                        int *karg);

	You can see the full changeset in the following commit (which is
	the last in the tree above)
	https://github.com/leitao/linux/commit/ad78da14601b078c4b6a9f63a86032467ab59bf7

2) Create a wrapper (sock_skprot_ioctl()) that should be called instead
of sk->sk_prot->ioctl(). For every exception, calls a specific function
for the exception (basically ipmr_ioctl and ipmr_ioctl) (see more on 3)

	This is the commit https://github.com/leitao/linux/commit/511592e549c39ef0de19efa2eb4382cac5786227

3) There are two exceptions, they are ip{6}mr_ioctl() and pn_ioctl().
ip{6}mr is the hardest one, and I implemented the exception flow for it.

	You could find ipmr changes here:
	https://github.com/leitao/linux/commit/659a76dc0547ab2170023f31e20115520ebe33d9

Is this what you had in mind?

Thank you!

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

Breno Leitao wrote:
> On Thu, Apr 13, 2023 at 10:24:31AM -0400, Willem de Bruijn wrote:
> > > How to handle these contradictory behaviour ahead of time (at callee
> > > time, where the buffers will be prepared)?
> > 
> > Ah you found a counter-example to the simple pattern of put_user.
> > 
> > The answer perhaps depends on how many such counter-examples you
> > encounter in the list you gave. If this is the only one, exceptions
> > in the wrapper are reasonable. Not if there are many.
> 
> 
> Hello Williem,
> 
> I spend sometime dealing with it, and the best way for me to figure out
> how much work this is, was implementing a PoC. You can find a basic PoC
> in the link below. It is not 100% complete (still need to convert 4
> simple ioctls), but, it deals with the most complicated cases. The
> missing parts are straighforward if we are OK with this approach.
> 
> 	https://github.com/leitao/linux/commits/ioctl_refactor
> 
> Details
> ===> 
> 1)  Change the ioctl callback to use kernel memory arguments. This
> changes a lot of files but most of them are trivial. This is the new
> ioctl callback:
> 
> struct proto {
> 
>         int                     (*ioctl)(struct sock *sk, int cmd,
> -                                        unsigned long arg);
> +                                        int *karg);
> 
> 	You can see the full changeset in the following commit (which is
> 	the last in the tree above)
> 	https://github.com/leitao/linux/commit/ad78da14601b078c4b6a9f63a86032467ab59bf7
> 
> 2) Create a wrapper (sock_skprot_ioctl()) that should be called instead
> of sk->sk_prot->ioctl(). For every exception, calls a specific function
> for the exception (basically ipmr_ioctl and ipmr_ioctl) (see more on 3)
> 
> 	This is the commit https://github.com/leitao/linux/commit/511592e549c39ef0de19efa2eb4382cac5786227
> 
> 3) There are two exceptions, they are ip{6}mr_ioctl() and pn_ioctl().
> ip{6}mr is the hardest one, and I implemented the exception flow for it.
> 
> 	You could find ipmr changes here:
> 	https://github.com/leitao/linux/commit/659a76dc0547ab2170023f31e20115520ebe33d9
> 
> Is this what you had in mind?
> 
> Thank you!

Thanks for the series, Breno. Yes, this looks very much what I hoped for.

The series shows two cases of ioctls: getters that return an int, and
combined getter/setters that take a struct of a certain size and
return the exact same.

I would deduplicate the four ipmr/ip6mr cases that constitute the second
type, by having a single helper for this type. sock_skprot_ioctl_struct,
which takes an argument for the struct size to copy in/out.

Did this series cover all proto ioctls, or is this still a subset just
for demonstration purposes -- and might there still be other types
lurking elsewhere?

If this is all, this looks like a reasonable amount of code churn to me.

Three small points

* please keep the __user annotation. Use make C=2 when unsure to warn
  about mismatched annotation
* minor: special case the ipmr (type 2) ioctls in sock_skprot_ioctl
  and treat the "return int" (type 1) ioctls as the default case.
* introduce code in a patch together with its use-case, so no separate
  patches for sock_skprot_ioctl and sock_skprot_ioctl_ipmr. Either one
  patch, or two, for each type of conversion.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Breno Leitao]

On Tue, Apr 18, 2023 at 03:41:24PM -0400, Willem de Bruijn wrote:
> Breno Leitao wrote:
> > On Thu, Apr 13, 2023 at 10:24:31AM -0400, Willem de Bruijn wrote:
> > > > How to handle these contradictory behaviour ahead of time (at callee
> > > > time, where the buffers will be prepared)?
> > > 
> > > Ah you found a counter-example to the simple pattern of put_user.
> > > 
> > > The answer perhaps depends on how many such counter-examples you
> > > encounter in the list you gave. If this is the only one, exceptions
> > > in the wrapper are reasonable. Not if there are many.
> > 
> > 
> > Hello Williem,
> > 
> > I spend sometime dealing with it, and the best way for me to figure out
> > how much work this is, was implementing a PoC. You can find a basic PoC
> > in the link below. It is not 100% complete (still need to convert 4
> > simple ioctls), but, it deals with the most complicated cases. The
> > missing parts are straighforward if we are OK with this approach.
> > 
> > 	https://github.com/leitao/linux/commits/ioctl_refactor
> > 
> > Details
> > ===> > 
> > 1)  Change the ioctl callback to use kernel memory arguments. This
> > changes a lot of files but most of them are trivial. This is the new
> > ioctl callback:
> > 
> > struct proto {
> > 
> >         int                     (*ioctl)(struct sock *sk, int cmd,
> > -                                        unsigned long arg);
> > +                                        int *karg);
> > 
> > 	You can see the full changeset in the following commit (which is
> > 	the last in the tree above)
> > 	https://github.com/leitao/linux/commit/ad78da14601b078c4b6a9f63a86032467ab59bf7
> > 
> > 2) Create a wrapper (sock_skprot_ioctl()) that should be called instead
> > of sk->sk_prot->ioctl(). For every exception, calls a specific function
> > for the exception (basically ipmr_ioctl and ipmr_ioctl) (see more on 3)
> > 
> > 	This is the commit https://github.com/leitao/linux/commit/511592e549c39ef0de19efa2eb4382cac5786227
> > 
> > 3) There are two exceptions, they are ip{6}mr_ioctl() and pn_ioctl().
> > ip{6}mr is the hardest one, and I implemented the exception flow for it.
> > 
> > 	You could find ipmr changes here:
> > 	https://github.com/leitao/linux/commit/659a76dc0547ab2170023f31e20115520ebe33d9
> > 
> > Is this what you had in mind?
> > 
> > Thank you!
> 
> Thanks for the series, Breno. Yes, this looks very much what I hoped for.

Awesome. Thanks.

> The series shows two cases of ioctls: getters that return an int, and
> combined getter/setters that take a struct of a certain size and
> return the exact same.
>
> I would deduplicate the four ipmr/ip6mr cases that constitute the second
> type, by having a single helper for this type. sock_skprot_ioctl_struct,
> which takes an argument for the struct size to copy in/out.

Ok, that is a good advice. Thanks!

> Did this series cover all proto ioctls, or is this still a subset just
> for demonstration purposes -- and might there still be other types
> lurking elsewhere?

It does not cover all the cases. I would say it cover 80% of the cases,
and the hardest cases.  These are the missing cases, and what they do:

* pn_ioctl     (getters/setter that reads/return an int)
* l2tp_ioctl   (getters that return an int)
* dgram_ioctl  (getters that return an int)
* sctp_ioctl   (getters that return an int)
* mptcp_ioctl  (getters that return an int)
* dccp_ioctl   (getters that return an int)
* dgram_ioctl  (getters that return an int)
* pep_ioctl    (getters that return an int)


Here is what I am using to get the full list:
 # ag  --no-filename -A 20 "struct proto \w* = {"  | grep .ioctl | cut -d "=" -f 2 | tr -d '\n'

 dccp_ioctl, dccp_ioctl, dgram_ioctl, tcp_ioctl, raw_ioctl, udp_ioctl,
 udp_ioctl, udp_ioctl, tcp_ioctl, l2tp_ioctl, rawv6_ioctl, l2tp_ioctl,
 mptcp_ioctl, pep_ioctl, pn_ioctl, rds_ioctl, sctp_ioctl, sctp_ioctl,
 sock_no_ioctl

> If this is all, this looks like a reasonable amount of code churn to me.

Should I proceed and create a final patch? I don't see a way to break up
the last patch, which changes the API , in smaller patches. I.e., the
last patch will be huge, right?

> Three small points
> 
> * please keep the __user annotation. Use make C=2 when unsure to warn
>   about mismatched annotation

ack!

> * minor: special case the ipmr (type 2) ioctls in sock_skprot_ioctl
>   and treat the "return int" (type 1) ioctls as the default case.

ack!

> * introduce code in a patch together with its use-case, so no separate
>   patches for sock_skprot_ioctl and sock_skprot_ioctl_ipmr. Either one
>   patch, or two, for each type of conversion.

I am not sure how to change the ABI (struct proto) without doing all the
protocol changes in the same patch. Otherwise compilation will be broken between
the patch that changes the "struct proto" and the patch that changes the
_ioctl for protocol X.  I mean, is it possible to break up changing
"struct proto" and the affected protocols?

Thank you for the review and suggestions!

PS: I will take some days off next week, and I am planning to send the
final patch when I come back.

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Willem de Bruijn]

Breno Leitao wrote:
> On Tue, Apr 18, 2023 at 03:41:24PM -0400, Willem de Bruijn wrote:
> > Breno Leitao wrote:
> > > On Thu, Apr 13, 2023 at 10:24:31AM -0400, Willem de Bruijn wrote:
> > > > > How to handle these contradictory behaviour ahead of time (at callee
> > > > > time, where the buffers will be prepared)?
> > > > 
> > > > Ah you found a counter-example to the simple pattern of put_user.
> > > > 
> > > > The answer perhaps depends on how many such counter-examples you
> > > > encounter in the list you gave. If this is the only one, exceptions
> > > > in the wrapper are reasonable. Not if there are many.
> > > 
> > > 
> > > Hello Williem,
> > > 
> > > I spend sometime dealing with it, and the best way for me to figure out
> > > how much work this is, was implementing a PoC. You can find a basic PoC
> > > in the link below. It is not 100% complete (still need to convert 4
> > > simple ioctls), but, it deals with the most complicated cases. The
> > > missing parts are straighforward if we are OK with this approach.
> > > 
> > > 	https://github.com/leitao/linux/commits/ioctl_refactor
> > > 
> > > Details
> > > ===> > > 
> > > 1)  Change the ioctl callback to use kernel memory arguments. This
> > > changes a lot of files but most of them are trivial. This is the new
> > > ioctl callback:
> > > 
> > > struct proto {
> > > 
> > >         int                     (*ioctl)(struct sock *sk, int cmd,
> > > -                                        unsigned long arg);
> > > +                                        int *karg);
> > > 
> > > 	You can see the full changeset in the following commit (which is
> > > 	the last in the tree above)
> > > 	https://github.com/leitao/linux/commit/ad78da14601b078c4b6a9f63a86032467ab59bf7
> > > 
> > > 2) Create a wrapper (sock_skprot_ioctl()) that should be called instead
> > > of sk->sk_prot->ioctl(). For every exception, calls a specific function
> > > for the exception (basically ipmr_ioctl and ipmr_ioctl) (see more on 3)
> > > 
> > > 	This is the commit https://github.com/leitao/linux/commit/511592e549c39ef0de19efa2eb4382cac5786227
> > > 
> > > 3) There are two exceptions, they are ip{6}mr_ioctl() and pn_ioctl().
> > > ip{6}mr is the hardest one, and I implemented the exception flow for it.
> > > 
> > > 	You could find ipmr changes here:
> > > 	https://github.com/leitao/linux/commit/659a76dc0547ab2170023f31e20115520ebe33d9
> > > 
> > > Is this what you had in mind?
> > > 
> > > Thank you!
> > 
> > Thanks for the series, Breno. Yes, this looks very much what I hoped for.
> 
> Awesome. Thanks.
> 
> > The series shows two cases of ioctls: getters that return an int, and
> > combined getter/setters that take a struct of a certain size and
> > return the exact same.
> >
> > I would deduplicate the four ipmr/ip6mr cases that constitute the second
> > type, by having a single helper for this type. sock_skprot_ioctl_struct,
> > which takes an argument for the struct size to copy in/out.
> 
> Ok, that is a good advice. Thanks!
> 
> > Did this series cover all proto ioctls, or is this still a subset just
> > for demonstration purposes -- and might there still be other types
> > lurking elsewhere?
> 
> It does not cover all the cases. I would say it cover 80% of the cases,
> and the hardest cases.  These are the missing cases, and what they do:
> 
> * pn_ioctl     (getters/setter that reads/return an int)
> * l2tp_ioctl   (getters that return an int)
> * dgram_ioctl  (getters that return an int)
> * sctp_ioctl   (getters that return an int)
> * mptcp_ioctl  (getters that return an int)
> * dccp_ioctl   (getters that return an int)
> * dgram_ioctl  (getters that return an int)
> * pep_ioctl    (getters that return an int)

Thanks for the thorough review.

So we have io_struct, io_int and o_int variants only. And the io_int
can use the proposed io_struct helper that takes an explicit length
to copy in and out.

 
> Here is what I am using to get the full list:
>  # ag  --no-filename -A 20 "struct proto \w* = {"  | grep .ioctl | cut -d "=" -f 2 | tr -d '\n'
> 
>  dccp_ioctl, dccp_ioctl, dgram_ioctl, tcp_ioctl, raw_ioctl, udp_ioctl,
>  udp_ioctl, udp_ioctl, tcp_ioctl, l2tp_ioctl, rawv6_ioctl, l2tp_ioctl,
>  mptcp_ioctl, pep_ioctl, pn_ioctl, rds_ioctl, sctp_ioctl, sctp_ioctl,
>  sock_no_ioctl
> 
> > If this is all, this looks like a reasonable amount of code churn to me.
> 
> Should I proceed and create a final patch? I don't see a way to break up
> the last patch, which changes the API , in smaller patches. I.e., the
> last patch will be huge, right?

Good point. So be it, then.
 
> > Three small points
> > 
> > * please keep the __user annotation. Use make C=2 when unsure to warn
> >   about mismatched annotation
> 
> ack!
> 
> > * minor: special case the ipmr (type 2) ioctls in sock_skprot_ioctl
> >   and treat the "return int" (type 1) ioctls as the default case.
> 
> ack!
> 
> > * introduce code in a patch together with its use-case, so no separate
> >   patches for sock_skprot_ioctl and sock_skprot_ioctl_ipmr. Either one
> >   patch, or two, for each type of conversion.
> 
> I am not sure how to change the ABI (struct proto) without doing all the
> protocol changes in the same patch. Otherwise compilation will be broken between
> the patch that changes the "struct proto" and the patch that changes the
> _ioctl for protocol X.  I mean, is it possible to break up changing
> "struct proto" and the affected protocols?
> 
> Thank you for the review and suggestions!
> 
> PS: I will take some days off next week, and I am planning to send the
> final patch when I come back.

RE: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Adrien Delorme]

From Adrien Delorme

> From: David Ahern 
> Sent: 12 April 2023 7:39 
> > Sent: 11 April 2023 16:28
> ....
> > Christoph's patch set a few years back that removed set_fs broke the
> > ability to do in-kernel ioctl and {s,g}setsockopt calls. I did not
> > follow that change; was it a deliberate intent to not allow these
> > in-kernel calls vs wanting to remove the set_fs? e.g., can we add a
> > kioctl variant for in-kernel use of the APIs?
>
> I think that was a side effect, and with no in-tree in-kernel
> users (apart from limited calls in bpf) it was deemed acceptable.
> (It is a PITA for any code trying to use SCTP in kernel.)
>
> One problem is that not all sockopt calls pass the correct length.
> And some of them can have very long buffers.
> Not to mention the ones that are read-modify-write.
>
> A plausible solution is to pass a 'fat pointer' that contains
> some, or all, of:
>       - A userspace buffer pointer.
>       - A kernel buffer pointer.
>       - The length supplied by the user.
>       - The length of the kernel buffer.
>       = The number of bytes to copy on completion.
> For simple user requests the syscall entry/exit code
> would copy the data to a short on-stack buffer.
> Kernel users just pass the kernel address.
> Odd requests can just use the user pointer.
>
> Probably needs accessors that add in an offset.
>
> It might also be that some of the problematic sockopt
> were in decnet - now removed.

Hello everyone,

I'm currently working on an implementation of {get,set} sockopt. 
Since this thread is already talking about it, I hope that I replying at the correct place. 

My implementation is rather simple using a struct that will be used to pass the necessary info throught sqe->cmd.

Here is my implementation based of kernel version 6.3 : 

Signed-off-by: Adrien Delorme <delorme.ade@outlook.com>

diff -uprN a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
--- a/include/uapi/linux/io_uring.h     2023-04-23 15:02:52.000000000 -0400
+++ b/include/uapi/linux/io_uring.h     2023-04-24 07:55:21.406981696 -0400
@@ -235,6 +235,25 @@ enum io_uring_op {
  */
#define IORING_URING_CMD_FIXED (1U << 0)

+/* struct io_uring_cmd->cmd_op flags for socket operations */
+#define IO_URING_CMD_OP_GETSOCKOPT 0x0
+#define IO_URING_CMD_OP_SETSOCKOPT 0x1
+
+/* Struct to pass args for IO_URING_CMD_OP_GETSOCKOPT and IO_URING_CMD_OP_SETSOCKOPT operations */
+struct args_setsockopt_uring{
+       int                             level;
+       int                     optname;
+       char __user *   user_optval;
+       int                     optlen;
+};
+
+struct args_getsockopt_uring{
+       int                             level;
+       int                     optname;
+       char __user *   user_optval;
+       int      __user *       optlen;
+};
+

/*
  * sqe->fsync_flags
diff -uprN a/net/socket.c b/net/socket.c
--- a/net/socket.c      2023-04-23 15:02:52.000000000 -0400
+++ b/net/socket.c      2023-04-24 08:06:44.800981696 -0400
@@ -108,6 +108,11 @@
#include <linux/ptp_clock_kernel.h>
#include <trace/events/sock.h>

+#ifdef CONFIG_IO_URING
+#include <uapi/linux/io_uring.h>
+#include <linux/io_uring.h>
+#endif
+
#ifdef CONFIG_NET_RX_BUSY_POLL
unsigned int sysctl_net_busy_read __read_mostly;
unsigned int sysctl_net_busy_poll __read_mostly;
@@ -132,6 +137,11 @@ static ssize_t sock_splice_read(struct f
                                struct pipe_inode_info *pipe, size_t len,
                                unsigned int flags);

+
+#ifdef CONFIG_IO_URING
+int socket_uring_cmd_handler(struct io_uring_cmd *cmd, unsigned int flags);
+#endif
+
#ifdef CONFIG_PROC_FS
static void sock_show_fdinfo(struct seq_file *m, struct file *f)
{
@@ -166,6 +176,9 @@ static const struct file_operations sock
        .splice_write = generic_splice_sendpage,
        .splice_read =  sock_splice_read,
        .show_fdinfo =  sock_show_fdinfo,
+#ifdef CONFIG_IO_URING
+       .uring_cmd = socket_uring_cmd_handler,
+#endif
};

static const char * const pf_family_names[] = {
@@ -2330,6 +2343,126 @@ SYSCALL_DEFINE5(getsockopt, int, fd, int
        return __sys_getsockopt(fd, level, optname, optval, optlen);
}

+#ifdef CONFIG_IO_URING
+
+/*
+ * Make getsockopt operation with io_uring.
+ * This fonction is based of the __sys_getsockopt without sockfd_lookup_light
+ * since io_uring retrieves it for us.
+ */
+int uring_cmd_getsockopt(struct socket *sock, int level, int optname, char __user *optval,
+               int __user *optlen)
+{
+       int err;
+       int max_optlen;
+
+       err = security_socket_getsockopt(sock, level, optname);
+       if (err)
+               goto out_put;
+
+       if (!in_compat_syscall())
+               max_optlen = BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen);
+
+       if (level == SOL_SOCKET)
+               err = sock_getsockopt(sock, level, optname, optval, optlen);
+       else if (unlikely(!sock->ops->getsockopt))
+               err = -EOPNOTSUPP;
+       else
+               err = sock->ops->getsockopt(sock, level, optname, optval,
+                                           optlen);
+
+       if (!in_compat_syscall())
+               err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level, optname,
+                                                    optval, optlen, max_optlen,
+                                                    err);
+out_put:
+       return err;
+}
+
+/*
+ * Make setsockopt operation with io_uring.
+ * This fonction is based of the __sys_setsockopt without sockfd_lookup_light
+ * since io_uring retrieves it for us.
+ */
+int uring_cmd_setsockopt(struct socket *sock, int level, int optname, char *user_optval,
+               int optlen)
+{
+       sockptr_t optval = USER_SOCKPTR(user_optval);
+       char *kernel_optval = NULL;
+       int err;
+
+       if (optlen < 0)
+               return -EINVAL;
+
+       err = security_socket_setsockopt(sock, level, optname);
+       if (err)
+               goto out_put;
+
+       if (!in_compat_syscall())
+               err = BPF_CGROUP_RUN_PROG_SETSOCKOPT(sock->sk, &level, &optname,
+                                                    user_optval, &optlen,
+                                                    &kernel_optval);
+       if (err < 0)
+               goto out_put;
+       if (err > 0) {
+               err = 0;
+               goto out_put;
+       }
+
+       if (kernel_optval)
+               optval = KERNEL_SOCKPTR(kernel_optval);
+       if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
+               err = sock_setsockopt(sock, level, optname, optval, optlen);
+       else if (unlikely(!sock->ops->setsockopt))
+               err = -EOPNOTSUPP;
+       else
+               err = sock->ops->setsockopt(sock, level, optname, optval,
+                                           optlen);
+       kfree(kernel_optval);
+out_put:
+       return err;
+}
+
+/*
+ * Handler uring_cmd socket file_operations.
+ *
+ * Operation code and struct are defined in /include/uapi/linux/io_uring.h
+ * The io_uring ring needs to be set with the flags : IORING_SETUP_SQE128 and IORING_SETUP_CQE32
+ *
+ */
+int socket_uring_cmd_handler(struct io_uring_cmd *cmd, unsigned int flags){
+
+       /* Retrieve socket */
+       struct socket *sock = sock_from_file(cmd->file);
+
+       if (!sock)
+               return -EINVAL;
+
+       /* Does the requested operation */
+       switch (cmd->cmd_op) {
+               case IO_URING_CMD_OP_GETSOCKOPT:
+                       struct args_getsockopt_uring *values_get = (struct args_getsockopt_uring *) cmd->cmd;
+                       return uring_cmd_getsockopt(sock,
+                                                                               values_get->level,
+                                                                               values_get->optname,
+                                                                               values_get->user_optval,
+                                                                               values_get->optlen);
+
+               case IO_URING_CMD_OP_SETSOCKOPT:
+                       struct args_setsockopt_uring *values_set = (struct args_setsockopt_uring *) cmd->cmd;
+                       return uring_cmd_setsockopt(sock,
+                                                                               values_set->level,
+                                                                               values_set->optname,
+                                                                               values_set->user_optval,
+                                                                               values_set->optlen);
+               default:
+                       break;
+
+       }
+       return -EINVAL;
+}
+#endif
+
/*
  *     Shutdown a socket.
  */

I would appreciate any feedback or advice you may have on this work. Hopefully it will be of some kind of help. Thank you for your time and consideration.

Adrien

Re: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Pavel Begunkov]

On 5/2/23 10:21, Adrien Delorme wrote:
>  From Adrien Delorme
> 
>> From: David Ahern
>> Sent: 12 April 2023 7:39
>>> Sent: 11 April 2023 16:28
>> ....
>>> Christoph's patch set a few years back that removed set_fs broke the
>>> ability to do in-kernel ioctl and {s,g}setsockopt calls. I did not
>>> follow that change; was it a deliberate intent to not allow these
>>> in-kernel calls vs wanting to remove the set_fs? e.g., can we add a
>>> kioctl variant for in-kernel use of the APIs?
>>
>> I think that was a side effect, and with no in-tree in-kernel
>> users (apart from limited calls in bpf) it was deemed acceptable.
>> (It is a PITA for any code trying to use SCTP in kernel.)
>>
>> One problem is that not all sockopt calls pass the correct length.
>> And some of them can have very long buffers.
>> Not to mention the ones that are read-modify-write.
>>
>> A plausible solution is to pass a 'fat pointer' that contains
>> some, or all, of:
>>        - A userspace buffer pointer.
>>        - A kernel buffer pointer.
>>        - The length supplied by the user.
>>        - The length of the kernel buffer.
>>        = The number of bytes to copy on completion.
>> For simple user requests the syscall entry/exit code
>> would copy the data to a short on-stack buffer.
>> Kernel users just pass the kernel address.
>> Odd requests can just use the user pointer.
>>
>> Probably needs accessors that add in an offset.
>>
>> It might also be that some of the problematic sockopt
>> were in decnet - now removed.
> 
> Hello everyone,
> 
> I'm currently working on an implementation of {get,set} sockopt.
> Since this thread is already talking about it, I hope that I replying at the correct place.

Hi Adrien, I believe Breno is working on set/getsockopt as well
and had similar patches for awhile, but that would need for some
problems to be solved first, e.g. try and decide whether it copies
to a ptr as the syscall versions or would get/return optval
directly in sqe/cqe. And also where to store bits that you pass
in struct args_setsockopt_uring, and whether to rely on SQE128
or not.


> My implementation is rather simple using a struct that will be used to pass the necessary info throught sqe->cmd.
> 
> Here is my implementation based of kernel version 6.3 :
> 
> Signed-off-by: Adrien Delorme <delorme.ade@outlook.com>
> 
> diff -uprN a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> --- a/include/uapi/linux/io_uring.h     2023-04-23 15:02:52.000000000 -0400
> +++ b/include/uapi/linux/io_uring.h     2023-04-24 07:55:21.406981696 -0400
> @@ -235,6 +235,25 @@ enum io_uring_op {
>    */
> #define IORING_URING_CMD_FIXED (1U << 0)
> 
> +/* struct io_uring_cmd->cmd_op flags for socket operations */
> +#define IO_URING_CMD_OP_GETSOCKOPT 0x0
> +#define IO_URING_CMD_OP_SETSOCKOPT 0x1
> +
> +/* Struct to pass args for IO_URING_CMD_OP_GETSOCKOPT and IO_URING_CMD_OP_SETSOCKOPT operations */
> +struct args_setsockopt_uring{

The name of the structure is quite inconsistent with the
rest. It's better to be io_[uring_]_sockopt_arg or some
variation.

> +       int                             level;
> +       int                     optname;
> +       char __user *   user_optval;
> +       int                     optlen;

That's uapi, there should not be __user, and field sizes
should be more portable, i.e. use __u32, __u64, etc, look
through the file.

Would need to look into the get/setsockopt implementation
before saying anything about uring_cmd_{set,get}sockopt.


> +};
> +
> +struct args_getsockopt_uring{
> +       int                             level;
> +       int                     optname;
> +       char __user *   user_optval;
> +       int      __user *       optlen;
> +};
> +
> 
> /*
>    * sqe->fsync_flags
> diff -uprN a/net/socket.c b/net/socket.c
> --- a/net/socket.c      2023-04-23 15:02:52.000000000 -0400
> +++ b/net/socket.c      2023-04-24 08:06:44.800981696 -0400
> @@ -108,6 +108,11 @@
> #include <linux/ptp_clock_kernel.h>
> #include <trace/events/sock.h>
> 
> +#ifdef CONFIG_IO_URING
> +#include <uapi/linux/io_uring.h>
> +#include <linux/io_uring.h>
> +#endif
> +
> #ifdef CONFIG_NET_RX_BUSY_POLL
> unsigned int sysctl_net_busy_read __read_mostly;
> unsigned int sysctl_net_busy_poll __read_mostly;
> @@ -132,6 +137,11 @@ static ssize_t sock_splice_read(struct f
>                                  struct pipe_inode_info *pipe, size_t len,
>                                  unsigned int flags);
> 
> +
> +#ifdef CONFIG_IO_URING
> +int socket_uring_cmd_handler(struct io_uring_cmd *cmd, unsigned int flags);
> +#endif
> +
> #ifdef CONFIG_PROC_FS
> static void sock_show_fdinfo(struct seq_file *m, struct file *f)
> {
> @@ -166,6 +176,9 @@ static const struct file_operations sock
>          .splice_write = generic_splice_sendpage,
>          .splice_read =  sock_splice_read,
>          .show_fdinfo =  sock_show_fdinfo,
> +#ifdef CONFIG_IO_URING
> +       .uring_cmd = socket_uring_cmd_handler,
> +#endif
> };
> 
> static const char * const pf_family_names[] = {
> @@ -2330,6 +2343,126 @@ SYSCALL_DEFINE5(getsockopt, int, fd, int
>          return __sys_getsockopt(fd, level, optname, optval, optlen);
> }
> 
> +#ifdef CONFIG_IO_URING
> +
> +/*
> + * Make getsockopt operation with io_uring.
> + * This fonction is based of the __sys_getsockopt without sockfd_lookup_light
> + * since io_uring retrieves it for us.
> + */
> +int uring_cmd_getsockopt(struct socket *sock, int level, int optname, char __user *optval,
> +               int __user *optlen)
> +{
> +       int err;
> +       int max_optlen;
> +
> +       err = security_socket_getsockopt(sock, level, optname);
> +       if (err)
> +               goto out_put;
> +
> +       if (!in_compat_syscall())
> +               max_optlen = BPF_CGROUP_GETSOCKOPT_MAX_OPTLEN(optlen);
> +
> +       if (level = SOL_SOCKET)
> +               err = sock_getsockopt(sock, level, optname, optval, optlen);
> +       else if (unlikely(!sock->ops->getsockopt))
> +               err = -EOPNOTSUPP;
> +       else
> +               err = sock->ops->getsockopt(sock, level, optname, optval,
> +                                           optlen);
> +
> +       if (!in_compat_syscall())
> +               err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level, optname,
> +                                                    optval, optlen, max_optlen,
> +                                                    err);
> +out_put:
> +       return err;
> +}
> +
> +/*
> + * Make setsockopt operation with io_uring.
> + * This fonction is based of the __sys_setsockopt without sockfd_lookup_light
> + * since io_uring retrieves it for us.
> + */
> +int uring_cmd_setsockopt(struct socket *sock, int level, int optname, char *user_optval,
> +               int optlen)
> +{
> +       sockptr_t optval = USER_SOCKPTR(user_optval);
> +       char *kernel_optval = NULL;
> +       int err;
> +
> +       if (optlen < 0)
> +               return -EINVAL;
> +
> +       err = security_socket_setsockopt(sock, level, optname);
> +       if (err)
> +               goto out_put;
> +
> +       if (!in_compat_syscall())
> +               err = BPF_CGROUP_RUN_PROG_SETSOCKOPT(sock->sk, &level, &optname,
> +                                                    user_optval, &optlen,
> +                                                    &kernel_optval);
> +       if (err < 0)
> +               goto out_put;
> +       if (err > 0) {
> +               err = 0;
> +               goto out_put;
> +       }
> +
> +       if (kernel_optval)
> +               optval = KERNEL_SOCKPTR(kernel_optval);
> +       if (level = SOL_SOCKET && !sock_use_custom_sol_socket(sock))
> +               err = sock_setsockopt(sock, level, optname, optval, optlen);
> +       else if (unlikely(!sock->ops->setsockopt))
> +               err = -EOPNOTSUPP;
> +       else
> +               err = sock->ops->setsockopt(sock, level, optname, optval,
> +                                           optlen);
> +       kfree(kernel_optval);
> +out_put:
> +       return err;
> +}
> +
> +/*
> + * Handler uring_cmd socket file_operations.
> + *
> + * Operation code and struct are defined in /include/uapi/linux/io_uring.h
> + * The io_uring ring needs to be set with the flags : IORING_SETUP_SQE128 and IORING_SETUP_CQE32
> + *
> + */
> +int socket_uring_cmd_handler(struct io_uring_cmd *cmd, unsigned int flags){
> +
> +       /* Retrieve socket */
> +       struct socket *sock = sock_from_file(cmd->file);
> +
> +       if (!sock)
> +               return -EINVAL;
> +
> +       /* Does the requested operation */
> +       switch (cmd->cmd_op) {
> +               case IO_URING_CMD_OP_GETSOCKOPT:
> +                       struct args_getsockopt_uring *values_get = (struct args_getsockopt_uring *) cmd->cmd;
> +                       return uring_cmd_getsockopt(sock,
> +                                                                               values_get->level,
> +                                                                               values_get->optname,
> +                                                                               values_get->user_optval,
> +                                                                               values_get->optlen);
> +
> +               case IO_URING_CMD_OP_SETSOCKOPT:
> +                       struct args_setsockopt_uring *values_set = (struct args_setsockopt_uring *) cmd->cmd;
> +                       return uring_cmd_setsockopt(sock,
> +                                                                               values_set->level,
> +                                                                               values_set->optname,
> +                                                                               values_set->user_optval,
> +                                                                               values_set->optlen);
> +               default:
> +                       break;
> +
> +       }
> +       return -EINVAL;
> +}
> +#endif
> +
> /*
>    *     Shutdown a socket.
>    */
> 
> I would appreciate any feedback or advice you may have on this work. Hopefully it will be of some kind of help. Thank you for your time and consideration.
> 
> Adrien

-- 
Pavel Begunkov

RE: [PATCH 0/5] add initial io_uring_cmd support for sockets

[Adrien Delorme]

DQpGcm9tIEFkcmllbiBEZWxvcm1lDQo+IEZyb23CoDogUGF2ZWwgQmVndW5rb3YgDQo+IFNlbnQg
OiAyIE1heSAyMDIzIDE1OjA0DQo+IE9uIDUvMi8yMyAxMDoyMSwgQWRyaWVuIERlbG9ybWUgd3Jv
dGU6DQo+ID4gIEZyb20gQWRyaWVuIERlbG9ybWUNCj4gPg0KPiA+PiBGcm9tOiBEYXZpZCBBaGVy
bg0KPiA+PiBTZW50OiAxMiBBcHJpbCAyMDIzIDc6MzkNCj4gPj4+IFNlbnQ6IDExIEFwcmlsIDIw
MjMgMTY6MjgNCj4gPj4gLi4uLg0KPiA+PiBPbmUgcHJvYmxlbSBpcyB0aGF0IG5vdCBhbGwgc29j
a29wdCBjYWxscyBwYXNzIHRoZSBjb3JyZWN0IGxlbmd0aC4NCj4gPj4gQW5kIHNvbWUgb2YgdGhl
bSBjYW4gaGF2ZSB2ZXJ5IGxvbmcgYnVmZmVycy4NCj4gPj4gTm90IHRvIG1lbnRpb24gdGhlIG9u
ZXMgdGhhdCBhcmUgcmVhZC1tb2RpZnktd3JpdGUuDQo+ID4+DQo+ID4+IEEgcGxhdXNpYmxlIHNv
bHV0aW9uIGlzIHRvIHBhc3MgYSAnZmF0IHBvaW50ZXInIHRoYXQgY29udGFpbnMgc29tZSwNCj4g
Pj4gb3IgYWxsLCBvZjoNCj4gPj4gICAgICAgIC0gQSB1c2Vyc3BhY2UgYnVmZmVyIHBvaW50ZXIu
DQo+ID4+ICAgICAgICAtIEEga2VybmVsIGJ1ZmZlciBwb2ludGVyLg0KPiA+PiAgICAgICAgLSBU
aGUgbGVuZ3RoIHN1cHBsaWVkIGJ5IHRoZSB1c2VyLg0KPiA+PiAgICAgICAgLSBUaGUgbGVuZ3Ro
IG9mIHRoZSBrZXJuZWwgYnVmZmVyLg0KPiA+PiAgICAgICAgPSBUaGUgbnVtYmVyIG9mIGJ5dGVz
IHRvIGNvcHkgb24gY29tcGxldGlvbi4NCj4gPj4gRm9yIHNpbXBsZSB1c2VyIHJlcXVlc3RzIHRo
ZSBzeXNjYWxsIGVudHJ5L2V4aXQgY29kZSB3b3VsZCBjb3B5IHRoZQ0KPiA+PiBkYXRhIHRvIGEg
c2hvcnQgb24tc3RhY2sgYnVmZmVyLg0KPiA+PiBLZXJuZWwgdXNlcnMganVzdCBwYXNzIHRoZSBr
ZXJuZWwgYWRkcmVzcy4NCj4gPj4gT2RkIHJlcXVlc3RzIGNhbiBqdXN0IHVzZSB0aGUgdXNlciBw
b2ludGVyLg0KPiA+Pg0KPiA+PiBQcm9iYWJseSBuZWVkcyBhY2Nlc3NvcnMgdGhhdCBhZGQgaW4g
YW4gb2Zmc2V0Lg0KPiA+Pg0KPiA+PiBJdCBtaWdodCBhbHNvIGJlIHRoYXQgc29tZSBvZiB0aGUg
cHJvYmxlbWF0aWMgc29ja29wdCB3ZXJlIGluIGRlY25ldA0KPiA+PiAtIG5vdyByZW1vdmVkLg0K
PiA+DQo+ID4gSGVsbG8gZXZlcnlvbmUsDQo+ID4NCj4gPiBJJ20gY3VycmVudGx5IHdvcmtpbmcg
b24gYW4gaW1wbGVtZW50YXRpb24gb2Yge2dldCxzZXR9IHNvY2tvcHQuDQo+ID4gU2luY2UgdGhp
cyB0aHJlYWQgaXMgYWxyZWFkeSB0YWxraW5nIGFib3V0IGl0LCBJIGhvcGUgdGhhdCBJIHJlcGx5
aW5nIGF0IHRoZQ0KPiBjb3JyZWN0IHBsYWNlLg0KPiANCj4gSGkgQWRyaWVuLCBJIGJlbGlldmUg
QnJlbm8gaXMgd29ya2luZyBvbiBzZXQvZ2V0c29ja29wdCBhcyB3ZWxsIGFuZCBoYWQNCj4gc2lt
aWxhciBwYXRjaGVzIGZvciBhd2hpbGUsIGJ1dCB0aGF0IHdvdWxkIG5lZWQgZm9yIHNvbWUgcHJv
YmxlbXMgdG8gYmUNCj4gc29sdmVkIGZpcnN0LCBlLmcuIHRyeSBhbmQgZGVjaWRlIHdoZXRoZXIg
aXQgY29waWVzIHRvIGEgcHRyIGFzIHRoZSBzeXNjYWxsDQo+IHZlcnNpb25zIG9yIHdvdWxkIGdl
dC9yZXR1cm4gb3B0dmFsIGRpcmVjdGx5IGluIHNxZS9jcWUuIEFuZCBhbHNvIHdoZXJlIHRvDQo+
IHN0b3JlIGJpdHMgdGhhdCB5b3UgcGFzcyBpbiBzdHJ1Y3QgYXJnc19zZXRzb2Nrb3B0X3VyaW5n
LCBhbmQgd2hldGhlciB0byByZWx5DQo+IG9uIFNRRTEyOCBvciBub3QuDQo+IA0KDQpIZWxsbyBQ
YXZlbCwNClRoYXQgaXMgZ29vZCB0byBoZWFyLiBJZiBwb3NzaWJsZSBJIHdvdWxkIGxpa2UgdG8g
cHJvdmlkZSBzb21lIGhlbHAuIA0KSSBsb29rZWQgYXQgdGhlIGdldHNvY2tvcHQgaW1wbGVtZW50
YXRpb24uIEZyb20gd2hhdCBJJ20gc2VlaW5nLCBJIGJlbGlldmUgdGhhdCBpdCB3b3VsZCBiZSBl
YXNpZXIgdG8gY29waWVzIHRvIGEgcHRyIGFzIHRoZSBzeXNjYWxsLg0KVGhlIGxlbmd0aCBvZiB0
aGUgb3V0cHV0IGlzIHVzdWFsbHkgNCBieXRlcyAoc29tZXRpbWVzIGxlc3MpIGJ1dCBpbiBhIGxv
dCBvZiBjYXNlcywgdGhpcyBsZW5ndGggaXMgdmFyaWFibGUuIFNvbWV0aW1lIGl0IGNhbiBldmVu
IGJlIGJpZ2dlciB0aGF0IHRoZSBTUUUxMjggcmluZy4NCg0KSGVyZSBpcyBhIG5vbi1leGhhdXN0
aXZlIGxpc3Qgb2YgdGhvc2UgY2FzZXMgOiANCi9uZXQvaXB2NC90Y3AuYyA6IGludCBkb190Y3Bf
Z2V0c29ja29wdCguLi4pDQogIC0gVENQX0lORk8gOiB1cCB0byAyNDAgYnl0ZXMNCiAgLSBUQ1Bf
Q0NfSU5GTyBhbmQgVENQX1JFUEFJUl9XSU5ET1cgOiB1cCB0byAyMCBieXRlcw0KICAtIFRDUF9D
T05HRVNUSU9OIGFuZCBUQ1BfVUxQIDogdXAgdG8gMTYgYnl0ZXMNCiAgLSBUQ1BfWkVST0NQT1lf
UkVDRUlWRSA6IHVwIHRvIDY0IGJ5dGVzICANCi9uZXQvYXRtL2NvbW11bi5jIDogaW50IHZjY19n
ZXRzb2Nrb3B0KC4uLikNCiAgLSBTT19BVE1RT1MgOiB1cCB0byA4OCBieXRlcw0KICAtIFNPX0FU
TVBWQyA6IHVwIHRvIDE2IGJ5dGVzDQovbmV0L2lwdjQvaW9fc29ja2dsdWUuYyA6IGludCBkb19p
cF9nZXRzb2Nrb3B0KC4uLikNCiAgLSBNQ0FTVF9NU0ZJTFRFUiA6IHVwIHRvIDE0NCBieXRlcw0K
ICAtIElQX01TRklMVEVSIDogMTYgYnl0ZXMgbWluaW11bQ0KDQpJIHdpbGwgbG9vayBpbnRvIHNl
dHNvY2tvcHQgYnV0IEkgYmVsaWV2ZSBpdCBtaWdodCBiZSB0aGUgc2FtZS4gDQpJZiBuZWVkZWQg
SSBjYW4gYWxzbyBjb21wbGV0ZSB0aGlzIGxpc3QuIA0KSG93ZXZlciB0aGVyZSBhcmUgc29tZSBj
YXNlcyB3aGVyZSBpdCBpcyBoYXJkIHRvIGRldGVybWluYXRlIGEgbWF4aW11bSBhbW91bnQgb2Yg
Ynl0ZXMgaW4gYWR2YW5jZS4gDQoNCkFzIHRvIHdoZXJlIHRoZSBieXRlcyBzaG91bGQgYmUgc3Rv
cmVkIEkgd2FzIHRoaW5raW5nIG9mIGVpdGhlciA6DQogIC0gQXMgYSBwb2ludGVyIGluIHNxZS0+
YWRkciBzbyB0aGUgU1FFMTI4IGlzIG5vdCBuZWVkZWQgDQogIC0gSW4gc3FlLT5jbWQgYXMgYSBz
dHJ1Y3QgYnV0IGZyb20gbXkgdW5kZXJzdGFuZGluZywgdGhlIFNRRTEyOCBpcyBuZWVkZWQNCj4g
DQo+ID4gTXkgaW1wbGVtZW50YXRpb24gaXMgcmF0aGVyIHNpbXBsZSB1c2luZyBhIHN0cnVjdCB0
aGF0IHdpbGwgYmUgdXNlZCB0byBwYXNzDQo+IHRoZSBuZWNlc3NhcnkgaW5mbyB0aHJvdWdodCBz
cWUtPmNtZC4NCj4gPg0KPiA+IEhlcmUgaXMgbXkgaW1wbGVtZW50YXRpb24gYmFzZWQgb2Yga2Vy
bmVsIHZlcnNpb24gNi4zIDoNCj4gPiAuLi4NCj4gPiArLyogU3RydWN0IHRvIHBhc3MgYXJncyBm
b3IgSU9fVVJJTkdfQ01EX09QX0dFVFNPQ0tPUFQgYW5kDQo+ID4gK0lPX1VSSU5HX0NNRF9PUF9T
RVRTT0NLT1BUIG9wZXJhdGlvbnMgKi8gc3RydWN0DQo+ID4gK2FyZ3Nfc2V0c29ja29wdF91cmlu
Z3sNCj4gDQo+IFRoZSBuYW1lIG9mIHRoZSBzdHJ1Y3R1cmUgaXMgcXVpdGUgaW5jb25zaXN0ZW50
IHdpdGggdGhlIHJlc3QuIEl0J3MgYmV0dGVyIHRvIGJlDQo+IGlvX1t1cmluZ19dX3NvY2tvcHRf
YXJnIG9yIHNvbWUgdmFyaWF0aW9uLg0KPiANCj4gPiArICAgICAgIGludCAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgbGV2ZWw7DQo+ID4gKyAgICAgICBpbnQgICAgICAgICAgICAgICAgICAg
ICBvcHRuYW1lOw0KPiA+ICsgICAgICAgY2hhciBfX3VzZXIgKiAgIHVzZXJfb3B0dmFsOw0KPiA+
ICsgICAgICAgaW50ICAgICAgICAgICAgICAgICAgICAgb3B0bGVuOw0KPiANCj4gVGhhdCdzIHVh
cGksIHRoZXJlIHNob3VsZCBub3QgYmUgX191c2VyLCBhbmQgZmllbGQgc2l6ZXMgc2hvdWxkIGJl
IG1vcmUNCj4gcG9ydGFibGUsIGkuZS4gdXNlIF9fdTMyLCBfX3U2NCwgZXRjLCBsb29rIHRocm91
Z2ggdGhlIGZpbGUuDQo+IA0KPiBXb3VsZCBuZWVkIHRvIGxvb2sgaW50byB0aGUgZ2V0L3NldHNv
Y2tvcHQgaW1wbGVtZW50YXRpb24gYmVmb3JlIHNheWluZw0KPiBhbnl0aGluZyBhYm91dCB1cmlu
Z19jbWRfe3NldCxnZXR9c29ja29wdC4NCj4gLi4uDQo+IFBhdmVsIEJlZ3Vua292DQoNClRoYW5r
IHlvdSBmb3IgdGhlIHJldmlldy4NCkFkcmllbiBEZWxvcm1lDQotLQ0KDQo

RE: [PATCH 0/5] add initial io_uring_cmd support for sockets

[David Laight]

RnJvbTogQWRyaWVuIERlbG9ybWUNCj4gU2VudDogMDMgTWF5IDIwMjMgMTQ6MTENCj4gDQo+IEZy
b20gQWRyaWVuIERlbG9ybWUNCj4gPiBGcm9twqA6IFBhdmVsIEJlZ3Vua292DQo+ID4gU2VudCA6
IDIgTWF5IDIwMjMgMTU6MDQNCj4gPiBPbiA1LzIvMjMgMTA6MjEsIEFkcmllbiBEZWxvcm1lIHdy
b3RlOg0KPiA+ID4gIEZyb20gQWRyaWVuIERlbG9ybWUNCj4gPiA+DQo+ID4gPj4gRnJvbTogRGF2
aWQgQWhlcm4NCj4gPiA+PiBTZW50OiAxMiBBcHJpbCAyMDIzIDc6MzkNCj4gPiA+Pj4gU2VudDog
MTEgQXByaWwgMjAyMyAxNjoyOA0KPiA+ID4+IC4uLi4NCj4gPiA+PiBPbmUgcHJvYmxlbSBpcyB0
aGF0IG5vdCBhbGwgc29ja29wdCBjYWxscyBwYXNzIHRoZSBjb3JyZWN0IGxlbmd0aC4NCj4gPiA+
PiBBbmQgc29tZSBvZiB0aGVtIGNhbiBoYXZlIHZlcnkgbG9uZyBidWZmZXJzLg0KPiA+ID4+IE5v
dCB0byBtZW50aW9uIHRoZSBvbmVzIHRoYXQgYXJlIHJlYWQtbW9kaWZ5LXdyaXRlLg0KPiA+ID4+
DQo+ID4gPj4gQSBwbGF1c2libGUgc29sdXRpb24gaXMgdG8gcGFzcyBhICdmYXQgcG9pbnRlcicg
dGhhdCBjb250YWlucyBzb21lLA0KPiA+ID4+IG9yIGFsbCwgb2Y6DQo+ID4gPj4gICAgICAgIC0g
QSB1c2Vyc3BhY2UgYnVmZmVyIHBvaW50ZXIuDQo+ID4gPj4gICAgICAgIC0gQSBrZXJuZWwgYnVm
ZmVyIHBvaW50ZXIuDQo+ID4gPj4gICAgICAgIC0gVGhlIGxlbmd0aCBzdXBwbGllZCBieSB0aGUg
dXNlci4NCj4gPiA+PiAgICAgICAgLSBUaGUgbGVuZ3RoIG9mIHRoZSBrZXJuZWwgYnVmZmVyLg0K
PiA+ID4+ICAgICAgICA9IFRoZSBudW1iZXIgb2YgYnl0ZXMgdG8gY29weSBvbiBjb21wbGV0aW9u
Lg0KPiA+ID4+IEZvciBzaW1wbGUgdXNlciByZXF1ZXN0cyB0aGUgc3lzY2FsbCBlbnRyeS9leGl0
IGNvZGUgd291bGQgY29weSB0aGUNCj4gPiA+PiBkYXRhIHRvIGEgc2hvcnQgb24tc3RhY2sgYnVm
ZmVyLg0KPiA+ID4+IEtlcm5lbCB1c2VycyBqdXN0IHBhc3MgdGhlIGtlcm5lbCBhZGRyZXNzLg0K
PiA+ID4+IE9kZCByZXF1ZXN0cyBjYW4ganVzdCB1c2UgdGhlIHVzZXIgcG9pbnRlci4NCj4gPiA+
Pg0KPiA+ID4+IFByb2JhYmx5IG5lZWRzIGFjY2Vzc29ycyB0aGF0IGFkZCBpbiBhbiBvZmZzZXQu
DQo+ID4gPj4NCj4gPiA+PiBJdCBtaWdodCBhbHNvIGJlIHRoYXQgc29tZSBvZiB0aGUgcHJvYmxl
bWF0aWMgc29ja29wdCB3ZXJlIGluIGRlY25ldA0KPiA+ID4+IC0gbm93IHJlbW92ZWQuDQo+ID4g
Pg0KPiA+ID4gSGVsbG8gZXZlcnlvbmUsDQo+ID4gPg0KPiA+ID4gSSdtIGN1cnJlbnRseSB3b3Jr
aW5nIG9uIGFuIGltcGxlbWVudGF0aW9uIG9mIHtnZXQsc2V0fSBzb2Nrb3B0Lg0KPiA+ID4gU2lu
Y2UgdGhpcyB0aHJlYWQgaXMgYWxyZWFkeSB0YWxraW5nIGFib3V0IGl0LCBJIGhvcGUgdGhhdCBJ
IHJlcGx5aW5nIGF0IHRoZQ0KPiA+IGNvcnJlY3QgcGxhY2UuDQo+ID4NCj4gPiBIaSBBZHJpZW4s
IEkgYmVsaWV2ZSBCcmVubyBpcyB3b3JraW5nIG9uIHNldC9nZXRzb2Nrb3B0IGFzIHdlbGwgYW5k
IGhhZA0KPiA+IHNpbWlsYXIgcGF0Y2hlcyBmb3IgYXdoaWxlLCBidXQgdGhhdCB3b3VsZCBuZWVk
IGZvciBzb21lIHByb2JsZW1zIHRvIGJlDQo+ID4gc29sdmVkIGZpcnN0LCBlLmcuIHRyeSBhbmQg
ZGVjaWRlIHdoZXRoZXIgaXQgY29waWVzIHRvIGEgcHRyIGFzIHRoZSBzeXNjYWxsDQo+ID4gdmVy
c2lvbnMgb3Igd291bGQgZ2V0L3JldHVybiBvcHR2YWwgZGlyZWN0bHkgaW4gc3FlL2NxZS4gQW5k
IGFsc28gd2hlcmUgdG8NCj4gPiBzdG9yZSBiaXRzIHRoYXQgeW91IHBhc3MgaW4gc3RydWN0IGFy
Z3Nfc2V0c29ja29wdF91cmluZywgYW5kIHdoZXRoZXIgdG8gcmVseQ0KPiA+IG9uIFNRRTEyOCBv
ciBub3QuDQo+ID4NCj4gDQo+IEhlbGxvIFBhdmVsLA0KPiBUaGF0IGlzIGdvb2QgdG8gaGVhci4g
SWYgcG9zc2libGUgSSB3b3VsZCBsaWtlIHRvIHByb3ZpZGUgc29tZSBoZWxwLg0KPiBJIGxvb2tl
ZCBhdCB0aGUgZ2V0c29ja29wdCBpbXBsZW1lbnRhdGlvbi4gRnJvbSB3aGF0IEknbSBzZWVpbmcs
IEkgYmVsaWV2ZSB0aGF0IGl0IHdvdWxkIGJlIGVhc2llciB0bw0KPiBjb3BpZXMgdG8gYSBwdHIg
YXMgdGhlIHN5c2NhbGwuDQo+IFRoZSBsZW5ndGggb2YgdGhlIG91dHB1dCBpcyB1c3VhbGx5IDQg
Ynl0ZXMgKHNvbWV0aW1lcyBsZXNzKSBidXQgaW4gYSBsb3Qgb2YgY2FzZXMsIHRoaXMgbGVuZ3Ro
IGlzDQo+IHZhcmlhYmxlLiBTb21ldGltZSBpdCBjYW4gZXZlbiBiZSBiaWdnZXIgdGhhdCB0aGUg
U1FFMTI4IHJpbmcuDQo+IA0KPiBIZXJlIGlzIGEgbm9uLWV4aGF1c3RpdmUgbGlzdCBvZiB0aG9z
ZSBjYXNlcyA6DQo+IC9uZXQvaXB2NC90Y3AuYyA6IGludCBkb190Y3BfZ2V0c29ja29wdCguLi4p
DQo+ICAgLSBUQ1BfSU5GTyA6IHVwIHRvIDI0MCBieXRlcw0KPiAgIC0gVENQX0NDX0lORk8gYW5k
IFRDUF9SRVBBSVJfV0lORE9XIDogdXAgdG8gMjAgYnl0ZXMNCj4gICAtIFRDUF9DT05HRVNUSU9O
IGFuZCBUQ1BfVUxQIDogdXAgdG8gMTYgYnl0ZXMNCj4gICAtIFRDUF9aRVJPQ1BPWV9SRUNFSVZF
IDogdXAgdG8gNjQgYnl0ZXMNCj4gL25ldC9hdG0vY29tbXVuLmMgOiBpbnQgdmNjX2dldHNvY2tv
cHQoLi4uKQ0KPiAgIC0gU09fQVRNUU9TIDogdXAgdG8gODggYnl0ZXMNCj4gICAtIFNPX0FUTVBW
QyA6IHVwIHRvIDE2IGJ5dGVzDQo+IC9uZXQvaXB2NC9pb19zb2NrZ2x1ZS5jIDogaW50IGRvX2lw
X2dldHNvY2tvcHQoLi4uKQ0KPiAgIC0gTUNBU1RfTVNGSUxURVIgOiB1cCB0byAxNDQgYnl0ZXMN
Cj4gICAtIElQX01TRklMVEVSIDogMTYgYnl0ZXMgbWluaW11bQ0KPiANCj4gSSB3aWxsIGxvb2sg
aW50byBzZXRzb2Nrb3B0IGJ1dCBJIGJlbGlldmUgaXQgbWlnaHQgYmUgdGhlIHNhbWUuDQo+IElm
IG5lZWRlZCBJIGNhbiBhbHNvIGNvbXBsZXRlIHRoaXMgbGlzdC4NCj4gSG93ZXZlciB0aGVyZSBh
cmUgc29tZSBjYXNlcyB3aGVyZSBpdCBpcyBoYXJkIHRvIGRldGVybWluYXRlIGEgbWF4aW11bSBh
bW91bnQgb2YgYnl0ZXMgaW4gYWR2YW5jZS4NCg0KQWxzbyBsb29rIGF0IFNDVFAgLSBpdCBoYXMg
c29tZSB2ZXJ5IGxvbmcgYnVmZmVycy4NCkFsbW9zdCBhbnkgY29kZSB0aGF0IHVzZXMgU0NUUCBu
ZWVkcyB0byB1c2UgdGhlIFNDVFBfU1RBVFVTDQpyZXF1ZXN0IHRvIGdldCB0aGUgbmVnb3RpYXRl
ZCBudW1iZXIgb2YgZGF0YSBzdHJlYW1zDQoodGhhdCBvbmUgaXMgcmVsYXRpdmVseSBzaG9ydCku
DQpJSVJDIHRoZXJlIGFyZSBhbHNvIGdldHNvY2tvcHQoKSB0aGF0IGFyZSByZWFkL21vZGlmeS93
cml0ZSENCg0KVGhlcmUgd2lsbCBhbHNvIGJlIHVzZXIgY29kZSB0aGF0IHN1cHBsaWVzIGEgdmVy
eSBsb25nIGJ1ZmZlcg0KKHRvbyBsb25nIHRvIGFsbG9jYXRlIGluIGtlcm5lbCkgZm9yIHNvbWUg
dmFyaWFibGUgbGVuZ3RoIHJlcXVlc3RzLg0KDQpTbyB0aGUgZ2VuZXJpYyBzeXN0ZW0gY2FsbCBj
b2RlIGNhbiBhbGxvY2F0ZSBhIHNob3J0IChlZyBvbi1zdGFjaykNCmJ1ZmZlciBmb3Igc2hvcnQg
cmVxdWVzdHMgYW5kIHRoZW4gcGFzcyBib3RoIHRoZSB1c2VyIGFuZCBrZXJuZWwNCmFkZHJlc3Nl
cyAoYW5kIGxlbmd0aHMpIHRocm91Z2ggdG8gdGhlIHByb3RvY29sIGZ1bmN0aW9ucy4NCkFueXRo
aW5nIHRoYXQgbmVlZHMgYSBiaWcgYnVmZmVyIGNhbiBkaXJlY3RseSBjb3B5IHRvL2Zyb20NCmFu
ZCB1c2VyIGJ1ZmZlcnMsIGtlcm5lbCBjYWxsZXJzIHdvdWxkIG5lZWQgdG8gcGFzcyBhIGJpZyBl
bm91Z2gNCmJ1ZmZlci4NCg0KQnV0IHRoZSBjb2RlIGZvciBzbWFsbCBidWZmZXJzIHdvdWxkIGJl
IG11Y2ggc2ltcGxpZmllZCBmb3INCmJvdGgga2VybmVsIGFuZCB1c2VyIGFjY2Vzcy4NCg0KCURh
dmlkDQoNCi0NClJlZ2lzdGVyZWQgQWRkcmVzcyBMYWtlc2lkZSwgQnJhbWxleSBSb2FkLCBNb3Vu
dCBGYXJtLCBNaWx0b24gS2V5bmVzLCBNSzEgMVBULCBVSw0KUmVnaXN0cmF0aW9uIE5vOiAxMzk3
Mzg2IChXYWxlcykNCg=