public inbox for [email protected]
 help / color / mirror / Atom feed
* [RFC 0/6] io_uring simplify zerocopy send API
@ 2022-09-01 10:53 Pavel Begunkov
  2022-09-01 10:54 ` [RFC 1/6] selftests/net: temporarily disable io_uring zc test Pavel Begunkov
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-09-01 10:53 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe, asml.silence

We're changing zerocopy send API making it a bit less flexible but
much simpler based on the feedback we've got from people trying it
out. We replace slots and flushing with a per request notifications.
The API change is described in 5/6 in more details.
more in 5/6.

The only real functional change is in 5/6, 2-4 are reverts, and patches
1 and 6 are fixing selftests.

Pavel Begunkov (6):
  selftests/net: temporarily disable io_uring zc test
  Revert "io_uring: add zc notification flush requests"
  Revert "io_uring: rename IORING_OP_FILES_UPDATE"
  io_uring/notif: remove notif registration
  io_uring/net: simplify zerocopy send user API
  selftests/net: return back io_uring zc send tests

 include/uapi/linux/io_uring.h                 |  28 ++---
 io_uring/io_uring.c                           |  14 +--
 io_uring/net.c                                |  57 ++++++----
 io_uring/net.h                                |   1 +
 io_uring/notif.c                              |  83 +-------------
 io_uring/notif.h                              |  54 +---------
 io_uring/opdef.c                              |  12 +--
 io_uring/rsrc.c                               |  55 +---------
 io_uring/rsrc.h                               |   4 +-
 .../selftests/net/io_uring_zerocopy_tx.c      | 101 +++++++-----------
 .../selftests/net/io_uring_zerocopy_tx.sh     |  10 +-
 11 files changed, 98 insertions(+), 321 deletions(-)

-- 
2.37.2


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

* [RFC 1/6] selftests/net: temporarily disable io_uring zc test
  2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
@ 2022-09-01 10:54 ` Pavel Begunkov
  2022-09-01 10:54 ` [RFC 2/6] Revert "io_uring: add zc notification flush requests" Pavel Begunkov
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-09-01 10:54 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe, asml.silence

We're going to change API, to avoid build problems with a couple of
following commits, disable io_uring testing.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 tools/testing/selftests/net/io_uring_zerocopy_tx.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/testing/selftests/net/io_uring_zerocopy_tx.c b/tools/testing/selftests/net/io_uring_zerocopy_tx.c
index 9d64c560a2d6..7446ef364e9f 100644
--- a/tools/testing/selftests/net/io_uring_zerocopy_tx.c
+++ b/tools/testing/selftests/net/io_uring_zerocopy_tx.c
@@ -36,6 +36,8 @@
 #include <sys/un.h>
 #include <sys/wait.h>
 
+#if 0
+
 #define NOTIF_TAG 0xfffffffULL
 #define NONZC_TAG 0
 #define ZC_TAG 1
@@ -603,3 +605,10 @@ int main(int argc, char **argv)
 		error(1, 0, "unknown cfg_test %s", cfg_test);
 	return 0;
 }
+
+#else
+int main(int argc, char **argv)
+{
+	return 0;
+}
+#endif
-- 
2.37.2


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

* [RFC 2/6] Revert "io_uring: add zc notification flush requests"
  2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
  2022-09-01 10:54 ` [RFC 1/6] selftests/net: temporarily disable io_uring zc test Pavel Begunkov
@ 2022-09-01 10:54 ` Pavel Begunkov
  2022-09-01 10:54 ` [RFC 3/6] Revert "io_uring: rename IORING_OP_FILES_UPDATE" Pavel Begunkov
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-09-01 10:54 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe, asml.silence

This reverts commit 492dddb4f6e3a5839c27d41ff1fecdbe6c3ab851.

