[PATCH liburing 0/4] zc changes

[PATCH liburing 0/4] zc changes

[Pavel Begunkov]

We're decoupling notifications generation from whether the request
failed or not. Adjust the tests, so nobody is confused reading it,
and put a note in man.

Also, 4/4 adds a test for just sent zc sendmsg.

Pavel Begunkov (4):
  examples: fix sendzc notif handling
  test: fix zc tests
  man: note about notification generation
  tests: add sendmsg_zc tests

 examples/send-zerocopy.c        |  17 +++---
 man/io_uring_enter.2            |   5 +-
 src/include/liburing/io_uring.h |   1 +
 test/send-zerocopy.c            | 101 ++++++++++++++++++++++++--------
 4 files changed, 88 insertions(+), 36 deletions(-)

-- 
2.37.2

[PATCH liburing 1/4] examples: fix sendzc notif handling

[Pavel Begunkov]

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 examples/send-zerocopy.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/examples/send-zerocopy.c b/examples/send-zerocopy.c
index 4ed0f67..7f5f2b1 100644
--- a/examples/send-zerocopy.c
+++ b/examples/send-zerocopy.c
@@ -190,8 +190,6 @@ static void do_tx(int domain, int type, int protocol)
 				sqe->flags |= IOSQE_FIXED_FILE;
 			}
 		}
-		if (cfg_zc)
-			compl_cqes += cfg_nr_reqs;
 
 		ret = io_uring_submit(&ring);
 		if (ret != cfg_nr_reqs)
@@ -205,19 +203,20 @@ static void do_tx(int domain, int type, int protocol)
 					error(1, -EINVAL, "F_MORE notif");
 				compl_cqes--;
 				i--;
-			} else if (cqe->res >= 0) {
-				if (!(cqe->flags & IORING_CQE_F_MORE) && cfg_zc)
-					error(1, -EINVAL, "no F_MORE");
+				io_uring_cqe_seen(&ring, cqe);
+				continue;
+			}
+			if (cqe->flags & IORING_CQE_F_MORE)
+				compl_cqes++;
+
+			if (cqe->res >= 0) {
 				packets++;
 				bytes += cqe->res;
-			} else if (cqe->res == -EAGAIN) {
-				if (cfg_zc)
-					compl_cqes--;
 			} else if (cqe->res == -ECONNREFUSED || cqe->res == -EPIPE ||
 				   cqe->res == -ECONNRESET) {
 				fprintf(stderr, "Connection failure");
 				goto out_fail;
-			} else {
+			} else if (cqe->res != -EAGAIN) {
 				error(1, cqe->res, "send failed");
 			}
 			io_uring_cqe_seen(&ring, cqe);
-- 
2.37.2

[PATCH liburing 2/4] test: fix zc tests

[Pavel Begunkov]

Decouple result from F_NOTIF/F_MORE, even failed requests may produce a
notification.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 test/send-zerocopy.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/test/send-zerocopy.c b/test/send-zerocopy.c
index 39c5c5d..e34e0c1 100644
--- a/test/send-zerocopy.c
+++ b/test/send-zerocopy.c
@@ -117,7 +117,7 @@ static int test_send_faults(struct io_uring *ring, int sock_tx, int sock_rx)
 	int msg_flags = 0;
 	unsigned zc_flags = 0;
 	int payload_size = 100;
