Re: [PATCH RFC net-next v3 8/8] tests: add vsock dgram tests

Re: [PATCH RFC net-next v3 8/8] tests: add vsock dgram tests

[Arseniy Krasnov]

Sorry, CC mailing lists

On 06.06.2023 12:29, Arseniy Krasnov wrote:
> Hello Bobby and Jiang! Small remarks(sorry for this letter layout, I add multiple newline over comments):
> 
> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> index 01b636d3039a..45e35da48b40 100644
> --- a/tools/testing/vsock/util.c
> +++ b/tools/testing/vsock/util.c
> @@ -260,6 +260,57 @@ void send_byte(int fd, int expected_ret, int flags)
>  	}
>  }
>  
> +/* Transmit one byte and check the return value.
> + *
> + * expected_ret:
> + *  <0 Negative errno (for testing errors)
> + *   0 End-of-file
> + *   1 Success
> + */
> +void sendto_byte(int fd, const struct sockaddr *dest_addr, int len, int expected_ret,
> +		 int flags)
> +{
> +	const uint8_t byte = 'A';
> +	ssize_t nwritten;
> +
> +	timeout_begin(TIMEOUT);
> +	do {
> +		nwritten = sendto(fd, &byte, sizeof(byte), flags, dest_addr,
> +				  len);
> +		timeout_check("write");
> +	} while (nwritten < 0 && errno == EINTR);
> +	timeout_end();
> +
> +	if (expected_ret < 0) {
> +		if (nwritten != -1) {
> +			fprintf(stderr, "bogus sendto(2) return value %zd\n",
> +				nwritten);
> +			exit(EXIT_FAILURE);
> +		}
> +		if (errno != -expected_ret) {
> +			perror("write");
> +			exit(EXIT_FAILURE);
> +		}
> +		return;
> +	}
> +
> +	if (nwritten < 0) {
> +		perror("write");
> +		exit(EXIT_FAILURE);
> +	}
> +	if (nwritten == 0) {
> +		if (expected_ret == 0)
> +			return;
> +
> +		fprintf(stderr, "unexpected EOF while sending byte\n");
> +		exit(EXIT_FAILURE);
> +	}
> +	if (nwritten != sizeof(byte)) {
> +		fprintf(stderr, "bogus sendto(2) return value %zd\n", nwritten);
> +		exit(EXIT_FAILURE);
> +
> 	}
> 
> 
> 
> ^^^
> May be short check that 'nwritten' != 'expected_ret' will be enough? Then print expected and
> real value. Here and in 'recvfrom_byte()' below.
> 
> 
> 
> 
> +}
> +
>  /* Receive one byte and check the return value.
>   *
>   * expected_ret:
> @@ -313,6 +364,60 @@ void recv_byte(int fd, int expected_ret, int flags)
>  	}
>  }
>  
> +/* Receive one byte and check the return value.
> + *
> + * expected_ret:
> + *  <0 Negative errno (for testing errors)
> + *   0 End-of-file
> + *   1 Success
> + */
> +void recvfrom_byte(int fd, struct sockaddr *src_addr, socklen_t *addrlen,
> +		   int expected_ret, int flags)
> +{
> +	uint8_t byte;
> +	ssize_t nread;
> +
> +	timeout_begin(TIMEOUT);
> +	do {
> +		nread = recvfrom(fd, &byte, sizeof(byte), flags, src_addr, addrlen);
> +		timeout_check("read");
> +	} while (nread < 0 && errno == EINTR);
> +	timeout_end();
> +
> +	if (expected_ret < 0) {
> +		if (nread != -1) {
> +			fprintf(stderr, "bogus recvfrom(2) return value %zd\n",
> +				nread);
> +			exit(EXIT_FAILURE);
> +		}
> +		if (errno != -expected_ret) {
> +			perror("read");
> +			exit(EXIT_FAILURE);
> +		}
> +		return;
> +	}
> +
> +	if (nread < 0) {
> +		perror("read");
> +		exit(EXIT_FAILURE);
> +	}
> +	if (nread == 0) {
> +		if (expected_ret == 0)
> +			return;
> +
> +		fprintf(stderr, "unexpected EOF while receiving byte\n");
> +		exit(EXIT_FAILURE);
> +	}
> +	if (nread != sizeof(byte)) {
> +		fprintf(stderr, "bogus recvfrom(2) return value %zd\n", nread);
> +		exit(EXIT_FAILURE);
> +	}
> +	if (byte != 'A') {
> +		fprintf(stderr, "unexpected byte read %c\n", byte);
> +		exit(EXIT_FAILURE);
> +	}
> +}
> +
>  /* Run test cases.  The program terminates if a failure occurs. */
>  void run_tests(const struct test_case *test_cases,
>  	       const struct test_opts *opts)
> diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
> index fb99208a95ea..6e5cd610bf05 100644
> --- a/tools/testing/vsock/util.h
> +++ b/tools/testing/vsock/util.h
> @@ -43,7 +43,11 @@ int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
>  			   struct sockaddr_vm *clientaddrp);
>  void vsock_wait_remote_close(int fd);
>  void send_byte(int fd, int expected_ret, int flags);
> +void sendto_byte(int fd, const struct sockaddr *dest_addr, int len, int expected_ret,
> +		 int flags);
>  void recv_byte(int fd, int expected_ret, int flags);
> +void recvfrom_byte(int fd, struct sockaddr *src_addr, socklen_t *addrlen,
> +		   int expected_ret, int flags);
>  void run_tests(const struct test_case *test_cases,
>  	       const struct test_opts *opts);
>  void list_tests(const struct test_case *test_cases);
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index ac1bd3ac1533..851c3d65178d 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -202,6 +202,113 @@ static void test_stream_server_close_server(const struct test_opts *opts)
>  	close(fd);
>  }
>  
> +static void test_dgram_sendto_client(const struct test_opts *opts)
> +{
> +	union {
> +		struct sockaddr sa;
> +		struct sockaddr_vm svm;
> +	} addr = {
> +		.svm = {
> +			.svm_family = AF_VSOCK,
> +			.svm_port = 1234,
> +			.svm_cid = opts->peer_cid,
> +		},
> +	};
> +	int fd;
> +
> +	/* Wait for the server to be ready */
> +	control_expectln("BIND");
> +
> +	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
> +	if (fd < 0) {
> +		perror("socket");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	sendto_byte(fd, &addr.sa, sizeof(addr.svm), 1, 0);
> +
> +	/* Notify the server that the client has finished */
> +	control_writeln("DONE");
> +
> +	close(fd);
> +}
> +
> +static void test_dgram_sendto_server(const struct test_opts *opts)
> +{
> +	union {
> +		struct sockaddr sa;
> +		struct sockaddr_vm svm;
> +	} addr = {
> +		.svm = {
> +			.svm_family = AF_VSOCK,
> +			.svm_port = 1234,
> +			.svm_cid = VMADDR_CID_ANY,
> +		},
> +	};
> +	int fd;
> +	int len = sizeof(addr.sa);
> +
> +	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
> 
> 
> 
> ^^^
> I think we can check 'socket()' return value;
> 
> 
> 
> 
> +
> +	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
> +		perror("bind");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	/* Notify the client that the server is ready */
> +	control_writeln("BIND");
> +
> +	recvfrom_byte(fd, &addr.sa, &len, 1, 0);
> +
> +	/* Wait for the client to finish */
> +	control_expectln("DONE");
> +
> +	close(fd);
> +}
> +
> +static void test_dgram_connect_client(const struct test_opts *opts)
> +{
> +	union {
> +		struct sockaddr sa;
> +		struct sockaddr_vm svm;
> +	} addr = {
> +		.svm = {
> +			.svm_family = AF_VSOCK,
> +			.svm_port = 1234,
> +			.svm_cid = opts->peer_cid,
> +		},
> +	};
> +	int fd;
> +	int ret;
> +
> +	/* Wait for the server to be ready */
> +	control_expectln("BIND");
> +
> +	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
> +	if (fd < 0) {
> +		perror("bind");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	ret = connect(fd, &addr.sa, sizeof(addr.svm));
> +	if (ret < 0) {
> +		perror("connect");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	send_byte(fd, 1, 0);
> +
> +	/* Notify the server that the client has finished */
> +	control_writeln("DONE");
> +
> +	close(fd);
> +}
> +
> +static void test_dgram_connect_server(const struct test_opts *opts)
> +{
> +	test_dgram_sendto_server(opts);
> +}
> +
>  /* With the standard socket sizes, VMCI is able to support about 100
>   * concurrent stream connections.
>   */
> @@ -255,6 +362,77 @@ static void test_stream_multiconn_server(const struct test_opts *opts)
>  		close(fds[i]);
>  }
>  
> +static void test_dgram_multiconn_client(const struct test_opts *opts)
> +{
> +	int fds[MULTICONN_NFDS];
> +	int i;
> +	union {
> +		struct sockaddr sa;
> +		struct sockaddr_vm svm;
> +	} addr = {
> +		.svm = {
> +			.svm_family = AF_VSOCK,
> +			.svm_port = 1234,
> +			.svm_cid = opts->peer_cid,
> +		},
> +	};
> +
> +	/* Wait for the server to be ready */
> +	control_expectln("BIND");
> +
> +	for (i = 0; i < MULTICONN_NFDS; i++) {
> +		fds[i] = socket(AF_VSOCK, SOCK_DGRAM, 0);
> +		if (fds[i] < 0) {
> +			perror("socket");
> +			exit(EXIT_FAILURE);
> +		}
> +	}
> +
> +	for (i = 0; i < MULTICONN_NFDS; i++)
> +		sendto_byte(fds[i], &addr.sa, sizeof(addr.svm), 1, 0);
> +
> +	/* Notify the server that the client has finished */
> +	control_writeln("DONE");
> +
> +	for (i = 0; i < MULTICONN_NFDS; i++)
> +		close(fds[i]);
> +}
> +
> +static void test_dgram_multiconn_server(const struct test_opts *opts)
> +{
> +	union {
> +		struct sockaddr sa;
> +		struct sockaddr_vm svm;
> +	} addr = {
> +		.svm = {
> +			.svm_family = AF_VSOCK,
> +			.svm_port = 1234,
> +			.svm_cid = VMADDR_CID_ANY,
> +		},
> +	};
> +	int fd;
> +	int len = sizeof(addr.sa);
> +	int i;
> +
> +	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
> 
> 
> 
> ^^^
> I think we can check 'socket()' return value;
> 
> 
> 
> +
> +	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
> +		perror("bind");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	/* Notify the client that the server is ready */
> +	control_writeln("BIND");
> +
> +	for (i = 0; i < MULTICONN_NFDS; i++)
> +		recvfrom_byte(fd, &addr.sa, &len, 1, 0);
> +
> +	/* Wait for the client to finish */
> +	control_expectln("DONE");
> +
> +	close(fd);
> +}
> +
>  static void test_stream_msg_peek_client(const struct test_opts *opts)
>  {
>  	int fd;
> @@ -1128,6 +1306,21 @@ static struct test_case test_cases[] = {
>  		.run_client = test_stream_virtio_skb_merge_client,
>  		.run_server = test_stream_virtio_skb_merge_server,
>  	},
> +	{
> +		.name = "SOCK_DGRAM client close",
> +		.run_client = test_dgram_sendto_client,
> +		.run_server = test_dgram_sendto_server,
> +	},
> +	{
> +		.name = "SOCK_DGRAM client connect",
> +		.run_client = test_dgram_connect_client,
> +		.run_server = test_dgram_connect_server,
> +	},
> +	{
> +		.name = "SOCK_DGRAM multiple connections",
> +		.run_client = test_dgram_multiconn_client,
> +		.run_server = test_dgram_multiconn_server,
> +	},
> 
> 
> 
> 
> SOCK_DGRAM guarantees message bounds, I think it will be good to add such test like in SOCK_SEQPACKET tests.
> 
> Thanks, Arseniy
> 
> 
>  	{},
>  };
>  
> 

Re: [PATCH RFC net-next v3 8/8] tests: add vsock dgram tests

[Bobby Eshleman]

On Tue, Jun 06, 2023 at 12:34:22PM +0300, Arseniy Krasnov wrote:
> Sorry, CC mailing lists
> 
> On 06.06.2023 12:29, Arseniy Krasnov wrote:
> > Hello Bobby and Jiang! Small remarks(sorry for this letter layout, I add multiple newline over comments):
> > 

Hey Arseniy!

> > diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> > index 01b636d3039a..45e35da48b40 100644
> > --- a/tools/testing/vsock/util.c
> > +++ b/tools/testing/vsock/util.c
> > @@ -260,6 +260,57 @@ void send_byte(int fd, int expected_ret, int flags)
> >  	}
> >  }
> >  
> > +/* Transmit one byte and check the return value.
> > + *
> > + * expected_ret:
> > + *  <0 Negative errno (for testing errors)
> > + *   0 End-of-file
> > + *   1 Success
> > + */
> > +void sendto_byte(int fd, const struct sockaddr *dest_addr, int len, int expected_ret,
> > +		 int flags)
> > +{
> > +	const uint8_t byte = 'A';
> > +	ssize_t nwritten;
> > +
> > +	timeout_begin(TIMEOUT);
> > +	do {
> > +		nwritten = sendto(fd, &byte, sizeof(byte), flags, dest_addr,
> > +				  len);
> > +		timeout_check("write");
> > +	} while (nwritten < 0 && errno == EINTR);
> > +	timeout_end();
> > +
> > +	if (expected_ret < 0) {
> > +		if (nwritten != -1) {
> > +			fprintf(stderr, "bogus sendto(2) return value %zd\n",
> > +				nwritten);
> > +			exit(EXIT_FAILURE);
> > +		}
> > +		if (errno != -expected_ret) {
> > +			perror("write");
> > +			exit(EXIT_FAILURE);
> > +		}
> > +		return;
> > +	}
> > +
> > +	if (nwritten < 0) {
> > +		perror("write");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +	if (nwritten == 0) {
> > +		if (expected_ret == 0)
> > +			return;
> > +
> > +		fprintf(stderr, "unexpected EOF while sending byte\n");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +	if (nwritten != sizeof(byte)) {
> > +		fprintf(stderr, "bogus sendto(2) return value %zd\n", nwritten);
> > +		exit(EXIT_FAILURE);
> > +
> > 	}
> > 
> > 
> > 
> > ^^^
> > May be short check that 'nwritten' != 'expected_ret' will be enough? Then print expected and
> > real value. Here and in 'recvfrom_byte()' below.
> > 

Right now this is really just a copy/paste of the send_byte() that
stream uses, so that would probably make the two report errors slightly
different. If desired for some specific reason, I'm open to it.

> > 
> > 
> > 
> > +}
> > +
> >  /* Receive one byte and check the return value.
> >   *
> >   * expected_ret:
> > @@ -313,6 +364,60 @@ void recv_byte(int fd, int expected_ret, int flags)
> >  	}
> >  }
> >  
> > +/* Receive one byte and check the return value.
> > + *
> > + * expected_ret:
> > + *  <0 Negative errno (for testing errors)
> > + *   0 End-of-file
> > + *   1 Success
> > + */
> > +void recvfrom_byte(int fd, struct sockaddr *src_addr, socklen_t *addrlen,
> > +		   int expected_ret, int flags)
> > +{
> > +	uint8_t byte;
> > +	ssize_t nread;
> > +
> > +	timeout_begin(TIMEOUT);
> > +	do {
> > +		nread = recvfrom(fd, &byte, sizeof(byte), flags, src_addr, addrlen);
> > +		timeout_check("read");
> > +	} while (nread < 0 && errno == EINTR);
> > +	timeout_end();
> > +
> > +	if (expected_ret < 0) {
> > +		if (nread != -1) {
> > +			fprintf(stderr, "bogus recvfrom(2) return value %zd\n",
> > +				nread);
> > +			exit(EXIT_FAILURE);
> > +		}
> > +		if (errno != -expected_ret) {
> > +			perror("read");
> > +			exit(EXIT_FAILURE);
> > +		}
> > +		return;
> > +	}
> > +
> > +	if (nread < 0) {
> > +		perror("read");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +	if (nread == 0) {
> > +		if (expected_ret == 0)
> > +			return;
> > +
> > +		fprintf(stderr, "unexpected EOF while receiving byte\n");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +	if (nread != sizeof(byte)) {
> > +		fprintf(stderr, "bogus recvfrom(2) return value %zd\n", nread);
> > +		exit(EXIT_FAILURE);
> > +	}
> > +	if (byte != 'A') {
> > +		fprintf(stderr, "unexpected byte read %c\n", byte);
> > +		exit(EXIT_FAILURE);
> > +	}
> > +}
> > +
> >  /* Run test cases.  The program terminates if a failure occurs. */
> >  void run_tests(const struct test_case *test_cases,
> >  	       const struct test_opts *opts)
> > diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
> > index fb99208a95ea..6e5cd610bf05 100644
> > --- a/tools/testing/vsock/util.h
> > +++ b/tools/testing/vsock/util.h
> > @@ -43,7 +43,11 @@ int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
> >  			   struct sockaddr_vm *clientaddrp);
> >  void vsock_wait_remote_close(int fd);
> >  void send_byte(int fd, int expected_ret, int flags);
> > +void sendto_byte(int fd, const struct sockaddr *dest_addr, int len, int expected_ret,
> > +		 int flags);
> >  void recv_byte(int fd, int expected_ret, int flags);
> > +void recvfrom_byte(int fd, struct sockaddr *src_addr, socklen_t *addrlen,
> > +		   int expected_ret, int flags);
> >  void run_tests(const struct test_case *test_cases,
> >  	       const struct test_opts *opts);
> >  void list_tests(const struct test_case *test_cases);
> > diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> > index ac1bd3ac1533..851c3d65178d 100644
> > --- a/tools/testing/vsock/vsock_test.c
> > +++ b/tools/testing/vsock/vsock_test.c
> > @@ -202,6 +202,113 @@ static void test_stream_server_close_server(const struct test_opts *opts)
> >  	close(fd);
> >  }
> >  
> > +static void test_dgram_sendto_client(const struct test_opts *opts)
> > +{
> > +	union {
> > +		struct sockaddr sa;
> > +		struct sockaddr_vm svm;
> > +	} addr = {
> > +		.svm = {
> > +			.svm_family = AF_VSOCK,
> > +			.svm_port = 1234,
> > +			.svm_cid = opts->peer_cid,
> > +		},
> > +	};
> > +	int fd;
> > +
> > +	/* Wait for the server to be ready */
> > +	control_expectln("BIND");
> > +
> > +	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
> > +	if (fd < 0) {
> > +		perror("socket");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +
> > +	sendto_byte(fd, &addr.sa, sizeof(addr.svm), 1, 0);
> > +
> > +	/* Notify the server that the client has finished */
> > +	control_writeln("DONE");
> > +
> > +	close(fd);
> > +}
> > +
> > +static void test_dgram_sendto_server(const struct test_opts *opts)
> > +{
> > +	union {
> > +		struct sockaddr sa;
> > +		struct sockaddr_vm svm;
> > +	} addr = {
> > +		.svm = {
> > +			.svm_family = AF_VSOCK,
> > +			.svm_port = 1234,
> > +			.svm_cid = VMADDR_CID_ANY,
> > +		},
> > +	};
> > +	int fd;
> > +	int len = sizeof(addr.sa);
> > +
> > +	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
> > 
> > 
> > 
> > ^^^
> > I think we can check 'socket()' return value;
> > 

Gotcha, I'll add in next rev.

> > 
> > 
> > 
> > +
> > +	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
> > +		perror("bind");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +
> > +	/* Notify the client that the server is ready */
> > +	control_writeln("BIND");
> > +
> > +	recvfrom_byte(fd, &addr.sa, &len, 1, 0);
> > +
> > +	/* Wait for the client to finish */
> > +	control_expectln("DONE");
> > +
> > +	close(fd);
> > +}
> > +
> > +static void test_dgram_connect_client(const struct test_opts *opts)
> > +{
> > +	union {
> > +		struct sockaddr sa;
> > +		struct sockaddr_vm svm;
> > +	} addr = {
> > +		.svm = {
> > +			.svm_family = AF_VSOCK,
> > +			.svm_port = 1234,
> > +			.svm_cid = opts->peer_cid,
> > +		},
> > +	};
> > +	int fd;
> > +	int ret;
> > +
> > +	/* Wait for the server to be ready */
> > +	control_expectln("BIND");
> > +
> > +	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
> > +	if (fd < 0) {
> > +		perror("bind");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +
> > +	ret = connect(fd, &addr.sa, sizeof(addr.svm));
> > +	if (ret < 0) {
> > +		perror("connect");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +
> > +	send_byte(fd, 1, 0);
> > +
> > +	/* Notify the server that the client has finished */
> > +	control_writeln("DONE");
> > +
> > +	close(fd);
> > +}
> > +
> > +static void test_dgram_connect_server(const struct test_opts *opts)
> > +{
> > +	test_dgram_sendto_server(opts);
> > +}
> > +
> >  /* With the standard socket sizes, VMCI is able to support about 100
> >   * concurrent stream connections.
> >   */
> > @@ -255,6 +362,77 @@ static void test_stream_multiconn_server(const struct test_opts *opts)
> >  		close(fds[i]);
> >  }
> >  
> > +static void test_dgram_multiconn_client(const struct test_opts *opts)
> > +{
> > +	int fds[MULTICONN_NFDS];
> > +	int i;
> > +	union {
> > +		struct sockaddr sa;
> > +		struct sockaddr_vm svm;
> > +	} addr = {
> > +		.svm = {
> > +			.svm_family = AF_VSOCK,
> > +			.svm_port = 1234,
> > +			.svm_cid = opts->peer_cid,
> > +		},
> > +	};
> > +
> > +	/* Wait for the server to be ready */
> > +	control_expectln("BIND");
> > +
> > +	for (i = 0; i < MULTICONN_NFDS; i++) {
> > +		fds[i] = socket(AF_VSOCK, SOCK_DGRAM, 0);
> > +		if (fds[i] < 0) {
> > +			perror("socket");
> > +			exit(EXIT_FAILURE);
> > +		}
> > +	}
> > +
> > +	for (i = 0; i < MULTICONN_NFDS; i++)
> > +		sendto_byte(fds[i], &addr.sa, sizeof(addr.svm), 1, 0);
> > +
> > +	/* Notify the server that the client has finished */
> > +	control_writeln("DONE");
> > +
> > +	for (i = 0; i < MULTICONN_NFDS; i++)
> > +		close(fds[i]);
> > +}
> > +
> > +static void test_dgram_multiconn_server(const struct test_opts *opts)
> > +{
> > +	union {
> > +		struct sockaddr sa;
> > +		struct sockaddr_vm svm;
> > +	} addr = {
> > +		.svm = {
> > +			.svm_family = AF_VSOCK,
> > +			.svm_port = 1234,
> > +			.svm_cid = VMADDR_CID_ANY,
> > +		},
> > +	};
> > +	int fd;
> > +	int len = sizeof(addr.sa);
> > +	int i;
> > +
> > +	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
> > 
> > 
> > 
> > ^^^
> > I think we can check 'socket()' return value;
> > 
> > 
> > 
> > +
> > +	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
> > +		perror("bind");
> > +		exit(EXIT_FAILURE);
> > +	}
> > +
> > +	/* Notify the client that the server is ready */
> > +	control_writeln("BIND");
> > +
> > +	for (i = 0; i < MULTICONN_NFDS; i++)
> > +		recvfrom_byte(fd, &addr.sa, &len, 1, 0);
> > +
> > +	/* Wait for the client to finish */
> > +	control_expectln("DONE");
> > +
> > +	close(fd);
> > +}
> > +
> >  static void test_stream_msg_peek_client(const struct test_opts *opts)
> >  {
> >  	int fd;
> > @@ -1128,6 +1306,21 @@ static struct test_case test_cases[] = {
> >  		.run_client = test_stream_virtio_skb_merge_client,
> >  		.run_server = test_stream_virtio_skb_merge_server,
> >  	},
> > +	{
> > +		.name = "SOCK_DGRAM client close",
> > +		.run_client = test_dgram_sendto_client,
> > +		.run_server = test_dgram_sendto_server,
> > +	},
> > +	{
> > +		.name = "SOCK_DGRAM client connect",
> > +		.run_client = test_dgram_connect_client,
> > +		.run_server = test_dgram_connect_server,
> > +	},
> > +	{
> > +		.name = "SOCK_DGRAM multiple connections",
> > +		.run_client = test_dgram_multiconn_client,
> > +		.run_server = test_dgram_multiconn_server,
> > +	},
> > 
> > 
> > 
> > 
> > SOCK_DGRAM guarantees message bounds, I think it will be good to add such test like in SOCK_SEQPACKET tests.

Agreed, I'll write one for the next rev.

> > 
> > Thanks, Arseniy

Thanks for the review!
> > 
> > 
> >  	{},
> >  };
> >  
> > 

[PATCH RFC net-next v3 0/8] virtio/vsock: support datagrams

[Bobby Eshleman]

Hey all!

This series introduces support for datagrams to virtio/vsock.

It is a spin-off (and smaller version) of this series from the summer: 
  https://lore.kernel.org/all/cover.1660362668.git.bobby.eshleman@bytedance.com/

Please note that this is an RFC and should not be merged until
associated changes are made to the virtio specification, which will
follow after discussion from this series.

This series first supports datagrams in a basic form for virtio, and
then optimizes the sendpath for all transports.

The result is a very fast datagram communication protocol that
outperforms even UDP on multi-queue virtio-net w/ vhost on a variety
of multi-threaded workload samples.

For those that are curious, some summary data comparing UDP and VSOCK
DGRAM (N=5):

	vCPUS: 16
	virtio-net queues: 16
	payload size: 4KB
	Setup: bare metal + vm (non-nested)

	UDP: 287.59 MB/s
	VSOCK DGRAM: 509.2 MB/s

Some notes about the implementation...

This datagram implementation forces datagrams to self-throttle according
to the threshold set by sk_sndbuf. It behaves similar to the credits
used by streams in its effect on throughput and memory consumption, but
it is not influenced by the receiving socket as credits are.

The device drops packets silently.

As discussed previously, this series introduces datagrams and defers
fairness to future work. See discussion in v2 for more context around
datagrams, fairness, and this implementation.

Signed-off-by: Bobby Eshleman <bobby.eshleman@bytedance.com>
---
Changes in v3:
- Support multi-transport dgram, changing logic in connect/bind
  to support VMCI case
- Support per-pkt transport lookup for sendto() case
- Fix dgram_allow() implementation
- Fix dgram feature bit number (now it is 3)
- Fix binding so dgram and connectible (cid,port) spaces are
  non-overlapping
- RCU protect transport ptr so connect() calls never leave
  a lockless read of the transport and remote_addr are always
  in sync
- Link to v2: https://lore.kernel.org/r/20230413-b4-vsock-dgram-v2-0-079cc7cee62e@bytedance.com

---
Bobby Eshleman (7):
      vsock/dgram: generalize recvmsg and drop transport->dgram_dequeue
      vsock: refactor transport lookup code
      vsock: support multi-transport datagrams
      vsock: make vsock bind reusable
      virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
      virtio/vsock: support dgrams
      vsock: Add lockless sendmsg() support

Jiang Wang (1):
      tests: add vsock dgram tests

 drivers/vhost/vsock.c                   |  44 ++-
 include/linux/virtio_vsock.h            |  13 +-
 include/net/af_vsock.h                  |  53 ++-
 include/uapi/linux/virtio_vsock.h       |   2 +
 net/vmw_vsock/af_vsock.c                | 615 ++++++++++++++++++++++++++------
 net/vmw_vsock/diag.c                    |  10 +-
 net/vmw_vsock/hyperv_transport.c        |  42 ++-
 net/vmw_vsock/virtio_transport.c        |  28 +-
 net/vmw_vsock/virtio_transport_common.c | 227 +++++++++---
 net/vmw_vsock/vmci_transport.c          | 152 ++++----
 net/vmw_vsock/vsock_bpf.c               |  10 +-
 net/vmw_vsock/vsock_loopback.c          |  13 +-
 tools/testing/vsock/util.c              | 105 ++++++
 tools/testing/vsock/util.h              |   4 +
 tools/testing/vsock/vsock_test.c        | 193 ++++++++++
 15 files changed, 1259 insertions(+), 252 deletions(-)
---
base-commit: ed72bd5a6790a0c3747cb32b0427f921bd03bb71
change-id: 20230413-b4-vsock-dgram-3b6eba6a64e5

Best regards,
-- 
Bobby Eshleman <bobby.eshleman@bytedance.com>

[PATCH RFC net-next v3 8/8] tests: add vsock dgram tests

[Bobby Eshleman]

From: Jiang Wang <jiang.wang@bytedance.com>

This patch adds tests for vsock datagram.

Signed-off-by: Bobby Eshleman <bobby.eshleman@bytedance.com>
Signed-off-by: Jiang Wang <jiang.wang@bytedance.com>
---
 tools/testing/vsock/util.c       | 105 +++++++++++++++++++++
 tools/testing/vsock/util.h       |   4 +
 tools/testing/vsock/vsock_test.c | 193 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 302 insertions(+)

diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 01b636d3039a..45e35da48b40 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -260,6 +260,57 @@ void send_byte(int fd, int expected_ret, int flags)
 	}
 }
 
+/* Transmit one byte and check the return value.
+ *
+ * expected_ret:
+ *  <0 Negative errno (for testing errors)
+ *   0 End-of-file
+ *   1 Success
+ */
+void sendto_byte(int fd, const struct sockaddr *dest_addr, int len, int expected_ret,
+		 int flags)
+{
+	const uint8_t byte = 'A';
+	ssize_t nwritten;
+
+	timeout_begin(TIMEOUT);
+	do {
+		nwritten = sendto(fd, &byte, sizeof(byte), flags, dest_addr,
+				  len);
+		timeout_check("write");
+	} while (nwritten < 0 && errno == EINTR);
+	timeout_end();
+
+	if (expected_ret < 0) {
+		if (nwritten != -1) {
+			fprintf(stderr, "bogus sendto(2) return value %zd\n",
+				nwritten);
+			exit(EXIT_FAILURE);
+		}
+		if (errno != -expected_ret) {
+			perror("write");
+			exit(EXIT_FAILURE);
+		}
+		return;
+	}
+
+	if (nwritten < 0) {
+		perror("write");
+		exit(EXIT_FAILURE);
+	}
+	if (nwritten == 0) {
+		if (expected_ret == 0)
+			return;
+
+		fprintf(stderr, "unexpected EOF while sending byte\n");
+		exit(EXIT_FAILURE);
+	}
+	if (nwritten != sizeof(byte)) {
+		fprintf(stderr, "bogus sendto(2) return value %zd\n", nwritten);
+		exit(EXIT_FAILURE);
+	}
+}
+
 /* Receive one byte and check the return value.
  *
  * expected_ret:
@@ -313,6 +364,60 @@ void recv_byte(int fd, int expected_ret, int flags)
 	}
 }
 
+/* Receive one byte and check the return value.
+ *
+ * expected_ret:
+ *  <0 Negative errno (for testing errors)
+ *   0 End-of-file
+ *   1 Success
+ */
+void recvfrom_byte(int fd, struct sockaddr *src_addr, socklen_t *addrlen,
+		   int expected_ret, int flags)
+{
+	uint8_t byte;
+	ssize_t nread;
+
+	timeout_begin(TIMEOUT);
+	do {
+		nread = recvfrom(fd, &byte, sizeof(byte), flags, src_addr, addrlen);
+		timeout_check("read");
+	} while (nread < 0 && errno == EINTR);
+	timeout_end();
+
+	if (expected_ret < 0) {
+		if (nread != -1) {
+			fprintf(stderr, "bogus recvfrom(2) return value %zd\n",
+				nread);
+			exit(EXIT_FAILURE);
+		}
+		if (errno != -expected_ret) {
+			perror("read");
+			exit(EXIT_FAILURE);
+		}
+		return;
+	}
+
+	if (nread < 0) {
+		perror("read");
+		exit(EXIT_FAILURE);
+	}
+	if (nread == 0) {
+		if (expected_ret == 0)
+			return;
+
+		fprintf(stderr, "unexpected EOF while receiving byte\n");
+		exit(EXIT_FAILURE);
+	}
+	if (nread != sizeof(byte)) {
+		fprintf(stderr, "bogus recvfrom(2) return value %zd\n", nread);
+		exit(EXIT_FAILURE);
+	}
+	if (byte != 'A') {
+		fprintf(stderr, "unexpected byte read %c\n", byte);
+		exit(EXIT_FAILURE);
+	}
+}
+
 /* Run test cases.  The program terminates if a failure occurs. */
 void run_tests(const struct test_case *test_cases,
 	       const struct test_opts *opts)
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index fb99208a95ea..6e5cd610bf05 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -43,7 +43,11 @@ int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
 			   struct sockaddr_vm *clientaddrp);
 void vsock_wait_remote_close(int fd);
 void send_byte(int fd, int expected_ret, int flags);
