Re: [PATCH] io_uring: add support for sendto(2) and recvfrom(2)

[Jens Axboe <axboe@xxxxxxxxx>]

On 7/6/20 2:44 PM, Jens Axboe wrote:
> On 7/6/20 12:09 PM, Alex Nash wrote:
>> This adds IORING_OP_SENDTO for sendto(2) support, and IORING_OP_RECVFROM
>> for recvfrom(2) support.
> 
> I'll ask the basic question that Jann also asked last week - why sendto
> and recvfrom, when you can use recvmsg and sendmsg to achieve the same
> thing?

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.

For the patch itself, a few comments below:

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 4c9a494c9f9f..b0783fbe1638 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -415,7 +415,14 @@ struct io_sr_msg {
 	struct file			*file;
 	union {
 		struct user_msghdr __user *msg;
-		void __user		*buf;
+		struct {
+			void __user	*buf;
+			struct sockaddr __user *addr;
+			union {
+			void __user	*recvfrom_addr_len;
+			size_t		sendto_addr_len;
+			};
+		} sr;
 	};
 	int				msg_flags;
 	int				bgid;

Some inconsistent indentation here.

@@ -876,6 +883,18 @@ static const struct io_op_def io_op_defs[] = {
 		.hash_reg_file		= 1,
 		.unbound_nonreg_file	= 1,
 	},
+	[IORING_OP_SENDTO] = {
+		.needs_mm		= 1,
+		.needs_file		= 1,
+		.unbound_nonreg_file	= 1,
+		.pollout		= 1,
+	},
+	[IORING_OP_RECVFROM] = {
+		.needs_mm		= 1,
+		.needs_file		= 1,
+		.unbound_nonreg_file	= 1,
+		.pollout		= 1,
+	},
 };

RECVFROM should have pollin set, not pollout?
 
 enum io_mem_account {
@@ -3910,6 +3929,11 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		sr->msg_flags |= MSG_CMSG_COMPAT;
 #endif
 
+	if (req->opcode == IORING_OP_SENDTO) {
+		sr->sr.addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+		sr->sr.sendto_addr_len = READ_ONCE(sqe->sendto_addr_len);
+		return 0;
+	}
 	if (!io || req->opcode == IORING_OP_SEND)
 		return 0;

Might be better with a switch for the opcode here since we're now doing
three ops in that prep?

@@ -4148,6 +4181,11 @@ static int io_recvmsg_prep(struct io_kiocb *req,
 		sr->msg_flags |= MSG_CMSG_COMPAT;
 #endif
 
+	if (req->opcode == IORING_OP_RECVFROM) {
+		sr->sr.addr = u64_to_user_ptr(READ_ONCE(sqe->addr2));
+		sr->sr.recvfrom_addr_len = u64_to_user_ptr(READ_ONCE(sqe->recvfrom_addr_len));
+		return 0;
+	}
 	if (!io || req->opcode == IORING_OP_RECV)
 		return 0;

Same comment here.

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 8d033961cb78..62605e4aabd2 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -56,6 +56,10 @@ struct io_uring_sqe {
 			/* personality to use, if used */
 			__u16	personality;
 			__s32	splice_fd_in;
+			union {
+				__u32	sendto_addr_len;
+				__u64	recvfrom_addr_len;
+			};
 		};
 		__u64	__pad2[3];
 	};

I guess we have no more room in the "regular" parts of the SQE with
recvfrom/sendto taking 6 arguments?

-- 
Jens Axboe