[RESEND PATCH liburing v1 00/12] Introducing t_bind_ephemeral_port() function

[RESEND PATCH liburing v1 00/12] Introducing t_bind_ephemeral_port() function

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

[ RESEND to the correct address now. ]

Hi,

After discussing an intermittent bind() issue with Dylan, I decided to
introduce a new helper function, t_bind_ephemeral_port().

## Problem:
We have many places where we need to bind() a socket to any unused port
number. To achieve that, the current approach does one of the following
mechanisms:

  1) Randomly brute force the port number until the bind() syscall
     succeeds.

  2) Use a static port at compile time (randomly chosen too).

This is not reliable and it results in an intermittent issue (test
fails when the selected port is in use).

## Solution:
Setting @addr->sin_port to zero on a bind() syscall lets the kernel
choose a port number that is not in use. The caller then can know the
port number to be bound by invoking a getsockname() syscall after
bind() succeeds.

Wrap this procedure in a new function called t_bind_ephemeral_port().
The selected port will be returned into @addr->sin_port, the caller
can use it later to connect() or whatever they need.

## Patchset summary:
There are 12 patches in this series, summary:
1) Patch #1 introduces a new helper function t_bind_ephemeral_port().
2) Patch #2 to #6 get rid of the port number brute force mechanism.
3) Patch #7 to #12 stop using a static port number.

Link: https://lore.kernel.org/r/918facd1-78ba-2de7-693a-5f8c65ea2fcd@gnuweeb.org
Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Ammar Faizi (12):
  test/helpers: Add `t_bind_ephemeral_port()` function
  t/poll-link: Don't brute force the port number
  t/socket-rw: Don't brute force the port number
  t/socket-rw-eagain: Don't brute force the port number
  t/socket-rw-offset: Don't brute force the port number
  t/files-exit-hang-poll: Don't brute force the port number
  t/socket: Don't use a static port number
  t/connect: Don't use a static port number
  t/shutdown: Don't use a static port number
  t/recv-msgall: Don't use a static port number
  t/232c93d07b74: Don't use a static port number
  t/recv-msgall-stream: Don't use a static port number

 test/232c93d07b74.c         | 10 ++++------
 test/accept.c               |  5 +----
 test/files-exit-hang-poll.c | 23 +++--------------------
 test/helpers.c              | 18 ++++++++++++++++++
 test/helpers.h              |  7 +++++++
 test/poll-link.c            | 20 ++++++--------------
 test/recv-msgall-stream.c   | 22 ++++++++++------------
 test/recv-msgall.c          | 10 ++++------
 test/shutdown.c             |  7 +++----
 test/socket-rw-eagain.c     | 14 ++------------
 test/socket-rw-offset.c     | 13 ++-----------
 test/socket-rw.c            | 13 ++-----------
 test/socket.c               | 11 ++++++-----
 13 files changed, 68 insertions(+), 105 deletions(-)


base-commit: b8c37f02662faa4f2b61840b123201ccc5678fb1
-- 
Ammar Faizi

[RESEND PATCH liburing v1 01/12] test/helpers: Add `t_bind_ephemeral_port()` function

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

This is a prep patch to fix an intermittent issue with the port number.

We have many places where we need to bind() a socket to any unused port
number. To achieve that, the current approach does one of the following
mechanisms:

  1) Randomly brute force the port number until the bind() syscall
     succeeds.

  2) Use a static port at compile time (randomly chosen too).

This is not reliable and it results in an intermittent issue (test
fails when the selected port is in use).

Setting @addr->sin_port to zero on a bind() syscall lets the kernel
choose a port number that is not in use. The caller then can know the
port number to be bound by invoking a getsockname() syscall after
bind() succeeds.

Wrap this procedure in a new function called t_bind_ephemeral_port().
The selected port will be returned into @addr->sin_port, the caller
can use it later to connect() or whatever they need.

Link: https://lore.kernel.org/r/918facd1-78ba-2de7-693a-5f8c65ea2fcd@gnuweeb.org
Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/helpers.c | 18 ++++++++++++++++++
 test/helpers.h |  7 +++++++
 2 files changed, 25 insertions(+)

diff --git a/test/helpers.c b/test/helpers.c
index 0146533..4d5c402 100644
--- a/test/helpers.c
+++ b/test/helpers.c
@@ -19,24 +19,42 @@
 
 /*
  * Helper for allocating memory in tests.
  */
 void *t_malloc(size_t size)
 {
 	void *ret;
 	ret = malloc(size);
 	assert(ret);
 	return ret;
 }
 
