diff mbox series

[net,v2,3/6] vsock/test: Introduce vsock_bind()

Message ID 20250121-vsock-transport-vs-autobind-v2-3-aad6069a4e8c@rbox.co (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series vsock: Transport reassignment and error handling issues | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success Errors and warnings before: 0 (+0) this patch: 0 (+0)
netdev/cc_maintainers warning 1 maintainers not CCed: virtualization@lists.linux.dev
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 93 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Michal Luczaj Jan. 21, 2025, 2:44 p.m. UTC 
Add a helper for socket()+bind(). Adapt callers.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
---
 tools/testing/vsock/util.c       | 56 +++++++++++++++++-----------------------
 tools/testing/vsock/util.h       |  1 +
 tools/testing/vsock/vsock_test.c | 17 +-----------
 3 files changed, 25 insertions(+), 49 deletions(-)

Comments

Luigi Leonardi Jan. 22, 2025, 4:01 p.m. UTC | #1 
On Tue, Jan 21, 2025 at 03:44:04PM +0100, Michal Luczaj wrote:
>Add a helper for socket()+bind(). Adapt callers.
>
>Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>---
> tools/testing/vsock/util.c       | 56 +++++++++++++++++-----------------------
> tools/testing/vsock/util.h       |  1 +
> tools/testing/vsock/vsock_test.c | 17 +-----------
> 3 files changed, 25 insertions(+), 49 deletions(-)
>
>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>index 34e9dac0a105f8aeb8c9af379b080d5ce8cb2782..31ee1767c8b73c05cfd219c3d520a677df6e66a6 100644
>--- a/tools/testing/vsock/util.c
>+++ b/tools/testing/vsock/util.c
>@@ -96,33 +96,42 @@ void vsock_wait_remote_close(int fd)
> 	close(epollfd);
> }
>
>-/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
>-int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
If you need to send a v3, it would be nice to have a comment for 
vsock_bind, as there used to be one.
>+int vsock_bind(unsigned int cid, unsigned int port, int type)
> {
>-	struct sockaddr_vm sa_client = {
>-		.svm_family = AF_VSOCK,
>-		.svm_cid = VMADDR_CID_ANY,
>-		.svm_port = bind_port,
>-	};
>-	struct sockaddr_vm sa_server = {
>+	struct sockaddr_vm sa = {
> 		.svm_family = AF_VSOCK,
> 		.svm_cid = cid,
> 		.svm_port = port,
> 	};
>+	int fd;
>
>-	int client_fd, ret;
>-
>-	client_fd = socket(AF_VSOCK, type, 0);
>-	if (client_fd < 0) {
>+	fd = socket(AF_VSOCK, type, 0);
>+	if (fd < 0) {
> 		perror("socket");
> 		exit(EXIT_FAILURE);
> 	}
>
>-	if (bind(client_fd, (struct sockaddr *)&sa_client, sizeof(sa_client))) {
>+	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa))) {
> 		perror("bind");
> 		exit(EXIT_FAILURE);
> 	}
>
>+	return fd;
>+}
>+
>+/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
>+int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
>+{
>+	struct sockaddr_vm sa_server = {
>+		.svm_family = AF_VSOCK,
>+		.svm_cid = cid,
>+		.svm_port = port,
>+	};
>+
>+	int client_fd, ret;
>+
>+	client_fd = vsock_bind(VMADDR_CID_ANY, bind_port, type);
>+
> 	timeout_begin(TIMEOUT);
> 	do {
> 		ret = connect(client_fd, (struct sockaddr *)&sa_server, sizeof(sa_server));
>@@ -192,28 +201,9 @@ int vsock_seqpacket_connect(unsigned int cid, unsigned int port)
> /* Listen on <cid, port> and return the file descriptor. */
> static int vsock_listen(unsigned int cid, unsigned int port, int type)
> {
>-	union {
>-		struct sockaddr sa;
>-		struct sockaddr_vm svm;
>-	} addr = {
>-		.svm = {
>-			.svm_family = AF_VSOCK,
>-			.svm_port = port,
>-			.svm_cid = cid,
>-		},
>-	};
> 	int fd;
>
>-	fd = socket(AF_VSOCK, type, 0);
>-	if (fd < 0) {
>-		perror("socket");
>-		exit(EXIT_FAILURE);
>-	}
>-
>-	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
>-		perror("bind");
>-		exit(EXIT_FAILURE);
>-	}
>+	fd = vsock_bind(cid, port, type);
>
> 	if (listen(fd, 1) < 0) {
> 		perror("listen");
>diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>index ba84d296d8b71e1bcba2abdad337e07aac45e75e..7736594a15d29449d98bd1e9e19c3acd1a623443 100644
>--- a/tools/testing/vsock/util.h
>+++ b/tools/testing/vsock/util.h
>@@ -43,6 +43,7 @@ int vsock_connect(unsigned int cid, unsigned int port, int type);
> int vsock_accept(unsigned int cid, unsigned int port,
> 		 struct sockaddr_vm *clientaddrp, int type);
> int vsock_stream_connect(unsigned int cid, unsigned int port);
>+int vsock_bind(unsigned int cid, unsigned int port, int type);
> int vsock_bind_connect(unsigned int cid, unsigned int port,
> 		       unsigned int bind_port, int type);
> int vsock_seqpacket_connect(unsigned int cid, unsigned int port);
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 48f17641ca504316d1199926149c9bd62eb2921d..28a5083bbfd600cf84a1a85cec2f272ce6912dd3 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -108,24 +108,9 @@ static void test_stream_bind_only_client(const struct test_opts *opts)
>
> static void test_stream_bind_only_server(const struct test_opts *opts)
> {
>-	union {
>-		struct sockaddr sa;
>-		struct sockaddr_vm svm;
>-	} addr = {
>-		.svm = {
>-			.svm_family = AF_VSOCK,
>-			.svm_port = opts->peer_port,
>-			.svm_cid = VMADDR_CID_ANY,
>-		},
>-	};
> 	int fd;
>
>-	fd = socket(AF_VSOCK, SOCK_STREAM, 0);
>-
>-	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
>-		perror("bind");
>-		exit(EXIT_FAILURE);
>-	}
>+	fd = vsock_bind(VMADDR_CID_ANY, opts->peer_port, SOCK_STREAM);
>
> 	/* Notify the client that the server is ready */
> 	control_writeln("BIND");
>
>-- 
>2.48.1
>

Thanks for the patch, I left small a comment, but if no v3 is needed 
then:

Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
Michal Luczaj Jan. 22, 2025, 8:11 p.m. UTC | #2 
On 1/22/25 17:01, Luigi Leonardi wrote:
> On Tue, Jan 21, 2025 at 03:44:04PM +0100, Michal Luczaj wrote:
>> Add a helper for socket()+bind(). Adapt callers.
>>
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>> ---
>> tools/testing/vsock/util.c       | 56 +++++++++++++++++-----------------------
>> tools/testing/vsock/util.h       |  1 +
>> tools/testing/vsock/vsock_test.c | 17 +-----------
>> 3 files changed, 25 insertions(+), 49 deletions(-)
>>
>> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>> index 34e9dac0a105f8aeb8c9af379b080d5ce8cb2782..31ee1767c8b73c05cfd219c3d520a677df6e66a6 100644
>> --- a/tools/testing/vsock/util.c
>> +++ b/tools/testing/vsock/util.c
>> @@ -96,33 +96,42 @@ void vsock_wait_remote_close(int fd)
>> 	close(epollfd);
>> }
>>
>> -/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
>> -int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
>
> If you need to send a v3, it would be nice to have a comment for 
> vsock_bind, as there used to be one.

Comment for vsock_bind_connect() remains, see below. As for vsock_bind(),
perhaps it's time to start using kernel-doc comments? v3 isn't coming, it
seems, but I'll comment the function later.

Thanks,
Michal

>> +/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
>> +int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
>> +{
>> +	struct sockaddr_vm sa_server = {
>> +		.svm_family = AF_VSOCK,
>> +		.svm_cid = cid,
>> +		.svm_port = port,
>> +	};
>> +
>> +	int client_fd, ret;
>> +
>> +	client_fd = vsock_bind(VMADDR_CID_ANY, bind_port, type);
>> +
>> 	timeout_begin(TIMEOUT);
>> 	do {
>> 		ret = connect(client_fd, (struct sockaddr *)&sa_server, sizeof(sa_server));
Luigi Leonardi Jan. 23, 2025, 11:02 a.m. UTC | #3 
On Wed, Jan 22, 2025 at 09:11:30PM +0100, Michal Luczaj wrote:
>On 1/22/25 17:01, Luigi Leonardi wrote:
>> On Tue, Jan 21, 2025 at 03:44:04PM +0100, Michal Luczaj wrote:
>>> Add a helper for socket()+bind(). Adapt callers.
>>>
>>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>>> ---
>>> tools/testing/vsock/util.c       | 56 +++++++++++++++++-----------------------
>>> tools/testing/vsock/util.h       |  1 +
>>> tools/testing/vsock/vsock_test.c | 17 +-----------
>>> 3 files changed, 25 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>>> index 34e9dac0a105f8aeb8c9af379b080d5ce8cb2782..31ee1767c8b73c05cfd219c3d520a677df6e66a6 100644
>>> --- a/tools/testing/vsock/util.c
>>> +++ b/tools/testing/vsock/util.c
>>> @@ -96,33 +96,42 @@ void vsock_wait_remote_close(int fd)
>>> 	close(epollfd);
>>> }
>>>
>>> -/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
>>> -int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
>>
>> If you need to send a v3, it would be nice to have a comment for
>> vsock_bind, as there used to be one.
>
>Comment for vsock_bind_connect() remains, see below. As for vsock_bind(),
>perhaps it's time to start using kernel-doc comments? 
This is a good idea! @Stefano WDYT?
Sticking to tests, IIRC someone mentioned (maybe Stefano?) about moving 
to selftests for vsock, but I don't think it's going to happen anytime 
soon.

>v3 isn't coming, it seems, but I'll comment the function later.
Thank you :)

Cheers,
Luigi
Stefano Garzarella Jan. 23, 2025, 11:07 a.m. UTC | #4 
On Thu, Jan 23, 2025 at 12:02:36PM +0100, Luigi Leonardi wrote:
>On Wed, Jan 22, 2025 at 09:11:30PM +0100, Michal Luczaj wrote:
>>On 1/22/25 17:01, Luigi Leonardi wrote:
>>>On Tue, Jan 21, 2025 at 03:44:04PM +0100, Michal Luczaj wrote:
>>>>Add a helper for socket()+bind(). Adapt callers.
>>>>
>>>>Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>>>>Signed-off-by: Michal Luczaj <mhal@rbox.co>
>>>>---
>>>>tools/testing/vsock/util.c       | 56 +++++++++++++++++-----------------------
>>>>tools/testing/vsock/util.h       |  1 +
>>>>tools/testing/vsock/vsock_test.c | 17 +-----------
>>>>3 files changed, 25 insertions(+), 49 deletions(-)
>>>>
>>>>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>>>>index 34e9dac0a105f8aeb8c9af379b080d5ce8cb2782..31ee1767c8b73c05cfd219c3d520a677df6e66a6 100644
>>>>--- a/tools/testing/vsock/util.c
>>>>+++ b/tools/testing/vsock/util.c
>>>>@@ -96,33 +96,42 @@ void vsock_wait_remote_close(int fd)
>>>>	close(epollfd);
>>>>}
>>>>
>>>>-/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
>>>>-int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
>>>
>>>If you need to send a v3, it would be nice to have a comment for
>>>vsock_bind, as there used to be one.
>>
>>Comment for vsock_bind_connect() remains, see below. As for vsock_bind(),
>>perhaps it's time to start using kernel-doc comments?
>This is a good idea! @Stefano WDYT?

I'm not sure it's worth it since they are just tests, but if someone
wants to do it, absolutely no objection.

Thanks,
Stefano
diff mbox series

Patch

diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 34e9dac0a105f8aeb8c9af379b080d5ce8cb2782..31ee1767c8b73c05cfd219c3d520a677df6e66a6 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -96,33 +96,42 @@  void vsock_wait_remote_close(int fd)
 	close(epollfd);
 }
 