Soon we won't have the very notion of notification flushing, so remove
notification flushing requests.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 include/uapi/linux/io_uring.h |  1 -
 io_uring/rsrc.c               | 38 -----------------------------------
 2 files changed, 39 deletions(-)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 9e0b5c8d92ce..18ae5caf1773 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -301,7 +301,6 @@ enum io_uring_op {
  */
 enum {
 	IORING_RSRC_UPDATE_FILES,
-	IORING_RSRC_UPDATE_NOTIF,
 };
 
 /*
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 71359a4d0bd4..048f7483fe8a 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -15,7 +15,6 @@
 #include "io_uring.h"
 #include "openclose.h"
 #include "rsrc.h"
-#include "notif.h"
 
 struct io_rsrc_update {
 	struct file			*file;
@@ -741,41 +740,6 @@ static int io_files_update(struct io_kiocb *req, unsigned int issue_flags)
 	return IOU_OK;
 }
 
-static int io_notif_update(struct io_kiocb *req, unsigned int issue_flags)
-{
-	struct io_rsrc_update *up = io_kiocb_to_cmd(req, struct io_rsrc_update);
-	struct io_ring_ctx *ctx = req->ctx;
-	unsigned len = up->nr_args;
-	unsigned idx_end, idx = up->offset;
-	int ret = 0;
-
-	io_ring_submit_lock(ctx, issue_flags);
-	if (unlikely(check_add_overflow(idx, len, &idx_end))) {
-		ret = -EOVERFLOW;
-		goto out;
-	}
-	if (unlikely(idx_end > ctx->nr_notif_slots)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	for (; idx < idx_end; idx++) {
-		struct io_notif_slot *slot = &ctx->notif_slots[idx];
-
-		if (!slot->notif)
-			continue;
-		if (up->arg)
-			slot->tag = up->arg;
-		io_notif_slot_flush_submit(slot, issue_flags);
-	}
-out:
-	io_ring_submit_unlock(ctx, issue_flags);
-	if (ret < 0)
-		req_set_fail(req);
-	io_req_set_res(req, ret, 0);
-	return IOU_OK;
-}
-
 int io_rsrc_update(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_rsrc_update *up = io_kiocb_to_cmd(req, struct io_rsrc_update);
@@ -783,8 +747,6 @@ int io_rsrc_update(struct io_kiocb *req, unsigned int issue_flags)
 	switch (up->type) {
 	case IORING_RSRC_UPDATE_FILES:
 		return io_files_update(req, issue_flags);
-	case IORING_RSRC_UPDATE_NOTIF:
-		return io_notif_update(req, issue_flags);
 	}
 	return -EINVAL;
 }
-- 
2.37.2


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

* [RFC 3/6] Revert "io_uring: rename IORING_OP_FILES_UPDATE"
  2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
  2022-09-01 10:54 ` [RFC 1/6] selftests/net: temporarily disable io_uring zc test Pavel Begunkov
  2022-09-01 10:54 ` [RFC 2/6] Revert "io_uring: add zc notification flush requests" Pavel Begunkov
@ 2022-09-01 10:54 ` Pavel Begunkov
  2022-09-01 10:54 ` [RFC 4/6] io_uring/notif: remove notif registration Pavel Begunkov
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-09-01 10:54 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe, asml.silence

This reverts commit 4379d5f15b3fd4224c37841029178aa8082a242e.

We removed notification flushing, also cleanup uapi preparation changes
to not pollute it.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 include/uapi/linux/io_uring.h | 12 +-----------
 io_uring/opdef.c              |  9 ++++-----
 io_uring/rsrc.c               | 17 ++---------------
 io_uring/rsrc.h               |  4 ++--
 4 files changed, 9 insertions(+), 33 deletions(-)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 18ae5caf1773..111b651366bd 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -178,8 +178,7 @@ enum io_uring_op {
 	IORING_OP_FALLOCATE,
 	IORING_OP_OPENAT,
 	IORING_OP_CLOSE,
-	IORING_OP_RSRC_UPDATE,
-	IORING_OP_FILES_UPDATE = IORING_OP_RSRC_UPDATE,
+	IORING_OP_FILES_UPDATE,
 	IORING_OP_STATX,
 	IORING_OP_READ,
 	IORING_OP_WRITE,
@@ -228,7 +227,6 @@ enum io_uring_op {
 #define IORING_TIMEOUT_ETIME_SUCCESS	(1U << 5)
 #define IORING_TIMEOUT_CLOCK_MASK	(IORING_TIMEOUT_BOOTTIME | IORING_TIMEOUT_REALTIME)
 #define IORING_TIMEOUT_UPDATE_MASK	(IORING_TIMEOUT_UPDATE | IORING_LINK_TIMEOUT_UPDATE)
-
 /*
  * sqe->splice_flags
  * extends splice(2) flags
@@ -295,14 +293,6 @@ enum io_uring_op {
  */
 #define IORING_ACCEPT_MULTISHOT	(1U << 0)
 
-
-/*
- * IORING_OP_RSRC_UPDATE flags
- */
-enum {
-	IORING_RSRC_UPDATE_FILES,
-};
-
 /*
  * IORING_OP_MSG_RING command types, stored in sqe->addr
  */
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 41410126c1c6..10b301ccf5cd 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -246,13 +246,12 @@ const struct io_op_def io_op_defs[] = {
 		.prep			= io_close_prep,
 		.issue			= io_close,
 	},
-	[IORING_OP_RSRC_UPDATE] = {
+	[IORING_OP_FILES_UPDATE] = {
 		.audit_skip		= 1,
 		.iopoll			= 1,
-		.name			= "RSRC_UPDATE",
-		.prep			= io_rsrc_update_prep,
-		.issue			= io_rsrc_update,
-		.ioprio			= 1,
+		.name			= "FILES_UPDATE",
+		.prep			= io_files_update_prep,
+		.issue			= io_files_update,
 	},
 	[IORING_OP_STATX] = {
 		.audit_skip		= 1,
diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index 048f7483fe8a..cf3272113214 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -21,7 +21,6 @@ struct io_rsrc_update {
 	u64				arg;
 	u32				nr_args;
 	u32				offset;
-	int				type;
 };
 
 static int io_sqe_buffer_register(struct io_ring_ctx *ctx, struct iovec *iov,
@@ -654,7 +653,7 @@ __cold int io_register_rsrc(struct io_ring_ctx *ctx, void __user *arg,
 	return -EINVAL;
 }
 
-int io_rsrc_update_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
+int io_files_update_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_rsrc_update *up = io_kiocb_to_cmd(req, struct io_rsrc_update);
 
@@ -668,7 +667,6 @@ int io_rsrc_update_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (!up->nr_args)
 		return -EINVAL;
 	up->arg = READ_ONCE(sqe->addr);
-	up->type = READ_ONCE(sqe->ioprio);
 	return 0;
 }
 
@@ -711,7 +709,7 @@ static int io_files_update_with_index_alloc(struct io_kiocb *req,
 	return ret;
 }
 
-static int io_files_update(struct io_kiocb *req, unsigned int issue_flags)
+int io_files_update(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_rsrc_update *up = io_kiocb_to_cmd(req, struct io_rsrc_update);
 	struct io_ring_ctx *ctx = req->ctx;
@@ -740,17 +738,6 @@ static int io_files_update(struct io_kiocb *req, unsigned int issue_flags)
 	return IOU_OK;
 }
 
-int io_rsrc_update(struct io_kiocb *req, unsigned int issue_flags)
-{
-	struct io_rsrc_update *up = io_kiocb_to_cmd(req, struct io_rsrc_update);
-
-	switch (up->type) {
-	case IORING_RSRC_UPDATE_FILES:
-		return io_files_update(req, issue_flags);
-	}
-	return -EINVAL;
-}
-
 int io_queue_rsrc_removal(struct io_rsrc_data *data, unsigned idx,
 			  struct io_rsrc_node *node, void *rsrc)
 {
diff --git a/io_uring/rsrc.h b/io_uring/rsrc.h
index f3a9a177941f..9bce15665444 100644
--- a/io_uring/rsrc.h
+++ b/io_uring/rsrc.h
@@ -167,8 +167,8 @@ static inline u64 *io_get_tag_slot(struct io_rsrc_data *data, unsigned int idx)
 	return &data->tags[table_idx][off];
 }
 
-int io_rsrc_update(struct io_kiocb *req, unsigned int issue_flags);
-int io_rsrc_update_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+int io_files_update(struct io_kiocb *req, unsigned int issue_flags);
+int io_files_update_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 
 int __io_account_mem(struct user_struct *user, unsigned long nr_pages);
 
-- 
2.37.2


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

* [RFC 4/6] io_uring/notif: remove notif registration
  2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
                   ` (2 preceding siblings ...)
  2022-09-01 10:54 ` [RFC 3/6] Revert "io_uring: rename IORING_OP_FILES_UPDATE" Pavel Begunkov
@ 2022-09-01 10:54 ` Pavel Begunkov
  2022-09-01 10:54 ` [RFC 5/6] io_uring/net: simplify zerocopy send user API Pavel Begunkov
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-09-01 10:54 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe, asml.silence

We're going to remove the userspace exposed zerocopy notification API,
remove notification registration.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 include/uapi/linux/io_uring.h |  8 ----
 io_uring/io_uring.c           | 10 -----
 io_uring/net.c                |  4 +-
 io_uring/notif.c              | 71 -----------------------------------
 io_uring/notif.h              | 11 ------
 5 files changed, 1 insertion(+), 103 deletions(-)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 111b651366bd..b11c57b0ebb5 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -279,14 +279,10 @@ enum io_uring_op {
  *
  * IORING_RECVSEND_FIXED_BUF	Use registered buffers, the index is stored in
  *				the buf_index field.
- *
- * IORING_RECVSEND_NOTIF_FLUSH	Flush a notification after a successful
- *				successful. Only for zerocopy sends.
  */
 #define IORING_RECVSEND_POLL_FIRST	(1U << 0)
 #define IORING_RECV_MULTISHOT		(1U << 1)
 #define IORING_RECVSEND_FIXED_BUF	(1U << 2)
-#define IORING_RECVSEND_NOTIF_FLUSH	(1U << 3)
 
 /*
  * accept flags stored in sqe->ioprio
@@ -474,10 +470,6 @@ enum {
 	/* register a range of fixed file slots for automatic slot allocation */
 	IORING_REGISTER_FILE_ALLOC_RANGE	= 25,
 
-	/* zerocopy notification API */
-	IORING_REGISTER_NOTIFIERS		= 26,
-	IORING_UNREGISTER_NOTIFIERS		= 27,
-
 	/* this goes last */
 	IORING_REGISTER_LAST
 };
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 77616279000b..c2e06a3aa18d 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -2640,7 +2640,6 @@ static __cold void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx)
 		io_unregister_personality(ctx, index);
 	if (ctx->rings)
 		io_poll_remove_all(ctx, NULL, true);
-	io_notif_unregister(ctx);
 	mutex_unlock(&ctx->uring_lock);
 
 	/* failed during ring init, it couldn't have issued any requests */
@@ -3839,15 +3838,6 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode,
 			break;
 		ret = io_register_file_alloc_range(ctx, arg);
 		break;
-	case IORING_REGISTER_NOTIFIERS:
-		ret = io_notif_register(ctx, arg, nr_args);
-		break;
-	case IORING_UNREGISTER_NOTIFIERS:
-		ret = -EINVAL;
-		if (arg || nr_args)
-			break;
-		ret = io_notif_unregister(ctx);
-		break;
 	default:
 		ret = -EINVAL;
 		break;
diff --git a/io_uring/net.c b/io_uring/net.c
index 7a5468cc905e..aac6997b7d88 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -889,7 +889,7 @@ int io_sendzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 
 	zc->flags = READ_ONCE(sqe->ioprio);
 	if (zc->flags & ~(IORING_RECVSEND_POLL_FIRST |
-			  IORING_RECVSEND_FIXED_BUF | IORING_RECVSEND_NOTIF_FLUSH))
+			  IORING_RECVSEND_FIXED_BUF))
 		return -EINVAL;
 	if (zc->flags & IORING_RECVSEND_FIXED_BUF) {
 		unsigned idx = READ_ONCE(sqe->buf_index);
@@ -1063,8 +1063,6 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags)
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
 		req_set_fail(req);