+/*
+ * Helper for binding socket to an ephemeral port.
+ * The port number to be bound is returned in @addr->sin_port.
+ */
+int t_bind_ephemeral_port(int fd, struct sockaddr_in *addr)
+{
+	socklen_t addrlen;
+
+	addr->sin_port = 0;
+	if (bind(fd, (struct sockaddr *)addr, sizeof(*addr)))
+		return -errno;
+
+	addrlen = sizeof(*addr);
+	assert(!getsockname(fd, (struct sockaddr *)addr, &addrlen));
+	assert(addr->sin_port != 0);
+	return 0;
+}
+
 /*
  * Helper for allocating size bytes aligned on a boundary.
  */
 void t_posix_memalign(void **memptr, size_t alignment, size_t size)
 {
 	int ret;
 	ret = posix_memalign(memptr, alignment, size);
 	assert(!ret);
 }
 
 /*
  * Helper for allocating space for an array of nmemb elements
diff --git a/test/helpers.h b/test/helpers.h
index 6d5726c..9ad9947 100644
--- a/test/helpers.h
+++ b/test/helpers.h
@@ -13,24 +13,31 @@ extern "C" {
 
 enum t_setup_ret {
 	T_SETUP_OK	= 0,
 	T_SETUP_SKIP,
 };
 
 enum t_test_result {
 	T_EXIT_PASS   = 0,
 	T_EXIT_FAIL   = 1,
 	T_EXIT_SKIP   = 77,
 };
 
+/*
+ * Helper for binding socket to an ephemeral port.
+ * The port number to be bound is returned in @addr->sin_port.
+ */
+int t_bind_ephemeral_port(int fd, struct sockaddr_in *addr);
+
+
 /*
  * Helper for allocating memory in tests.
  */
 void *t_malloc(size_t size);
 
 
 /*
  * Helper for allocating size bytes aligned on a boundary.
  */
 void t_posix_memalign(void **memptr, size_t alignment, size_t size);
 
 
-- 
Ammar Faizi

[RESEND PATCH liburing v1 02/12] t/poll-link: Don't brute force the port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't brute force the port number, use `t_bind_ephemeral_port()`,
much simpler and reliable for choosing a port number that is not
in use.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/poll-link.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/test/poll-link.c b/test/poll-link.c
index 197ad77..a6fe0de 100644
--- a/test/poll-link.c
+++ b/test/poll-link.c
@@ -4,24 +4,25 @@
 #include <unistd.h>
 #include <stdlib.h>
 #include <string.h>
 #include <fcntl.h>
 #include <assert.h>
 #include <pthread.h>
 #include <sys/socket.h>
 #include <netinet/tcp.h>
 #include <netinet/in.h>
 #include <poll.h>
 #include <arpa/inet.h>
 
+#include "helpers.h"
 #include "liburing.h"
 
 pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
 
 static int recv_thread_ready = 0;
 static int recv_thread_done = 0;
 
 static void signal_var(int *var)
 {
         pthread_mutex_lock(&mutex);
         *var = 1;
@@ -80,45 +81,37 @@ void *recv_thread(void *arg)
 	ret = io_uring_queue_init(8, &ring, 0);
 	assert(ret == 0);
 
 	int s0 = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
 	assert(s0 != -1);
 
 	int32_t val = 1;
 	ret = setsockopt(s0, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
 	assert(ret != -1);
 	ret = setsockopt(s0, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 	assert(ret != -1);
 
-	struct sockaddr_in addr;
+	struct sockaddr_in addr = { };
 
 	addr.sin_family = AF_INET;
 	data->addr = inet_addr("127.0.0.1");
 	addr.sin_addr.s_addr = data->addr;
 
-	i = 0;
-	do {
-		data->port = htons(1025 + (rand() % 64510));
-		addr.sin_port = data->port;
-
-		if (bind(s0, (struct sockaddr*)&addr, sizeof(addr)) != -1)
-			break;
-	} while (++i < 100);
-
-	if (i >= 100) {
-		fprintf(stderr, "Can't find good port, skipped\n");
+	if (t_bind_ephemeral_port(s0, &addr)) {
+		perror("bind");
 		data->stop = 1;
 		signal_var(&recv_thread_ready);
-		goto out;
+		goto err;
 	}
+	data->port = addr.sin_port;
 
 	ret = listen(s0, 128);
 	assert(ret != -1);
 
 	signal_var(&recv_thread_ready);
 
 	sqe = io_uring_get_sqe(&ring);
 	assert(sqe != NULL);
 
 	io_uring_prep_poll_add(sqe, s0, POLLIN | POLLHUP | POLLERR);
 	sqe->flags |= IOSQE_IO_LINK;
 	sqe->user_data = 1;
@@ -149,25 +142,24 @@ void *recv_thread(void *arg)
 					(uint64_t) cqe->user_data, cqe->res,
 					data->expected[idx]);
 			goto err;
 		} else if (!data->is_mask[idx] && cqe->res != data->expected[idx]) {
 			fprintf(stderr, "cqe %" PRIu64 " got %d, wanted %d\n",
 					(uint64_t) cqe->user_data, cqe->res,
 					data->expected[idx]);
 			goto err;
 		}
 		io_uring_cqe_seen(&ring, cqe);
 	}
 
-out:
 	signal_var(&recv_thread_done);
 	close(s0);
 	io_uring_queue_exit(&ring);
 	return NULL;
 err:
 	signal_var(&recv_thread_done);
 	close(s0);
 	io_uring_queue_exit(&ring);
 	return (void *) 1;
 }
 
 static int test_poll_timeout(int do_connect, unsigned long timeout)
-- 
Ammar Faizi

[RESEND PATCH liburing v1 03/12] t/socket-rw: Don't brute force the port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't brute force the port number, use `t_bind_ephemeral_port()`,
much simpler and reliable for choosing a port number that is not
in use.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/socket-rw.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/test/socket-rw.c b/test/socket-rw.c
index 4fbf032..6211b01 100644
--- a/test/socket-rw.c
+++ b/test/socket-rw.c
@@ -11,58 +11,49 @@
 #include <assert.h>
 
 #include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <netinet/tcp.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
 #include "liburing.h"