-/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
-int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
+int vsock_bind(unsigned int cid, unsigned int port, int type)
 {
-	struct sockaddr_vm sa_client = {
-		.svm_family = AF_VSOCK,
-		.svm_cid = VMADDR_CID_ANY,
-		.svm_port = bind_port,
-	};
-	struct sockaddr_vm sa_server = {
+	struct sockaddr_vm sa = {
 		.svm_family = AF_VSOCK,
 		.svm_cid = cid,
 		.svm_port = port,
 	};
+	int fd;
 
-	int client_fd, ret;
-
-	client_fd = socket(AF_VSOCK, type, 0);
-	if (client_fd < 0) {
+	fd = socket(AF_VSOCK, type, 0);
+	if (fd < 0) {
 		perror("socket");
 		exit(EXIT_FAILURE);
 	}
 
-	if (bind(client_fd, (struct sockaddr *)&sa_client, sizeof(sa_client))) {
+	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa))) {
 		perror("bind");
 		exit(EXIT_FAILURE);
 	}
 
+	return fd;
+}
+
+/* Bind to <bind_port>, connect to <cid, port> and return the file descriptor. */
+int vsock_bind_connect(unsigned int cid, unsigned int port, unsigned int bind_port, int type)
+{
+	struct sockaddr_vm sa_server = {
+		.svm_family = AF_VSOCK,
+		.svm_cid = cid,
+		.svm_port = port,
+	};
+
+	int client_fd, ret;
+
+	client_fd = vsock_bind(VMADDR_CID_ANY, bind_port, type);
+
 	timeout_begin(TIMEOUT);
 	do {
 		ret = connect(client_fd, (struct sockaddr *)&sa_server, sizeof(sa_server));
@@ -192,28 +201,9 @@  int vsock_seqpacket_connect(unsigned int cid, unsigned int port)
 /* Listen on <cid, port> and return the file descriptor. */
 static int vsock_listen(unsigned int cid, unsigned int port, int type)
 {
-	union {
-		struct sockaddr sa;
-		struct sockaddr_vm svm;
-	} addr = {
-		.svm = {
-			.svm_family = AF_VSOCK,
-			.svm_port = port,
-			.svm_cid = cid,
-		},
-	};
 	int fd;
 
-	fd = socket(AF_VSOCK, type, 0);
-	if (fd < 0) {
-		perror("socket");
-		exit(EXIT_FAILURE);
-	}
-
-	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
-		perror("bind");
-		exit(EXIT_FAILURE);
-	}
+	fd = vsock_bind(cid, port, type);
 
 	if (listen(fd, 1) < 0) {
 		perror("listen");
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index ba84d296d8b71e1bcba2abdad337e07aac45e75e..7736594a15d29449d98bd1e9e19c3acd1a623443 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -43,6 +43,7 @@  int vsock_connect(unsigned int cid, unsigned int port, int type);
 int vsock_accept(unsigned int cid, unsigned int port,
 		 struct sockaddr_vm *clientaddrp, int type);
 int vsock_stream_connect(unsigned int cid, unsigned int port);
+int vsock_bind(unsigned int cid, unsigned int port, int type);
 int vsock_bind_connect(unsigned int cid, unsigned int port,
 		       unsigned int bind_port, int type);
 int vsock_seqpacket_connect(unsigned int cid, unsigned int port);
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 48f17641ca504316d1199926149c9bd62eb2921d..28a5083bbfd600cf84a1a85cec2f272ce6912dd3 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -108,24 +108,9 @@  static void test_stream_bind_only_client(const struct test_opts *opts)
 
 static void test_stream_bind_only_server(const struct test_opts *opts)
 {
-	union {
-		struct sockaddr sa;
-		struct sockaddr_vm svm;
-	} addr = {
-		.svm = {
-			.svm_family = AF_VSOCK,
-			.svm_port = opts->peer_port,
-			.svm_cid = VMADDR_CID_ANY,
-		},
-	};
 	int fd;
 
-	fd = socket(AF_VSOCK, SOCK_STREAM, 0);
-
-	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
-		perror("bind");
-		exit(EXIT_FAILURE);
-	}
+	fd = vsock_bind(VMADDR_CID_ANY, opts->peer_port, SOCK_STREAM);
 
 	/* Notify the client that the server is ready */
 	control_writeln("BIND");