[RFC PATCH v1 0/3] io_uring: Add sendto(2) and recvfrom(2) support

All of lore.kernel.org
 help / color / mirror / Atom feed

* [RFC PATCH v1 0/3] io_uring: Add sendto(2) and recvfrom(2) support
@ 2021-12-30  1:35 Ammar Faizi
  2021-12-30  1:35 ` [RFC PATCH v1 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30  1:35 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi

Hello,

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

## Motivations:
1) By using `sendto()` and `recvfrom()` we can make the submission
   simpler compared to always using `sendmsg()` and `recvmsg()` from
   the userspace.

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

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

## Changes summary
There are 3 patches in this series.

PATCH 1/3 renames io_recv to io_recvfrom and io_send to io_sendto.
Note that

    send(sockfd, buf, len, flags);

  is equivalent to

    sendto(sockfd, buf, len, flags, NULL, 0);

and
    recv(sockfd, buf, len, flags);

  is equivalent to

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

So it is saner to have `send` and `recv` directed to `sendto` and
`recvfrom` instead of the opposite with respect to the name.

PATCH 2/3 makes `move_addr_to_user()` be a non static function. This
function lives in net/socket.c, we need to call this from io_uring
to add `recvfrom()` support for liburing. Added net files maintainers
to the CC list.

PATCH 3/3 adds `sendto(2)` and `recvfrom(2)` support for io_uring.
Added two new opcodes: IORING_OP_SENDTO and IORING_OP_RECVFROM.

## How to test

This patchset is based on "for-next" branch commit:

  aafc6e6eba29c890b0031267fc37c43490447c81 ("Merge branch 'for-5.17/io_uring' into for-next")

It is also available in the Git repository at:

  https://github.com/ammarfaizi2/linux-block ammarfaizi2/linux-block/io_uring-recvfrom-sendto

I also added the liburing support and test. The liburing support is
based on "xattr-getdents64" branch commit:

  18d71076f6c97e1b25aa0e3b0e12a913ec4717fa ("src/include/liburing.h: style cleanups")

It is available in the Git repository at:

  https://github.com/ammarfaizi2/liburing sendto-recvfrom

Link: https://github.com/axboe/liburing/issues/397 [1]
Link: https://lore.kernel.org/io-uring/a2399c89-2c45-375c-7395-b5caf556ec3d@kernel.dk/ [2]
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---
Ammar Faizi (3):
  io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
  net: Make `move_addr_to_user()` be a non static function
  io_uring: Add `sendto(2)` and `recvfrom(2)` support

 fs/io_uring.c                 | 88 +++++++++++++++++++++++++++++++----
 include/linux/socket.h        |  2 +
 include/uapi/linux/io_uring.h |  2 +
 net/socket.c                  |  4 +-
 4 files changed, 86 insertions(+), 10 deletions(-)

base-commit: aafc6e6eba29c890b0031267fc37c43490447c81
-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v1 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
  2021-12-30  1:35 [RFC PATCH v1 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
@ 2021-12-30  1:35 ` Ammar Faizi
  2021-12-30 11:59   ` [RFC PATCH v1 1/3] io_uring: Rename `io_{send, recv}` to `io_{sendto, recvfrom}` kernel test robot
  2021-12-30  1:35 ` [RFC PATCH v1 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
  2021-12-30  1:35 ` [RFC PATCH v1 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
  2 siblings, 1 reply; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30  1:35 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi

Currently we can perform `send` and `recv` via io_uring. And now, we
are going to add `sendto` and `recvfrom` support for io_uring.

Note that:
  Calling `send(fd, buf, len, flags)` is equivalent to calling
  `sendto(fd, buf, len, flags, NULL, 0)`. Therefore, `sendto`
  is a superset of `send`.

  Calling `recv(fd, buf, len, flags)` is equivalent to calling
  `recvfrom(fd, buf, len, flags, NULL, NULL)`. Therefore, `recvfrom`
  is a superset of `recv`.

As such, let's direct the current supported `IORING_OP_{SEND,RECV}` to
`io_{sendto,recvfrom}`. These functions will also be used for
`IORING_OP_{SENDTO,RECVFROM}` operation in the next patches.

Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---
 fs/io_uring.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 90002bb3fdf4..d564f98d5d3b 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5273,7 +5273,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 	return 0;
 }
 
-static int io_send(struct io_kiocb *req, unsigned int issue_flags)
+static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_sr_msg *sr = &req->sr_msg;
 	struct msghdr msg;
@@ -5499,7 +5499,7 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
 	return 0;
 }
 
-static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
+static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_buffer *kbuf;
 	struct io_sr_msg *sr = &req->sr_msg;
@@ -7061,13 +7061,13 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 		ret = io_sendmsg(req, issue_flags);
 		break;
 	case IORING_OP_SEND:
-		ret = io_send(req, issue_flags);
+		ret = io_sendto(req, issue_flags);
 		break;
 	case IORING_OP_RECVMSG:
 		ret = io_recvmsg(req, issue_flags);
 		break;
 	case IORING_OP_RECV:
-		ret = io_recv(req, issue_flags);
+		ret = io_recvfrom(req, issue_flags);
 		break;
 	case IORING_OP_TIMEOUT:
 		ret = io_timeout(req, issue_flags);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v1 2/3] net: Make `move_addr_to_user()` be a non static function
  2021-12-30  1:35 [RFC PATCH v1 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
  2021-12-30  1:35 ` [RFC PATCH v1 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
@ 2021-12-30  1:35 ` Ammar Faizi
  2021-12-30  1:35 ` [RFC PATCH v1 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
  2 siblings, 0 replies; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30  1:35 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi

In order to add recvfrom support for io_uring, we need to call
`move_addr_to_user()` in fs/io_uring.c.

This makes `move_addr_to_user()` be a non static function so we can
call it from io_uring.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---
 include/linux/socket.h | 2 ++
 net/socket.c           | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 8ef26d89ef49..0d0bc1ace50c 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -371,6 +371,8 @@ struct ucred {
 #define IPX_TYPE	1
 
 extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
+extern int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
+			     void __user *uaddr, int __user *ulen);
 extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
 
 struct timespec64;
diff --git a/net/socket.c b/net/socket.c
index 7f64a6eccf63..af521d351c8a 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -267,8 +267,8 @@ int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *k
  *	specified. Zero is returned for a success.
  */
 
-static int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
-			     void __user *uaddr, int __user *ulen)
+int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
+		      void __user *uaddr, int __user *ulen)
 {
 	int err;
 	int len;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v1 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support
  2021-12-30  1:35 [RFC PATCH v1 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
  2021-12-30  1:35 ` [RFC PATCH v1 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
  2021-12-30  1:35 ` [RFC PATCH v1 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
@ 2021-12-30  1:35 ` Ammar Faizi
  2021-12-30 12:00   ` [RFC PATCH v2 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
  2 siblings, 1 reply; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30  1:35 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi

This adds sendto(2) and recvfrom(2) support for io_uring.

New opcodes:
  IORING_OP_SENDTO
  IORING_OP_RECVFROM

Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---
 fs/io_uring.c                 | 80 +++++++++++++++++++++++++++++++++--
 include/uapi/linux/io_uring.h |  2 +
 2 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index d564f98d5d3b..4406c044798f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -575,7 +575,15 @@ struct io_sr_msg {
 	union {
 		struct compat_msghdr __user	*umsg_compat;
 		struct user_msghdr __user	*umsg;
-		void __user			*buf;
+
+		struct {
+			void __user		*buf;
+			struct sockaddr __user	*addr;
+			union {
+				int		sendto_addr_len;
+				int __user	*recvfrom_addr_len;
+			};
+		};
 	};
 	int				msg_flags;
 	int				bgid;
@@ -1133,6 +1141,19 @@ static const struct io_op_def io_op_defs[] = {
 		.needs_file = 1
 	},
 	[IORING_OP_GETXATTR] = {},
+	[IORING_OP_SENDTO] = {
+		.needs_file		= 1,
+		.unbound_nonreg_file	= 1,
+		.pollout		= 1,
+		.audit_skip		= 1,
+	},
+	[IORING_OP_RECVFROM] = {
+		.needs_file		= 1,
+		.unbound_nonreg_file	= 1,
+		.pollin			= 1,
+		.buffer_select		= 1,
+		.audit_skip		= 1,
+	},
 };
 
 /* requests with any of those set should undergo io_disarm_next() */
@@ -5216,12 +5237,24 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
 
+	/*
+	 * For IORING_OP_SEND{,TO}, the assignment to @sr->umsg
+	 * is equivalent to an assignment to @sr->buf.
+	 */
 	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
+
 	sr->len = READ_ONCE(sqe->len);
 	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
 	if (sr->msg_flags & MSG_DONTWAIT)
 		req->flags |= REQ_F_NOWAIT;
 
+	if (req->opcode == IORING_OP_SENDTO) {
+		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+		sr->sendto_addr_len = READ_ONCE(sqe->addr3);
+	} else {
+		sr->addr = (struct sockaddr __user *) NULL;
+	}
+
 #ifdef CONFIG_COMPAT
 	if (req->ctx->compat)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
@@ -5275,6 +5308,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 
 static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 {
+	struct sockaddr_storage address;
 	struct io_sr_msg *sr = &req->sr_msg;
 	struct msghdr msg;
 	struct iovec iov;
@@ -5291,10 +5325,20 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 	if (unlikely(ret))
 		return ret;
 
-	msg.msg_name = NULL;
+
 	msg.msg_control = NULL;
 	msg.msg_controllen = 0;
-	msg.msg_namelen = 0;
+	if (sr->addr) {
+		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
+					  &address);
+		if (unlikely(ret < 0))
+			goto fail;
+		msg.msg_name = (struct sockaddr *) &address;
+		msg.msg_namelen = sr->sendto_addr_len;
+	} else {
+		msg.msg_name = NULL;
+		msg.msg_namelen = 0;
+	}
 
 	flags = req->sr_msg.msg_flags;
 	if (issue_flags & IO_URING_F_NONBLOCK)
@@ -5309,6 +5353,7 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 			return -EAGAIN;
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
+	fail:
 		req_set_fail(req);
 	}
 	__io_req_complete(req, issue_flags, ret, 0);
@@ -5427,13 +5472,25 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
 
+	/*
+	 * For IORING_OP_RECV{,FROM}, the assignment to @sr->umsg
+	 * is equivalent to an assignment to @sr->buf.
+	 */
 	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
+
 	sr->len = READ_ONCE(sqe->len);
 	sr->bgid = READ_ONCE(sqe->buf_group);
 	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
 	if (sr->msg_flags & MSG_DONTWAIT)
 		req->flags |= REQ_F_NOWAIT;
 
+	if (req->opcode == IORING_OP_RECVFROM) {
+		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+		sr->recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr3));
+	} else {
+		sr->addr = (struct sockaddr __user *) NULL;
+	}
+
 #ifdef CONFIG_COMPAT
 	if (req->ctx->compat)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
@@ -5509,6 +5566,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 	struct iovec iov;
 	unsigned flags;
 	int ret, min_ret = 0;
+	struct sockaddr_storage address;
 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
 
 	sock = sock_from_file(req->file);
@@ -5526,7 +5584,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 	if (unlikely(ret))
 		goto out_free;
 
-	msg.msg_name = NULL;
+	msg.msg_name = sr->addr ? (struct sockaddr *) &address : NULL;
 	msg.msg_control = NULL;
 	msg.msg_controllen = 0;
 	msg.msg_namelen = 0;
@@ -5540,6 +5598,16 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 		min_ret = iov_iter_count(&msg.msg_iter);
 
 	ret = sock_recvmsg(sock, &msg, flags);
+
+	if (ret >= 0 && sr->addr != NULL) {
+		int tmp;
+
+		tmp = move_addr_to_user(&address, msg.msg_namelen, sr->addr,
+					sr->recvfrom_addr_len);
+		if (tmp < 0)
+			ret = tmp;
+	}
+
 out_free:
 	if (ret < min_ret) {
 		if (ret == -EAGAIN && force_nonblock)
@@ -6778,9 +6846,11 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	case IORING_OP_SYNC_FILE_RANGE:
 		return io_sfr_prep(req, sqe);
 	case IORING_OP_SENDMSG:
+	case IORING_OP_SENDTO:
 	case IORING_OP_SEND:
 		return io_sendmsg_prep(req, sqe);
 	case IORING_OP_RECVMSG:
+	case IORING_OP_RECVFROM:
 	case IORING_OP_RECV:
 		return io_recvmsg_prep(req, sqe);
 	case IORING_OP_CONNECT:
@@ -7060,12 +7130,14 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 	case IORING_OP_SENDMSG:
 		ret = io_sendmsg(req, issue_flags);
 		break;
+	case IORING_OP_SENDTO:
 	case IORING_OP_SEND:
 		ret = io_sendto(req, issue_flags);
 		break;
 	case IORING_OP_RECVMSG:
 		ret = io_recvmsg(req, issue_flags);
 		break;
+	case IORING_OP_RECVFROM:
 	case IORING_OP_RECV:
 		ret = io_recvfrom(req, issue_flags);
 		break;
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index efc7ac9b3a6b..a360069d1e8e 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -150,6 +150,8 @@ enum {
 	IORING_OP_SETXATTR,
 	IORING_OP_FGETXATTR,
 	IORING_OP_GETXATTR,
+	IORING_OP_SENDTO,
+	IORING_OP_RECVFROM,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v1 1/3] io_uring: Rename `io_{send, recv}` to `io_{sendto, recvfrom}`
  2021-12-30  1:35 ` [RFC PATCH v1 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
@ 2021-12-30 11:59   ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-12-30 11:59 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 8123 bytes --]

Hi Ammar,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on aafc6e6eba29c890b0031267fc37c43490447c81]

url:    https://github.com/0day-ci/linux/commits/Ammar-Faizi/io_uring-Add-sendto-2-and-recvfrom-2-support/20211230-093703
base:   aafc6e6eba29c890b0031267fc37c43490447c81
config: nds32-randconfig-r036-20211230 (https://download.01.org/0day-ci/archive/20211230/202112301905.cGMjzzKP-lkp(a)intel.com/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/9cfe685bad543158dec1c3a7560036b2e2f93f89
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Ammar-Faizi/io_uring-Add-sendto-2-and-recvfrom-2-support/20211230-093703
        git checkout 9cfe685bad543158dec1c3a7560036b2e2f93f89
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   fs/io_uring.c: In function '__io_submit_flush_completions':
   fs/io_uring.c:2541:40: warning: variable 'prev' set but not used [-Wunused-but-set-variable]
    2541 |         struct io_wq_work_node *node, *prev;
         |                                        ^~~~
   fs/io_uring.c: In function 'io_issue_sqe':
>> fs/io_uring.c:7064:23: error: implicit declaration of function 'io_sendto'; did you mean 'io_send'? [-Werror=implicit-function-declaration]
    7064 |                 ret = io_sendto(req, issue_flags);
         |                       ^~~~~~~~~
         |                       io_send
>> fs/io_uring.c:7070:23: error: implicit declaration of function 'io_recvfrom'; did you mean 'sys_recvfrom'? [-Werror=implicit-function-declaration]
    7070 |                 ret = io_recvfrom(req, issue_flags);
         |                       ^~~~~~~~~~~
         |                       sys_recvfrom
   At top level:
   fs/io_uring.c:5687:12: warning: 'io_recv' defined but not used [-Wunused-function]
    5687 | static int io_##op(struct io_kiocb *req, unsigned int issue_flags)      \
         |            ^~~
   fs/io_uring.c:5711:1: note: in expansion of macro 'IO_NETOP_FN'
    5711 | IO_NETOP_FN(recv);
         | ^~~~~~~~~~~
   fs/io_uring.c:5687:12: warning: 'io_send' defined but not used [-Wunused-function]
    5687 | static int io_##op(struct io_kiocb *req, unsigned int issue_flags)      \
         |            ^~~
   fs/io_uring.c:5710:1: note: in expansion of macro 'IO_NETOP_FN'
    5710 | IO_NETOP_FN(send);
         | ^~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +7064 fs/io_uring.c

  7022	
  7023	static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
  7024	{
  7025		const struct cred *creds = NULL;
  7026		int ret;
  7027	
  7028		if (unlikely((req->flags & REQ_F_CREDS) && req->creds != current_cred()))
  7029			creds = override_creds(req->creds);
  7030	
  7031		if (!io_op_defs[req->opcode].audit_skip)
  7032			audit_uring_entry(req->opcode);
  7033	
  7034		switch (req->opcode) {
  7035		case IORING_OP_NOP:
  7036			ret = io_nop(req, issue_flags);
  7037			break;
  7038		case IORING_OP_READV:
  7039		case IORING_OP_READ_FIXED:
  7040		case IORING_OP_READ:
  7041			ret = io_read(req, issue_flags);
  7042			break;
  7043		case IORING_OP_WRITEV:
  7044		case IORING_OP_WRITE_FIXED:
  7045		case IORING_OP_WRITE:
  7046			ret = io_write(req, issue_flags);
  7047			break;
  7048		case IORING_OP_FSYNC:
  7049			ret = io_fsync(req, issue_flags);
  7050			break;
  7051		case IORING_OP_POLL_ADD:
  7052			ret = io_poll_add(req, issue_flags);
  7053			break;
  7054		case IORING_OP_POLL_REMOVE:
  7055			ret = io_poll_update(req, issue_flags);
  7056			break;
  7057		case IORING_OP_SYNC_FILE_RANGE:
  7058			ret = io_sync_file_range(req, issue_flags);
  7059			break;
  7060		case IORING_OP_SENDMSG:
  7061			ret = io_sendmsg(req, issue_flags);
  7062			break;
  7063		case IORING_OP_SEND:
> 7064			ret = io_sendto(req, issue_flags);
  7065			break;
  7066		case IORING_OP_RECVMSG:
  7067			ret = io_recvmsg(req, issue_flags);
  7068			break;
  7069		case IORING_OP_RECV:
> 7070			ret = io_recvfrom(req, issue_flags);
  7071			break;
  7072		case IORING_OP_TIMEOUT:
  7073			ret = io_timeout(req, issue_flags);
  7074			break;
  7075		case IORING_OP_TIMEOUT_REMOVE:
  7076			ret = io_timeout_remove(req, issue_flags);
  7077			break;
  7078		case IORING_OP_ACCEPT:
  7079			ret = io_accept(req, issue_flags);
  7080			break;
  7081		case IORING_OP_CONNECT:
  7082			ret = io_connect(req, issue_flags);
  7083			break;
  7084		case IORING_OP_ASYNC_CANCEL:
  7085			ret = io_async_cancel(req, issue_flags);
  7086			break;
  7087		case IORING_OP_FALLOCATE:
  7088			ret = io_fallocate(req, issue_flags);
  7089			break;
  7090		case IORING_OP_OPENAT:
  7091			ret = io_openat(req, issue_flags);
  7092			break;
  7093		case IORING_OP_CLOSE:
  7094			ret = io_close(req, issue_flags);
  7095			break;
  7096		case IORING_OP_FILES_UPDATE:
  7097			ret = io_files_update(req, issue_flags);
  7098			break;
  7099		case IORING_OP_STATX:
  7100			ret = io_statx(req, issue_flags);
  7101			break;
  7102		case IORING_OP_FADVISE:
  7103			ret = io_fadvise(req, issue_flags);
  7104			break;
  7105		case IORING_OP_MADVISE:
  7106			ret = io_madvise(req, issue_flags);
  7107			break;
  7108		case IORING_OP_OPENAT2:
  7109			ret = io_openat2(req, issue_flags);
  7110			break;
  7111		case IORING_OP_EPOLL_CTL:
  7112			ret = io_epoll_ctl(req, issue_flags);
  7113			break;
  7114		case IORING_OP_SPLICE:
  7115			ret = io_splice(req, issue_flags);
  7116			break;
  7117		case IORING_OP_PROVIDE_BUFFERS:
  7118			ret = io_provide_buffers(req, issue_flags);
  7119			break;
  7120		case IORING_OP_REMOVE_BUFFERS:
  7121			ret = io_remove_buffers(req, issue_flags);
  7122			break;
  7123		case IORING_OP_TEE:
  7124			ret = io_tee(req, issue_flags);
  7125			break;
  7126		case IORING_OP_SHUTDOWN:
  7127			ret = io_shutdown(req, issue_flags);
  7128			break;
  7129		case IORING_OP_RENAMEAT:
  7130			ret = io_renameat(req, issue_flags);
  7131			break;
  7132		case IORING_OP_UNLINKAT:
  7133			ret = io_unlinkat(req, issue_flags);
  7134			break;
  7135		case IORING_OP_MKDIRAT:
  7136			ret = io_mkdirat(req, issue_flags);
  7137			break;
  7138		case IORING_OP_SYMLINKAT:
  7139			ret = io_symlinkat(req, issue_flags);
  7140			break;
  7141		case IORING_OP_LINKAT:
  7142			ret = io_linkat(req, issue_flags);
  7143			break;
  7144		case IORING_OP_GETDENTS:
  7145			ret = io_getdents(req, issue_flags);
  7146			break;
  7147		case IORING_OP_FSETXATTR:
  7148			ret = io_fsetxattr(req, issue_flags);
  7149			break;
  7150		case IORING_OP_SETXATTR:
  7151			ret = io_setxattr(req, issue_flags);
  7152			break;
  7153		case IORING_OP_FGETXATTR:
  7154			ret = io_fgetxattr(req, issue_flags);
  7155			break;
  7156		case IORING_OP_GETXATTR:
  7157			ret = io_getxattr(req, issue_flags);
  7158			break;
  7159		default:
  7160			ret = -EINVAL;
  7161			break;
  7162		}
  7163	
  7164		if (!io_op_defs[req->opcode].audit_skip)
  7165			audit_uring_exit(!ret, ret);
  7166	
  7167		if (creds)
  7168			revert_creds(creds);
  7169		if (ret)
  7170			return ret;
  7171		/* If the op doesn't have a file, we're not polling for it */
  7172		if ((req->ctx->flags & IORING_SETUP_IOPOLL) && req->file)
  7173			io_iopoll_req_issued(req, issue_flags);
  7174	
  7175		return 0;
  7176	}
  7177	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 0/3] io_uring: Add sendto(2) and recvfrom(2) support
  2021-12-30  1:35 ` [RFC PATCH v1 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
@ 2021-12-30 12:00   ` Ammar Faizi
  2021-12-30 12:00     ` [RFC PATCH v2 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
                       ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30 12:00 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi, Nugra

Hello,

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


## Motivations:
1) By using `sendto()` and `recvfrom()` we can make the submission
   simpler compared to always using `sendmsg()` and `recvmsg()` from
   the userspace.

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

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


## Changes summary
There are 3 patches in this series.

PATCH 1/3 renames io_recv to io_recvfrom and io_send to io_sendto.
Note that

    send(sockfd, buf, len, flags);

  is equivalent to

    sendto(sockfd, buf, len, flags, NULL, 0);

and
    recv(sockfd, buf, len, flags);

  is equivalent to

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

So it is saner to have `send` and `recv` directed to `sendto` and
`recvfrom` instead of the opposite with respect to the name.


PATCH 2/3 makes `move_addr_to_user()` be a non static function. This
function lives in net/socket.c, we need to call this from io_uring
to add `recvfrom()` support for liburing. Added net files maintainers
to the CC list.

PATCH 3/3 adds `sendto(2)` and `recvfrom(2)` support for io_uring.
Added two new opcodes: IORING_OP_SENDTO and IORING_OP_RECVFROM.


## How to test

This patchset is based on "for-next" branch commit:

  bb3294e22482db4b7ec7cfbb2d0f5b53c1adcf86 ("Merge branch 'for-5.17/drivers' into for-next")

It is also available in the Git repository at:

  https://github.com/ammarfaizi2/linux-block ammarfaizi2/linux-block/io_uring-recvfrom-sendto


I also added the liburing support and test. The liburing support is
based on "xattr-getdents64" branch commit:

  55a9bf979f27f3a5c9f456f26dcfe16c4791667b ("src/include/liburing.h: style cleanups")

It is available in the Git repository at:

  https://github.com/ammarfaizi2/liburing sendto-recvfrom

---
v2:
  - Rebased the work, now this patchset is based on commit
    bb3294e22482db4b7ec ("Merge branch 'for-5.17/drivers' into
    for-next").

  - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
    call as unlikely.

  - Fix build error when CONFIG_NET is undefined.

  - Update liburing test (the branch is still the same, just force
    pushed).

  - Added Nugra to CC list (tester).

---
RFC v1: https://lore.kernel.org/io-uring/20211230013154.102910-1-ammar.faizi@intel.com/
Link: https://github.com/axboe/liburing/issues/397 [1]
Link: https://lore.kernel.org/io-uring/a2399c89-2c45-375c-7395-b5caf556ec3d@kernel.dk/ [2]
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: Nugra <richiisei@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---
Ammar Faizi (3):
  io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
  net: Make `move_addr_to_user()` be a non static function
  io_uring: Add `sendto(2)` and `recvfrom(2)` support

 fs/io_uring.c                 | 92 +++++++++++++++++++++++++++++++----
 include/linux/socket.h        |  2 +
 include/uapi/linux/io_uring.h |  2 +
 net/socket.c                  |  4 +-
 4 files changed, 88 insertions(+), 12 deletions(-)


base-commit: bb3294e22482db4b7ec7cfbb2d0f5b53c1adcf86
-- 
2.32.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
  2021-12-30 12:00   ` [RFC PATCH v2 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
@ 2021-12-30 12:00     ` Ammar Faizi
  2021-12-30 12:00     ` [RFC PATCH v2 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
  2021-12-30 12:00     ` [RFC PATCH v2 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
  2 siblings, 0 replies; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30 12:00 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi, Nugra

Currently we can perform `send` and `recv` via io_uring. And now, we
are going to add `sendto` and `recvfrom` support for io_uring.

Note that:
  Calling `send(fd, buf, len, flags)` is equivalent to calling
  `sendto(fd, buf, len, flags, NULL, 0)`. Therefore, `sendto`
  is a superset of `send`.

  Calling `recv(fd, buf, len, flags)` is equivalent to calling
  `recvfrom(fd, buf, len, flags, NULL, NULL)`. Therefore, `recvfrom`
  is a superset of `recv`.

As such, let's direct the current supported `IORING_OP_{SEND,RECV}` to
`io_{sendto,recvfrom}`. These functions will also be used for
`IORING_OP_{SENDTO,RECVFROM}` operation in the next patches.

Cc: Nugra <richiisei@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---
 fs/io_uring.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 90002bb3fdf4..d564f98d5d3b 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5273,7 +5273,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 	return 0;
 }
 
-static int io_send(struct io_kiocb *req, unsigned int issue_flags)
+static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_sr_msg *sr = &req->sr_msg;
 	struct msghdr msg;
@@ -5499,7 +5499,7 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
 	return 0;
 }
 
-static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
+static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_buffer *kbuf;
 	struct io_sr_msg *sr = &req->sr_msg;
@@ -7061,13 +7061,13 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 		ret = io_sendmsg(req, issue_flags);
 		break;
 	case IORING_OP_SEND:
-		ret = io_send(req, issue_flags);
+		ret = io_sendto(req, issue_flags);
 		break;
 	case IORING_OP_RECVMSG:
 		ret = io_recvmsg(req, issue_flags);
 		break;
 	case IORING_OP_RECV:
-		ret = io_recv(req, issue_flags);
+		ret = io_recvfrom(req, issue_flags);
 		break;
 	case IORING_OP_TIMEOUT:
 		ret = io_timeout(req, issue_flags);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 2/3] net: Make `move_addr_to_user()` be a non static function
  2021-12-30 12:00   ` [RFC PATCH v2 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
  2021-12-30 12:00     ` [RFC PATCH v2 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
@ 2021-12-30 12:00     ` Ammar Faizi
  2021-12-30 12:00     ` [RFC PATCH v2 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
  2 siblings, 0 replies; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30 12:00 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi, Nugra

In order to add recvfrom support for io_uring, we need to call
`move_addr_to_user()` in fs/io_uring.c.

This makes `move_addr_to_user()` be a non static function so we can
call it from io_uring.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: Nugra <richiisei@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---
 include/linux/socket.h | 2 ++
 net/socket.c           | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 8ef26d89ef49..0d0bc1ace50c 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -371,6 +371,8 @@ struct ucred {
 #define IPX_TYPE	1
 
 extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
+extern int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
+			     void __user *uaddr, int __user *ulen);
 extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
 
 struct timespec64;
diff --git a/net/socket.c b/net/socket.c
index 7f64a6eccf63..af521d351c8a 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -267,8 +267,8 @@ int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *k
  *	specified. Zero is returned for a success.
  */
 
-static int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
-			     void __user *uaddr, int __user *ulen)
+int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
+		      void __user *uaddr, int __user *ulen)
 {
 	int err;
 	int len;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v2 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support
  2021-12-30 12:00   ` [RFC PATCH v2 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
  2021-12-30 12:00     ` [RFC PATCH v2 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
  2021-12-30 12:00     ` [RFC PATCH v2 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
@ 2021-12-30 12:00     ` Ammar Faizi
  2021-12-30 17:52       ` [RFC PATCH v3 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
  2 siblings, 1 reply; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30 12:00 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi, Nugra

This adds sendto(2) and recvfrom(2) support for io_uring.

New opcodes:
  IORING_OP_SENDTO
  IORING_OP_RECVFROM

Cc: Nugra <richiisei@gmail.com>
Link: https://github.com/axboe/liburing/issues/397
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---

v2:
  - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
    call as unlikely.

  - Fix build error when CONFIG_NET is undefined.

 fs/io_uring.c                 | 84 ++++++++++++++++++++++++++++++++---
 include/uapi/linux/io_uring.h |  2 +
 2 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index d564f98d5d3b..3726958f8f58 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -575,7 +575,15 @@ struct io_sr_msg {
 	union {
 		struct compat_msghdr __user	*umsg_compat;
 		struct user_msghdr __user	*umsg;
-		void __user			*buf;
+
+		struct {
+			void __user		*buf;
+			struct sockaddr __user	*addr;
+			union {
+				int		sendto_addr_len;
+				int __user	*recvfrom_addr_len;
+			};
+		};
 	};
 	int				msg_flags;
 	int				bgid;
@@ -1133,6 +1141,19 @@ static const struct io_op_def io_op_defs[] = {
 		.needs_file = 1
 	},
 	[IORING_OP_GETXATTR] = {},
+	[IORING_OP_SENDTO] = {
+		.needs_file		= 1,
+		.unbound_nonreg_file	= 1,
+		.pollout		= 1,
+		.audit_skip		= 1,
+	},
+	[IORING_OP_RECVFROM] = {
+		.needs_file		= 1,
+		.unbound_nonreg_file	= 1,
+		.pollin			= 1,
+		.buffer_select		= 1,
+		.audit_skip		= 1,
+	},
 };
 
 /* requests with any of those set should undergo io_disarm_next() */
@@ -5216,12 +5237,24 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
 
+	/*
+	 * For IORING_OP_SEND{,TO}, the assignment to @sr->umsg
+	 * is equivalent to an assignment to @sr->buf.
+	 */
 	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
+
 	sr->len = READ_ONCE(sqe->len);
 	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
 	if (sr->msg_flags & MSG_DONTWAIT)
 		req->flags |= REQ_F_NOWAIT;
 
+	if (req->opcode == IORING_OP_SENDTO) {
+		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+		sr->sendto_addr_len = READ_ONCE(sqe->addr3);
+	} else {
+		sr->addr = (struct sockaddr __user *) NULL;
+	}
+
 #ifdef CONFIG_COMPAT
 	if (req->ctx->compat)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
@@ -5275,6 +5308,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 
 static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 {
+	struct sockaddr_storage address;
 	struct io_sr_msg *sr = &req->sr_msg;
 	struct msghdr msg;
 	struct iovec iov;
@@ -5291,10 +5325,20 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 	if (unlikely(ret))
 		return ret;
 
-	msg.msg_name = NULL;
+
 	msg.msg_control = NULL;
 	msg.msg_controllen = 0;
-	msg.msg_namelen = 0;
+	if (sr->addr) {
+		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
+					  &address);
+		if (unlikely(ret < 0))
+			goto fail;
+		msg.msg_name = (struct sockaddr *) &address;
+		msg.msg_namelen = sr->sendto_addr_len;
+	} else {
+		msg.msg_name = NULL;
+		msg.msg_namelen = 0;
+	}
 
 	flags = req->sr_msg.msg_flags;
 	if (issue_flags & IO_URING_F_NONBLOCK)
@@ -5309,6 +5353,7 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 			return -EAGAIN;
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
+	fail:
 		req_set_fail(req);
 	}
 	__io_req_complete(req, issue_flags, ret, 0);
@@ -5427,13 +5472,25 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
 
+	/*
+	 * For IORING_OP_RECV{,FROM}, the assignment to @sr->umsg
+	 * is equivalent to an assignment to @sr->buf.
+	 */
 	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
+
 	sr->len = READ_ONCE(sqe->len);
 	sr->bgid = READ_ONCE(sqe->buf_group);
 	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
 	if (sr->msg_flags & MSG_DONTWAIT)
 		req->flags |= REQ_F_NOWAIT;
 
+	if (req->opcode == IORING_OP_RECVFROM) {
+		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+		sr->recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr3));
+	} else {
+		sr->addr = (struct sockaddr __user *) NULL;
+	}
+
 #ifdef CONFIG_COMPAT
 	if (req->ctx->compat)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
@@ -5509,6 +5566,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 	struct iovec iov;
 	unsigned flags;
 	int ret, min_ret = 0;
+	struct sockaddr_storage address;
 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
 
 	sock = sock_from_file(req->file);
@@ -5526,7 +5584,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 	if (unlikely(ret))
 		goto out_free;
 
-	msg.msg_name = NULL;
+	msg.msg_name = sr->addr ? (struct sockaddr *) &address : NULL;
 	msg.msg_control = NULL;
 	msg.msg_controllen = 0;
 	msg.msg_namelen = 0;
@@ -5540,6 +5598,16 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 		min_ret = iov_iter_count(&msg.msg_iter);
 
 	ret = sock_recvmsg(sock, &msg, flags);
+
+	if (ret >= 0 && sr->addr != NULL) {
+		int tmp;
+
+		tmp = move_addr_to_user(&address, msg.msg_namelen, sr->addr,
+					sr->recvfrom_addr_len);
+		if (unlikely(tmp < 0))
+			ret = tmp;
+	}
+
 out_free:
 	if (ret < min_ret) {
 		if (ret == -EAGAIN && force_nonblock)
@@ -5707,8 +5775,8 @@ IO_NETOP_PREP_ASYNC(sendmsg);
 IO_NETOP_PREP_ASYNC(recvmsg);
 IO_NETOP_PREP_ASYNC(connect);
 IO_NETOP_PREP(accept);
-IO_NETOP_FN(send);
-IO_NETOP_FN(recv);
+IO_NETOP_FN(sendto);
+IO_NETOP_FN(recvfrom);
 #endif /* CONFIG_NET */
 
 struct io_poll_table {
@@ -6778,9 +6846,11 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	case IORING_OP_SYNC_FILE_RANGE:
 		return io_sfr_prep(req, sqe);
 	case IORING_OP_SENDMSG:
+	case IORING_OP_SENDTO:
 	case IORING_OP_SEND:
 		return io_sendmsg_prep(req, sqe);
 	case IORING_OP_RECVMSG:
+	case IORING_OP_RECVFROM:
 	case IORING_OP_RECV:
 		return io_recvmsg_prep(req, sqe);
 	case IORING_OP_CONNECT:
@@ -7060,12 +7130,14 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 	case IORING_OP_SENDMSG:
 		ret = io_sendmsg(req, issue_flags);
 		break;
+	case IORING_OP_SENDTO:
 	case IORING_OP_SEND:
 		ret = io_sendto(req, issue_flags);
 		break;
 	case IORING_OP_RECVMSG:
 		ret = io_recvmsg(req, issue_flags);
 		break;
+	case IORING_OP_RECVFROM:
 	case IORING_OP_RECV:
 		ret = io_recvfrom(req, issue_flags);
 		break;
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index efc7ac9b3a6b..a360069d1e8e 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -150,6 +150,8 @@ enum {
 	IORING_OP_SETXATTR,
 	IORING_OP_FGETXATTR,
 	IORING_OP_GETXATTR,
+	IORING_OP_SENDTO,
+	IORING_OP_RECVFROM,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v3 0/3] io_uring: Add sendto(2) and recvfrom(2) support
  2021-12-30 12:00     ` [RFC PATCH v2 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
@ 2021-12-30 17:52       ` Ammar Faizi
  2021-12-30 17:52         ` [RFC PATCH v3 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
                           ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30 17:52 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi, Nugra

Hello,

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


## Motivations:
1) By using `sendto()` and `recvfrom()` we can make the submission
   simpler compared to always using `sendmsg()` and `recvmsg()` from
   the userspace.

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

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


## Changes summary
There are 3 patches in this series.

PATCH 1/3 renames io_recv to io_recvfrom and io_send to io_sendto.
Note that

    send(sockfd, buf, len, flags);

  is equivalent to

    sendto(sockfd, buf, len, flags, NULL, 0);

and
    recv(sockfd, buf, len, flags);

  is equivalent to

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

So it is saner to have `send` and `recv` directed to `sendto` and
`recvfrom` instead of the opposite with respect to the name.


PATCH 2/3 makes `move_addr_to_user()` be a non static function. This
function lives in net/socket.c, we need to call this from io_uring
to add `recvfrom()` support for liburing. Added net files maintainers
to the CC list.

PATCH 3/3 adds `sendto(2)` and `recvfrom(2)` support for io_uring.
Added two new opcodes: IORING_OP_SENDTO and IORING_OP_RECVFROM.


## How to test

This patchset is based on "for-next" branch commit:

  bb3294e22482db4b7ec7cfbb2d0f5b53c1adcf86 ("Merge branch 'for-5.17/drivers' into for-next")

It is also available in the Git repository at:

  https://github.com/ammarfaizi2/linux-block ammarfaizi2/linux-block/io_uring-recvfrom-sendto


I also added the liburing support and test. The liburing support is
based on "xattr-getdents64" branch commit:

  55a9bf979f27f3a5c9f456f26dcfe16c4791667b ("src/include/liburing.h: style cleanups")

It is available in the Git repository at:

  https://github.com/ammarfaizi2/liburing sendto-recvfrom

---
v3:
  - Fix build error when CONFIG_NET is undefined for PATCH 1/3. I
    tried to fix it in PATCH 3/3, but it should be fixed in PATCH 1/3,
    otherwise it breaks the build in PATCH 1/3.

  - Added `io_uring_prep_{sendto,recvfrom}` docs to the liburing.

v2:
  - Rebased the work, now this patchset is based on commit
    bb3294e22482db4b7ec ("Merge branch 'for-5.17/drivers' into
    for-next").

  - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
    call as unlikely.

  - Fix build error when CONFIG_NET is undefined.

  - Update liburing test (the branch is still the same, just force
    pushed).

  - Added Nugra to CC list (tester).

---
RFC v2: https://lore.kernel.org/io-uring/20211230114846.137954-1-ammar.faizi@intel.com/
RFC v1: https://lore.kernel.org/io-uring/20211230013154.102910-1-ammar.faizi@intel.com/
Link: https://github.com/axboe/liburing/issues/397 [1]
Link: https://lore.kernel.org/io-uring/a2399c89-2c45-375c-7395-b5caf556ec3d@kernel.dk/ [2]
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: Nugra <richiisei@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---

Ammar Faizi (3):
  io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
  net: Make `move_addr_to_user()` be a non static function
  io_uring: Add `sendto(2)` and `recvfrom(2)` support

 fs/io_uring.c                 | 92 +++++++++++++++++++++++++++++++----
 include/linux/socket.h        |  2 +
 include/uapi/linux/io_uring.h |  2 +
 net/socket.c                  |  4 +-
 4 files changed, 88 insertions(+), 12 deletions(-)


base-commit: bb3294e22482db4b7ec7cfbb2d0f5b53c1adcf86
-- 
Ammar Faizi


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH v3 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}`
  2021-12-30 17:52       ` [RFC PATCH v3 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
@ 2021-12-30 17:52         ` Ammar Faizi
  2021-12-30 17:52         ` [RFC PATCH v3 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
  2021-12-30 17:52         ` [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
  2 siblings, 0 replies; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30 17:52 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi, Nugra

Currently we can perform `send` and `recv` via io_uring. And now, we
are going to add `sendto` and `recvfrom` support for io_uring.

Note that:
  Calling `send(fd, buf, len, flags)` is equivalent to calling
  `sendto(fd, buf, len, flags, NULL, 0)`. Therefore, `sendto`
  is a superset of `send`.

  Calling `recv(fd, buf, len, flags)` is equivalent to calling
  `recvfrom(fd, buf, len, flags, NULL, NULL)`. Therefore, `recvfrom`
  is a superset of `recv`.

As such, let's direct the current supported `IORING_OP_{SEND,RECV}` to
`io_{sendto,recvfrom}`. These functions will also be used for
`IORING_OP_{SENDTO,RECVFROM}` operation in the next patches.

Cc: Nugra <richiisei@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---

v3:
  - Fix build error when CONFIG_NET is undefined for PATCH 1/3. I
    tried to fix it in PATCH 3/3, but it should be fixed in PATCH 1/3,
    otherwise it breaks the build in PATCH 1/3.

v2:
  - Added Nugra to CC list (tester).
---
 fs/io_uring.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 90002bb3fdf4..7adcb591398f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5273,7 +5273,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 	return 0;
 }
 
-static int io_send(struct io_kiocb *req, unsigned int issue_flags)
+static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_sr_msg *sr = &req->sr_msg;
 	struct msghdr msg;
@@ -5499,7 +5499,7 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
 	return 0;
 }
 
-static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
+static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_buffer *kbuf;
 	struct io_sr_msg *sr = &req->sr_msg;
@@ -5707,8 +5707,8 @@ IO_NETOP_PREP_ASYNC(sendmsg);
 IO_NETOP_PREP_ASYNC(recvmsg);
 IO_NETOP_PREP_ASYNC(connect);
 IO_NETOP_PREP(accept);
-IO_NETOP_FN(send);
-IO_NETOP_FN(recv);
+IO_NETOP_FN(sendto);
+IO_NETOP_FN(recvfrom);
 #endif /* CONFIG_NET */
 
 struct io_poll_table {
@@ -7061,13 +7061,13 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 		ret = io_sendmsg(req, issue_flags);
 		break;
 	case IORING_OP_SEND:
-		ret = io_send(req, issue_flags);
+		ret = io_sendto(req, issue_flags);
 		break;
 	case IORING_OP_RECVMSG:
 		ret = io_recvmsg(req, issue_flags);
 		break;
 	case IORING_OP_RECV:
-		ret = io_recv(req, issue_flags);
+		ret = io_recvfrom(req, issue_flags);
 		break;
 	case IORING_OP_TIMEOUT:
 		ret = io_timeout(req, issue_flags);
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v3 2/3] net: Make `move_addr_to_user()` be a non static function
  2021-12-30 17:52       ` [RFC PATCH v3 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
  2021-12-30 17:52         ` [RFC PATCH v3 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
@ 2021-12-30 17:52         ` Ammar Faizi
  2021-12-30 17:52         ` [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
  2 siblings, 0 replies; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30 17:52 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi, Nugra

In order to add recvfrom support for io_uring, we need to call
`move_addr_to_user()` in fs/io_uring.c.

This makes `move_addr_to_user()` be a non static function so we can
call it from io_uring.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: netdev@vger.kernel.org
Cc: Nugra <richiisei@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---

v3:
  * No changes *

v2:
  - Added Nugra to CC list (tester).
---
 include/linux/socket.h | 2 ++
 net/socket.c           | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 8ef26d89ef49..0d0bc1ace50c 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -371,6 +371,8 @@ struct ucred {
 #define IPX_TYPE	1
 
 extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
+extern int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
+			     void __user *uaddr, int __user *ulen);
 extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
 
 struct timespec64;
diff --git a/net/socket.c b/net/socket.c
index 7f64a6eccf63..af521d351c8a 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -267,8 +267,8 @@ int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *k
  *	specified. Zero is returned for a success.
  */
 
-static int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
-			     void __user *uaddr, int __user *ulen)
+int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
+		      void __user *uaddr, int __user *ulen)
 {
 	int err;
 	int len;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support
  2021-12-30 17:52       ` [RFC PATCH v3 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
  2021-12-30 17:52         ` [RFC PATCH v3 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
  2021-12-30 17:52         ` [RFC PATCH v3 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
@ 2021-12-30 17:52         ` Ammar Faizi
  2022-01-06 17:31           ` Praveen Kumar
  2 siblings, 1 reply; 19+ messages in thread
From: Ammar Faizi @ 2021-12-30 17:52 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Ammar Faizi, Nugra

This adds sendto(2) and recvfrom(2) support for io_uring.

New opcodes:
  IORING_OP_SENDTO
  IORING_OP_RECVFROM

Cc: Nugra <richiisei@gmail.com>
Tested-by: Nugra <richiisei@gmail.com>
Link: https://github.com/axboe/liburing/issues/397
Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
---

v3:
  - Fix build error when CONFIG_NET is undefined should be done in
    the first patch, not this patch.

  - Add Tested-by tag from Nugra.

v2:
  - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
    call as unlikely.

  - Fix build error when CONFIG_NET is undefined.

  - Added Nugra to CC list (tester).
---
 fs/io_uring.c                 | 80 +++++++++++++++++++++++++++++++++--
 include/uapi/linux/io_uring.h |  2 +
 2 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 7adcb591398f..3726958f8f58 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -575,7 +575,15 @@ struct io_sr_msg {
 	union {
 		struct compat_msghdr __user	*umsg_compat;
 		struct user_msghdr __user	*umsg;
-		void __user			*buf;
+
+		struct {
+			void __user		*buf;
+			struct sockaddr __user	*addr;
+			union {
+				int		sendto_addr_len;
+				int __user	*recvfrom_addr_len;
+			};
+		};
 	};
 	int				msg_flags;
 	int				bgid;
@@ -1133,6 +1141,19 @@ static const struct io_op_def io_op_defs[] = {
 		.needs_file = 1
 	},
 	[IORING_OP_GETXATTR] = {},
+	[IORING_OP_SENDTO] = {
+		.needs_file		= 1,
+		.unbound_nonreg_file	= 1,
+		.pollout		= 1,
+		.audit_skip		= 1,
+	},
+	[IORING_OP_RECVFROM] = {
+		.needs_file		= 1,
+		.unbound_nonreg_file	= 1,
+		.pollin			= 1,
+		.buffer_select		= 1,
+		.audit_skip		= 1,
+	},
 };
 
 /* requests with any of those set should undergo io_disarm_next() */
@@ -5216,12 +5237,24 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
 
+	/*
+	 * For IORING_OP_SEND{,TO}, the assignment to @sr->umsg
+	 * is equivalent to an assignment to @sr->buf.
+	 */
 	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
+
 	sr->len = READ_ONCE(sqe->len);
 	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
 	if (sr->msg_flags & MSG_DONTWAIT)
 		req->flags |= REQ_F_NOWAIT;
 
+	if (req->opcode == IORING_OP_SENDTO) {
+		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+		sr->sendto_addr_len = READ_ONCE(sqe->addr3);
+	} else {
+		sr->addr = (struct sockaddr __user *) NULL;
+	}
+
 #ifdef CONFIG_COMPAT
 	if (req->ctx->compat)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
@@ -5275,6 +5308,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
 
 static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 {
+	struct sockaddr_storage address;
 	struct io_sr_msg *sr = &req->sr_msg;
 	struct msghdr msg;
 	struct iovec iov;
@@ -5291,10 +5325,20 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 	if (unlikely(ret))
 		return ret;
 
-	msg.msg_name = NULL;
+
 	msg.msg_control = NULL;
 	msg.msg_controllen = 0;
-	msg.msg_namelen = 0;
+	if (sr->addr) {
+		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
+					  &address);
+		if (unlikely(ret < 0))
+			goto fail;
+		msg.msg_name = (struct sockaddr *) &address;
+		msg.msg_namelen = sr->sendto_addr_len;
+	} else {
+		msg.msg_name = NULL;
+		msg.msg_namelen = 0;
+	}
 
 	flags = req->sr_msg.msg_flags;
 	if (issue_flags & IO_URING_F_NONBLOCK)
@@ -5309,6 +5353,7 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
 			return -EAGAIN;
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
+	fail:
 		req_set_fail(req);
 	}
 	__io_req_complete(req, issue_flags, ret, 0);
@@ -5427,13 +5472,25 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
 		return -EINVAL;
 
+	/*
+	 * For IORING_OP_RECV{,FROM}, the assignment to @sr->umsg
+	 * is equivalent to an assignment to @sr->buf.
+	 */
 	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
+
 	sr->len = READ_ONCE(sqe->len);
 	sr->bgid = READ_ONCE(sqe->buf_group);
 	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
 	if (sr->msg_flags & MSG_DONTWAIT)
 		req->flags |= REQ_F_NOWAIT;
 
+	if (req->opcode == IORING_OP_RECVFROM) {
+		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+		sr->recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr3));
+	} else {
+		sr->addr = (struct sockaddr __user *) NULL;
+	}
+
 #ifdef CONFIG_COMPAT
 	if (req->ctx->compat)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
@@ -5509,6 +5566,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 	struct iovec iov;
 	unsigned flags;
 	int ret, min_ret = 0;
+	struct sockaddr_storage address;
 	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
 
 	sock = sock_from_file(req->file);
@@ -5526,7 +5584,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 	if (unlikely(ret))
 		goto out_free;
 
-	msg.msg_name = NULL;
+	msg.msg_name = sr->addr ? (struct sockaddr *) &address : NULL;
 	msg.msg_control = NULL;
 	msg.msg_controllen = 0;
 	msg.msg_namelen = 0;
@@ -5540,6 +5598,16 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
 		min_ret = iov_iter_count(&msg.msg_iter);
 
 	ret = sock_recvmsg(sock, &msg, flags);
+
+	if (ret >= 0 && sr->addr != NULL) {
+		int tmp;
+
+		tmp = move_addr_to_user(&address, msg.msg_namelen, sr->addr,
+					sr->recvfrom_addr_len);
+		if (unlikely(tmp < 0))
+			ret = tmp;
+	}
+
 out_free:
 	if (ret < min_ret) {
 		if (ret == -EAGAIN && force_nonblock)
@@ -6778,9 +6846,11 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	case IORING_OP_SYNC_FILE_RANGE:
 		return io_sfr_prep(req, sqe);
 	case IORING_OP_SENDMSG:
+	case IORING_OP_SENDTO:
 	case IORING_OP_SEND:
 		return io_sendmsg_prep(req, sqe);
 	case IORING_OP_RECVMSG:
+	case IORING_OP_RECVFROM:
 	case IORING_OP_RECV:
 		return io_recvmsg_prep(req, sqe);
 	case IORING_OP_CONNECT:
@@ -7060,12 +7130,14 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
 	case IORING_OP_SENDMSG:
 		ret = io_sendmsg(req, issue_flags);
 		break;
+	case IORING_OP_SENDTO:
 	case IORING_OP_SEND:
 		ret = io_sendto(req, issue_flags);
 		break;
 	case IORING_OP_RECVMSG:
 		ret = io_recvmsg(req, issue_flags);
 		break;
+	case IORING_OP_RECVFROM:
 	case IORING_OP_RECV:
 		ret = io_recvfrom(req, issue_flags);
 		break;
diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index efc7ac9b3a6b..a360069d1e8e 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -150,6 +150,8 @@ enum {
 	IORING_OP_SETXATTR,
 	IORING_OP_FGETXATTR,
 	IORING_OP_GETXATTR,
+	IORING_OP_SENDTO,
+	IORING_OP_RECVFROM,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support
  2021-12-30 17:52         ` [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
@ 2022-01-06 17:31           ` Praveen Kumar
  2022-01-06 20:38             ` Ammar Faizi
  0 siblings, 1 reply; 19+ messages in thread
From: Praveen Kumar @ 2022-01-06 17:31 UTC (permalink / raw)
  To: Ammar Faizi, Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Nugra

On 30-12-2021 23:22, Ammar Faizi wrote:
> This adds sendto(2) and recvfrom(2) support for io_uring.
> 
> New opcodes:
>   IORING_OP_SENDTO
>   IORING_OP_RECVFROM
> 
> Cc: Nugra <richiisei@gmail.com>
> Tested-by: Nugra <richiisei@gmail.com>
> Link: https://github.com/axboe/liburing/issues/397
> Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
> ---
> 
> v3:
>   - Fix build error when CONFIG_NET is undefined should be done in
>     the first patch, not this patch.
> 
>   - Add Tested-by tag from Nugra.
> 
> v2:
>   - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
>     call as unlikely.
> 
>   - Fix build error when CONFIG_NET is undefined.
> 
>   - Added Nugra to CC list (tester).
> ---
>  fs/io_uring.c                 | 80 +++++++++++++++++++++++++++++++++--
>  include/uapi/linux/io_uring.h |  2 +
>  2 files changed, 78 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 7adcb591398f..3726958f8f58 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -575,7 +575,15 @@ struct io_sr_msg {
>  	union {
>  		struct compat_msghdr __user	*umsg_compat;
>  		struct user_msghdr __user	*umsg;
> -		void __user			*buf;
> +
> +		struct {
> +			void __user		*buf;
> +			struct sockaddr __user	*addr;
> +			union {
> +				int		sendto_addr_len;
> +				int __user	*recvfrom_addr_len;
> +			};
> +		};
>  	};
>  	int				msg_flags;
>  	int				bgid;
> @@ -1133,6 +1141,19 @@ static const struct io_op_def io_op_defs[] = {
>  		.needs_file = 1
>  	},
>  	[IORING_OP_GETXATTR] = {},
> +	[IORING_OP_SENDTO] = {
> +		.needs_file		= 1,
> +		.unbound_nonreg_file	= 1,
> +		.pollout		= 1,
> +		.audit_skip		= 1,
> +	},
> +	[IORING_OP_RECVFROM] = {
> +		.needs_file		= 1,
> +		.unbound_nonreg_file	= 1,
> +		.pollin			= 1,
> +		.buffer_select		= 1,
> +		.audit_skip		= 1,
> +	},
>  };
>  
>  /* requests with any of those set should undergo io_disarm_next() */
> @@ -5216,12 +5237,24 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>  		return -EINVAL;
>  
> +	/*
> +	 * For IORING_OP_SEND{,TO}, the assignment to @sr->umsg
> +	 * is equivalent to an assignment to @sr->buf.
> +	 */
>  	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
> +
>  	sr->len = READ_ONCE(sqe->len);
>  	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>  	if (sr->msg_flags & MSG_DONTWAIT)
>  		req->flags |= REQ_F_NOWAIT;
>  
> +	if (req->opcode == IORING_OP_SENDTO) {
> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
> +		sr->sendto_addr_len = READ_ONCE(sqe->addr3);
> +	} else {
> +		sr->addr = (struct sockaddr __user *) NULL;

Let's have sendto_addr_len  = 0  

> +	}
> +
>  #ifdef CONFIG_COMPAT
>  	if (req->ctx->compat)
>  		sr->msg_flags |= MSG_CMSG_COMPAT;
> @@ -5275,6 +5308,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
>  
>  static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>  {
> +	struct sockaddr_storage address;
>  	struct io_sr_msg *sr = &req->sr_msg;
>  	struct msghdr msg;
>  	struct iovec iov;
> @@ -5291,10 +5325,20 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>  	if (unlikely(ret))
>  		return ret;
>  
> -	msg.msg_name = NULL;
> +
>  	msg.msg_control = NULL;
>  	msg.msg_controllen = 0;
> -	msg.msg_namelen = 0;
> +	if (sr->addr) {
> +		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
> +					  &address);
> +		if (unlikely(ret < 0))
> +			goto fail;
> +		msg.msg_name = (struct sockaddr *) &address;
> +		msg.msg_namelen = sr->sendto_addr_len;
> +	} else {
> +		msg.msg_name = NULL;
> +		msg.msg_namelen = 0;
> +	}
>  
>  	flags = req->sr_msg.msg_flags;
>  	if (issue_flags & IO_URING_F_NONBLOCK)
> @@ -5309,6 +5353,7 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>  			return -EAGAIN;
>  		if (ret == -ERESTARTSYS)
>  			ret = -EINTR;
> +	fail:
>  		req_set_fail(req);

I think there is a problem with "fail" goto statement. Not getting full clarity on this change. With latest kernel, I see req_set_fail(req) inside if check, which I don't see here. Can you please resend the patch on latest kernel version. Thanks.

>  	}
>  	__io_req_complete(req, issue_flags, ret, 0);
> @@ -5427,13 +5472,25 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>  		return -EINVAL;
>  
> +	/*
> +	 * For IORING_OP_RECV{,FROM}, the assignment to @sr->umsg
> +	 * is equivalent to an assignment to @sr->buf.
> +	 */
>  	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
> +
>  	sr->len = READ_ONCE(sqe->len);
>  	sr->bgid = READ_ONCE(sqe->buf_group);
>  	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>  	if (sr->msg_flags & MSG_DONTWAIT)
>  		req->flags |= REQ_F_NOWAIT;
>  
> +	if (req->opcode == IORING_OP_RECVFROM) {
> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
> +		sr->recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr3));
> +	} else {
> +		sr->addr = (struct sockaddr __user *) NULL;

I think recvfrom_addr_len should also be pointed to NULL, instead of garbage for this case.

> +	}
> +
>  #ifdef CONFIG_COMPAT
>  	if (req->ctx->compat)
>  		sr->msg_flags |= MSG_CMSG_COMPAT;
> @@ -5509,6 +5566,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>  	struct iovec iov;
>  	unsigned flags;
>  	int ret, min_ret = 0;
> +	struct sockaddr_storage address;
>  	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>  
>  	sock = sock_from_file(req->file);
> @@ -5526,7 +5584,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>  	if (unlikely(ret))
>  		goto out_free;
>  
> -	msg.msg_name = NULL;
> +	msg.msg_name = sr->addr ? (struct sockaddr *) &address : NULL;
>  	msg.msg_control = NULL;
>  	msg.msg_controllen = 0;
>  	msg.msg_namelen = 0;

I think namelen should also be updated ?

> @@ -5540,6 +5598,16 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>  		min_ret = iov_iter_count(&msg.msg_iter);
>  
>  	ret = sock_recvmsg(sock, &msg, flags);
> +
> +	if (ret >= 0 && sr->addr != NULL) {
> +		int tmp;
> +
> +		tmp = move_addr_to_user(&address, msg.msg_namelen, sr->addr,
> +					sr->recvfrom_addr_len);
> +		if (unlikely(tmp < 0))
> +			ret = tmp;
> +	}
> +
>  out_free:
>  	if (ret < min_ret) {
>  		if (ret == -EAGAIN && force_nonblock)
> @@ -6778,9 +6846,11 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  	case IORING_OP_SYNC_FILE_RANGE:
>  		return io_sfr_prep(req, sqe);
>  	case IORING_OP_SENDMSG:
> +	case IORING_OP_SENDTO:
>  	case IORING_OP_SEND:
>  		return io_sendmsg_prep(req, sqe);
>  	case IORING_OP_RECVMSG:
> +	case IORING_OP_RECVFROM:
>  	case IORING_OP_RECV:
>  		return io_recvmsg_prep(req, sqe);
>  	case IORING_OP_CONNECT:
> @@ -7060,12 +7130,14 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
>  	case IORING_OP_SENDMSG:
>  		ret = io_sendmsg(req, issue_flags);
>  		break;
> +	case IORING_OP_SENDTO:
>  	case IORING_OP_SEND:
>  		ret = io_sendto(req, issue_flags);
>  		break;
>  	case IORING_OP_RECVMSG:
>  		ret = io_recvmsg(req, issue_flags);
>  		break;
> +	case IORING_OP_RECVFROM:
>  	case IORING_OP_RECV:
>  		ret = io_recvfrom(req, issue_flags);
>  		break;
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index efc7ac9b3a6b..a360069d1e8e 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -150,6 +150,8 @@ enum {
>  	IORING_OP_SETXATTR,
>  	IORING_OP_FGETXATTR,
>  	IORING_OP_GETXATTR,
> +	IORING_OP_SENDTO,
> +	IORING_OP_RECVFROM,
>  
>  	/* this goes last, obviously */
>  	IORING_OP_LAST,


Regards,

~Praveen.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support
  2022-01-06 17:31           ` Praveen Kumar
@ 2022-01-06 20:38             ` Ammar Faizi
  2022-01-06 20:48               ` Ammar Faizi
  2022-01-07  8:33               ` Praveen Kumar
  0 siblings, 2 replies; 19+ messages in thread
From: Ammar Faizi @ 2022-01-06 20:38 UTC (permalink / raw)
  To: Praveen Kumar, Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Nugra, Ammar Faizi,
	Ammar Faizi


On Thu, 6 Jan 2022 at 23:01:59 +0530, Praveen Kumar <kpraveen.lkml@gmail.com> wrote:
> On 30-12-2021 23:22, Ammar Faizi wrote:
>> This adds sendto(2) and recvfrom(2) support for io_uring.
>> 
>> New opcodes:
>>   IORING_OP_SENDTO
>>   IORING_OP_RECVFROM
>> 
>> Cc: Nugra <richiisei@gmail.com>
>> Tested-by: Nugra <richiisei@gmail.com>
>> Link: https://github.com/axboe/liburing/issues/397
>> Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
>> ---
>> 
>> v3:
>>   - Fix build error when CONFIG_NET is undefined should be done in
>>     the first patch, not this patch.
>> 
>>   - Add Tested-by tag from Nugra.
>> 
>> v2:
>>   - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
>>     call as unlikely.
>> 
>>   - Fix build error when CONFIG_NET is undefined.
>> 
>>   - Added Nugra to CC list (tester).
>> ---
>>  fs/io_uring.c                 | 80 +++++++++++++++++++++++++++++++++--
>>  include/uapi/linux/io_uring.h |  2 +
>>  2 files changed, 78 insertions(+), 4 deletions(-)
>> 
>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>> index 7adcb591398f..3726958f8f58 100644
>> --- a/fs/io_uring.c
>> +++ b/fs/io_uring.c
>> @@ -575,7 +575,15 @@ struct io_sr_msg {
>>  	union {
>>  		struct compat_msghdr __user	*umsg_compat;
>>  		struct user_msghdr __user	*umsg;
>> -		void __user			*buf;
>> +
>> +		struct {
>> +			void __user		*buf;
>> +			struct sockaddr __user	*addr;
>> +			union {
>> +				int		sendto_addr_len;
>> +				int __user	*recvfrom_addr_len;
>> +			};
>> +		};
>>  	};
>>  	int				msg_flags;
>>  	int				bgid;
>> @@ -1133,6 +1141,19 @@ static const struct io_op_def io_op_defs[] = {
>>  		.needs_file = 1
>>  	},
>>  	[IORING_OP_GETXATTR] = {},
>> +	[IORING_OP_SENDTO] = {
>> +		.needs_file		= 1,
>> +		.unbound_nonreg_file	= 1,
>> +		.pollout		= 1,
>> +		.audit_skip		= 1,
>> +	},
>> +	[IORING_OP_RECVFROM] = {
>> +		.needs_file		= 1,
>> +		.unbound_nonreg_file	= 1,
>> +		.pollin			= 1,
>> +		.buffer_select		= 1,
>> +		.audit_skip		= 1,
>> +	},
>>  };
>>  
>>  /* requests with any of those set should undergo io_disarm_next() */
>> @@ -5216,12 +5237,24 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>  	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>  		return -EINVAL;
>>  
>> +	/*
>> +	 * For IORING_OP_SEND{,TO}, the assignment to @sr->umsg
>> +	 * is equivalent to an assignment to @sr->buf.
>> +	 */
>>  	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
>> +
>>  	sr->len = READ_ONCE(sqe->len);
>>  	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>>  	if (sr->msg_flags & MSG_DONTWAIT)
>>  		req->flags |= REQ_F_NOWAIT;
>>  
>> +	if (req->opcode == IORING_OP_SENDTO) {
>> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
>> +		sr->sendto_addr_len = READ_ONCE(sqe->addr3);
>> +	} else {
>> +		sr->addr = (struct sockaddr __user *) NULL;
> 
> Let's have sendto_addr_len  = 0  

Will do in the RFC v5.

> 
>> +	}
>> +
>>  #ifdef CONFIG_COMPAT
>>  	if (req->ctx->compat)
>>  		sr->msg_flags |= MSG_CMSG_COMPAT;
>> @@ -5275,6 +5308,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
>>  
>>  static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>  {
>> +	struct sockaddr_storage address;
>>  	struct io_sr_msg *sr = &req->sr_msg;
>>  	struct msghdr msg;
>>  	struct iovec iov;
>> @@ -5291,10 +5325,20 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>  	if (unlikely(ret))
>>  		return ret;
>>  
>> -	msg.msg_name = NULL;
>> +
>>  	msg.msg_control = NULL;
>>  	msg.msg_controllen = 0;
>> -	msg.msg_namelen = 0;
>> +	if (sr->addr) {
>> +		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
>> +					  &address);
>> +		if (unlikely(ret < 0))
>> +			goto fail;
>> +		msg.msg_name = (struct sockaddr *) &address;
>> +		msg.msg_namelen = sr->sendto_addr_len;
>> +	} else {
>> +		msg.msg_name = NULL;
>> +		msg.msg_namelen = 0;
>> +	}
>>  
>>  	flags = req->sr_msg.msg_flags;
>>  	if (issue_flags & IO_URING_F_NONBLOCK)
>> @@ -5309,6 +5353,7 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>  			return -EAGAIN;
>>  		if (ret == -ERESTARTSYS)
>>  			ret = -EINTR;
>> +	fail:
>>  		req_set_fail(req);
> 
> I think there is a problem with "fail" goto statement. Not getting
> full clarity on this change. With latest kernel, I see
> req_set_fail(req) inside if check, which I don't see here. Can you
> please resend the patch on latest kernel version. Thanks.

I will send the v5 on top of "for-next" branch in Jens' tree soon.

That is already inside an "if check" anyway. We go to that label when
the move_addr_to_kernel() fails (most of the time it is -EFAULT or
-EINVAL).

That part looks like this (note the if check before the goto):
----------------------------------------------------------------------
	msg.msg_control = NULL;
	msg.msg_controllen = 0;
	if (sr->addr) {
		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
					  &address);
		if (unlikely(ret < 0))
			goto fail;
		msg.msg_name = (struct sockaddr *) &address;
		msg.msg_namelen = sr->sendto_addr_len;
	} else {
		msg.msg_name = NULL;
		msg.msg_namelen = 0;
	}

	flags = req->sr_msg.msg_flags;
	if (issue_flags & IO_URING_F_NONBLOCK)
		flags |= MSG_DONTWAIT;
	if (flags & MSG_WAITALL)
		min_ret = iov_iter_count(&msg.msg_iter);

	msg.msg_flags = flags;
	ret = sock_sendmsg(sock, &msg);
	if (ret < min_ret) {
		if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
			return -EAGAIN;
		if (ret == -ERESTARTSYS)
			ret = -EINTR;
	fail:
		req_set_fail(req);
	}
	__io_req_complete(req, issue_flags, ret, 0);
	return 0;
----------------------------------------------------------------------

>>  	}
>>  	__io_req_complete(req, issue_flags, ret, 0);
>> @@ -5427,13 +5472,25 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>  	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>  		return -EINVAL;
>>  
>> +	/*
>> +	 * For IORING_OP_RECV{,FROM}, the assignment to @sr->umsg
>> +	 * is equivalent to an assignment to @sr->buf.
>> +	 */
>>  	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
>> +
>>  	sr->len = READ_ONCE(sqe->len);
>>  	sr->bgid = READ_ONCE(sqe->buf_group);
>>  	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>>  	if (sr->msg_flags & MSG_DONTWAIT)
>>  		req->flags |= REQ_F_NOWAIT;
>>  
>> +	if (req->opcode == IORING_OP_RECVFROM) {
>> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
>> +		sr->recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr3));
>> +	} else {
>> +		sr->addr = (struct sockaddr __user *) NULL;
> 
> I think recvfrom_addr_len should also be pointed to NULL, instead of
> garbage for this case.

Will do in the RFC v5.

> 
>> +	}
>> +
>>  #ifdef CONFIG_COMPAT
>>  	if (req->ctx->compat)
>>  		sr->msg_flags |= MSG_CMSG_COMPAT;
>> @@ -5509,6 +5566,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>  	struct iovec iov;
>>  	unsigned flags;
>>  	int ret, min_ret = 0;
>> +	struct sockaddr_storage address;
>>  	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>  
>>  	sock = sock_from_file(req->file);
>> @@ -5526,7 +5584,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>  	if (unlikely(ret))
>>  		goto out_free;
>>  
>> -	msg.msg_name = NULL;
>> +	msg.msg_name = sr->addr ? (struct sockaddr *) &address : NULL;
>>  	msg.msg_control = NULL;
>>  	msg.msg_controllen = 0;
>>  	msg.msg_namelen = 0;
> 
> I think namelen should also be updated ?

It doesn't have to be updated. From net/socket.c there is a comment
like this:

	/* We assume all kernel code knows the size of sockaddr_storage */
	msg.msg_namelen = 0;

Full __sys_recvfrom() source code, see here:
https://github.com/torvalds/linux/blob/v5.16-rc8/net/socket.c#L2085-L2088

I will add the same comment in next series to clarify this one.

> 
>> @@ -5540,6 +5598,16 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>  		min_ret = iov_iter_count(&msg.msg_iter);
>>  
>>  	ret = sock_recvmsg(sock, &msg, flags);
>> +
>> +	if (ret >= 0 && sr->addr != NULL) {
>> +		int tmp;
>> +
>> +		tmp = move_addr_to_user(&address, msg.msg_namelen, sr->addr,
>> +					sr->recvfrom_addr_len);
>> +		if (unlikely(tmp < 0))
>> +			ret = tmp;
>> +	}
>> +
>>  out_free:
>>  	if (ret < min_ret) {
>>  		if (ret == -EAGAIN && force_nonblock)
>> @@ -6778,9 +6846,11 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>  	case IORING_OP_SYNC_FILE_RANGE:
>>  		return io_sfr_prep(req, sqe);
>>  	case IORING_OP_SENDMSG:
>> +	case IORING_OP_SENDTO:
>>  	case IORING_OP_SEND:
>>  		return io_sendmsg_prep(req, sqe);
>>  	case IORING_OP_RECVMSG:
>> +	case IORING_OP_RECVFROM:
>>  	case IORING_OP_RECV:
>>  		return io_recvmsg_prep(req, sqe);
>>  	case IORING_OP_CONNECT:
>> @@ -7060,12 +7130,14 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
>>  	case IORING_OP_SENDMSG:
>>  		ret = io_sendmsg(req, issue_flags);
>>  		break;
>> +	case IORING_OP_SENDTO:
>>  	case IORING_OP_SEND:
>>  		ret = io_sendto(req, issue_flags);
>>  		break;
>>  	case IORING_OP_RECVMSG:
>>  		ret = io_recvmsg(req, issue_flags);
>>  		break;
>> +	case IORING_OP_RECVFROM:
>>  	case IORING_OP_RECV:
>>  		ret = io_recvfrom(req, issue_flags);
>>  		break;
>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>> index efc7ac9b3a6b..a360069d1e8e 100644
>> --- a/include/uapi/linux/io_uring.h
>> +++ b/include/uapi/linux/io_uring.h
>> @@ -150,6 +150,8 @@ enum {
>>  	IORING_OP_SETXATTR,
>>  	IORING_OP_FGETXATTR,
>>  	IORING_OP_GETXATTR,
>> +	IORING_OP_SENDTO,
>> +	IORING_OP_RECVFROM,
>>  
>>  	/* this goes last, obviously */
>>  	IORING_OP_LAST,
> 
> 
> Regards,
> 
> ~Praveen.
> 

Thanks for the review. I will send the RFC v5 soon.

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support
  2022-01-06 20:38             ` Ammar Faizi
@ 2022-01-06 20:48               ` Ammar Faizi
  2022-01-07  8:33               ` Praveen Kumar
  1 sibling, 0 replies; 19+ messages in thread
From: Ammar Faizi @ 2022-01-06 20:48 UTC (permalink / raw)
  To: Praveen Kumar, Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Nugra, Ammar Faizi,
	Ammar Faizi

On Fri, 7 Jan 2022 at 03:38:50 +0700, Ammar Faizi <ammarfaizi2@gnuweeb.org> wrote:
> On Thu, 6 Jan 2022 at 23:01:59 +0530, Praveen Kumar <kpraveen.lkml@gmail.com> wrote:
>> On 30-12-2021 23:22, Ammar Faizi wrote:
>>> This adds sendto(2) and recvfrom(2) support for io_uring.
>>> 
>>> New opcodes:
>>>   IORING_OP_SENDTO
>>>   IORING_OP_RECVFROM
>>> 
>>> Cc: Nugra <richiisei@gmail.com>
>>> Tested-by: Nugra <richiisei@gmail.com>
>>> Link: https://github.com/axboe/liburing/issues/397
>>> Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
>>> ---
>>> 
>>> v3:
>>>   - Fix build error when CONFIG_NET is undefined should be done in
>>>     the first patch, not this patch.
>>> 
>>>   - Add Tested-by tag from Nugra.
>>> 
>>> v2:
>>>   - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
>>>     call as unlikely.
>>> 
>>>   - Fix build error when CONFIG_NET is undefined.
>>> 
>>>   - Added Nugra to CC list (tester).
>>> ---
>>>  fs/io_uring.c                 | 80 +++++++++++++++++++++++++++++++++--
>>>  include/uapi/linux/io_uring.h |  2 +
>>>  2 files changed, 78 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>> index 7adcb591398f..3726958f8f58 100644
>>> --- a/fs/io_uring.c
>>> +++ b/fs/io_uring.c
>>> @@ -575,7 +575,15 @@ struct io_sr_msg {
>>>  	union {
>>>  		struct compat_msghdr __user	*umsg_compat;
>>>  		struct user_msghdr __user	*umsg;
>>> -		void __user			*buf;
>>> +
>>> +		struct {
>>> +			void __user		*buf;
>>> +			struct sockaddr __user	*addr;
>>> +			union {
>>> +				int		sendto_addr_len;
>>> +				int __user	*recvfrom_addr_len;
>>> +			};
>>> +		};
>>>  	};
>>>  	int				msg_flags;
>>>  	int				bgid;
>>> @@ -1133,6 +1141,19 @@ static const struct io_op_def io_op_defs[] = {
>>>  		.needs_file = 1
>>>  	},
>>>  	[IORING_OP_GETXATTR] = {},
>>> +	[IORING_OP_SENDTO] = {
>>> +		.needs_file		= 1,
>>> +		.unbound_nonreg_file	= 1,
>>> +		.pollout		= 1,
>>> +		.audit_skip		= 1,
>>> +	},
>>> +	[IORING_OP_RECVFROM] = {
>>> +		.needs_file		= 1,
>>> +		.unbound_nonreg_file	= 1,
>>> +		.pollin			= 1,
>>> +		.buffer_select		= 1,
>>> +		.audit_skip		= 1,
>>> +	},
>>>  };
>>>  
>>>  /* requests with any of those set should undergo io_disarm_next() */
>>> @@ -5216,12 +5237,24 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>>  		return -EINVAL;
>>>  
>>> +	/*
>>> +	 * For IORING_OP_SEND{,TO}, the assignment to @sr->umsg
>>> +	 * is equivalent to an assignment to @sr->buf.
>>> +	 */
>>>  	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>> +
>>>  	sr->len = READ_ONCE(sqe->len);
>>>  	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>>>  	if (sr->msg_flags & MSG_DONTWAIT)
>>>  		req->flags |= REQ_F_NOWAIT;
>>>  
>>> +	if (req->opcode == IORING_OP_SENDTO) {
>>> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
>>> +		sr->sendto_addr_len = READ_ONCE(sqe->addr3);
>>> +	} else {
>>> +		sr->addr = (struct sockaddr __user *) NULL;
>> 
>> Let's have sendto_addr_len  = 0  
> 
> Will do in the RFC v5.
> 
>> 
>>> +	}
>>> +
>>>  #ifdef CONFIG_COMPAT
>>>  	if (req->ctx->compat)
>>>  		sr->msg_flags |= MSG_CMSG_COMPAT;
>>> @@ -5275,6 +5308,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
>>>  
>>>  static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>  {
>>> +	struct sockaddr_storage address;
>>>  	struct io_sr_msg *sr = &req->sr_msg;
>>>  	struct msghdr msg;
>>>  	struct iovec iov;
>>> @@ -5291,10 +5325,20 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>  	if (unlikely(ret))
>>>  		return ret;
>>>  
>>> -	msg.msg_name = NULL;
>>> +
>>>  	msg.msg_control = NULL;
>>>  	msg.msg_controllen = 0;
>>> -	msg.msg_namelen = 0;
>>> +	if (sr->addr) {
>>> +		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
>>> +					  &address);
>>> +		if (unlikely(ret < 0))
>>> +			goto fail;
>>> +		msg.msg_name = (struct sockaddr *) &address;
>>> +		msg.msg_namelen = sr->sendto_addr_len;
>>> +	} else {
>>> +		msg.msg_name = NULL;
>>> +		msg.msg_namelen = 0;
>>> +	}
>>>  
>>>  	flags = req->sr_msg.msg_flags;
>>>  	if (issue_flags & IO_URING_F_NONBLOCK)
>>> @@ -5309,6 +5353,7 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>  			return -EAGAIN;
>>>  		if (ret == -ERESTARTSYS)
>>>  			ret = -EINTR;
>>> +	fail:
>>>  		req_set_fail(req);
>> 
>> I think there is a problem with "fail" goto statement. Not getting
>> full clarity on this change. With latest kernel, I see
>> req_set_fail(req) inside if check, which I don't see here. Can you
>> please resend the patch on latest kernel version. Thanks.
> 
> I will send the v5 on top of "for-next" branch in Jens' tree soon.
> 
> That is already inside an "if check" anyway. We go to that label when
> the move_addr_to_kernel() fails (most of the time it is -EFAULT or
> -EINVAL).
> 
> That part looks like this (note the if check before the goto):
> ----------------------------------------------------------------------
> 	msg.msg_control = NULL;
> 	msg.msg_controllen = 0;
> 	if (sr->addr) {
> 		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
> 					  &address);
> 		if (unlikely(ret < 0))
> 			goto fail;
> 		msg.msg_name = (struct sockaddr *) &address;
> 		msg.msg_namelen = sr->sendto_addr_len;
> 	} else {
> 		msg.msg_name = NULL;
> 		msg.msg_namelen = 0;
> 	}
> 
> 	flags = req->sr_msg.msg_flags;
> 	if (issue_flags & IO_URING_F_NONBLOCK)
> 		flags |= MSG_DONTWAIT;
> 	if (flags & MSG_WAITALL)
> 		min_ret = iov_iter_count(&msg.msg_iter);
> 
> 	msg.msg_flags = flags;
> 	ret = sock_sendmsg(sock, &msg);
> 	if (ret < min_ret) {
> 		if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
> 			return -EAGAIN;
> 		if (ret == -ERESTARTSYS)
> 			ret = -EINTR;
> 	fail:
> 		req_set_fail(req);
> 	}
> 	__io_req_complete(req, issue_flags, ret, 0);
> 	return 0;
> ----------------------------------------------------------------------
> 
>>>  	}
>>>  	__io_req_complete(req, issue_flags, ret, 0);
>>> @@ -5427,13 +5472,25 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>>  		return -EINVAL;
>>>  
>>> +	/*
>>> +	 * For IORING_OP_RECV{,FROM}, the assignment to @sr->umsg
>>> +	 * is equivalent to an assignment to @sr->buf.
>>> +	 */
>>>  	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>> +
>>>  	sr->len = READ_ONCE(sqe->len);
>>>  	sr->bgid = READ_ONCE(sqe->buf_group);
>>>  	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>>>  	if (sr->msg_flags & MSG_DONTWAIT)
>>>  		req->flags |= REQ_F_NOWAIT;
>>>  
>>> +	if (req->opcode == IORING_OP_RECVFROM) {
>>> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
>>> +		sr->recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr3));
>>> +	} else {
>>> +		sr->addr = (struct sockaddr __user *) NULL;
>> 
>> I think recvfrom_addr_len should also be pointed to NULL, instead of
>> garbage for this case.
> 
> Will do in the RFC v5.
> 
>> 
>>> +	}
>>> +
>>>  #ifdef CONFIG_COMPAT
>>>  	if (req->ctx->compat)
>>>  		sr->msg_flags |= MSG_CMSG_COMPAT;
>>> @@ -5509,6 +5566,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>  	struct iovec iov;
>>>  	unsigned flags;
>>>  	int ret, min_ret = 0;
>>> +	struct sockaddr_storage address;
>>>  	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>>  
>>>  	sock = sock_from_file(req->file);
>>> @@ -5526,7 +5584,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>  	if (unlikely(ret))
>>>  		goto out_free;
>>>  
>>> -	msg.msg_name = NULL;
>>> +	msg.msg_name = sr->addr ? (struct sockaddr *) &address : NULL;
>>>  	msg.msg_control = NULL;
>>>  	msg.msg_controllen = 0;
>>>  	msg.msg_namelen = 0;
>> 
>> I think namelen should also be updated ?
> 
> It doesn't have to be updated. From net/socket.c there is a comment
> like this:
> 
> 	/* We assume all kernel code knows the size of sockaddr_storage */
> 	msg.msg_namelen = 0;
> 
> Full __sys_recvfrom() source code, see here:
> https://github.com/torvalds/linux/blob/v5.16-rc8/net/socket.c#L2085-L2088
> 
> I will add the same comment in next series to clarify this one.
> 
>> 
>>> @@ -5540,6 +5598,16 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>  		min_ret = iov_iter_count(&msg.msg_iter);
>>>  
>>>  	ret = sock_recvmsg(sock, &msg, flags);
>>> +
>>> +	if (ret >= 0 && sr->addr != NULL) {
>>> +		int tmp;
>>> +
>>> +		tmp = move_addr_to_user(&address, msg.msg_namelen, sr->addr,
>>> +					sr->recvfrom_addr_len);
>>> +		if (unlikely(tmp < 0))
>>> +			ret = tmp;
>>> +	}
>>> +
>>>  out_free:
>>>  	if (ret < min_ret) {
>>>  		if (ret == -EAGAIN && force_nonblock)
>>> @@ -6778,9 +6846,11 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	case IORING_OP_SYNC_FILE_RANGE:
>>>  		return io_sfr_prep(req, sqe);
>>>  	case IORING_OP_SENDMSG:
>>> +	case IORING_OP_SENDTO:
>>>  	case IORING_OP_SEND:
>>>  		return io_sendmsg_prep(req, sqe);
>>>  	case IORING_OP_RECVMSG:
>>> +	case IORING_OP_RECVFROM:
>>>  	case IORING_OP_RECV:
>>>  		return io_recvmsg_prep(req, sqe);
>>>  	case IORING_OP_CONNECT:
>>> @@ -7060,12 +7130,14 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
>>>  	case IORING_OP_SENDMSG:
>>>  		ret = io_sendmsg(req, issue_flags);
>>>  		break;
>>> +	case IORING_OP_SENDTO:
>>>  	case IORING_OP_SEND:
>>>  		ret = io_sendto(req, issue_flags);
>>>  		break;
>>>  	case IORING_OP_RECVMSG:
>>>  		ret = io_recvmsg(req, issue_flags);
>>>  		break;
>>> +	case IORING_OP_RECVFROM:
>>>  	case IORING_OP_RECV:
>>>  		ret = io_recvfrom(req, issue_flags);
>>>  		break;
>>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>>> index efc7ac9b3a6b..a360069d1e8e 100644
>>> --- a/include/uapi/linux/io_uring.h
>>> +++ b/include/uapi/linux/io_uring.h
>>> @@ -150,6 +150,8 @@ enum {
>>>  	IORING_OP_SETXATTR,
>>>  	IORING_OP_FGETXATTR,
>>>  	IORING_OP_GETXATTR,
>>> +	IORING_OP_SENDTO,
>>> +	IORING_OP_RECVFROM,
>>>  
>>>  	/* this goes last, obviously */
>>>  	IORING_OP_LAST,
>> 
>> 
>> Regards,
>> 
>> ~Praveen.
>> 
> 
> Thanks for the review. I will send the RFC v5 soon.
> 
> -- 
> Ammar Faizi

s/v5/v4/g

-- 
Ammar Faizi

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support
  2022-01-06 20:38             ` Ammar Faizi
  2022-01-06 20:48               ` Ammar Faizi
@ 2022-01-07  8:33               ` Praveen Kumar
  2022-01-07 11:02                 ` Ammar Faizi
  1 sibling, 1 reply; 19+ messages in thread
From: Praveen Kumar @ 2022-01-07  8:33 UTC (permalink / raw)
  To: Ammar Faizi, Jens Axboe
  Cc: io-uring Mailing List, Pavel Begunkov, David S . Miller,
	Jakub Kicinski, netdev, linux-kernel, Nugra, Ammar Faizi

On 07-01-2022 02:08, Ammar Faizi wrote:
> 
> On Thu, 6 Jan 2022 at 23:01:59 +0530, Praveen Kumar <kpraveen.lkml@gmail.com> wrote:
>> On 30-12-2021 23:22, Ammar Faizi wrote:
>>> This adds sendto(2) and recvfrom(2) support for io_uring.
>>>
>>> New opcodes:
>>>   IORING_OP_SENDTO
>>>   IORING_OP_RECVFROM
>>>
>>> Cc: Nugra <richiisei@gmail.com>
>>> Tested-by: Nugra <richiisei@gmail.com>
>>> Link: https://github.com/axboe/liburing/issues/397
>>> Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
>>> ---
>>>
>>> v3:
>>>   - Fix build error when CONFIG_NET is undefined should be done in
>>>     the first patch, not this patch.
>>>
>>>   - Add Tested-by tag from Nugra.
>>>
>>> v2:
>>>   - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
>>>     call as unlikely.
>>>
>>>   - Fix build error when CONFIG_NET is undefined.
>>>
>>>   - Added Nugra to CC list (tester).
>>> ---
>>>  fs/io_uring.c                 | 80 +++++++++++++++++++++++++++++++++--
>>>  include/uapi/linux/io_uring.h |  2 +
>>>  2 files changed, 78 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>> index 7adcb591398f..3726958f8f58 100644
>>> --- a/fs/io_uring.c
>>> +++ b/fs/io_uring.c
>>> @@ -575,7 +575,15 @@ struct io_sr_msg {
>>>  	union {
>>>  		struct compat_msghdr __user	*umsg_compat;
>>>  		struct user_msghdr __user	*umsg;
>>> -		void __user			*buf;
>>> +
>>> +		struct {
>>> +			void __user		*buf;
>>> +			struct sockaddr __user	*addr;
>>> +			union {
>>> +				int		sendto_addr_len;
>>> +				int __user	*recvfrom_addr_len;
>>> +			};
>>> +		};
>>>  	};
>>>  	int				msg_flags;
>>>  	int				bgid;
>>> @@ -1133,6 +1141,19 @@ static const struct io_op_def io_op_defs[] = {
>>>  		.needs_file = 1
>>>  	},
>>>  	[IORING_OP_GETXATTR] = {},
>>> +	[IORING_OP_SENDTO] = {
>>> +		.needs_file		= 1,
>>> +		.unbound_nonreg_file	= 1,
>>> +		.pollout		= 1,
>>> +		.audit_skip		= 1,
>>> +	},
>>> +	[IORING_OP_RECVFROM] = {
>>> +		.needs_file		= 1,
>>> +		.unbound_nonreg_file	= 1,
>>> +		.pollin			= 1,
>>> +		.buffer_select		= 1,
>>> +		.audit_skip		= 1,
>>> +	},
>>>  };
>>>  
>>>  /* requests with any of those set should undergo io_disarm_next() */
>>> @@ -5216,12 +5237,24 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>>  		return -EINVAL;
>>>  
>>> +	/*
>>> +	 * For IORING_OP_SEND{,TO}, the assignment to @sr->umsg
>>> +	 * is equivalent to an assignment to @sr->buf.
>>> +	 */
>>>  	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>> +
>>>  	sr->len = READ_ONCE(sqe->len);
>>>  	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>>>  	if (sr->msg_flags & MSG_DONTWAIT)
>>>  		req->flags |= REQ_F_NOWAIT;
>>>  
>>> +	if (req->opcode == IORING_OP_SENDTO) {
>>> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
>>> +		sr->sendto_addr_len = READ_ONCE(sqe->addr3);
>>> +	} else {
>>> +		sr->addr = (struct sockaddr __user *) NULL;
>>
>> Let's have sendto_addr_len  = 0  
> 
> Will do in the RFC v5.
> 
>>
>>> +	}
>>> +
>>>  #ifdef CONFIG_COMPAT
>>>  	if (req->ctx->compat)
>>>  		sr->msg_flags |= MSG_CMSG_COMPAT;
>>> @@ -5275,6 +5308,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
>>>  
>>>  static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>  {
>>> +	struct sockaddr_storage address;
>>>  	struct io_sr_msg *sr = &req->sr_msg;
>>>  	struct msghdr msg;
>>>  	struct iovec iov;
>>> @@ -5291,10 +5325,20 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>  	if (unlikely(ret))
>>>  		return ret;
>>>  
>>> -	msg.msg_name = NULL;
>>> +
>>>  	msg.msg_control = NULL;
>>>  	msg.msg_controllen = 0;
>>> -	msg.msg_namelen = 0;
>>> +	if (sr->addr) {
>>> +		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
>>> +					  &address);
>>> +		if (unlikely(ret < 0))
>>> +			goto fail;
>>> +		msg.msg_name = (struct sockaddr *) &address;
>>> +		msg.msg_namelen = sr->sendto_addr_len;
>>> +	} else {
>>> +		msg.msg_name = NULL;
>>> +		msg.msg_namelen = 0;
>>> +	}
>>>  
>>>  	flags = req->sr_msg.msg_flags;
>>>  	if (issue_flags & IO_URING_F_NONBLOCK)
>>> @@ -5309,6 +5353,7 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>  			return -EAGAIN;
>>>  		if (ret == -ERESTARTSYS)
>>>  			ret = -EINTR;
>>> +	fail:
>>>  		req_set_fail(req);
>>
>> I think there is a problem with "fail" goto statement. Not getting
>> full clarity on this change. With latest kernel, I see
>> req_set_fail(req) inside if check, which I don't see here. Can you
>> please resend the patch on latest kernel version. Thanks.
> 
> I will send the v5 on top of "for-next" branch in Jens' tree soon.
> 
> That is already inside an "if check" anyway. We go to that label when
> the move_addr_to_kernel() fails (most of the time it is -EFAULT or
> -EINVAL).
> 
> That part looks like this (note the if check before the goto):
> ----------------------------------------------------------------------
> 	msg.msg_control = NULL;
> 	msg.msg_controllen = 0;
> 	if (sr->addr) {
> 		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
> 					  &address);
> 		if (unlikely(ret < 0))
> 			goto fail;
> 		msg.msg_name = (struct sockaddr *) &address;
> 		msg.msg_namelen = sr->sendto_addr_len;
> 	} else {
> 		msg.msg_name = NULL;
> 		msg.msg_namelen = 0;
> 	}
> 
> 	flags = req->sr_msg.msg_flags;
> 	if (issue_flags & IO_URING_F_NONBLOCK)
> 		flags |= MSG_DONTWAIT;
> 	if (flags & MSG_WAITALL)
> 		min_ret = iov_iter_count(&msg.msg_iter);
> 
> 	msg.msg_flags = flags;
> 	ret = sock_sendmsg(sock, &msg);
> 	if (ret < min_ret) {
> 		if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
> 			return -EAGAIN;
> 		if (ret == -ERESTARTSYS)
> 			ret = -EINTR;
> 	fail:

Thanks for sending this. IMO this goto label should be just before the "if (ret < min_ret)" statement.

> 		req_set_fail(req);
> 	}
> 	__io_req_complete(req, issue_flags, ret, 0);
> 	return 0;
> ----------------------------------------------------------------------
> 
>>>  	}
>>>  	__io_req_complete(req, issue_flags, ret, 0);
>>> @@ -5427,13 +5472,25 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>>  		return -EINVAL;
>>>  
>>> +	/*
>>> +	 * For IORING_OP_RECV{,FROM}, the assignment to @sr->umsg
>>> +	 * is equivalent to an assignment to @sr->buf.
>>> +	 */
>>>  	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>> +
>>>  	sr->len = READ_ONCE(sqe->len);
>>>  	sr->bgid = READ_ONCE(sqe->buf_group);
>>>  	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>>>  	if (sr->msg_flags & MSG_DONTWAIT)
>>>  		req->flags |= REQ_F_NOWAIT;
>>>  
>>> +	if (req->opcode == IORING_OP_RECVFROM) {
>>> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
>>> +		sr->recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr3));
>>> +	} else {
>>> +		sr->addr = (struct sockaddr __user *) NULL;
>>
>> I think recvfrom_addr_len should also be pointed to NULL, instead of
>> garbage for this case.
> 
> Will do in the RFC v5.
> 
>>
>>> +	}
>>> +
>>>  #ifdef CONFIG_COMPAT
>>>  	if (req->ctx->compat)
>>>  		sr->msg_flags |= MSG_CMSG_COMPAT;
>>> @@ -5509,6 +5566,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>  	struct iovec iov;
>>>  	unsigned flags;
>>>  	int ret, min_ret = 0;
>>> +	struct sockaddr_storage address;
>>>  	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>>  
>>>  	sock = sock_from_file(req->file);
>>> @@ -5526,7 +5584,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>  	if (unlikely(ret))
>>>  		goto out_free;
>>>  
>>> -	msg.msg_name = NULL;
>>> +	msg.msg_name = sr->addr ? (struct sockaddr *) &address : NULL;
>>>  	msg.msg_control = NULL;
>>>  	msg.msg_controllen = 0;
>>>  	msg.msg_namelen = 0;
>>
>> I think namelen should also be updated ?
> 
> It doesn't have to be updated. From net/socket.c there is a comment
> like this:
> 
> 	/* We assume all kernel code knows the size of sockaddr_storage */
> 	msg.msg_namelen = 0;
> 
> Full __sys_recvfrom() source code, see here:
> https://github.com/torvalds/linux/blob/v5.16-rc8/net/socket.c#L2085-L2088
> 
> I will add the same comment in next series to clarify this one.
> 
>>
>>> @@ -5540,6 +5598,16 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>  		min_ret = iov_iter_count(&msg.msg_iter);
>>>  
>>>  	ret = sock_recvmsg(sock, &msg, flags);
>>> +
>>> +	if (ret >= 0 && sr->addr != NULL) {
>>> +		int tmp;
>>> +
>>> +		tmp = move_addr_to_user(&address, msg.msg_namelen, sr->addr,
>>> +					sr->recvfrom_addr_len);
>>> +		if (unlikely(tmp < 0))
>>> +			ret = tmp;
>>> +	}
>>> +
>>>  out_free:
>>>  	if (ret < min_ret) {
>>>  		if (ret == -EAGAIN && force_nonblock)
>>> @@ -6778,9 +6846,11 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>  	case IORING_OP_SYNC_FILE_RANGE:
>>>  		return io_sfr_prep(req, sqe);
>>>  	case IORING_OP_SENDMSG:
>>> +	case IORING_OP_SENDTO:
>>>  	case IORING_OP_SEND:
>>>  		return io_sendmsg_prep(req, sqe);
>>>  	case IORING_OP_RECVMSG:
>>> +	case IORING_OP_RECVFROM:
>>>  	case IORING_OP_RECV:
>>>  		return io_recvmsg_prep(req, sqe);
>>>  	case IORING_OP_CONNECT:
>>> @@ -7060,12 +7130,14 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
>>>  	case IORING_OP_SENDMSG:
>>>  		ret = io_sendmsg(req, issue_flags);
>>>  		break;
>>> +	case IORING_OP_SENDTO:
>>>  	case IORING_OP_SEND:
>>>  		ret = io_sendto(req, issue_flags);
>>>  		break;
>>>  	case IORING_OP_RECVMSG:
>>>  		ret = io_recvmsg(req, issue_flags);
>>>  		break;
>>> +	case IORING_OP_RECVFROM:
>>>  	case IORING_OP_RECV:
>>>  		ret = io_recvfrom(req, issue_flags);
>>>  		break;
>>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>>> index efc7ac9b3a6b..a360069d1e8e 100644
>>> --- a/include/uapi/linux/io_uring.h
>>> +++ b/include/uapi/linux/io_uring.h
>>> @@ -150,6 +150,8 @@ enum {
>>>  	IORING_OP_SETXATTR,
>>>  	IORING_OP_FGETXATTR,
>>>  	IORING_OP_GETXATTR,
>>> +	IORING_OP_SENDTO,
>>> +	IORING_OP_RECVFROM,
>>>  
>>>  	/* this goes last, obviously */
>>>  	IORING_OP_LAST,
>>
>>
>> Regards,
>>
>> ~Praveen.
>>
> 
> Thanks for the review. I will send the RFC v5 soon.
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support
  2022-01-07  8:33               ` Praveen Kumar
@ 2022-01-07 11:02                 ` Ammar Faizi
  0 siblings, 0 replies; 19+ messages in thread
From: Ammar Faizi @ 2022-01-07 11:02 UTC (permalink / raw)
  To: Praveen Kumar, Jens Axboe
  Cc: Ammar Faizi, io-uring Mailing List, netdev Mailing List,
	GNU/Weeb Mailing List, Linux Kernel Mailing List, Pavel Begunkov,
	David S. Miller, Jakub Kicinski, Nugra, Ammar Faizi


[-- Attachment #1.1.1: Type: text/plain, Size: 11433 bytes --]

On 1/7/22 3:33 PM, Praveen Kumar wrote:
> On 07-01-2022 02:08, Ammar Faizi wrote:
>> On Thu, 6 Jan 2022 at 23:01:59 +0530, Praveen Kumar <kpraveen.lkml@gmail.com> wrote:
>>> On 30-12-2021 23:22, Ammar Faizi wrote:
>>>> This adds sendto(2) and recvfrom(2) support for io_uring.
>>>>
>>>> New opcodes:
>>>>    IORING_OP_SENDTO
>>>>    IORING_OP_RECVFROM
>>>>
>>>> Cc: Nugra <richiisei@gmail.com>
>>>> Tested-by: Nugra <richiisei@gmail.com>
>>>> Link: https://github.com/axboe/liburing/issues/397
>>>> Signed-off-by: Ammar Faizi <ammarfaizi2@gmail.com>
>>>> ---
>>>>
>>>> v3:
>>>>    - Fix build error when CONFIG_NET is undefined should be done in
>>>>      the first patch, not this patch.
>>>>
>>>>    - Add Tested-by tag from Nugra.
>>>>
>>>> v2:
>>>>    - In `io_recvfrom()`, mark the error check of `move_addr_to_user()`
>>>>      call as unlikely.
>>>>
>>>>    - Fix build error when CONFIG_NET is undefined.
>>>>
>>>>    - Added Nugra to CC list (tester).
>>>> ---
>>>>   fs/io_uring.c                 | 80 +++++++++++++++++++++++++++++++++--
>>>>   include/uapi/linux/io_uring.h |  2 +
>>>>   2 files changed, 78 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>>> index 7adcb591398f..3726958f8f58 100644
>>>> --- a/fs/io_uring.c
>>>> +++ b/fs/io_uring.c
>>>> @@ -575,7 +575,15 @@ struct io_sr_msg {
>>>>   	union {
>>>>   		struct compat_msghdr __user	*umsg_compat;
>>>>   		struct user_msghdr __user	*umsg;
>>>> -		void __user			*buf;
>>>> +
>>>> +		struct {
>>>> +			void __user		*buf;
>>>> +			struct sockaddr __user	*addr;
>>>> +			union {
>>>> +				int		sendto_addr_len;
>>>> +				int __user	*recvfrom_addr_len;
>>>> +			};
>>>> +		};
>>>>   	};
>>>>   	int				msg_flags;
>>>>   	int				bgid;
>>>> @@ -1133,6 +1141,19 @@ static const struct io_op_def io_op_defs[] = {
>>>>   		.needs_file = 1
>>>>   	},
>>>>   	[IORING_OP_GETXATTR] = {},
>>>> +	[IORING_OP_SENDTO] = {
>>>> +		.needs_file		= 1,
>>>> +		.unbound_nonreg_file	= 1,
>>>> +		.pollout		= 1,
>>>> +		.audit_skip		= 1,
>>>> +	},
>>>> +	[IORING_OP_RECVFROM] = {
>>>> +		.needs_file		= 1,
>>>> +		.unbound_nonreg_file	= 1,
>>>> +		.pollin			= 1,
>>>> +		.buffer_select		= 1,
>>>> +		.audit_skip		= 1,
>>>> +	},
>>>>   };
>>>>   
>>>>   /* requests with any of those set should undergo io_disarm_next() */
>>>> @@ -5216,12 +5237,24 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>>   	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>>>   		return -EINVAL;
>>>>   
>>>> +	/*
>>>> +	 * For IORING_OP_SEND{,TO}, the assignment to @sr->umsg
>>>> +	 * is equivalent to an assignment to @sr->buf.
>>>> +	 */
>>>>   	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>>> +
>>>>   	sr->len = READ_ONCE(sqe->len);
>>>>   	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>>>>   	if (sr->msg_flags & MSG_DONTWAIT)
>>>>   		req->flags |= REQ_F_NOWAIT;
>>>>   
>>>> +	if (req->opcode == IORING_OP_SENDTO) {
>>>> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
>>>> +		sr->sendto_addr_len = READ_ONCE(sqe->addr3);
>>>> +	} else {
>>>> +		sr->addr = (struct sockaddr __user *) NULL;
>>>
>>> Let's have sendto_addr_len  = 0
>>
>> Will do in the RFC v5.
>>
>>>
>>>> +	}
>>>> +
>>>>   #ifdef CONFIG_COMPAT
>>>>   	if (req->ctx->compat)
>>>>   		sr->msg_flags |= MSG_CMSG_COMPAT;
>>>> @@ -5275,6 +5308,7 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
>>>>   
>>>>   static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>>   {
>>>> +	struct sockaddr_storage address;
>>>>   	struct io_sr_msg *sr = &req->sr_msg;
>>>>   	struct msghdr msg;
>>>>   	struct iovec iov;
>>>> @@ -5291,10 +5325,20 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>>   	if (unlikely(ret))
>>>>   		return ret;
>>>>   
>>>> -	msg.msg_name = NULL;
>>>> +
>>>>   	msg.msg_control = NULL;
>>>>   	msg.msg_controllen = 0;
>>>> -	msg.msg_namelen = 0;
>>>> +	if (sr->addr) {
>>>> +		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
>>>> +					  &address);
>>>> +		if (unlikely(ret < 0))
>>>> +			goto fail;
>>>> +		msg.msg_name = (struct sockaddr *) &address;
>>>> +		msg.msg_namelen = sr->sendto_addr_len;
>>>> +	} else {
>>>> +		msg.msg_name = NULL;
>>>> +		msg.msg_namelen = 0;
>>>> +	}
>>>>   
>>>>   	flags = req->sr_msg.msg_flags;
>>>>   	if (issue_flags & IO_URING_F_NONBLOCK)
>>>> @@ -5309,6 +5353,7 @@ static int io_sendto(struct io_kiocb *req, unsigned int issue_flags)
>>>>   			return -EAGAIN;
>>>>   		if (ret == -ERESTARTSYS)
>>>>   			ret = -EINTR;
>>>> +	fail:
>>>>   		req_set_fail(req);
>>>
>>> I think there is a problem with "fail" goto statement. Not getting
>>> full clarity on this change. With latest kernel, I see
>>> req_set_fail(req) inside if check, which I don't see here. Can you
>>> please resend the patch on latest kernel version. Thanks.
>>
>> I will send the v5 on top of "for-next" branch in Jens' tree soon.
>>
>> That is already inside an "if check" anyway. We go to that label when
>> the move_addr_to_kernel() fails (most of the time it is -EFAULT or
>> -EINVAL).
>>
>> That part looks like this (note the if check before the goto):
>> ----------------------------------------------------------------------
>> 	msg.msg_control = NULL;
>> 	msg.msg_controllen = 0;
>> 	if (sr->addr) {
>> 		ret = move_addr_to_kernel(sr->addr, sr->sendto_addr_len,
>> 					  &address);
>> 		if (unlikely(ret < 0))
>> 			goto fail;
>> 		msg.msg_name = (struct sockaddr *) &address;
>> 		msg.msg_namelen = sr->sendto_addr_len;
>> 	} else {
>> 		msg.msg_name = NULL;
>> 		msg.msg_namelen = 0;
>> 	}
>>
>> 	flags = req->sr_msg.msg_flags;
>> 	if (issue_flags & IO_URING_F_NONBLOCK)
>> 		flags |= MSG_DONTWAIT;
>> 	if (flags & MSG_WAITALL)
>> 		min_ret = iov_iter_count(&msg.msg_iter);
>>
>> 	msg.msg_flags = flags;
>> 	ret = sock_sendmsg(sock, &msg);
>> 	if (ret < min_ret) {
>> 		if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
>> 			return -EAGAIN;
>> 		if (ret == -ERESTARTSYS)
>> 			ret = -EINTR;
>> 	fail:
> 
> Thanks for sending this. IMO this goto label should be just before
> the "if (ret < min_ret)" statement.

AFAICT, error returned by move_addr_to_kernel are only -EFAULT and -EINVAL,
so the check against -EAGAIN and -ERESTARTSYS is unnecessary for that
case. We can skip that.

RFC v4 here (rebased on top of "for-next" Jens' tree):
https://lore.kernel.org/io-uring/20220107000006.1194026-1-ammarfaizi2@gnuweeb.org/T/

> 
>> 		req_set_fail(req);
>> 	}
>> 	__io_req_complete(req, issue_flags, ret, 0);
>> 	return 0;
>> ----------------------------------------------------------------------
>>
>>>>   	}
>>>>   	__io_req_complete(req, issue_flags, ret, 0);
>>>> @@ -5427,13 +5472,25 @@ static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>>   	if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
>>>>   		return -EINVAL;
>>>>   
>>>> +	/*
>>>> +	 * For IORING_OP_RECV{,FROM}, the assignment to @sr->umsg
>>>> +	 * is equivalent to an assignment to @sr->buf.
>>>> +	 */
>>>>   	sr->umsg = u64_to_user_ptr(READ_ONCE(sqe->addr));
>>>> +
>>>>   	sr->len = READ_ONCE(sqe->len);
>>>>   	sr->bgid = READ_ONCE(sqe->buf_group);
>>>>   	sr->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
>>>>   	if (sr->msg_flags & MSG_DONTWAIT)
>>>>   		req->flags |= REQ_F_NOWAIT;
>>>>   
>>>> +	if (req->opcode == IORING_OP_RECVFROM) {
>>>> +		sr->addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
>>>> +		sr->recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->addr3));
>>>> +	} else {
>>>> +		sr->addr = (struct sockaddr __user *) NULL;
>>>
>>> I think recvfrom_addr_len should also be pointed to NULL, instead of
>>> garbage for this case.
>>
>> Will do in the RFC v5.
>>
>>>
>>>> +	}
>>>> +
>>>>   #ifdef CONFIG_COMPAT
>>>>   	if (req->ctx->compat)
>>>>   		sr->msg_flags |= MSG_CMSG_COMPAT;
>>>> @@ -5509,6 +5566,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>>   	struct iovec iov;
>>>>   	unsigned flags;
>>>>   	int ret, min_ret = 0;
>>>> +	struct sockaddr_storage address;
>>>>   	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
>>>>   
>>>>   	sock = sock_from_file(req->file);
>>>> @@ -5526,7 +5584,7 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>>   	if (unlikely(ret))
>>>>   		goto out_free;
>>>>   
>>>> -	msg.msg_name = NULL;
>>>> +	msg.msg_name = sr->addr ? (struct sockaddr *) &address : NULL;
>>>>   	msg.msg_control = NULL;
>>>>   	msg.msg_controllen = 0;
>>>>   	msg.msg_namelen = 0;
>>>
>>> I think namelen should also be updated ?
>>
>> It doesn't have to be updated. From net/socket.c there is a comment
>> like this:
>>
>> 	/* We assume all kernel code knows the size of sockaddr_storage */
>> 	msg.msg_namelen = 0;
>>
>> Full __sys_recvfrom() source code, see here:
>> https://github.com/torvalds/linux/blob/v5.16-rc8/net/socket.c#L2085-L2088
>>
>> I will add the same comment in next series to clarify this one.
>>
>>>
>>>> @@ -5540,6 +5598,16 @@ static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
>>>>   		min_ret = iov_iter_count(&msg.msg_iter);
>>>>   
>>>>   	ret = sock_recvmsg(sock, &msg, flags);
>>>> +
>>>> +	if (ret >= 0 && sr->addr != NULL) {
>>>> +		int tmp;
>>>> +
>>>> +		tmp = move_addr_to_user(&address, msg.msg_namelen, sr->addr,
>>>> +					sr->recvfrom_addr_len);
>>>> +		if (unlikely(tmp < 0))
>>>> +			ret = tmp;
>>>> +	}
>>>> +
>>>>   out_free:
>>>>   	if (ret < min_ret) {
>>>>   		if (ret == -EAGAIN && force_nonblock)
>>>> @@ -6778,9 +6846,11 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>>>   	case IORING_OP_SYNC_FILE_RANGE:
>>>>   		return io_sfr_prep(req, sqe);
>>>>   	case IORING_OP_SENDMSG:
>>>> +	case IORING_OP_SENDTO:
>>>>   	case IORING_OP_SEND:
>>>>   		return io_sendmsg_prep(req, sqe);
>>>>   	case IORING_OP_RECVMSG:
>>>> +	case IORING_OP_RECVFROM:
>>>>   	case IORING_OP_RECV:
>>>>   		return io_recvmsg_prep(req, sqe);
>>>>   	case IORING_OP_CONNECT:
>>>> @@ -7060,12 +7130,14 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
>>>>   	case IORING_OP_SENDMSG:
>>>>   		ret = io_sendmsg(req, issue_flags);
>>>>   		break;
>>>> +	case IORING_OP_SENDTO:
>>>>   	case IORING_OP_SEND:
>>>>   		ret = io_sendto(req, issue_flags);
>>>>   		break;
>>>>   	case IORING_OP_RECVMSG:
>>>>   		ret = io_recvmsg(req, issue_flags);
>>>>   		break;
>>>> +	case IORING_OP_RECVFROM:
>>>>   	case IORING_OP_RECV:
>>>>   		ret = io_recvfrom(req, issue_flags);
>>>>   		break;
>>>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>>>> index efc7ac9b3a6b..a360069d1e8e 100644
>>>> --- a/include/uapi/linux/io_uring.h
>>>> +++ b/include/uapi/linux/io_uring.h
>>>> @@ -150,6 +150,8 @@ enum {
>>>>   	IORING_OP_SETXATTR,
>>>>   	IORING_OP_FGETXATTR,
>>>>   	IORING_OP_GETXATTR,
>>>> +	IORING_OP_SENDTO,
>>>> +	IORING_OP_RECVFROM,
>>>>   
>>>>   	/* this goes last, obviously */
>>>>   	IORING_OP_LAST,
>>>
>>>
>>> Regards,
>>>
>>> ~Praveen.
>>>
>>
>> Thanks for the review. I will send the RFC v5 soon.
>>
> 

-- 
Ammar Faizi


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 1793 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH v1 1/3] io_uring: Rename `io_{send, recv}` to `io_{sendto, recvfrom}`
@ 2021-12-30 16:54 kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-12-30 16:54 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 5797 bytes --]

CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211230013250.103267-1-ammar.faizi@intel.com>
References: <20211230013250.103267-1-ammar.faizi@intel.com>
TO: Ammar Faizi <ammarfaizi2@gmail.com>

Hi Ammar,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on aafc6e6eba29c890b0031267fc37c43490447c81]

url:    https://github.com/0day-ci/linux/commits/Ammar-Faizi/io_uring-Add-sendto-2-and-recvfrom-2-support/20211230-093703
base:   aafc6e6eba29c890b0031267fc37c43490447c81
:::::: branch date: 15 hours ago
:::::: commit date: 15 hours ago
config: i386-randconfig-m021-20211230 (https://download.01.org/0day-ci/archive/20211231/202112310016.rALniPPT-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
fs/io_uring.c:5550 io_recvfrom() error: uninitialized symbol 'flags'.

Old smatch warnings:
fs/io_uring.c:6364 io_timeout_cancel() warn: passing a valid pointer to 'PTR_ERR'
fs/io_uring.c:6421 io_timeout_update() warn: passing a valid pointer to 'PTR_ERR'
fs/io_uring.c:8722 io_sqe_files_register() error: we previously assumed 'ctx->file_data' could be null (see line 8694)

vim +/flags +5550 fs/io_uring.c

0fa03c624d8fc9 Jens Axboe        2019-04-19  5501  
9cfe685bad5431 Ammar Faizi       2021-12-30  5502  static int io_recvfrom(struct io_kiocb *req, unsigned int issue_flags)
fddafacee287b3 Jens Axboe        2020-01-04  5503  {
6b754c8b912a16 Pavel Begunkov    2020-07-16  5504  	struct io_buffer *kbuf;
fddafacee287b3 Jens Axboe        2020-01-04  5505  	struct io_sr_msg *sr = &req->sr_msg;
fddafacee287b3 Jens Axboe        2020-01-04  5506  	struct msghdr msg;
7a7cacba8b4560 Pavel Begunkov    2020-07-16  5507  	void __user *buf = sr->buf;
7a7cacba8b4560 Pavel Begunkov    2020-07-16  5508  	struct socket *sock;
fddafacee287b3 Jens Axboe        2020-01-04  5509  	struct iovec iov;
fddafacee287b3 Jens Axboe        2020-01-04  5510  	unsigned flags;
d1fd1c201d7507 Pavel Begunkov    2021-12-05  5511  	int ret, min_ret = 0;
45d189c6062922 Pavel Begunkov    2021-02-10  5512  	bool force_nonblock = issue_flags & IO_URING_F_NONBLOCK;
7a7cacba8b4560 Pavel Begunkov    2020-07-16  5513  
dba4a9256bb4d7 Florent Revest    2020-12-04  5514  	sock = sock_from_file(req->file);
7a7cacba8b4560 Pavel Begunkov    2020-07-16  5515  	if (unlikely(!sock))
dba4a9256bb4d7 Florent Revest    2020-12-04  5516  		return -ENOTSOCK;
fddafacee287b3 Jens Axboe        2020-01-04  5517  
bc02ef3325e3ef Pavel Begunkov    2020-07-16  5518  	if (req->flags & REQ_F_BUFFER_SELECT) {
51aac424aef980 Pavel Begunkov    2021-10-14  5519  		kbuf = io_recv_buffer_select(req, issue_flags);
bcda7baaa3f15c Jens Axboe        2020-02-23  5520  		if (IS_ERR(kbuf))
bcda7baaa3f15c Jens Axboe        2020-02-23  5521  			return PTR_ERR(kbuf);
bcda7baaa3f15c Jens Axboe        2020-02-23  5522  		buf = u64_to_user_ptr(kbuf->addr);
bcda7baaa3f15c Jens Axboe        2020-02-23  5523  	}
fddafacee287b3 Jens Axboe        2020-01-04  5524  
7a7cacba8b4560 Pavel Begunkov    2020-07-16  5525  	ret = import_single_range(READ, buf, sr->len, &iov, &msg.msg_iter);
14c32eee928662 Pavel Begunkov    2020-07-16  5526  	if (unlikely(ret))
14c32eee928662 Pavel Begunkov    2020-07-16  5527  		goto out_free;
fddafacee287b3 Jens Axboe        2020-01-04  5528  
fddafacee287b3 Jens Axboe        2020-01-04  5529  	msg.msg_name = NULL;
fddafacee287b3 Jens Axboe        2020-01-04  5530  	msg.msg_control = NULL;
fddafacee287b3 Jens Axboe        2020-01-04  5531  	msg.msg_controllen = 0;
fddafacee287b3 Jens Axboe        2020-01-04  5532  	msg.msg_namelen = 0;
fddafacee287b3 Jens Axboe        2020-01-04  5533  	msg.msg_iocb = NULL;
fddafacee287b3 Jens Axboe        2020-01-04  5534  	msg.msg_flags = 0;
fddafacee287b3 Jens Axboe        2020-01-04  5535  
044118069a23fd Pavel Begunkov    2021-04-01  5536  	flags = req->sr_msg.msg_flags;
044118069a23fd Pavel Begunkov    2021-04-01  5537  	if (force_nonblock)
fddafacee287b3 Jens Axboe        2020-01-04  5538  		flags |= MSG_DONTWAIT;
0031275d119efe Stefan Metzmacher 2021-03-20  5539  	if (flags & MSG_WAITALL)
0031275d119efe Stefan Metzmacher 2021-03-20  5540  		min_ret = iov_iter_count(&msg.msg_iter);
0031275d119efe Stefan Metzmacher 2021-03-20  5541  
0b7b21e42ba2d6 Jens Axboe        2020-01-31  5542  	ret = sock_recvmsg(sock, &msg, flags);
7297ce3d59449d Pavel Begunkov    2021-11-23  5543  out_free:
7297ce3d59449d Pavel Begunkov    2021-11-23  5544  	if (ret < min_ret) {
7297ce3d59449d Pavel Begunkov    2021-11-23  5545  		if (ret == -EAGAIN && force_nonblock)
fddafacee287b3 Jens Axboe        2020-01-04  5546  			return -EAGAIN;
fddafacee287b3 Jens Axboe        2020-01-04  5547  		if (ret == -ERESTARTSYS)
fddafacee287b3 Jens Axboe        2020-01-04  5548  			ret = -EINTR;
7297ce3d59449d Pavel Begunkov    2021-11-23  5549  		req_set_fail(req);
7297ce3d59449d Pavel Begunkov    2021-11-23 @5550  	} else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
7297ce3d59449d Pavel Begunkov    2021-11-23  5551  		req_set_fail(req);
7297ce3d59449d Pavel Begunkov    2021-11-23  5552  	}
d1fd1c201d7507 Pavel Begunkov    2021-12-05  5553  
d1fd1c201d7507 Pavel Begunkov    2021-12-05  5554  	__io_req_complete(req, issue_flags, ret, io_put_kbuf(req));
fddafacee287b3 Jens Axboe        2020-01-04  5555  	return 0;
fddafacee287b3 Jens Axboe        2020-01-04  5556  }
fddafacee287b3 Jens Axboe        2020-01-04  5557  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-01-07 11:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-30  1:35 [RFC PATCH v1 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
2021-12-30  1:35 ` [RFC PATCH v1 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
2021-12-30 11:59   ` [RFC PATCH v1 1/3] io_uring: Rename `io_{send, recv}` to `io_{sendto, recvfrom}` kernel test robot
2021-12-30  1:35 ` [RFC PATCH v1 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
2021-12-30  1:35 ` [RFC PATCH v1 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
2021-12-30 12:00   ` [RFC PATCH v2 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
2021-12-30 12:00     ` [RFC PATCH v2 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
2021-12-30 12:00     ` [RFC PATCH v2 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
2021-12-30 12:00     ` [RFC PATCH v2 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
2021-12-30 17:52       ` [RFC PATCH v3 0/3] io_uring: Add sendto(2) and recvfrom(2) support Ammar Faizi
2021-12-30 17:52         ` [RFC PATCH v3 1/3] io_uring: Rename `io_{send,recv}` to `io_{sendto,recvfrom}` Ammar Faizi
2021-12-30 17:52         ` [RFC PATCH v3 2/3] net: Make `move_addr_to_user()` be a non static function Ammar Faizi
2021-12-30 17:52         ` [RFC PATCH v3 3/3] io_uring: Add `sendto(2)` and `recvfrom(2)` support Ammar Faizi
2022-01-06 17:31           ` Praveen Kumar
2022-01-06 20:38             ` Ammar Faizi
2022-01-06 20:48               ` Ammar Faizi
2022-01-07  8:33               ` Praveen Kumar
2022-01-07 11:02                 ` Ammar Faizi
2021-12-30 16:54 [RFC PATCH v1 1/3] io_uring: Rename `io_{send, recv}` to `io_{sendto, recvfrom}` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.