+void sendto_byte(int fd, const struct sockaddr *dest_addr, int len, int expected_ret,
+		 int flags);
 void recv_byte(int fd, int expected_ret, int flags);
+void recvfrom_byte(int fd, struct sockaddr *src_addr, socklen_t *addrlen,
+		   int expected_ret, int flags);
 void run_tests(const struct test_case *test_cases,
 	       const struct test_opts *opts);
 void list_tests(const struct test_case *test_cases);
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index ac1bd3ac1533..851c3d65178d 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -202,6 +202,113 @@ static void test_stream_server_close_server(const struct test_opts *opts)
 	close(fd);
 }
 
+static void test_dgram_sendto_client(const struct test_opts *opts)
+{
+	union {
+		struct sockaddr sa;
+		struct sockaddr_vm svm;
+	} addr = {
+		.svm = {
+			.svm_family = AF_VSOCK,
+			.svm_port = 1234,
+			.svm_cid = opts->peer_cid,
+		},
+	};
+	int fd;
+
+	/* Wait for the server to be ready */
+	control_expectln("BIND");
+
+	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
+	if (fd < 0) {
+		perror("socket");
+		exit(EXIT_FAILURE);
+	}
+
+	sendto_byte(fd, &addr.sa, sizeof(addr.svm), 1, 0);
+
+	/* Notify the server that the client has finished */
+	control_writeln("DONE");
+
+	close(fd);
+}
+
+static void test_dgram_sendto_server(const struct test_opts *opts)
+{
+	union {
+		struct sockaddr sa;
+		struct sockaddr_vm svm;
+	} addr = {
+		.svm = {
+			.svm_family = AF_VSOCK,
+			.svm_port = 1234,
+			.svm_cid = VMADDR_CID_ANY,
+		},
+	};
+	int fd;
+	int len = sizeof(addr.sa);
+
+	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
+
+	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
+		perror("bind");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Notify the client that the server is ready */
+	control_writeln("BIND");
+
+	recvfrom_byte(fd, &addr.sa, &len, 1, 0);
+
+	/* Wait for the client to finish */
+	control_expectln("DONE");
+
+	close(fd);
+}
+
+static void test_dgram_connect_client(const struct test_opts *opts)
+{
+	union {
+		struct sockaddr sa;
+		struct sockaddr_vm svm;
+	} addr = {
+		.svm = {
+			.svm_family = AF_VSOCK,
+			.svm_port = 1234,
+			.svm_cid = opts->peer_cid,
+		},
+	};
+	int fd;
+	int ret;
+
+	/* Wait for the server to be ready */
+	control_expectln("BIND");
+
+	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
+	if (fd < 0) {
+		perror("bind");
+		exit(EXIT_FAILURE);
+	}
+
+	ret = connect(fd, &addr.sa, sizeof(addr.svm));
+	if (ret < 0) {
+		perror("connect");
+		exit(EXIT_FAILURE);
+	}
+
+	send_byte(fd, 1, 0);
+
+	/* Notify the server that the client has finished */
+	control_writeln("DONE");
+
+	close(fd);
+}
+
+static void test_dgram_connect_server(const struct test_opts *opts)
+{
+	test_dgram_sendto_server(opts);
+}
+
 /* With the standard socket sizes, VMCI is able to support about 100
  * concurrent stream connections.
  */