-	} else if (zc->flags & IORING_RECVSEND_NOTIF_FLUSH) {
-		io_notif_slot_flush_submit(notif_slot, 0);
 	}
 
 	if (ret >= 0)
diff --git a/io_uring/notif.c b/io_uring/notif.c
index 96f076b175e0..11f45640684a 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -86,74 +86,3 @@ void io_notif_slot_flush(struct io_notif_slot *slot)
 		io_req_task_work_add(notif);
 	}
 }
-
-__cold int io_notif_unregister(struct io_ring_ctx *ctx)
-	__must_hold(&ctx->uring_lock)
-{
-	int i;
-
-	if (!ctx->notif_slots)
-		return -ENXIO;
-
-	for (i = 0; i < ctx->nr_notif_slots; i++) {
-		struct io_notif_slot *slot = &ctx->notif_slots[i];
-		struct io_kiocb *notif = slot->notif;
-		struct io_notif_data *nd;
-
-		if (!notif)
-			continue;
-		nd = io_notif_to_data(notif);
-		slot->notif = NULL;
-		if (!refcount_dec_and_test(&nd->uarg.refcnt))
-			continue;
-		notif->io_task_work.func = __io_notif_complete_tw;
-		io_req_task_work_add(notif);
-	}
-
-	kvfree(ctx->notif_slots);
-	ctx->notif_slots = NULL;
-	ctx->nr_notif_slots = 0;
-	return 0;
-}
-
-__cold int io_notif_register(struct io_ring_ctx *ctx,
-			     void __user *arg, unsigned int size)
-	__must_hold(&ctx->uring_lock)
-{
-	struct io_uring_notification_slot __user *slots;
-	struct io_uring_notification_slot slot;
-	struct io_uring_notification_register reg;
-	unsigned i;
-
-	if (ctx->nr_notif_slots)
-		return -EBUSY;
-	if (size != sizeof(reg))
-		return -EINVAL;
-	if (copy_from_user(&reg, arg, sizeof(reg)))
-		return -EFAULT;
-	if (!reg.nr_slots || reg.nr_slots > IORING_MAX_NOTIF_SLOTS)
-		return -EINVAL;
-	if (reg.resv || reg.resv2 || reg.resv3)
-		return -EINVAL;
-
-	slots = u64_to_user_ptr(reg.data);
-	ctx->notif_slots = kvcalloc(reg.nr_slots, sizeof(ctx->notif_slots[0]),
-				GFP_KERNEL_ACCOUNT);
-	if (!ctx->notif_slots)
-		return -ENOMEM;
-
-	for (i = 0; i < reg.nr_slots; i++, ctx->nr_notif_slots++) {
-		struct io_notif_slot *notif_slot = &ctx->notif_slots[i];
-
-		if (copy_from_user(&slot, &slots[i], sizeof(slot))) {
-			io_notif_unregister(ctx);
-			return -EFAULT;
-		}
-		if (slot.resv[0] | slot.resv[1] | slot.resv[2]) {
-			io_notif_unregister(ctx);
-			return -EINVAL;
-		}
-		notif_slot->tag = slot.tag;
-	}
-	return 0;
-}
diff --git a/io_uring/notif.h b/io_uring/notif.h
index 80f6445e0c2b..8380eeff2f2e 100644
--- a/io_uring/notif.h
+++ b/io_uring/notif.h
@@ -8,7 +8,6 @@
 #include "rsrc.h"
 
 #define IO_NOTIF_SPLICE_BATCH	32
