diff mbox series

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

Message ID 20220902011548.2506938-3-ammar.faizi@intel.com (mailing list archive)
State New
Headers show
Series Introducing t_bind_ephemeral_port() function | expand

Commit Message

Ammar Faizi Sept. 2, 2022, 1:17 a.m. UTC 
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(-)

Comments

Alviro Iskandar Setiawan Sept. 2, 2022, 6:04 a.m. UTC | #1 
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
Ammar Faizi Sept. 2, 2022, 6:18 a.m. UTC | #2 
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!
diff mbox series

Patch

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)