Commit 0e21a47c authored by Paolo Abeni's avatar Paolo Abeni
Browse files

Merge branch 'vsock-test-fix-wrong-setsockopt-parameters' 

Konstantin Shkolnyy says:

====================
vsock/test: fix wrong setsockopt() parameters

Parameters were created using wrong C types, which caused them to be of
wrong size on some architectures, causing problems.

The problem with SO_RCVLOWAT was found on s390 (big endian), while x86-64
didn't show it. After the fix, all tests pass on s390.
Then Stefano Garzarella pointed out that SO_VM_SOCKETS_* calls might have
a similar problem, which turned out to be true, hence, the second patch.

Changes for v8:
- Fix whitespace warnings from "checkpatch.pl --strict"
- Add maintainers to Cc:
Changes for v7:
- Rebase on top of https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git
- Add the "net" tags to the subjects
Changes for v6:
- rework the patch #3 to avoid creating a new file for new functions,
and exclude vsock_perf from calling the new functions.
- add "Reviewed-by:" to the patch #2.
Changes for v5:
- in the patch #2 replace the introduced uint64_t with unsigned long long
to match documentation
- add a patch #3 that verifies every setsockopt() call.
Changes for v4:
- add "Reviewed-by:" to the first patch, and add a second patch fixing
SO_VM_SOCKETS_* calls, which depends on the first one (hence, it's now
a patch series.)
Changes for v3:
- fix the same problem in vsock_perf and update commit message
Changes for v2:
- add "Fixes:" lines to the commit message
====================

Link: https://patch.msgid.link/20241203150656.287028-1-kshk@linux.ibm.com


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parents 4615855e 86814d8f

tools/testing/vsock/control.c

+3 −6
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ 


#include "timeout.h"

#include "control.h"

#include "util.h"



static int control_fd = -1;
@@ -50,7 +51,6 @@ void control_init(const char *control_host, 


	for (ai = result; ai; ai = ai->ai_next) {

		int fd;

		int val = 1;



		fd = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);

		if (fd < 0)
@@ -65,11 +65,8 @@ void control_init(const char *control_host, 
			break;

		}



		if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR,

			       &val, sizeof(val)) < 0) {

			perror("setsockopt");

			exit(EXIT_FAILURE);

		}

		setsockopt_int_check(fd, SOL_SOCKET, SO_REUSEADDR, 1,

				     "setsockopt SO_REUSEADDR");



		if (bind(fd, ai->ai_addr, ai->ai_addrlen) < 0)

			goto next;

tools/testing/vsock/msg_zerocopy_common.c

+0 −10
Original line number Diff line number Diff line
@@ -14,16 +14,6 @@ 


#include "msg_zerocopy_common.h"



void enable_so_zerocopy(int fd)

{

	int val = 1;



	if (setsockopt(fd, SOL_SOCKET, SO_ZEROCOPY, &val, sizeof(val))) {

		perror("setsockopt");

		exit(EXIT_FAILURE);

	}

}



void vsock_recv_completion(int fd, const bool *zerocopied)