-#define IORING_MAX_NOTIF_SLOTS	(1U << 15)
 
 struct io_notif_data {
 	struct file		*file;
@@ -36,10 +35,6 @@ struct io_notif_slot {
 	u32			seq;
 };
 
-int io_notif_register(struct io_ring_ctx *ctx,
-		      void __user *arg, unsigned int size);
-int io_notif_unregister(struct io_ring_ctx *ctx);
-
 void io_notif_slot_flush(struct io_notif_slot *slot);
 struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx,
 				struct io_notif_slot *slot);
@@ -67,12 +62,6 @@ static inline struct io_notif_slot *io_get_notif_slot(struct io_ring_ctx *ctx,
 	return &ctx->notif_slots[idx];
 }
 
-static inline void io_notif_slot_flush_submit(struct io_notif_slot *slot,
-					      unsigned int issue_flags)
-{
-	io_notif_slot_flush(slot);
-}
-
 static inline int io_notif_account_mem(struct io_kiocb *notif, unsigned len)
 {
 	struct io_ring_ctx *ctx = notif->ctx;
-- 
2.37.2


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

* [RFC 5/6] io_uring/net: simplify zerocopy send user API
  2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
                   ` (3 preceding siblings ...)
  2022-09-01 10:54 ` [RFC 4/6] io_uring/notif: remove notif registration Pavel Begunkov
@ 2022-09-01 10:54 ` Pavel Begunkov
  2022-09-01 10:54 ` [RFC 6/6] selftests/net: return back io_uring zc send tests Pavel Begunkov
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-09-01 10:54 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe, asml.silence

Following user feedback, this patch simplifies zerocopy send API. One of
the main complaints is that the current API is difficult with the
userspace managing notification slots, and then send retries with error
handling make it even worse.

Instead of keeping notification slots change it to the per-request
notifications model, which posts both completion and notification CQEs
for each request when any data has been sent, and only one CQE if it
fails. All notification CQEs will have IORING_CQE_F_NOTIF set and
IORING_CQE_F_MORE in completion CQEs indicates whether to wait a
notification or not.

IOSQE_CQE_SKIP_SUCCESS is disallowed with zerocopy sends for now.

This is less flexible, but greatly simplifies the user API and also the
kernel implementation. We reuse notif helpers in this patch, but in the
future there won't be need for keeping two requests.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 include/uapi/linux/io_uring.h |  7 +++--
 io_uring/io_uring.c           |  4 +--
 io_uring/net.c                | 53 ++++++++++++++++++++++-------------
 io_uring/net.h                |  1 +
 io_uring/notif.c              | 12 ++------
 io_uring/notif.h              | 43 ++--------------------------
 io_uring/opdef.c              |  3 +-
 7 files changed, 47 insertions(+), 76 deletions(-)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index b11c57b0ebb5..6b83177fd41d 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -71,8 +71,8 @@ struct io_uring_sqe {
 		__s32	splice_fd_in;
 		__u32	file_index;
 		struct {
-			__u16	notification_idx;
 			__u16	addr_len;
+			__u16	__pad3[1];
 		};
 	};
 	union {
@@ -205,7 +205,7 @@ enum io_uring_op {
 	IORING_OP_GETXATTR,
 	IORING_OP_SOCKET,
 	IORING_OP_URING_CMD,
-	IORING_OP_SENDZC_NOTIF,
+	IORING_OP_SEND_ZC,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
@@ -326,10 +326,13 @@ struct io_uring_cqe {
  * IORING_CQE_F_BUFFER	If set, the upper 16 bits are the buffer ID
  * IORING_CQE_F_MORE	If set, parent SQE will generate more CQE entries
  * IORING_CQE_F_SOCK_NONEMPTY	If set, more data to read after socket recv
+ * IORING_CQE_F_NOTIF	Set for notification CQEs. Can be used to distinct
+ * 			them from sends.
  */
 #define IORING_CQE_F_BUFFER		(1U << 0)
 #define IORING_CQE_F_MORE		(1U << 1)
 #define IORING_CQE_F_SOCK_NONEMPTY	(1U << 2)
+#define IORING_CQE_F_NOTIF		(1U << 3)
 
 enum {
 	IORING_CQE_BUFFER_SHIFT		= 16,
diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index c2e06a3aa18d..f9be9b7eb654 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -3923,8 +3923,8 @@ static int __init io_uring_init(void)
 	BUILD_BUG_SQE_ELEM(42, __u16,  personality);
 	BUILD_BUG_SQE_ELEM(44, __s32,  splice_fd_in);
 	BUILD_BUG_SQE_ELEM(44, __u32,  file_index);
-	BUILD_BUG_SQE_ELEM(44, __u16,  notification_idx);
-	BUILD_BUG_SQE_ELEM(46, __u16,  addr_len);
+	BUILD_BUG_SQE_ELEM(44, __u16,  addr_len);
+	BUILD_BUG_SQE_ELEM(46, __u16,  __pad3[0]);
 	BUILD_BUG_SQE_ELEM(48, __u64,  addr3);
 	BUILD_BUG_SQE_ELEM_SIZE(48, 0, cmd);
 	BUILD_BUG_SQE_ELEM(56, __u64,  __pad2);
diff --git a/io_uring/net.c b/io_uring/net.c
index aac6997b7d88..7047c1342541 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -65,12 +65,12 @@ struct io_sendzc {
 	struct file			*file;
 	void __user			*buf;
 	size_t				len;
-	u16				slot_idx;
 	unsigned			msg_flags;
 	unsigned			flags;
 	unsigned			addr_len;
 	void __user			*addr;
 	size_t				done_io;
+	struct io_kiocb 		*notif;
 };
 
 #define IO_APOLL_MULTI_POLLED (REQ_F_APOLL_MULTISHOT | REQ_F_POLLED)
@@ -879,12 +879,26 @@ int io_recv(struct io_kiocb *req, unsigned int issue_flags)
 	return ret;
 }
 
+void io_sendzc_cleanup(struct io_kiocb *req)
+{
+	struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc);
+
+	zc->notif->flags |= REQ_F_CQE_SKIP;
+	io_notif_flush(zc->notif);
+	zc->notif = NULL;
+}
+
 int io_sendzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc);
 	struct io_ring_ctx *ctx = req->ctx;
+	struct io_kiocb *notif;
 
-	if (READ_ONCE(sqe->__pad2[0]) || READ_ONCE(sqe->addr3))
+	if (READ_ONCE(sqe->__pad2[0]) || READ_ONCE(sqe->addr3) ||
+	    READ_ONCE(sqe->__pad3[0]))
+		return -EINVAL;
+	/* we don't support IOSQE_CQE_SKIP_SUCCESS just yet */
+	if (req->flags & REQ_F_CQE_SKIP)
 		return -EINVAL;
 
 	zc->flags = READ_ONCE(sqe->ioprio);
@@ -900,11 +914,17 @@ int io_sendzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 		req->imu = READ_ONCE(ctx->user_bufs[idx]);
 		io_req_set_rsrc_node(req, ctx, 0);
 	}
+	notif = zc->notif = io_alloc_notif(ctx);
+	if (!notif)
+		return -ENOMEM;
+	notif->cqe.user_data = req->cqe.user_data;
+	notif->cqe.res = 0;
+	notif->cqe.flags = IORING_CQE_F_NOTIF;
+	req->flags |= REQ_F_NEED_CLEANUP;
 
 	zc->buf = u64_to_user_ptr(READ_ONCE(sqe->addr));
 	zc->len = READ_ONCE(sqe->len);
 	zc->msg_flags = READ_ONCE(sqe->msg_flags) | MSG_NOSIGNAL;
-	zc->slot_idx = READ_ONCE(sqe->notification_idx);
 	if (zc->msg_flags & MSG_DONTWAIT)
 		req->flags |= REQ_F_NOWAIT;
 
@@ -976,33 +996,20 @@ static int io_sg_from_iter(struct sock *sk, struct sk_buff *skb,
 int io_sendzc(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct sockaddr_storage __address, *addr = NULL;
-	struct io_ring_ctx *ctx = req->ctx;
 	struct io_sendzc *zc = io_kiocb_to_cmd(req, struct io_sendzc);
-	struct io_notif_slot *notif_slot;
-	struct io_kiocb *notif;
 	struct msghdr msg;
 	struct iovec iov;
 	struct socket *sock;
-	unsigned msg_flags;
+	unsigned msg_flags, cflags;
 	int ret, min_ret = 0;
 
 	if (!(req->flags & REQ_F_POLLED) &&
 	    (zc->flags & IORING_RECVSEND_POLL_FIRST))
 		return -EAGAIN;
-
-	if (issue_flags & IO_URING_F_UNLOCKED)
-		return -EAGAIN;
 	sock = sock_from_file(req->file);
 	if (unlikely(!sock))
 		return -ENOTSOCK;
 
-	notif_slot = io_get_notif_slot(ctx, zc->slot_idx);
-	if (!notif_slot)
-		return -EINVAL;
-	notif = io_get_notif(ctx, notif_slot);
-	if (!notif)
-		return -ENOMEM;
-
 	msg.msg_name = NULL;
 	msg.msg_control = NULL;
 	msg.msg_controllen = 0;
@@ -1033,7 +1040,7 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags)
 					  &msg.msg_iter);
 		if (unlikely(ret))
 			return ret;
-		ret = io_notif_account_mem(notif, zc->len);
+		ret = io_notif_account_mem(zc->notif, zc->len);
 		if (unlikely(ret))
 			return ret;
 	}