-	int ret, i;
+	int ret, i, nr_cqes = 2;
 
 	sqe = io_uring_get_sqe(ring);
 	io_uring_prep_send_zc(sqe, sock_tx, (void *)1UL, payload_size,
@@ -125,7 +125,7 @@ static int test_send_faults(struct io_uring *ring, int sock_tx, int sock_rx)
 	sqe->user_data = 1;
 
 	sqe = io_uring_get_sqe(ring);
-	io_uring_prep_send_zc(sqe, sock_tx, (void *)1UL, payload_size,
+	io_uring_prep_send_zc(sqe, sock_tx, tx_buffer, payload_size,
 			      msg_flags, zc_flags);
 	sqe->user_data = 2;
 	io_uring_prep_send_set_addr(sqe, (const struct sockaddr *)1UL,
@@ -134,12 +134,18 @@ static int test_send_faults(struct io_uring *ring, int sock_tx, int sock_rx)
 	ret = io_uring_submit(ring);
 	assert(ret == 2);
 
-	for (i = 0; i < 2; i++) {
+	for (i = 0; i < nr_cqes; i++) {
 		ret = io_uring_wait_cqe(ring, &cqe);
 		assert(!ret);
 		assert(cqe->user_data <= 2);
-		assert(cqe->res == -EFAULT);
-		assert(!(cqe->flags & IORING_CQE_F_MORE));
+
+		if (cqe->flags & IORING_CQE_F_NOTIF) {
+			assert(ret > 0);
+		} else {
+			assert(cqe->res == -EFAULT);
+			if (cqe->flags & IORING_CQE_F_MORE)
+				nr_cqes++;
+		}
 		io_uring_cqe_seen(ring, cqe);
 	}
 	assert(check_cq_empty(ring));
-- 
2.37.2

[PATCH liburing 3/4] man: note about notification generation

[Pavel Begunkov]

Add a small note to clarify expectations on when to expect a
notification.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 man/io_uring_enter.2 | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/man/io_uring_enter.2 b/man/io_uring_enter.2
index d87f254..e0ce051 100644
--- a/man/io_uring_enter.2
+++ b/man/io_uring_enter.2
@@ -1091,7 +1091,10 @@ long time, e.g. waiting for a TCP ACK, and having a separate cqe for request
 completions allows userspace to push more data without extra delays. Note,
 notifications are only responsible for controlling the lifetime of the buffers,
 and as such don't mean anything about whether the data has atually been sent
-out or received by the other end.
+out or received by the other end. Even errored requests may generate a
+notification, and the user must check for
+.B IORING_CQE_F_MORE
+rather than relying on the result.
 
 .I fd
 must be set to the socket file descriptor,
-- 
2.37.2

[PATCH liburing 4/4] tests: add sendmsg_zc tests

[Pavel Begunkov]

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 src/include/liburing/io_uring.h |  1 +
 test/send-zerocopy.c            | 85 +++++++++++++++++++++++++--------
 2 files changed, 65 insertions(+), 21 deletions(-)

diff --git a/src/include/liburing/io_uring.h b/src/include/liburing/io_uring.h
index 972b179..92f29d9 100644
--- a/src/include/liburing/io_uring.h
+++ b/src/include/liburing/io_uring.h
@@ -213,6 +213,7 @@ enum io_uring_op {
 	IORING_OP_SOCKET,
 	IORING_OP_URING_CMD,
 	IORING_OP_SEND_ZC,
+	IORING_OP_SENDMSG_ZC,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
diff --git a/test/send-zerocopy.c b/test/send-zerocopy.c
index e34e0c1..80723de 100644
--- a/test/send-zerocopy.c
+++ b/test/send-zerocopy.c
@@ -44,6 +44,7 @@
 #define HOST	"127.0.0.1"
 #define HOSTV6	"::1"
 
+#define CORK_REQS 5
 #define RX_TAG 10000
 #define BUFFER_OFFSET 41
 
@@ -60,6 +61,7 @@ enum {
 
 static char *tx_buffer, *rx_buffer;
 static struct iovec buffers_iov[4];
+static bool has_sendmsg;
 
 static bool check_cq_empty(struct io_uring *ring)
 {
@@ -252,18 +254,27 @@ static int prepare_ip(struct sockaddr_storage *addr, int *sock_client, int *sock
 static int do_test_inet_send(struct io_uring *ring, int sock_client, int sock_server,
 			     bool fixed_buf, struct sockaddr_storage *addr,
 			     bool cork, bool mix_register,
-			     int buf_idx, bool force_async)
+			     int buf_idx, bool force_async, bool use_sendmsg)
 {
+	struct iovec iov[CORK_REQS];
+	struct msghdr msghdr[CORK_REQS];
 	const unsigned zc_flags = 0;
 	struct io_uring_sqe *sqe;
 	struct io_uring_cqe *cqe;
-	int nr_reqs = cork ? 5 : 1;
-	int i, ret, nr_cqes;
+	int nr_reqs = cork ? CORK_REQS : 1;
+	int i, ret, nr_cqes, addr_len = 0;
 	size_t send_size = buffers_iov[buf_idx].iov_len;
 	size_t chunk_size = send_size / nr_reqs;
 	size_t chunk_size_last = send_size - chunk_size * (nr_reqs - 1);
 	char *buf = buffers_iov[buf_idx].iov_base;
 
+	if (addr) {
+		sa_family_t fam = ((struct sockaddr_in *)addr)->sin_family;
+
+		addr_len = (fam == AF_INET) ? sizeof(struct sockaddr_in) :
+					      sizeof(struct sockaddr_in6);
+	}
+
 	memset(rx_buffer, 0, send_size);
 
 	for (i = 0; i < nr_reqs; i++) {
@@ -280,25 +291,35 @@ static int do_test_inet_send(struct io_uring *ring, int sock_client, int sock_se
 			cur_size = chunk_size_last;
 
 		sqe = io_uring_get_sqe(ring);
-		io_uring_prep_send_zc(sqe, sock_client, buf + i * chunk_size,
-				      cur_size, msg_flags, zc_flags);
-		sqe->user_data = i;
-
-		if (real_fixed_buf) {
-			sqe->ioprio |= IORING_RECVSEND_FIXED_BUF;
-			sqe->buf_index = buf_idx;
-		}
-		if (addr) {
-			sa_family_t fam = ((struct sockaddr_in *)addr)->sin_family;
-			int addr_len = fam == AF_INET ? sizeof(struct sockaddr_in) :
-							sizeof(struct sockaddr_in6);
 
-			io_uring_prep_send_set_addr(sqe, (const struct sockaddr *)addr,
-						    addr_len);
+		if (!use_sendmsg) {
+			io_uring_prep_send_zc(sqe, sock_client, buf + i * chunk_size,
+					      cur_size, msg_flags, zc_flags);
+			if (real_fixed_buf) {
+				sqe->ioprio |= IORING_RECVSEND_FIXED_BUF;
+				sqe->buf_index = buf_idx;
+			}
+			if (addr)
+				io_uring_prep_send_set_addr(sqe, (const struct sockaddr *)addr,
+							    addr_len);
+		} else {
+			io_uring_prep_sendmsg(sqe, sock_client, &msghdr[i], msg_flags);
+			sqe->opcode = IORING_OP_SENDMSG_ZC;
+
+			memset(&msghdr[i], 0, sizeof(msghdr[i]));
+			iov[i].iov_len = cur_size;
+			iov[i].iov_base = buf + i * chunk_size;
+			msghdr[i].msg_iov = &iov[i];
+			msghdr[i].msg_iovlen = 1;
+			if (addr) {
+				msghdr[i].msg_name = addr;
+				msghdr[i].msg_namelen = addr_len;
+			}
 		}
+		sqe->user_data = i;
 		if (force_async)
 			sqe->flags |= IOSQE_ASYNC;
-		if (cork && i != nr_reqs - 1)
+		if (i != nr_reqs - 1)
 			sqe->flags |= IOSQE_IO_LINK;
 	}
 
@@ -346,7 +367,8 @@ static int do_test_inet_send(struct io_uring *ring, int sock_client, int sock_se
 		}
 		if ((cqe->flags & IORING_CQE_F_MORE) ==
 		    (cqe->flags & IORING_CQE_F_NOTIF)) {
-			fprintf(stderr, "unexpected cflags %i\n", cqe->flags);
+			fprintf(stderr, "unexpected cflags %i res %i\n",
+					cqe->flags, cqe->res);
 			return 1;
 		}
 		io_uring_cqe_seen(ring, cqe);
@@ -384,13 +406,14 @@ static int test_inet_send(struct io_uring *ring)
 			return 1;
 		}
 
-		for (i = 0; i < 128; i++) {
+		for (i = 0; i < 256; i++) {
 			int buf_flavour = i & 3;
 			bool fixed_buf = i & 4;
 			struct sockaddr_storage *addr_arg = (i & 8) ? &addr : NULL;
 			bool cork = i & 16;
 			bool mix_register = i & 32;
 			bool force_async = i & 64;
+			bool use_sendmsg = i & 128;
 
 			if (buf_flavour == BUF_T_LARGE && !tcp)
 				continue;
@@ -402,10 +425,12 @@ static int test_inet_send(struct io_uring *ring)
 				continue;
 			if (!client_connect && addr_arg == NULL)
 				continue;
+			if (use_sendmsg && (mix_register || fixed_buf || !has_sendmsg))
+				continue;
 
 			ret = do_test_inet_send(ring, sock_client, sock_server, fixed_buf,
 						addr_arg, cork, mix_register,
-						buf_flavour, force_async);
+						buf_flavour, force_async, use_sendmsg);
 			if (ret) {
 				fprintf(stderr, "send failed fixed buf %i, conn %i, addr %i, "
 					"cork %i\n",
@@ -492,6 +517,22 @@ static int test_async_addr(struct io_uring *ring)
 	return 0;
 }
 
+static bool io_check_zc_sendmsg(struct io_uring *ring)
+{
+	struct io_uring_probe *p;
+	int ret;
+
+	p = t_calloc(1, sizeof(*p) + 256 * sizeof(struct io_uring_probe_op));
+	if (!p) {
+		fprintf(stderr, "probe allocation failed\n");
+		return false;
+	}
+	ret = io_uring_register_probe(ring, p, 256);
+	if (ret)
+		return false;
+	return p->ops_len > IORING_OP_SENDMSG_ZC;
+}
+
 int main(int argc, char *argv[])
 {
 	struct io_uring ring;
@@ -550,6 +591,8 @@ int main(int argc, char *argv[])
 		return T_EXIT_FAIL;
 	}
 
+	has_sendmsg = io_check_zc_sendmsg(&ring);
+
 	ret = test_send_faults(&ring, sp[0], sp[1]);
 	if (ret) {
 		fprintf(stderr, "test_send_faults() failed\n");
-- 
2.37.2

Re: [PATCH liburing 0/4] zc changes

[Jens Axboe]

On Wed, 21 Sep 2022 12:21:54 +0100, Pavel Begunkov wrote:
> We're decoupling notifications generation from whether the request
> failed or not. Adjust the tests, so nobody is confused reading it,
> and put a note in man.
> 
> Also, 4/4 adds a test for just sent zc sendmsg.
> 
> Pavel Begunkov (4):
>   examples: fix sendzc notif handling
>   test: fix zc tests
>   man: note about notification generation
>   tests: add sendmsg_zc tests
> 
> [...]

Applied, thanks!

[1/4] examples: fix sendzc notif handling
      commit: a3a35cf05c6ed670f0f14c3181c10683d22d98da
[2/4] test: fix zc tests
      commit: 5127b05e5acf530020518d198401f09c32e09f9c
[3/4] man: note about notification generation
      commit: ccde465295cd07d0a4b25ae892de957d50424b50
[4/4] tests: add sendmsg_zc tests
      commit: dec0b1db70e8fadf9666289fa00c1ef508a1e8a2

Best regards,
-- 
Jens Axboe