{

	struct sock_extended_err *serr;

tools/testing/vsock/msg_zerocopy_common.h

+0 −1
Original line number Diff line number Diff line
@@ -12,7 +12,6 @@ 
#define VSOCK_RECVERR	1

#endif



void enable_so_zerocopy(int fd);

void vsock_recv_completion(int fd, const bool *zerocopied);



#endif /* MSG_ZEROCOPY_COMMON_H */

tools/testing/vsock/util.c

+142 −0
Original line number Diff line number Diff line
@@ -651,3 +651,145 @@ void free_test_iovec(const struct iovec *test_iovec, 


	free(iovec);

}



/* Set "unsigned long long" socket option and check that it's indeed set */

void setsockopt_ull_check(int fd, int level, int optname,

			  unsigned long long val, char const *errmsg)

{

	unsigned long long chkval;

	socklen_t chklen;

	int err;



	err = setsockopt(fd, level, optname, &val, sizeof(val));

	if (err) {

		fprintf(stderr, "setsockopt err: %s (%d)\n",

			strerror(errno), errno);

		goto fail;

	}



	chkval = ~val; /* just make storage != val */

	chklen = sizeof(chkval);



	err = getsockopt(fd, level, optname, &chkval, &chklen);

	if (err) {

		fprintf(stderr, "getsockopt err: %s (%d)\n",

			strerror(errno), errno);

		goto fail;

	}



	if (chklen != sizeof(chkval)) {

		fprintf(stderr, "size mismatch: set %zu got %d\n", sizeof(val),

			chklen);

		goto fail;

	}



	if (chkval != val) {

		fprintf(stderr, "value mismatch: set %llu got %llu\n", val,

			chkval);

		goto fail;

	}

	return;

fail:

	fprintf(stderr, "%s  val %llu\n", errmsg, val);

	exit(EXIT_FAILURE);

;

}



/* Set "int" socket option and check that it's indeed set */

void setsockopt_int_check(int fd, int level, int optname, int val,

			  char const *errmsg)

{

	int chkval;

	socklen_t chklen;

	int err;



	err = setsockopt(fd, level, optname, &val, sizeof(val));

	if (err) {

		fprintf(stderr, "setsockopt err: %s (%d)\n",

			strerror(errno), errno);

		goto fail;

	}



	chkval = ~val; /* just make storage != val */

	chklen = sizeof(chkval);



	err = getsockopt(fd, level, optname, &chkval, &chklen);

	if (err) {

		fprintf(stderr, "getsockopt err: %s (%d)\n",

			strerror(errno), errno);

		goto fail;

	}



	if (chklen != sizeof(chkval)) {

		fprintf(stderr, "size mismatch: set %zu got %d\n", sizeof(val),

			chklen);

		goto fail;

	}



	if (chkval != val) {

		fprintf(stderr, "value mismatch: set %d got %d\n", val, chkval);

		goto fail;

	}

	return;

fail:

	fprintf(stderr, "%s val %d\n", errmsg, val);

	exit(EXIT_FAILURE);

}



static void mem_invert(unsigned char *mem, size_t size)

{

	size_t i;



	for (i = 0; i < size; i++)

		mem[i] = ~mem[i];

}



/* Set "timeval" socket option and check that it's indeed set */

void setsockopt_timeval_check(int fd, int level, int optname,

			      struct timeval val, char const *errmsg)

{

	struct timeval chkval;

	socklen_t chklen;

	int err;



	err = setsockopt(fd, level, optname, &val, sizeof(val));

	if (err) {

		fprintf(stderr, "setsockopt err: %s (%d)\n",

			strerror(errno), errno);

		goto fail;

	}



	 /* just make storage != val */

	chkval = val;

	mem_invert((unsigned char *)&chkval, sizeof(chkval));

	chklen = sizeof(chkval);



	err = getsockopt(fd, level, optname, &chkval, &chklen);

	if (err) {

		fprintf(stderr, "getsockopt err: %s (%d)\n",

			strerror(errno), errno);

		goto fail;

	}



	if (chklen != sizeof(chkval)) {

		fprintf(stderr, "size mismatch: set %zu got %d\n", sizeof(val),

			chklen);

		goto fail;

	}



	if (memcmp(&chkval, &val, sizeof(val)) != 0) {

		fprintf(stderr, "value mismatch: set %ld:%ld got %ld:%ld\n",

			val.tv_sec, val.tv_usec, chkval.tv_sec, chkval.tv_usec);

		goto fail;

	}

	return;

fail:

	fprintf(stderr, "%s val %ld:%ld\n", errmsg, val.tv_sec, val.tv_usec);

	exit(EXIT_FAILURE);

}



void enable_so_zerocopy_check(int fd)

{

	setsockopt_int_check(fd, SOL_SOCKET, SO_ZEROCOPY, 1,

			     "setsockopt SO_ZEROCOPY");

}

tools/testing/vsock/util.h

+7 −0
Original line number Diff line number Diff line
@@ -68,4 +68,11 @@ unsigned long iovec_hash_djb2(const struct iovec *iov, size_t iovnum); 
struct iovec *alloc_test_iovec(const struct iovec *test_iovec, int iovnum);

void free_test_iovec(const struct iovec *test_iovec,

		     struct iovec *iovec, int iovnum);

void setsockopt_ull_check(int fd, int level, int optname,

			  unsigned long long val, char const *errmsg);

void setsockopt_int_check(int fd, int level, int optname, int val,

			  char const *errmsg);

void setsockopt_timeval_check(int fd, int level, int optname,

			      struct timeval val, char const *errmsg);

void enable_so_zerocopy_check(int fd);

#endif /* UTIL_H */