@@ -1045,7 +1052,7 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags)
 		min_ret = iov_iter_count(&msg.msg_iter);
 
 	msg.msg_flags = msg_flags;
-	msg.msg_ubuf = &io_notif_to_data(notif)->uarg;
+	msg.msg_ubuf = &io_notif_to_data(zc->notif)->uarg;
 	msg.sg_from_iter = io_sg_from_iter;
 	ret = sock_sendmsg(sock, &msg);
 
@@ -1060,6 +1067,8 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags)
 			req->flags |= REQ_F_PARTIAL_IO;
 			return io_setup_async_addr(req, addr, issue_flags);
 		}
+		if (ret < 0 && !zc->done_io)
+			zc->notif->flags |= REQ_F_CQE_SKIP;
 		if (ret == -ERESTARTSYS)
 			ret = -EINTR;
 		req_set_fail(req);
@@ -1069,7 +1078,11 @@ int io_sendzc(struct io_kiocb *req, unsigned int issue_flags)
 		ret += zc->done_io;
 	else if (zc->done_io)
 		ret = zc->done_io;
-	io_req_set_res(req, ret, 0);
+
+	io_notif_flush(zc->notif);
+	req->flags &= ~REQ_F_NEED_CLEANUP;
+	cflags = ret >= 0 ? IORING_CQE_F_MORE : 0;
+	io_req_set_res(req, ret, cflags);
 	return IOU_OK;
 }
 
diff --git a/io_uring/net.h b/io_uring/net.h
index f91f56c6eeac..d744a0a874e7 100644
--- a/io_uring/net.h
+++ b/io_uring/net.h
@@ -55,6 +55,7 @@ int io_connect(struct io_kiocb *req, unsigned int issue_flags);
 
 int io_sendzc(struct io_kiocb *req, unsigned int issue_flags);
 int io_sendzc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
+void io_sendzc_cleanup(struct io_kiocb *req);
 
 void io_netmsg_cache_free(struct io_cache_entry *entry);
 #else
diff --git a/io_uring/notif.c b/io_uring/notif.c
index 11f45640684a..38d77165edc3 100644
--- a/io_uring/notif.c
+++ b/io_uring/notif.c
@@ -42,8 +42,7 @@ static void io_uring_tx_zerocopy_callback(struct sk_buff *skb,
 	}
 }
 
-struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx,
-				struct io_notif_slot *slot)
+struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx)
 	__must_hold(&ctx->uring_lock)
 {
 	struct io_kiocb *notif;
@@ -59,27 +58,20 @@ struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx,
 	io_get_task_refs(1);
 	notif->rsrc_node = NULL;
 	io_req_set_rsrc_node(notif, ctx, 0);
-	notif->cqe.user_data = slot->tag;
-	notif->cqe.flags = slot->seq++;
-	notif->cqe.res = 0;
 
 	nd = io_notif_to_data(notif);
 	nd->account_pages = 0;
 	nd->uarg.flags = SKBFL_ZEROCOPY_FRAG | SKBFL_DONT_ORPHAN;
 	nd->uarg.callback = io_uring_tx_zerocopy_callback;
-	/* master ref owned by io_notif_slot, will be dropped on flush */
 	refcount_set(&nd->uarg.refcnt, 1);
 	return notif;
 }
 