+#include "helpers.h"
 
 int main(int argc, char *argv[])
 {
 	int p_fd[2], ret;
 	int32_t recv_s0;
 	int32_t val = 1;
 	struct sockaddr_in addr;
 	struct iovec iov_r[1], iov_w[1];
 
 	if (argc > 1)
 		return 0;
 
 	srand(getpid());
 
 	recv_s0 = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 
 	ret = setsockopt(recv_s0, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
 	assert(ret != -1);
 	ret = setsockopt(recv_s0, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 	assert(ret != -1);
 
 	addr.sin_family = AF_INET;
 	addr.sin_addr.s_addr = inet_addr("127.0.0.1");
-
-	do {
-		addr.sin_port = htons((rand() % 61440) + 4096);
-		ret = bind(recv_s0, (struct sockaddr*)&addr, sizeof(addr));
-		if (!ret)
-			break;
-		if (errno != EADDRINUSE) {
-			perror("bind");
-			exit(1);
-		}
-	} while (1);
+	assert(!t_bind_ephemeral_port(recv_s0, &addr));
 	ret = listen(recv_s0, 128);
 	assert(ret != -1);
 
 
 	p_fd[1] = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 
 	val = 1;
 	ret = setsockopt(p_fd[1], IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val));
 	assert(ret != -1);
 
 	int32_t flags = fcntl(p_fd[1], F_GETFL, 0);
 	assert(flags != -1);
-- 
Ammar Faizi

[RESEND PATCH liburing v1 04/12] t/socket-rw-eagain: Don't brute force the port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't brute force the port number, use `t_bind_ephemeral_port()`,
much simpler and reliable for choosing a port number that is not
in use.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/socket-rw-eagain.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/test/socket-rw-eagain.c b/test/socket-rw-eagain.c
index 2d6a817..a12c70d 100644
--- a/test/socket-rw-eagain.c
+++ b/test/socket-rw-eagain.c
@@ -9,59 +9,49 @@
 #include <assert.h>
 
 #include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <netinet/tcp.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
 #include "liburing.h"
+#include "helpers.h"
 
 int main(int argc, char *argv[])
 {
 	int p_fd[2], ret;
 	int32_t recv_s0;
 	int32_t val = 1;
 	struct sockaddr_in addr;
 	struct iovec iov_r[1], iov_w[1];
 
 	if (argc > 1)
 		return 0;
 
 	srand(getpid());
 
 	recv_s0 = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 
 	ret = setsockopt(recv_s0, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
 	assert(ret != -1);
 	ret = setsockopt(recv_s0, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 	assert(ret != -1);
 
 	addr.sin_family = AF_INET;
 	addr.sin_addr.s_addr = inet_addr("127.0.0.1");
-
-	do {
-		addr.sin_port = htons((rand() % 61440) + 4096);
-		ret = bind(recv_s0, (struct sockaddr*)&addr, sizeof(addr));
-		if (!ret)
-			break;
-		if (errno != EADDRINUSE) {
-			perror("bind");
-			exit(1);
-		}
-	} while (1);
-
+	assert(!t_bind_ephemeral_port(recv_s0, &addr));
 	ret = listen(recv_s0, 128);
 	assert(ret != -1);
 
 	p_fd[1] = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 
 	val = 1;
 	ret = setsockopt(p_fd[1], IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val));
 	assert(ret != -1);
 
 	int32_t flags = fcntl(p_fd[1], F_GETFL, 0);
 	assert(flags != -1);
 
-- 
Ammar Faizi

[RESEND PATCH liburing v1 05/12] t/socket-rw-offset: Don't brute force the port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't brute force the port number, use `t_bind_ephemeral_port()`,
much simpler and reliable for choosing a port number that is not
in use.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/socket-rw-offset.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/test/socket-rw-offset.c b/test/socket-rw-offset.c
index 987b6c9..c422442 100644
--- a/test/socket-rw-offset.c
+++ b/test/socket-rw-offset.c
@@ -11,58 +11,49 @@
 #include <assert.h>
 
 #include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <netinet/tcp.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
 #include "liburing.h"
+#include "helpers.h"
 
 int main(int argc, char *argv[])
 {
 	int p_fd[2], ret;
 	int32_t recv_s0;
 	int32_t val = 1;
 	struct sockaddr_in addr;
 	struct iovec iov_r[1], iov_w[1];
 
 	if (argc > 1)
 		return 0;
 
 	srand(getpid());
 
 	recv_s0 = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 
 	ret = setsockopt(recv_s0, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
 	assert(ret != -1);
 	ret = setsockopt(recv_s0, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 	assert(ret != -1);
 
 	addr.sin_family = AF_INET;
 	addr.sin_addr.s_addr = inet_addr("127.0.0.1");
-
-	do {
-		addr.sin_port = htons((rand() % 61440) + 4096);
-		ret = bind(recv_s0, (struct sockaddr*)&addr, sizeof(addr));
-		if (!ret)
-			break;
-		if (errno != EADDRINUSE) {
-			perror("bind");
-			exit(1);
-		}
-	} while (1);
+	assert(!t_bind_ephemeral_port(recv_s0, &addr));
 	ret = listen(recv_s0, 128);
 	assert(ret != -1);
 
 
 	p_fd[1] = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 
 	val = 1;
 	ret = setsockopt(p_fd[1], IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val));
 	assert(ret != -1);
 
 	int32_t flags = fcntl(p_fd[1], F_GETFL, 0);
 	assert(flags != -1);
-- 
Ammar Faizi

[RESEND PATCH liburing v1 06/12] t/files-exit-hang-poll: Don't brute force the port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't brute force the port number, use `t_bind_ephemeral_port()`,
much simpler and reliable for choosing a port number that is not
in use.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/files-exit-hang-poll.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/test/files-exit-hang-poll.c b/test/files-exit-hang-poll.c
index 0c609f1..04febc8 100644
--- a/test/files-exit-hang-poll.c
+++ b/test/files-exit-hang-poll.c
@@ -10,26 +10,24 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <strings.h>
 #include <sys/socket.h>
 #include <unistd.h>
 #include <poll.h>
 #include "liburing.h"
 #include "helpers.h"
 
 #define BACKLOG 512
 
-#define PORT 9100
-
 static struct io_uring ring;
 
 static void add_poll(struct io_uring *ring, int fd)
 {
 	struct io_uring_sqe *sqe;
 
 	sqe = io_uring_get_sqe(ring);
 	io_uring_prep_poll_add(sqe, fd, POLLIN);
 	sqe->flags |= IOSQE_IO_LINK;
 }
 
 static void add_accept(struct io_uring *ring, int fd)
@@ -55,55 +53,43 @@ static int setup_io_uring(void)
 
 static void alarm_sig(int sig)
 {
 	exit(0);
 }
 
 int main(int argc, char *argv[])
 {
 	struct sockaddr_in serv_addr;
 	struct io_uring_cqe *cqe;
 	int ret, sock_listen_fd;
 	const int val = 1;
-	int i;
 
 	if (argc > 1)
 		return T_EXIT_SKIP;
 
 	sock_listen_fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
 	if (sock_listen_fd < 0) {
 		perror("socket");
 		return T_EXIT_FAIL;
 	}
 
 	setsockopt(sock_listen_fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 
 	memset(&serv_addr, 0, sizeof(serv_addr));
 	serv_addr.sin_family = AF_INET;
 	serv_addr.sin_addr.s_addr = INADDR_ANY;
 
-	for (i = 0; i < 100; i++) {
-		serv_addr.sin_port = htons(PORT + i);
-
-		ret = bind(sock_listen_fd, (struct sockaddr *)&serv_addr, sizeof(serv_addr));
-		if (!ret)
-			break;
-		if (errno != EADDRINUSE) {
-			fprintf(stderr, "bind: %s\n", strerror(errno));
-			return T_EXIT_FAIL;
-		}
-		if (i == 99) {
-			printf("Gave up on finding a port, skipping\n");
-			goto skip;
-		}
+	if (t_bind_ephemeral_port(sock_listen_fd, &serv_addr)) {
+		perror("bind");
+		return T_EXIT_FAIL;
 	}
 
 	if (listen(sock_listen_fd, BACKLOG) < 0) {
 		perror("Error listening on socket\n");
 		return T_EXIT_FAIL;
 	}
 
 	if (setup_io_uring())
 		return T_EXIT_FAIL;
 
 	add_poll(&ring, sock_listen_fd);
 	add_accept(&ring, sock_listen_fd);
@@ -116,16 +102,13 @@ int main(int argc, char *argv[])
 
 	signal(SIGALRM, alarm_sig);
 	alarm(1);
 
 	ret = io_uring_wait_cqe(&ring, &cqe);
 	if (ret) {
 		fprintf(stderr, "wait_cqe=%d\n", ret);
 		return T_EXIT_FAIL;
 	}
 
 	io_uring_queue_exit(&ring);
 	return T_EXIT_PASS;
-skip:
-	io_uring_queue_exit(&ring);
-	return T_EXIT_SKIP;
 }
-- 
Ammar Faizi

[RESEND PATCH liburing v1 07/12] t/socket: Don't use a static port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't use a static port number. It might already be in use, resulting
in a test failure. Use an ephemeral port to make this test reliable.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/socket.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/test/socket.c b/test/socket.c
index 6a3ea09..94c8e9f 100644
--- a/test/socket.c
+++ b/test/socket.c
@@ -2,63 +2,63 @@
 /*
  * Simple test case using the socket op
  */
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <arpa/inet.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <pthread.h>
+#include <assert.h>
 
 #include "liburing.h"
 #include "helpers.h"
 
 static char str[] = "This is a test of send and recv over io_uring!";
 
 #define MAX_MSG	128
 
-#define PORT	10202
 #define HOST	"127.0.0.1"
 
 static int no_socket;
+static __be32 g_port;
 
 static int recv_prep(struct io_uring *ring, struct iovec *iov, int *sock,
 		     int registerfiles)
 {
 	struct sockaddr_in saddr;
 	struct io_uring_sqe *sqe;
 	int sockfd, ret, val, use_fd;
 
 	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_family = AF_INET;
 	saddr.sin_addr.s_addr = htonl(INADDR_ANY);
-	saddr.sin_port = htons(PORT);
 
 	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
 	if (sockfd < 0) {
 		perror("socket");
 		return 1;
 	}
 
 	val = 1;
 	setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 
-	ret = bind(sockfd, (struct sockaddr *)&saddr, sizeof(saddr));
-	if (ret < 0) {
+	if (t_bind_ephemeral_port(sockfd, &saddr)) {
 		perror("bind");
 		goto err;
 	}
+	g_port = saddr.sin_port;
 
 	if (registerfiles) {
 		ret = io_uring_register_files(ring, &sockfd, 1);
 		if (ret) {
 			fprintf(stderr, "file reg failed\n");
 			goto err;
 		}
 		use_fd = 0;
 	} else {
 		use_fd = sockfd;
 	}
 
@@ -235,27 +235,28 @@ static int do_send(int socket_direct, int alloc)
 		fprintf(stderr, "queue init failed: %d\n", ret);
 		return 1;
 	}
 
 	if (socket_direct) {
 		ret = io_uring_register_files(&ring, &fd, 1);
 		if (ret) {
 			fprintf(stderr, "file register %d\n", ret);
 			return 1;
 		}
 	}
 
+	assert(g_port != 0);
 	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_family = AF_INET;
-	saddr.sin_port = htons(PORT);
+	saddr.sin_port = g_port;
 	inet_pton(AF_INET, HOST, &saddr.sin_addr);
 
 	sqe = io_uring_get_sqe(&ring);
 	if (socket_direct) {
 		unsigned file_index = 0;
 		if (alloc)
 			file_index = IORING_FILE_INDEX_ALLOC - 1;
 		io_uring_prep_socket_direct(sqe, AF_INET, SOCK_DGRAM, 0,
 						file_index, 0);
 	} else {
 		io_uring_prep_socket(sqe, AF_INET, SOCK_DGRAM, 0, 0);
 	}
-- 
Ammar Faizi

[RESEND PATCH liburing v1 08/12] t/connect: Don't use a static port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't use a static port number. It might already be in use, resulting
in a test failure. Use an ephemeral port to make this test reliable.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/accept.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/test/accept.c b/test/accept.c
index b35ded4..1821faa 100644
--- a/test/accept.c
+++ b/test/accept.c
@@ -185,29 +185,26 @@ static int start_accept_listen(struct sockaddr_in *addr, int port_off,
 	int32_t val = 1;
 	ret = setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
 	assert(ret != -1);
 	ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 	assert(ret != -1);
 
 	struct sockaddr_in laddr;
 
 	if (!addr)
 		addr = &laddr;
 
 	addr->sin_family = AF_INET;
-	addr->sin_port = htons(0x1235 + port_off);
 	addr->sin_addr.s_addr = inet_addr("127.0.0.1");
-
-	ret = bind(fd, (struct sockaddr*)addr, sizeof(*addr));
-	assert(ret != -1);
+	assert(!t_bind_ephemeral_port(fd, addr));
 	ret = listen(fd, 128);
 	assert(ret != -1);
 
 	return fd;
 }
 
 static int set_client_fd(struct sockaddr_in *addr)
 {
 	int32_t val;
 	int fd, ret;
 
 	fd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
-- 
Ammar Faizi

[RESEND PATCH liburing v1 09/12] t/shutdown: Don't use a static port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't use a static port number. It might already be in use, resulting
in a test failure. Use an ephemeral port to make this test reliable.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/shutdown.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/test/shutdown.c b/test/shutdown.c
index 14c7407..064ee36 100644
--- a/test/shutdown.c
+++ b/test/shutdown.c
@@ -10,54 +10,53 @@
 #include <assert.h>
 
 #include <errno.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <netinet/tcp.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
 #include "liburing.h"
+#include "helpers.h"
 
 static void sig_pipe(int sig)
 {
 }
 
 int main(int argc, char *argv[])
 {
 	int p_fd[2], ret;
 	int32_t recv_s0;
 	int32_t val = 1;
-	struct sockaddr_in addr;
+	struct sockaddr_in addr = { };
 
 	if (argc > 1)
 		return 0;
 
 	srand(getpid());
 
 	recv_s0 = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 
 	ret = setsockopt(recv_s0, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
 	assert(ret != -1);
 	ret = setsockopt(recv_s0, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 	assert(ret != -1);
 
 	addr.sin_family = AF_INET;
-	addr.sin_port = htons((rand() % 61440) + 4096);
 	addr.sin_addr.s_addr = inet_addr("127.0.0.1");
 
-	ret = bind(recv_s0, (struct sockaddr*)&addr, sizeof(addr));
-	assert(ret != -1);
+	assert(!t_bind_ephemeral_port(recv_s0, &addr));
 	ret = listen(recv_s0, 128);
 	assert(ret != -1);
 
 	p_fd[1] = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 
 	val = 1;
 	ret = setsockopt(p_fd[1], IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val));
 	assert(ret != -1);
 
 	int32_t flags = fcntl(p_fd[1], F_GETFL, 0);
 	assert(flags != -1);
 
-- 
Ammar Faizi

[RESEND PATCH liburing v1 10/12] t/recv-msgall: Don't use a static port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't use a static port number. It might already be in use, resulting
in a test failure. Use an ephemeral port to make this test reliable.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/recv-msgall.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/test/recv-msgall.c b/test/recv-msgall.c
index a6f7cfc..ae123e4 100644
--- a/test/recv-msgall.c
+++ b/test/recv-msgall.c
@@ -7,55 +7,53 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <arpa/inet.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <pthread.h>
 
 #include "liburing.h"
 #include "helpers.h"
 
 #define MAX_MSG	128
-
-#define PORT	10201
 #define HOST	"127.0.0.1"
+static __be16 bind_port;
 
 static int recv_prep(struct io_uring *ring, struct iovec *iov, int *sock,
 		     int use_recvmsg)
 {
 	struct sockaddr_in saddr;
 	struct io_uring_sqe *sqe;
 	int sockfd, ret, val;
 	struct msghdr msg = { };
 
 	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_family = AF_INET;
 	saddr.sin_addr.s_addr = htonl(INADDR_ANY);
-	saddr.sin_port = htons(PORT);
 
 	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
 	if (sockfd < 0) {
 		perror("socket");
 		return 1;
 	}
 
 	val = 1;
 	setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 
-	ret = bind(sockfd, (struct sockaddr *)&saddr, sizeof(saddr));
-	if (ret < 0) {
+	if (t_bind_ephemeral_port(sockfd, &saddr)) {
 		perror("bind");
 		goto err;
 	}
+	bind_port = saddr.sin_port;
 
 	sqe = io_uring_get_sqe(ring);
 	if (!use_recvmsg) {
 		io_uring_prep_recv(sqe, sockfd, iov->iov_base, iov->iov_len,
 					MSG_WAITALL);
 	} else {
 		msg.msg_namelen = sizeof(struct sockaddr_in);
 		msg.msg_iov = iov;
 		msg.msg_iovlen = 1;
 		io_uring_prep_recvmsg(sqe, sockfd, &msg, MSG_WAITALL);
 	}
 
@@ -156,25 +154,25 @@ static int do_send(void)
 	ret = io_uring_queue_init(2, &ring, 0);
 	if (ret) {
 		fprintf(stderr, "queue init failed: %d\n", ret);
 		return 1;
 	}
 
 	buf = malloc(MAX_MSG * sizeof(int));
 	for (i = 0; i < MAX_MSG; i++)
 		buf[i] = i;
 
 	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_family = AF_INET;
-	saddr.sin_port = htons(PORT);
+	saddr.sin_port = bind_port;
 	inet_pton(AF_INET, HOST, &saddr.sin_addr);
 
 	sockfd = socket(AF_INET, SOCK_DGRAM, 0);
 	if (sockfd < 0) {
 		perror("socket");
 		return 1;
 	}
 
 	ret = connect(sockfd, (struct sockaddr *)&saddr, sizeof(saddr));
 	if (ret < 0) {
 		perror("connect");
 		return 1;
-- 
Ammar Faizi

[RESEND PATCH liburing v1 11/12] t/232c93d07b74: Don't use a static port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't use a static port number. It might already be in use, resulting
in a test failure. Use an ephemeral port to make this test reliable.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/232c93d07b74.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/test/232c93d07b74.c b/test/232c93d07b74.c
index c99491f..74cc063 100644
--- a/test/232c93d07b74.c
+++ b/test/232c93d07b74.c
@@ -18,29 +18,28 @@
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <netinet/tcp.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 
 #include "helpers.h"
 #include "liburing.h"
 
 #define RECV_BUFF_SIZE 2
 #define SEND_BUFF_SIZE 3
 
-#define PORT	0x1234
-
 struct params {
 	int tcp;
 	int non_blocking;
+	__be16 bind_port;
 };
 
 pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
 int rcv_ready = 0;
 
 static void set_rcv_ready(void)
 {
 	pthread_mutex_lock(&mutex);
 
 	rcv_ready = 1;
 	pthread_cond_signal(&cond);
@@ -68,28 +67,27 @@ static void *rcv(void *arg)
 		int val = 1;
                 
 
 		s0 = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 		res = setsockopt(s0, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
 		assert(res != -1);
 		res = setsockopt(s0, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 		assert(res != -1);
 
 		struct sockaddr_in addr;
 
 		addr.sin_family = AF_INET;
-		addr.sin_port = htons(PORT);
 		addr.sin_addr.s_addr = inet_addr("127.0.0.1");
-		res = bind(s0, (struct sockaddr *) &addr, sizeof(addr));
-		assert(res != -1);
+		assert(t_bind_ephemeral_port(s0, &addr) == 0);
+		p->bind_port = addr.sin_port;
 	} else {
 		s0 = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
 		assert(s0 != -1);
 
 		struct sockaddr_un addr;
 		memset(&addr, 0, sizeof(addr));
 
 		addr.sun_family = AF_UNIX;
 		memcpy(addr.sun_path, "\0sock", 6);
 		res = bind(s0, (struct sockaddr *) &addr, sizeof(addr));
 		assert(res != -1);
 	}
@@ -183,25 +181,25 @@ static void *snd(void *arg)
 	wait_for_rcv_ready();
 
 	if (p->tcp) {
 		int val = 1;
 
 		s0 = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 		ret = setsockopt(s0, IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val));
 		assert(ret != -1);
 
 		struct sockaddr_in addr;
 
 		addr.sin_family = AF_INET;
-		addr.sin_port = htons(PORT);
+		addr.sin_port = p->bind_port;
 		addr.sin_addr.s_addr = inet_addr("127.0.0.1");
 		ret = connect(s0, (struct sockaddr*) &addr, sizeof(addr));
 		assert(ret != -1);
 	} else {
 		s0 = socket(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0);
 		assert(s0 != -1);
 
 		struct sockaddr_un addr;
 		memset(&addr, 0, sizeof(addr));
 
 		addr.sun_family = AF_UNIX;
 		memcpy(addr.sun_path, "\0sock", 6);
-- 
Ammar Faizi

[RESEND PATCH liburing v1 12/12] t/recv-msgall-stream: Don't use a static port number

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Don't use a static port number. It might already be in use, resulting
in a test failure. Use an ephemeral port to make this test reliable.

Cc: Dylan Yudaken <dylany@fb.com>
Cc: Facebook Kernel Team <kernel-team@fb.com>
Cc: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/recv-msgall-stream.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/test/recv-msgall-stream.c b/test/recv-msgall-stream.c
index a188cc1..65b4d22 100644
--- a/test/recv-msgall-stream.c
+++ b/test/recv-msgall-stream.c
@@ -1,67 +1,65 @@
 /* SPDX-License-Identifier: MIT */
 /*
  * Test MSG_WAITALL for recv/recvmsg and include normal sync versions just
  * for comparison.
  */
+#include <assert.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <arpa/inet.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <pthread.h>
 
 #include "liburing.h"
 #include "helpers.h"
 
 #define MAX_MSG	128
 
-static int port = 31200;
-
 struct recv_data {
 	pthread_mutex_t mutex;
 	int use_recvmsg;
 	int use_sync;
-	int port;
+	__be16 port;
 };
 
 static int get_conn_sock(struct recv_data *rd, int *sockout)
 {
 	struct sockaddr_in saddr;
 	int sockfd, ret, val;
 
 	memset(&saddr, 0, sizeof(saddr));
 	saddr.sin_family = AF_INET;
 	saddr.sin_addr.s_addr = htonl(INADDR_ANY);
-	saddr.sin_port = htons(rd->port);
 
 	sockfd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 	if (sockfd < 0) {
 		perror("socket");
 		goto err;
 	}
 
 	val = 1;
 	setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
 	setsockopt(sockfd, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
 
-	ret = bind(sockfd, (struct sockaddr *)&saddr, sizeof(saddr));
-	if (ret < 0) {
+	if (t_bind_ephemeral_port(sockfd, &saddr)) {
 		perror("bind");
 		goto err;
 	}
+	rd->port = saddr.sin_port;
 
 	ret = listen(sockfd, 16);
 	if (ret < 0) {
 		perror("listen");
 		goto err;
 	}
 
 	pthread_mutex_unlock(&rd->mutex);
 
 	ret = accept(sockfd, NULL, NULL);
 	if (ret < 0) {
 		perror("accept");
@@ -270,36 +268,36 @@ static int do_send(struct recv_data *rd)
 	int *buf;
 
 	ret = io_uring_queue_init(2, &ring, 0);
 	if (ret) {
 		fprintf(stderr, "queue init failed: %d\n", ret);
 		return 1;
 	}
 
 	buf = malloc(MAX_MSG * sizeof(int));
 	for (i = 0; i < MAX_MSG; i++)
 		buf[i] = i;
 
-	memset(&saddr, 0, sizeof(saddr));
-	saddr.sin_family = AF_INET;
-	saddr.sin_port = htons(rd->port);
-	inet_pton(AF_INET, "127.0.0.1", &saddr.sin_addr);
-
 	sockfd = socket(AF_INET, SOCK_STREAM | SOCK_CLOEXEC, IPPROTO_TCP);
 	if (sockfd < 0) {
 		perror("socket");
 		return 1;
 	}
 
 	pthread_mutex_lock(&rd->mutex);
+	assert(rd->port != 0);
+	memset(&saddr, 0, sizeof(saddr));
+	saddr.sin_family = AF_INET;
+	saddr.sin_port = rd->port;
+	inet_pton(AF_INET, "127.0.0.1", &saddr.sin_addr);
 
 	ret = connect(sockfd, (struct sockaddr *)&saddr, sizeof(saddr));
 	if (ret < 0) {
 		perror("connect");
 		return 1;
 	}
 
 	iov.iov_base = buf;
 	iov.iov_len = MAX_MSG * sizeof(int) / 2;
 	for (i = 0; i < 2; i++) {
 		sqe = io_uring_get_sqe(&ring);
 		io_uring_prep_send(sqe, sockfd, iov.iov_base, iov.iov_len, 0);
@@ -342,25 +340,25 @@ static int test(int use_recvmsg, int use_sync)
 	pthread_mutexattr_t attr;
 	pthread_t recv_thread;
 	struct recv_data rd;
 	int ret;
 	void *retval;
 
 	pthread_mutexattr_init(&attr);
 	pthread_mutexattr_setpshared(&attr, 1);
 	pthread_mutex_init(&rd.mutex, &attr);
 	pthread_mutex_lock(&rd.mutex);
 	rd.use_recvmsg = use_recvmsg;
 	rd.use_sync = use_sync;
-	rd.port = port++;
+	rd.port = 0;
 
 	ret = pthread_create(&recv_thread, NULL, recv_fn, &rd);
 	if (ret) {
 		fprintf(stderr, "Thread create failed: %d\n", ret);
 		pthread_mutex_unlock(&rd.mutex);
 		return 1;
 	}
 
 	do_send(&rd);
 	pthread_join(recv_thread, &retval);
 	return (intptr_t)retval;
 }
-- 
Ammar Faizi

Re: [RESEND PATCH liburing v1 01/12] test/helpers: Add `t_bind_ephemeral_port()` function

[Alviro Iskandar Setiawan]

On Fri, Sep 2, 2022 at 8:18 AM Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>
> This is a prep patch to fix an intermittent issue with the port number.
>
> We have many places where we need to bind() a socket to any unused port
> number. To achieve that, the current approach does one of the following
> mechanisms:
>
>   1) Randomly brute force the port number until the bind() syscall
>      succeeds.
>
>   2) Use a static port at compile time (randomly chosen too).
>
> This is not reliable and it results in an intermittent issue (test
> fails when the selected port is in use).
>
> Setting @addr->sin_port to zero on a bind() syscall lets the kernel
> choose a port number that is not in use. The caller then can know the
> port number to be bound by invoking a getsockname() syscall after
> bind() succeeds.
>
> Wrap this procedure in a new function called t_bind_ephemeral_port().
> The selected port will be returned into @addr->sin_port, the caller
> can use it later to connect() or whatever they need.
>
> Link: https://lore.kernel.org/r/918facd1-78ba-2de7-693a-5f8c65ea2fcd@gnuweeb.org
> Cc: Dylan Yudaken <dylany@fb.com>
> Cc: Facebook Kernel Team <kernel-team@fb.com>
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

tq

-- Viro

Re: [RESEND PATCH liburing v1 02/12] t/poll-link: Don't brute force the port number

[Alviro Iskandar Setiawan]

On Fri, Sep 2, 2022 at 8:18 AM Ammar Faizi wrote:
>  static void signal_var(int *var)
>  {
>          pthread_mutex_lock(&mutex);
>          *var = 1;
> @@ -80,45 +81,37 @@ void *recv_thread(void *arg)
>         ret = io_uring_queue_init(8, &ring, 0);
>         assert(ret == 0);
>
>         int s0 = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
>         assert(s0 != -1);
>
>         int32_t val = 1;
>         ret = setsockopt(s0, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
>         assert(ret != -1);
>         ret = setsockopt(s0, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
>         assert(ret != -1);
>
> -       struct sockaddr_in addr;
> +       struct sockaddr_in addr = { };

move this variable to the top plz, with that fixed:

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

tq

-- Viro

Re: [RESEND PATCH liburing v1 03/12] t/socket-rw: Don't brute force the port number

[Alviro Iskandar Setiawan]

On Fri, Sep 2, 2022 at 8:18 AM Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>
> Don't brute force the port number, use `t_bind_ephemeral_port()`,
> much simpler and reliable for choosing a port number that is not
> in use.
>
> Cc: Dylan Yudaken <dylany@fb.com>
> Cc: Facebook Kernel Team <kernel-team@fb.com>
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

tq

-- Viro

Re: [RESEND PATCH liburing v1 04/12] t/socket-rw-eagain: Don't brute force the port number

[Alviro Iskandar Setiawan]

On Fri, Sep 2, 2022 at 8:18 AM Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>
> Don't brute force the port number, use `t_bind_ephemeral_port()`,
> much simpler and reliable for choosing a port number that is not
> in use.
>
> Cc: Dylan Yudaken <dylany@fb.com>
> Cc: Facebook Kernel Team <kernel-team@fb.com>
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

tq

-- Viro

Re: [RESEND PATCH liburing v1 05/12] t/socket-rw-offset: Don't brute force the port number

[Alviro Iskandar Setiawan]

On Fri, Sep 2, 2022 at 8:18 AM Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>
> Don't brute force the port number, use `t_bind_ephemeral_port()`,
> much simpler and reliable for choosing a port number that is not
> in use.
>
> Cc: Dylan Yudaken <dylany@fb.com>
> Cc: Facebook Kernel Team <kernel-team@fb.com>
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

tq

-- Viro

Re: [RESEND PATCH liburing v1 02/12] t/poll-link: Don't brute force the port number

[Ammar Faizi]

On 9/2/22 1:04 PM, Alviro Iskandar Setiawan wrote:
> On Fri, Sep 2, 2022 at 8:18 AM Ammar Faizi wrote:
>>   static void signal_var(int *var)
>>   {
>>           pthread_mutex_lock(&mutex);
>>           *var = 1;
>> @@ -80,45 +81,37 @@ void *recv_thread(void *arg)
>>          ret = io_uring_queue_init(8, &ring, 0);
>>          assert(ret == 0);
>>
>>          int s0 = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
>>          assert(s0 != -1);
>>
>>          int32_t val = 1;
>>          ret = setsockopt(s0, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val));
>>          assert(ret != -1);
>>          ret = setsockopt(s0, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val));
>>          assert(ret != -1);
>>
>> -       struct sockaddr_in addr;
>> +       struct sockaddr_in addr = { };
> 
> move this variable to the top plz, with that fixed:
> 
> Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

Will do that in v2 revision. Thanks!

-- 
Ammar Faizi

Re: [RESEND PATCH liburing v1 06/12] t/files-exit-hang-poll: Don't brute force the port number

[Alviro Iskandar Setiawan]

On Fri, Sep 2, 2022 at 8:18 AM Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>
> Don't brute force the port number, use `t_bind_ephemeral_port()`,
> much simpler and reliable for choosing a port number that is not
> in use.
>
> Cc: Dylan Yudaken <dylany@fb.com>
> Cc: Facebook Kernel Team <kernel-team@fb.com>
> Cc: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

tq

-- Viro

Re: [RESEND PATCH liburing v1 00/12] Introducing t_bind_ephemeral_port() function

[Alviro Iskandar Setiawan]

On Fri, Sep 2, 2022 at 8:18 AM Ammar Faizi wrote:
> ## Solution:
> Setting @addr->sin_port to zero on a bind() syscall lets the kernel
> choose a port number that is not in use. The caller then can know the
> port number to be bound by invoking a getsockname() syscall after
> bind() succeeds.
>
> Wrap this procedure in a new function called t_bind_ephemeral_port().
> The selected port will be returned into @addr->sin_port, the caller
> can use it later to connect() or whatever they need.

with variable placement fix, for all patches:

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>
Tested-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

(tested on 5.19)

tq

-- Viro