@@ -255,6 +362,77 @@ static void test_stream_multiconn_server(const struct test_opts *opts)
 		close(fds[i]);
 }
 
+static void test_dgram_multiconn_client(const struct test_opts *opts)
+{
+	int fds[MULTICONN_NFDS];
+	int i;
+	union {
+		struct sockaddr sa;
+		struct sockaddr_vm svm;
+	} addr = {
+		.svm = {
+			.svm_family = AF_VSOCK,
+			.svm_port = 1234,
+			.svm_cid = opts->peer_cid,
+		},
+	};
+
+	/* Wait for the server to be ready */
+	control_expectln("BIND");
+
+	for (i = 0; i < MULTICONN_NFDS; i++) {
+		fds[i] = socket(AF_VSOCK, SOCK_DGRAM, 0);
+		if (fds[i] < 0) {
+			perror("socket");
+			exit(EXIT_FAILURE);
+		}
+	}
+
+	for (i = 0; i < MULTICONN_NFDS; i++)
+		sendto_byte(fds[i], &addr.sa, sizeof(addr.svm), 1, 0);
+
+	/* Notify the server that the client has finished */
+	control_writeln("DONE");
+
+	for (i = 0; i < MULTICONN_NFDS; i++)
+		close(fds[i]);
+}
+
+static void test_dgram_multiconn_server(const struct test_opts *opts)
+{
+	union {
+		struct sockaddr sa;
+		struct sockaddr_vm svm;
+	} addr = {
+		.svm = {
+			.svm_family = AF_VSOCK,
+			.svm_port = 1234,
+			.svm_cid = VMADDR_CID_ANY,
+		},
+	};
+	int fd;
+	int len = sizeof(addr.sa);
+	int i;
+
+	fd = socket(AF_VSOCK, SOCK_DGRAM, 0);
+
+	if (bind(fd, &addr.sa, sizeof(addr.svm)) < 0) {
+		perror("bind");
+		exit(EXIT_FAILURE);
+	}
+
+	/* Notify the client that the server is ready */
+	control_writeln("BIND");
+
+	for (i = 0; i < MULTICONN_NFDS; i++)
+		recvfrom_byte(fd, &addr.sa, &len, 1, 0);
+
+	/* Wait for the client to finish */
+	control_expectln("DONE");
+
+	close(fd);
+}
+
 static void test_stream_msg_peek_client(const struct test_opts *opts)
 {
 	int fd;
@@ -1128,6 +1306,21 @@ static struct test_case test_cases[] = {
 		.run_client = test_stream_virtio_skb_merge_client,
 		.run_server = test_stream_virtio_skb_merge_server,
 	},
+	{
+		.name = "SOCK_DGRAM client close",
+		.run_client = test_dgram_sendto_client,
+		.run_server = test_dgram_sendto_server,
+	},
+	{
+		.name = "SOCK_DGRAM client connect",
+		.run_client = test_dgram_connect_client,
+		.run_server = test_dgram_connect_server,
+	},
+	{
+		.name = "SOCK_DGRAM multiple connections",
+		.run_client = test_dgram_multiconn_client,
+		.run_server = test_dgram_multiconn_server,
+	},
 	{},
 };
 

-- 
2.30.2