-void io_notif_slot_flush(struct io_notif_slot *slot)
+void io_notif_flush(struct io_kiocb *notif)
 	__must_hold(&slot->notif->ctx->uring_lock)
 {
-	struct io_kiocb *notif = slot->notif;
 	struct io_notif_data *nd = io_notif_to_data(notif);
 
-	slot->notif = NULL;
-
 	/* drop slot's master ref */
 	if (refcount_dec_and_test(&nd->uarg.refcnt)) {
 		notif->io_task_work.func = __io_notif_complete_tw;
diff --git a/io_uring/notif.h b/io_uring/notif.h
index 8380eeff2f2e..5b4d710c8ca5 100644
--- a/io_uring/notif.h
+++ b/io_uring/notif.h
@@ -15,53 +15,14 @@ struct io_notif_data {
 	unsigned long		account_pages;
 };
 
-struct io_notif_slot {
-	/*
-	 * Current/active notifier. A slot holds only one active notifier at a
-	 * time and keeps one reference to it. Flush releases the reference and
-	 * lazily replaces it with a new notifier.
-	 */
-	struct io_kiocb		*notif;
-
-	/*
-	 * Default ->user_data for this slot notifiers CQEs
-	 */
-	u64			tag;
-	/*
-	 * Notifiers of a slot live in generations, we create a new notifier
-	 * only after flushing the previous one. Track the sequential number
-	 * for all notifiers and copy it into notifiers's cqe->cflags
-	 */
-	u32			seq;
-};
-
-void io_notif_slot_flush(struct io_notif_slot *slot);
-struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx,
-				struct io_notif_slot *slot);
+void io_notif_flush(struct io_kiocb *notif);
+struct io_kiocb *io_alloc_notif(struct io_ring_ctx *ctx);
 
 static inline struct io_notif_data *io_notif_to_data(struct io_kiocb *notif)
 {
 	return io_kiocb_to_cmd(notif, struct io_notif_data);
 }
 
-static inline struct io_kiocb *io_get_notif(struct io_ring_ctx *ctx,
-					    struct io_notif_slot *slot)
-{
-	if (!slot->notif)
-		slot->notif = io_alloc_notif(ctx, slot);
-	return slot->notif;
-}
-
-static inline struct io_notif_slot *io_get_notif_slot(struct io_ring_ctx *ctx,
-						      unsigned idx)
-	__must_hold(&ctx->uring_lock)
-{
-	if (idx >= ctx->nr_notif_slots)
-		return NULL;
-	idx = array_index_nospec(idx, ctx->nr_notif_slots);
-	return &ctx->notif_slots[idx];
-}
-
 static inline int io_notif_account_mem(struct io_kiocb *notif, unsigned len)
 {
 	struct io_ring_ctx *ctx = notif->ctx;
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 10b301ccf5cd..c61494e0a602 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -470,7 +470,7 @@ const struct io_op_def io_op_defs[] = {
 		.issue			= io_uring_cmd,
 		.prep_async		= io_uring_cmd_prep_async,
 	},
-	[IORING_OP_SENDZC_NOTIF] = {
+	[IORING_OP_SEND_ZC] = {
 		.name			= "SENDZC_NOTIF",
 		.needs_file		= 1,
 		.unbound_nonreg_file	= 1,
@@ -483,6 +483,7 @@ const struct io_op_def io_op_defs[] = {
 		.prep			= io_sendzc_prep,
 		.issue			= io_sendzc,
 		.prep_async		= io_sendzc_prep_async,
+		.cleanup		= io_sendzc_cleanup,
 #else
 		.prep			= io_eopnotsupp_prep,
 #endif
-- 
2.37.2


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

* [RFC 6/6] selftests/net: return back io_uring zc send tests
  2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
                   ` (4 preceding siblings ...)
  2022-09-01 10:54 ` [RFC 5/6] io_uring/net: simplify zerocopy send user API Pavel Begunkov
@ 2022-09-01 10:54 ` Pavel Begunkov
  2022-09-01 15:13 ` [RFC 0/6] io_uring simplify zerocopy send API Jens Axboe
  2022-09-01 15:54 ` Jens Axboe
  7 siblings, 0 replies; 9+ messages in thread
From: Pavel Begunkov @ 2022-09-01 10:54 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe, asml.silence

Enable io_uring zerocopy send tests back and fix them up to follow the
new inteface.

Signed-off-by: Pavel Begunkov <[email protected]>
---
 .../selftests/net/io_uring_zerocopy_tx.c      | 110 ++++++------------
 .../selftests/net/io_uring_zerocopy_tx.sh     |  10 +-
 2 files changed, 41 insertions(+), 79 deletions(-)

diff --git a/tools/testing/selftests/net/io_uring_zerocopy_tx.c b/tools/testing/selftests/net/io_uring_zerocopy_tx.c
index 7446ef364e9f..8ce48aca8321 100644
--- a/tools/testing/selftests/net/io_uring_zerocopy_tx.c
+++ b/tools/testing/selftests/net/io_uring_zerocopy_tx.c
@@ -36,8 +36,6 @@
 #include <sys/un.h>
 #include <sys/wait.h>
 
-#if 0
-
 #define NOTIF_TAG 0xfffffffULL
 #define NONZC_TAG 0
 #define ZC_TAG 1
@@ -49,7 +47,6 @@ enum {
 	MODE_MIXED	= 3,
 };
 
-static bool cfg_flush		= false;
 static bool cfg_cork		= false;
 static int  cfg_mode		= MODE_ZC_FIXED;
 static int  cfg_nr_reqs		= 8;
@@ -168,21 +165,6 @@ static int io_uring_register_buffers(struct io_uring *ring,
 	return (ret < 0) ? -errno : ret;
 }
 
-static int io_uring_register_notifications(struct io_uring *ring,
-					   unsigned nr,
-					   struct io_uring_notification_slot *slots)
-{
-	int ret;
-	struct io_uring_notification_register r = {
-		.nr_slots = nr,
-		.data = (unsigned long)slots,
-	};
-
-	ret = syscall(__NR_io_uring_register, ring->ring_fd,
-		      IORING_REGISTER_NOTIFIERS, &r, sizeof(r));
-	return (ret < 0) ? -errno : ret;
-}
-
 static int io_uring_mmap(int fd, struct io_uring_params *p,
 			 struct io_uring_sq *sq, struct io_uring_cq *cq)
 {
@@ -299,11 +281,10 @@ static inline void io_uring_prep_send(struct io_uring_sqe *sqe, int sockfd,
 
 static inline void io_uring_prep_sendzc(struct io_uring_sqe *sqe, int sockfd,
 				        const void *buf, size_t len, int flags,
-				        unsigned slot_idx, unsigned zc_flags)
+				        unsigned zc_flags)
 {
 	io_uring_prep_send(sqe, sockfd, buf, len, flags);
-	sqe->opcode = (__u8) IORING_OP_SENDZC_NOTIF;
-	sqe->notification_idx = slot_idx;
+	sqe->opcode = (__u8) IORING_OP_SEND_ZC;
 	sqe->ioprio = zc_flags;
 }
 
@@ -376,7 +357,6 @@ static int do_setup_tx(int domain, int type, int protocol)
 
 static void do_tx(int domain, int type, int protocol)
 {
-	struct io_uring_notification_slot b[1] = {{.tag = NOTIF_TAG}};
 	struct io_uring_sqe *sqe;
 	struct io_uring_cqe *cqe;
 	unsigned long packets = 0, bytes = 0;
@@ -392,10 +372,6 @@ static void do_tx(int domain, int type, int protocol)
 	if (ret)
 		error(1, ret, "io_uring: queue init");
 
-	ret = io_uring_register_notifications(&ring, 1, b);
-	if (ret)
-		error(1, ret, "io_uring: tx ctx registration");
-
 	iov.iov_base = payload;
 	iov.iov_len = cfg_payload_len;
 
@@ -411,9 +387,8 @@ static void do_tx(int domain, int type, int protocol)
 		for (i = 0; i < cfg_nr_reqs; i++) {
 			unsigned zc_flags = 0;
 			unsigned buf_idx = 0;
-			unsigned slot_idx = 0;
 			unsigned mode = cfg_mode;
-			unsigned msg_flags = 0;
+			unsigned msg_flags = MSG_WAITALL;
 
 			if (cfg_mode == MODE_MIXED)
 				mode = rand() % 3;
@@ -425,13 +400,10 @@ static void do_tx(int domain, int type, int protocol)
 						   cfg_payload_len, msg_flags);
 				sqe->user_data = NONZC_TAG;
 			} else {
-				if (cfg_flush) {
-					zc_flags |= IORING_RECVSEND_NOTIF_FLUSH;
-					compl_cqes++;
-				}
+				compl_cqes++;
 				io_uring_prep_sendzc(sqe, fd, payload,
 						     cfg_payload_len,
-						     msg_flags, slot_idx, zc_flags);
+						     msg_flags, zc_flags);
 				if (mode == MODE_ZC_FIXED) {
 					sqe->ioprio |= IORING_RECVSEND_FIXED_BUF;
 					sqe->buf_index = buf_idx;
@@ -444,51 +416,57 @@ static void do_tx(int domain, int type, int protocol)
 		if (ret != cfg_nr_reqs)
 			error(1, ret, "submit");
 
+		if (cfg_cork)
+			do_setsockopt(fd, IPPROTO_UDP, UDP_CORK, 0);
 		for (i = 0; i < cfg_nr_reqs; i++) {
 			ret = io_uring_wait_cqe(&ring, &cqe);
 			if (ret)
 				error(1, ret, "wait cqe");
 
-			if (cqe->user_data == NOTIF_TAG) {
+			if (cqe->user_data != NONZC_TAG &&
+			    cqe->user_data != ZC_TAG)
+				error(1, -EINVAL, "invalid cqe->user_data");
+
+			if (cqe->flags & IORING_CQE_F_NOTIF) {
+				if (cqe->flags & IORING_CQE_F_MORE)
+					error(1, -EINVAL, "invalid notif flags");
 				compl_cqes--;
 				i--;
-			} else if (cqe->user_data != NONZC_TAG &&
-				   cqe->user_data != ZC_TAG) {
-				error(1, cqe->res, "invalid user_data");
-			} else if (cqe->res <= 0 && cqe->res != -EAGAIN) {
+			} else if (cqe->res <= 0) {
+				if (cqe->flags & IORING_CQE_F_MORE)
+					error(1, cqe->res, "more with a failed send");
 				error(1, cqe->res, "send failed");
 			} else {
-				if (cqe->res > 0) {
-					packets++;
-					bytes += cqe->res;
-				}
-				/* failed requests don't flush */
-				if (cfg_flush &&
-				    cqe->res <= 0 &&
-				    cqe->user_data == ZC_TAG)
-					compl_cqes--;
+				if (cqe->user_data == ZC_TAG &&
+				    !(cqe->flags & IORING_CQE_F_MORE))
+					error(1, cqe->res, "missing more flag");
+				packets++;
+				bytes += cqe->res;
 			}
 			io_uring_cqe_seen(&ring);
 		}
-		if (cfg_cork)
-			do_setsockopt(fd, IPPROTO_UDP, UDP_CORK, 0);
 	} while (gettimeofday_ms() < tstop);
 
-	if (close(fd))
-		error(1, errno, "close");
-
-	fprintf(stderr, "tx=%lu (MB=%lu), tx/s=%lu (MB/s=%lu)\n",
-			packets, bytes >> 20,
-			packets / (cfg_runtime_ms / 1000),
-			(bytes >> 20) / (cfg_runtime_ms / 1000));
-
 	while (compl_cqes) {
 		ret = io_uring_wait_cqe(&ring, &cqe);
 		if (ret)
 			error(1, ret, "wait cqe");
+		if (cqe->flags & IORING_CQE_F_MORE)
+			error(1, -EINVAL, "invalid notif flags");
+		if (!(cqe->flags & IORING_CQE_F_NOTIF))
+			error(1, -EINVAL, "missing notif flag");
+
 		io_uring_cqe_seen(&ring);
 		compl_cqes--;
 	}
+
+	fprintf(stderr, "tx=%lu (MB=%lu), tx/s=%lu (MB/s=%lu)\n",
+			packets, bytes >> 20,
+			packets / (cfg_runtime_ms / 1000),
+			(bytes >> 20) / (cfg_runtime_ms / 1000));
+
+	if (close(fd))
+		error(1, errno, "close");
 }
 
 static void do_test(int domain, int type, int protocol)
@@ -502,8 +480,8 @@ static void do_test(int domain, int type, int protocol)
 
 static void usage(const char *filepath)
 {
-	error(1, 0, "Usage: %s [-f] [-n<N>] [-z0] [-s<payload size>] "
-		    "(-4|-6) [-t<time s>] -D<dst_ip> udp", filepath);
+	error(1, 0, "Usage: %s (-4|-6) (udp|tcp) -D<dst_ip> [-s<payload size>] "
+		    "[-t<time s>] [-n<batch>] [-p<port>] [-m<mode>]", filepath);
 }
 
 static void parse_opts(int argc, char **argv)
@@ -521,7 +499,7 @@ static void parse_opts(int argc, char **argv)
 		usage(argv[0]);
 	cfg_payload_len = max_payload_len;
 
-	while ((c = getopt(argc, argv, "46D:p:s:t:n:fc:m:")) != -1) {
+	while ((c = getopt(argc, argv, "46D:p:s:t:n:c:m:")) != -1) {
 		switch (c) {
 		case '4':
 			if (cfg_family != PF_UNSPEC)
@@ -550,9 +528,6 @@ static void parse_opts(int argc, char **argv)
 		case 'n':
 			cfg_nr_reqs = strtoul(optarg, NULL, 0);
 			break;
-		case 'f':
-			cfg_flush = 1;
-			break;
 		case 'c':
 			cfg_cork = strtol(optarg, NULL, 0);
 			break;
@@ -585,8 +560,6 @@ static void parse_opts(int argc, char **argv)
 
 	if (cfg_payload_len > max_payload_len)
 		error(1, 0, "-s: payload exceeds max (%d)", max_payload_len);
-	if (cfg_mode == MODE_NONZC && cfg_flush)
-		error(1, 0, "-f: only zerocopy modes support notifications");
 	if (optind != argc - 1)
 		usage(argv[0]);
 }
@@ -605,10 +578,3 @@ int main(int argc, char **argv)
 		error(1, 0, "unknown cfg_test %s", cfg_test);
 	return 0;
 }
-
-#else
-int main(int argc, char **argv)
-{
-	return 0;
-}
-#endif
diff --git a/tools/testing/selftests/net/io_uring_zerocopy_tx.sh b/tools/testing/selftests/net/io_uring_zerocopy_tx.sh
index 6a65e4437640..32aa6e9dacc2 100755
--- a/tools/testing/selftests/net/io_uring_zerocopy_tx.sh
+++ b/tools/testing/selftests/net/io_uring_zerocopy_tx.sh
@@ -25,15 +25,11 @@ readonly path_sysctl_mem="net.core.optmem_max"
 # No arguments: automated test
 if [[ "$#" -eq "0" ]]; then
 	IPs=( "4" "6" )
-	protocols=( "tcp" "udp" )
 
 	for IP in "${IPs[@]}"; do
-		for proto in "${protocols[@]}"; do
-			for mode in $(seq 1 3); do
-				$0 "$IP" "$proto" -m "$mode" -t 1 -n 32
-				$0 "$IP" "$proto" -m "$mode" -t 1 -n 32 -f
-				$0 "$IP" "$proto" -m "$mode" -t 1 -n 32 -c -f
-			done
+		for mode in $(seq 1 3); do
+			$0 "$IP" udp -m "$mode" -t 1 -n 32
+			$0 "$IP" tcp -m "$mode" -t 1 -n 32
 		done
 	done
 
-- 
2.37.2


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

* Re: [RFC 0/6] io_uring simplify zerocopy send API
  2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
                   ` (5 preceding siblings ...)
  2022-09-01 10:54 ` [RFC 6/6] selftests/net: return back io_uring zc send tests Pavel Begunkov
@ 2022-09-01 15:13 ` Jens Axboe
  2022-09-01 15:54 ` Jens Axboe
  7 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2022-09-01 15:13 UTC (permalink / raw)
  To: Pavel Begunkov, io-uring

On 9/1/22 4:53 AM, Pavel Begunkov wrote:
> We're changing zerocopy send API making it a bit less flexible but
> much simpler based on the feedback we've got from people trying it
> out. We replace slots and flushing with a per request notifications.
> The API change is described in 5/6 in more details.
> more in 5/6.
> 
> The only real functional change is in 5/6, 2-4 are reverts, and patches
> 1 and 6 are fixing selftests.

Let's go for this, we can always bring back notification slots in a
later release if it's deemed necessary or beneficial, however we
can't take it out if we release 6.0 with the change...

-- 
Jens Axboe



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

* Re: [RFC 0/6] io_uring simplify zerocopy send API
  2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
                   ` (6 preceding siblings ...)
  2022-09-01 15:13 ` [RFC 0/6] io_uring simplify zerocopy send API Jens Axboe
@ 2022-09-01 15:54 ` Jens Axboe
  7 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2022-09-01 15:54 UTC (permalink / raw)
  To: io-uring, Pavel Begunkov

On Thu, 1 Sep 2022 11:53:59 +0100, Pavel Begunkov wrote:
> We're changing zerocopy send API making it a bit less flexible but
> much simpler based on the feedback we've got from people trying it
> out. We replace slots and flushing with a per request notifications.
> The API change is described in 5/6 in more details.
> more in 5/6.
> 
> The only real functional change is in 5/6, 2-4 are reverts, and patches
> 1 and 6 are fixing selftests.
> 
> [...]

Applied, thanks!

[1/6] selftests/net: temporarily disable io_uring zc test
      (no commit info)
[2/6] Revert "io_uring: add zc notification flush requests"
      (no commit info)
[3/6] Revert "io_uring: rename IORING_OP_FILES_UPDATE"
      (no commit info)
[4/6] io_uring/notif: remove notif registration
      (no commit info)
[5/6] io_uring/net: simplify zerocopy send user API
      (no commit info)
[6/6] selftests/net: return back io_uring zc send tests
      (no commit info)

Best regards,
-- 
Jens Axboe



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

end of thread, other threads:[~2022-09-01 15:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-01 10:53 [RFC 0/6] io_uring simplify zerocopy send API Pavel Begunkov
2022-09-01 10:54 ` [RFC 1/6] selftests/net: temporarily disable io_uring zc test Pavel Begunkov
2022-09-01 10:54 ` [RFC 2/6] Revert "io_uring: add zc notification flush requests" Pavel Begunkov
2022-09-01 10:54 ` [RFC 3/6] Revert "io_uring: rename IORING_OP_FILES_UPDATE" Pavel Begunkov
2022-09-01 10:54 ` [RFC 4/6] io_uring/notif: remove notif registration Pavel Begunkov
2022-09-01 10:54 ` [RFC 5/6] io_uring/net: simplify zerocopy send user API Pavel Begunkov
2022-09-01 10:54 ` [RFC 6/6] selftests/net: return back io_uring zc send tests Pavel Begunkov
2022-09-01 15:13 ` [RFC 0/6] io_uring simplify zerocopy send API Jens Axboe
2022-09-01 15:54 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox