[PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[Stanislav Fomichev]

The goal of the series is to simplify and make it possible to use
ncdevmem in an automated way from the ksft python wrapper.

ncdevmem is slowly mutated into a state where it uses stdout
to print the payload and the python wrapper is added to
make sure the arrived payload matches the expected one.

v6:
- fix compilation issue in 'Unify error handling' patch (Jakub)

v5:
- properly handle errors from inet_pton() and socket() (Paolo)
- remove unneeded import from python selftest (Paolo)

v4:
- keep usage example with validation (Mina)
- fix compilation issue in one patch (s/start_queues/start_queue/)

v3:
- keep and refine the comment about ncdevmem invocation (Mina)
- add the comment about not enforcing exit status for ntuple reset (Mina)
- make configure_headersplit more robust (Mina)
- use num_queues/2 in selftest and let the users override it (Mina)
- remove memory_provider.memcpy_to_device (Mina)
- keep ksft as is (don't use -v validate flags): we are gonna
  need a --debug-disable flag to make it less chatty; otherwise
  it times out when sending too much data; so leaving it as
  a separate follow up

v2:
- don't remove validation (Mina)
- keep 5-tuple flow steering but use it only when -c is provided (Mina)
- remove separate flag for probing (Mina)
- move ncdevmem under drivers/net/hw, not drivers/net (Jakub)

Cc: Mina Almasry <almasrymina@google.com>

Stanislav Fomichev (12):
  selftests: ncdevmem: Redirect all non-payload output to stderr
  selftests: ncdevmem: Separate out dmabuf provider
  selftests: ncdevmem: Unify error handling
  selftests: ncdevmem: Make client_ip optional
  selftests: ncdevmem: Remove default arguments
  selftests: ncdevmem: Switch to AF_INET6
  selftests: ncdevmem: Properly reset flow steering
  selftests: ncdevmem: Use YNL to enable TCP header split
  selftests: ncdevmem: Remove hard-coded queue numbers
  selftests: ncdevmem: Run selftest when none of the -s or -c has been
    provided
  selftests: ncdevmem: Move ncdevmem under drivers/net/hw
  selftests: ncdevmem: Add automated test

 .../selftests/drivers/net/hw/.gitignore       |   1 +
 .../testing/selftests/drivers/net/hw/Makefile |   9 +
 .../selftests/drivers/net/hw/devmem.py        |  45 +
 .../selftests/drivers/net/hw/ncdevmem.c       | 773 ++++++++++++++++++
 tools/testing/selftests/net/.gitignore        |   1 -
 tools/testing/selftests/net/Makefile          |   8 -
 tools/testing/selftests/net/ncdevmem.c        | 570 -------------
 7 files changed, 828 insertions(+), 579 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/hw/.gitignore
 create mode 100755 tools/testing/selftests/drivers/net/hw/devmem.py
 create mode 100644 tools/testing/selftests/drivers/net/hw/ncdevmem.c
 delete mode 100644 tools/testing/selftests/net/ncdevmem.c

-- 
2.47.0

[PATCH net-next v6 01/12] selftests: ncdevmem: Redirect all non-payload output to stderr

[Stanislav Fomichev]

That should make it possible to do expected payload validation on
the caller side.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 61 +++++++++++++-------------
 1 file changed, 30 insertions(+), 31 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index 64d6805381c5..9245d3f158dd 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -88,7 +88,6 @@ void print_nonzero_bytes(void *ptr, size_t size)
 
 	for (i = 0; i < size; i++)
 		putchar(p[i]);
-	printf("\n");
 }
 
 void validate_buffer(void *line, size_t size)
@@ -120,7 +119,7 @@ void validate_buffer(void *line, size_t size)
 		char command[256];                                      \
 		memset(command, 0, sizeof(command));                    \
 		snprintf(command, sizeof(command), cmd, ##__VA_ARGS__); \
-		printf("Running: %s\n", command);                       \
+		fprintf(stderr, "Running: %s\n", command);                       \
 		system(command);                                        \
 	})
 
@@ -128,22 +127,22 @@ static int reset_flow_steering(void)
 {
 	int ret = 0;
 
-	ret = run_command("sudo ethtool -K %s ntuple off", ifname);
+	ret = run_command("sudo ethtool -K %s ntuple off >&2", ifname);
 	if (ret)
 		return ret;
 
-	return run_command("sudo ethtool -K %s ntuple on", ifname);
+	return run_command("sudo ethtool -K %s ntuple on >&2", ifname);
 }
 
 static int configure_headersplit(bool on)
 {
-	return run_command("sudo ethtool -G %s tcp-data-split %s", ifname,
+	return run_command("sudo ethtool -G %s tcp-data-split %s >&2", ifname,
 			   on ? "on" : "off");
 }
 
 static int configure_rss(void)
 {
-	return run_command("sudo ethtool -X %s equal %d", ifname, start_queue);
+	return run_command("sudo ethtool -X %s equal %d >&2", ifname, start_queue);
 }
 
 static int configure_channels(unsigned int rx, unsigned int tx)
@@ -153,7 +152,7 @@ static int configure_channels(unsigned int rx, unsigned int tx)
 
 static int configure_flow_steering(void)
 {
-	return run_command("sudo ethtool -N %s flow-type tcp4 src-ip %s dst-ip %s src-port %s dst-port %s queue %d",
+	return run_command("sudo ethtool -N %s flow-type tcp4 src-ip %s dst-ip %s src-port %s dst-port %s queue %d >&2",
 			   ifname, client_ip, server_ip, port, port, start_queue);
 }
 
@@ -187,7 +186,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd,
 		goto err_close;
 	}
 
-	printf("got dmabuf id=%d\n", rsp->id);
+	fprintf(stderr, "got dmabuf id=%d\n", rsp->id);
 	dmabuf_id = rsp->id;
 
 	netdev_bind_rx_req_free(req);
@@ -314,8 +313,8 @@ int do_server(void)
 	if (ret)
 		error(errno, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
 
-	printf("binding to address %s:%d\n", server_ip,
-	       ntohs(server_sin.sin_port));
+	fprintf(stderr, "binding to address %s:%d\n", server_ip,
+		ntohs(server_sin.sin_port));
 
 	ret = bind(socket_fd, &server_sin, sizeof(server_sin));
 	if (ret)
@@ -329,14 +328,14 @@ int do_server(void)
 
 	inet_ntop(server_sin.sin_family, &server_sin.sin_addr, buffer,
 		  sizeof(buffer));
-	printf("Waiting or connection on %s:%d\n", buffer,
-	       ntohs(server_sin.sin_port));
+	fprintf(stderr, "Waiting or connection on %s:%d\n", buffer,
+		ntohs(server_sin.sin_port));
 	client_fd = accept(socket_fd, &client_addr, &client_addr_len);
 
 	inet_ntop(client_addr.sin_family, &client_addr.sin_addr, buffer,
 		  sizeof(buffer));
-	printf("Got connection from %s:%d\n", buffer,
-	       ntohs(client_addr.sin_port));
+	fprintf(stderr, "Got connection from %s:%d\n", buffer,
+		ntohs(client_addr.sin_port));
 
 	while (1) {
 		struct iovec iov = { .iov_base = iobuf,
@@ -349,14 +348,13 @@ int do_server(void)
 		ssize_t ret;
 
 		is_devmem = false;
-		printf("\n\n");
 
 		msg.msg_iov = &iov;
 		msg.msg_iovlen = 1;
 		msg.msg_control = ctrl_data;
 		msg.msg_controllen = sizeof(ctrl_data);
 		ret = recvmsg(client_fd, &msg, MSG_SOCK_DEVMEM);
-		printf("recvmsg ret=%ld\n", ret);
+		fprintf(stderr, "recvmsg ret=%ld\n", ret);
 		if (ret < 0 && (errno == EAGAIN || errno == EWOULDBLOCK))
 			continue;
 		if (ret < 0) {
@@ -364,7 +362,7 @@ int do_server(void)
 			continue;
 		}
 		if (ret == 0) {
-			printf("client exited\n");
+			fprintf(stderr, "client exited\n");
 			goto cleanup;
 		}
 
@@ -373,7 +371,7 @@ int do_server(void)
 			if (cm->cmsg_level != SOL_SOCKET ||
 			    (cm->cmsg_type != SCM_DEVMEM_DMABUF &&
 			     cm->cmsg_type != SCM_DEVMEM_LINEAR)) {
-				fprintf(stdout, "skipping non-devmem cmsg\n");
+				fprintf(stderr, "skipping non-devmem cmsg\n");
 				continue;
 			}
 
@@ -384,7 +382,7 @@ int do_server(void)
 				/* TODO: process data copied from skb's linear
 				 * buffer.
 				 */
-				fprintf(stdout,
+				fprintf(stderr,
 					"SCM_DEVMEM_LINEAR. dmabuf_cmsg->frag_size=%u\n",
 					dmabuf_cmsg->frag_size);
 
@@ -395,12 +393,13 @@ int do_server(void)
 			token.token_count = 1;
 
 			total_received += dmabuf_cmsg->frag_size;
-			printf("received frag_page=%llu, in_page_offset=%llu, frag_offset=%llu, frag_size=%u, token=%u, total_received=%lu, dmabuf_id=%u\n",
-			       dmabuf_cmsg->frag_offset >> PAGE_SHIFT,
-			       dmabuf_cmsg->frag_offset % getpagesize(),
-			       dmabuf_cmsg->frag_offset, dmabuf_cmsg->frag_size,
-			       dmabuf_cmsg->frag_token, total_received,
-			       dmabuf_cmsg->dmabuf_id);
+			fprintf(stderr,
+				"received frag_page=%llu, in_page_offset=%llu, frag_offset=%llu, frag_size=%u, token=%u, total_received=%lu, dmabuf_id=%u\n",
+				dmabuf_cmsg->frag_offset >> PAGE_SHIFT,
+				dmabuf_cmsg->frag_offset % getpagesize(),
+				dmabuf_cmsg->frag_offset,
+				dmabuf_cmsg->frag_size, dmabuf_cmsg->frag_token,
+				total_received, dmabuf_cmsg->dmabuf_id);
 
 			if (dmabuf_cmsg->dmabuf_id != dmabuf_id)
 				error(1, 0,
@@ -438,15 +437,15 @@ int do_server(void)
 		if (!is_devmem)
 			error(1, 0, "flow steering error\n");
 
-		printf("total_received=%lu\n", total_received);
+		fprintf(stderr, "total_received=%lu\n", total_received);
 	}
 
-	fprintf(stdout, "%s: ok\n", TEST_PREFIX);
+	fprintf(stderr, "%s: ok\n", TEST_PREFIX);
 
-	fprintf(stdout, "page_aligned_frags=%lu, non_page_aligned_frags=%lu\n",
+	fprintf(stderr, "page_aligned_frags=%lu, non_page_aligned_frags=%lu\n",
 		page_aligned_frags, non_page_aligned_frags);
 
-	fprintf(stdout, "page_aligned_frags=%lu, non_page_aligned_frags=%lu\n",
+	fprintf(stderr, "page_aligned_frags=%lu, non_page_aligned_frags=%lu\n",
 		page_aligned_frags, non_page_aligned_frags);
 
 cleanup:
@@ -551,7 +550,7 @@ int main(int argc, char *argv[])
 			ifname = optarg;
 			break;
 		case '?':
-			printf("unknown option: %c\n", optopt);
+			fprintf(stderr, "unknown option: %c\n", optopt);
 			break;
 		}
 	}
@@ -559,7 +558,7 @@ int main(int argc, char *argv[])
 	ifindex = if_nametoindex(ifname);
 
 	for (; optind < argc; optind++)
-		printf("extra arguments: %s\n", argv[optind]);
+		fprintf(stderr, "extra arguments: %s\n", argv[optind]);
 
 	run_devmem_tests();
 
-- 
2.47.0

[PATCH net-next v6 02/12] selftests: ncdevmem: Separate out dmabuf provider

[Stanislav Fomichev]

So we can plug the other ones in the future if needed.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 198 +++++++++++++++----------
 1 file changed, 117 insertions(+), 81 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index 9245d3f158dd..9b3ca6398a9d 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -71,17 +71,101 @@ static char *ifname = "eth1";
 static unsigned int ifindex;
 static unsigned int dmabuf_id;
 
-void print_bytes(void *ptr, size_t size)
+struct memory_buffer {
+	int fd;
+	size_t size;
+
+	int devfd;
+	int memfd;
+	char *buf_mem;
+};
+
+struct memory_provider {
+	struct memory_buffer *(*alloc)(size_t size);
+	void (*free)(struct memory_buffer *ctx);
+	void (*memcpy_from_device)(void *dst, struct memory_buffer *src,
+				   size_t off, int n);
+};
+
+static struct memory_buffer *udmabuf_alloc(size_t size)
 {
-	unsigned char *p = ptr;
-	int i;
+	struct udmabuf_create create;
+	struct memory_buffer *ctx;
+	int ret;
 
-	for (i = 0; i < size; i++)
-		printf("%02hhX ", p[i]);
-	printf("\n");
+	ctx = malloc(sizeof(*ctx));
+	if (!ctx)
+		error(1, ENOMEM, "malloc failed");
+
+	ctx->size = size;
+
+	ctx->devfd = open("/dev/udmabuf", O_RDWR);
+	if (ctx->devfd < 0)
+		error(1, errno,
+		      "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n",
+		      TEST_PREFIX);
+
+	ctx->memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING);
+	if (ctx->memfd < 0)
+		error(1, errno, "%s: [skip,no-memfd]\n", TEST_PREFIX);
+
+	ret = fcntl(ctx->memfd, F_ADD_SEALS, F_SEAL_SHRINK);
+	if (ret < 0)
+		error(1, errno, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
+
+	ret = ftruncate(ctx->memfd, size);
+	if (ret == -1)
+		error(1, errno, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX);
+
+	memset(&create, 0, sizeof(create));
+
+	create.memfd = ctx->memfd;
+	create.offset = 0;
+	create.size = size;
+	ctx->fd = ioctl(ctx->devfd, UDMABUF_CREATE, &create);
+	if (ctx->fd < 0)
+		error(1, errno, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX);
+
+	ctx->buf_mem = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED,
+			    ctx->fd, 0);
+	if (ctx->buf_mem == MAP_FAILED)
+		error(1, errno, "%s: [FAIL, map udmabuf]\n", TEST_PREFIX);
+
+	return ctx;
+}
+
+static void udmabuf_free(struct memory_buffer *ctx)
+{
+	munmap(ctx->buf_mem, ctx->size);
+	close(ctx->fd);
+	close(ctx->memfd);
+	close(ctx->devfd);
+	free(ctx);
 }
 
-void print_nonzero_bytes(void *ptr, size_t size)
+static void udmabuf_memcpy_from_device(void *dst, struct memory_buffer *src,
+				       size_t off, int n)
+{
+	struct dma_buf_sync sync = {};
+
+	sync.flags = DMA_BUF_SYNC_START;
+	ioctl(src->fd, DMA_BUF_IOCTL_SYNC, &sync);
+
+	memcpy(dst, src->buf_mem + off, n);
+
+	sync.flags = DMA_BUF_SYNC_END;
+	ioctl(src->fd, DMA_BUF_IOCTL_SYNC, &sync);
+}
+
+static struct memory_provider udmabuf_memory_provider = {
+	.alloc = udmabuf_alloc,
+	.free = udmabuf_free,
+	.memcpy_from_device = udmabuf_memcpy_from_device,
+};
+
+static struct memory_provider *provider = &udmabuf_memory_provider;
+
+static void print_nonzero_bytes(void *ptr, size_t size)
 {
 	unsigned char *p = ptr;
 	unsigned int i;
@@ -201,42 +285,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd,
 	return -1;
 }
 
-static void create_udmabuf(int *devfd, int *memfd, int *buf, size_t dmabuf_size)
-{
-	struct udmabuf_create create;
-	int ret;
-
-	*devfd = open("/dev/udmabuf", O_RDWR);
-	if (*devfd < 0) {
-		error(70, 0,
-		      "%s: [skip,no-udmabuf: Unable to access DMA buffer device file]\n",
-		      TEST_PREFIX);
-	}
-
-	*memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING);
-	if (*memfd < 0)
-		error(70, 0, "%s: [skip,no-memfd]\n", TEST_PREFIX);
-
-	/* Required for udmabuf */
-	ret = fcntl(*memfd, F_ADD_SEALS, F_SEAL_SHRINK);
-	if (ret < 0)
-		error(73, 0, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
-
-	ret = ftruncate(*memfd, dmabuf_size);
-	if (ret == -1)
-		error(74, 0, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX);
-
-	memset(&create, 0, sizeof(create));
-
-	create.memfd = *memfd;
-	create.offset = 0;
-	create.size = dmabuf_size;
-	*buf = ioctl(*devfd, UDMABUF_CREATE, &create);
-	if (*buf < 0)
-		error(75, 0, "%s: [FAIL, create udmabuf]\n", TEST_PREFIX);
-}
-
-int do_server(void)
+int do_server(struct memory_buffer *mem)
 {
 	char ctrl_data[sizeof(int) * 20000];
 	struct netdev_queue_id *queues;
@@ -244,23 +293,18 @@ int do_server(void)
 	struct sockaddr_in client_addr;
 	struct sockaddr_in server_sin;
 	size_t page_aligned_frags = 0;
-	int devfd, memfd, buf, ret;
 	size_t total_received = 0;
 	socklen_t client_addr_len;
 	bool is_devmem = false;
-	char *buf_mem = NULL;
+	char *tmp_mem = NULL;
 	struct ynl_sock *ys;
-	size_t dmabuf_size;
 	char iobuf[819200];
 	char buffer[256];
 	int socket_fd;
 	int client_fd;
 	size_t i = 0;
 	int opt = 1;
-
-	dmabuf_size = getpagesize() * NUM_PAGES;
-
-	create_udmabuf(&devfd, &memfd, &buf, dmabuf_size);
+	int ret;
 
 	if (reset_flow_steering())
 		error(1, 0, "Failed to reset flow steering\n");
@@ -284,13 +328,12 @@ int do_server(void)
 		queues[i].id = start_queue + i;
 	}
 
-	if (bind_rx_queue(ifindex, buf, queues, num_queues, &ys))
+	if (bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys))
 		error(1, 0, "Failed to bind\n");
 
-	buf_mem = mmap(NULL, dmabuf_size, PROT_READ | PROT_WRITE, MAP_SHARED,
-		       buf, 0);
-	if (buf_mem == MAP_FAILED)
-		error(1, 0, "mmap()");
+	tmp_mem = malloc(mem->size);
+	if (!tmp_mem)
+		error(1, ENOMEM, "malloc failed");
 
 	server_sin.sin_family = AF_INET;
 	server_sin.sin_port = htons(atoi(port));
@@ -341,7 +384,6 @@ int do_server(void)
 		struct iovec iov = { .iov_base = iobuf,
 				     .iov_len = sizeof(iobuf) };
 		struct dmabuf_cmsg *dmabuf_cmsg = NULL;
-		struct dma_buf_sync sync = { 0 };
 		struct cmsghdr *cm = NULL;
 		struct msghdr msg = { 0 };
 		struct dmabuf_token token;
@@ -410,22 +452,17 @@ int do_server(void)
 			else
 				page_aligned_frags++;
 
-			sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_START;
-			ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync);
+			provider->memcpy_from_device(tmp_mem, mem,
+						     dmabuf_cmsg->frag_offset,
+						     dmabuf_cmsg->frag_size);
 
 			if (do_validation)
 				validate_buffer(
-					((unsigned char *)buf_mem) +
+					((unsigned char *)tmp_mem) +
 						dmabuf_cmsg->frag_offset,
 					dmabuf_cmsg->frag_size);
 			else
-				print_nonzero_bytes(
-					((unsigned char *)buf_mem) +
-						dmabuf_cmsg->frag_offset,
-					dmabuf_cmsg->frag_size);
-
-			sync.flags = DMA_BUF_SYNC_READ | DMA_BUF_SYNC_END;
-			ioctl(buf, DMA_BUF_IOCTL_SYNC, &sync);
+				print_nonzero_bytes(tmp_mem, dmabuf_cmsg->frag_size);
 
 			ret = setsockopt(client_fd, SOL_SOCKET,
 					 SO_DEVMEM_DONTNEED, &token,
@@ -450,12 +487,9 @@ int do_server(void)
 
 cleanup:
 
-	munmap(buf_mem, dmabuf_size);
+	free(tmp_mem);
 	close(client_fd);
 	close(socket_fd);
-	close(buf);
-	close(memfd);
-	close(devfd);
 	ynl_sock_destroy(ys);
 
 	return 0;
@@ -464,14 +498,11 @@ int do_server(void)
 void run_devmem_tests(void)
 {
 	struct netdev_queue_id *queues;
-	int devfd, memfd, buf;
+	struct memory_buffer *mem;
 	struct ynl_sock *ys;
-	size_t dmabuf_size;
 	size_t i = 0;
 
-	dmabuf_size = getpagesize() * NUM_PAGES;
-
-	create_udmabuf(&devfd, &memfd, &buf, dmabuf_size);
+	mem = provider->alloc(getpagesize() * NUM_PAGES);
 
 	/* Configure RSS to divert all traffic from our devmem queues */
 	if (configure_rss())
@@ -482,7 +513,7 @@ void run_devmem_tests(void)
 	if (configure_headersplit(1))
 		error(1, 0, "Failed to configure header split\n");
 
-	if (!bind_rx_queue(ifindex, buf, queues, num_queues, &ys))
+	if (!bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys))
 		error(1, 0, "Binding empty queues array should have failed\n");
 
 	for (i = 0; i < num_queues; i++) {
@@ -495,7 +526,7 @@ void run_devmem_tests(void)
 	if (configure_headersplit(0))
 		error(1, 0, "Failed to configure header split\n");
 
-	if (!bind_rx_queue(ifindex, buf, queues, num_queues, &ys))
+	if (!bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys))
 		error(1, 0, "Configure dmabuf with header split off should have failed\n");
 
 	if (configure_headersplit(1))
@@ -508,7 +539,7 @@ void run_devmem_tests(void)
 		queues[i].id = start_queue + i;
 	}
 
-	if (bind_rx_queue(ifindex, buf, queues, num_queues, &ys))
+	if (bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys))
 		error(1, 0, "Failed to bind\n");
 
 	/* Deactivating a bound queue should not be legal */
@@ -517,11 +548,15 @@ void run_devmem_tests(void)
 
 	/* Closing the netlink socket does an implicit unbind */
 	ynl_sock_destroy(ys);
+
+	provider->free(mem);
 }
 
 int main(int argc, char *argv[])
 {
+	struct memory_buffer *mem;
 	int is_server = 0, opt;
+	int ret;
 
 	while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:")) != -1) {
 		switch (opt) {
@@ -562,8 +597,9 @@ int main(int argc, char *argv[])
 
 	run_devmem_tests();
 
-	if (is_server)
-		return do_server();
+	mem = provider->alloc(getpagesize() * NUM_PAGES);
+	ret = is_server ? do_server(mem) : 1;
+	provider->free(mem);
 
-	return 0;
+	return ret;
 }
-- 
2.47.0

[PATCH net-next v6 03/12] selftests: ncdevmem: Unify error handling

[Stanislav Fomichev]

There is a bunch of places where error() calls look out of place.
Use the same error(1, errno, ...) pattern everywhere.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index 9b3ca6398a9d..b89b62445158 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -339,33 +339,33 @@ int do_server(struct memory_buffer *mem)
 	server_sin.sin_port = htons(atoi(port));
 
 	ret = inet_pton(server_sin.sin_family, server_ip, &server_sin.sin_addr);
-	if (socket < 0)
-		error(79, 0, "%s: [FAIL, create socket]\n", TEST_PREFIX);
+	if (ret < 0)
+		error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX);
 
 	socket_fd = socket(server_sin.sin_family, SOCK_STREAM, 0);
-	if (socket < 0)
-		error(errno, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX);
+	if (socket_fd < 0)
+		error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX);
 
 	ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEPORT, &opt,
 			 sizeof(opt));
 	if (ret)
-		error(errno, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
+		error(1, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
 
 	ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEADDR, &opt,
 			 sizeof(opt));
 	if (ret)
-		error(errno, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
+		error(1, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
 
 	fprintf(stderr, "binding to address %s:%d\n", server_ip,
 		ntohs(server_sin.sin_port));
 
 	ret = bind(socket_fd, &server_sin, sizeof(server_sin));
 	if (ret)
-		error(errno, errno, "%s: [FAIL, bind]\n", TEST_PREFIX);
+		error(1, errno, "%s: [FAIL, bind]\n", TEST_PREFIX);
 
 	ret = listen(socket_fd, 1);
 	if (ret)
-		error(errno, errno, "%s: [FAIL, listen]\n", TEST_PREFIX);
+		error(1, errno, "%s: [FAIL, listen]\n", TEST_PREFIX);
 
 	client_addr_len = sizeof(client_addr);
 
-- 
2.47.0

[PATCH net-next v6 04/12] selftests: ncdevmem: Make client_ip optional

[Stanislav Fomichev]

Support 3-tuple filtering by making client_ip optional. When -c is
not passed, don't specify src-ip/src-port in the filter.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index b89b62445158..b94f7c4a53ed 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -62,7 +62,7 @@
  */
 
 static char *server_ip = "192.168.1.4";
-static char *client_ip = "192.168.1.2";
+static char *client_ip;
 static char *port = "5201";
 static size_t do_validation;
 static int start_queue = 8;
@@ -236,8 +236,14 @@ static int configure_channels(unsigned int rx, unsigned int tx)
 
 static int configure_flow_steering(void)
 {
-	return run_command("sudo ethtool -N %s flow-type tcp4 src-ip %s dst-ip %s src-port %s dst-port %s queue %d >&2",
-			   ifname, client_ip, server_ip, port, port, start_queue);
+	return run_command("sudo ethtool -N %s flow-type tcp4 %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
+			   ifname,
+			   client_ip ? "src-ip" : "",
+			   client_ip ?: "",
+			   server_ip,
+			   client_ip ? "src-port" : "",
+			   client_ip ? port : "",
+			   port, start_queue);
 }
 
 static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd,
-- 
2.47.0

[PATCH net-next v6 05/12] selftests: ncdevmem: Remove default arguments

[Stanislav Fomichev]

To make it clear what's required and what's not. Also, some of the
values don't seem like a good defaults; for example eth1.

Move the invocation comment to the top, add missing -s to the client
and cleanup the client invocation a bit to make more readable.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 61 ++++++++++++++++----------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index b94f7c4a53ed..037fb6e97d29 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -1,4 +1,31 @@
 // SPDX-License-Identifier: GPL-2.0
+/*
+ * tcpdevmem netcat. Works similarly to netcat but does device memory TCP
+ * instead of regular TCP. Uses udmabuf to mock a dmabuf provider.
+ *
+ * Usage:
+ *
+ *     On server:
+ *     ncdevmem -s <server IP> [-c <client IP>] -f eth1 -l -p 5201
+ *
+ *     On client:
+ *     echo -n "hello\nworld" | nc -s <server IP> 5201 -p 5201
+ *
+ * Test data validation:
+ *
+ *     On server:
+ *     ncdevmem -s <server IP> [-c <client IP>] -f eth1 -l -p 5201 -v 7
+ *
+ *     On client:
+ *     yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | \
+ *             tr \\n \\0 | \
+ *             head -c 5G | \
+ *             nc <server IP> 5201 -p 5201
+ *
+ *
+ * Note this is compatible with regular netcat. i.e. the sender or receiver can
+ * be replaced with regular netcat to test the RX or TX path in isolation.
+ */
 #define _GNU_SOURCE
 #define __EXPORTED_HEADERS__
 
@@ -42,32 +69,13 @@
 #define MSG_SOCK_DEVMEM 0x2000000
 #endif
 
-/*
- * tcpdevmem netcat. Works similarly to netcat but does device memory TCP
- * instead of regular TCP. Uses udmabuf to mock a dmabuf provider.
- *
- * Usage:
- *
- *	On server:
- *	ncdevmem -s <server IP> -c <client IP> -f eth1 -l -p 5201 -v 7
- *
- *	On client:
- *	yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | \
- *		tr \\n \\0 | \
- *		head -c 5G | \
- *		nc <server IP> 5201 -p 5201
- *
- * Note this is compatible with regular netcat. i.e. the sender or receiver can
- * be replaced with regular netcat to test the RX or TX path in isolation.
- */
-
-static char *server_ip = "192.168.1.4";
+static char *server_ip;
 static char *client_ip;
-static char *port = "5201";
+static char *port;
 static size_t do_validation;
 static int start_queue = 8;
 static int num_queues = 8;
-static char *ifname = "eth1";
+static char *ifname;
 static unsigned int ifindex;
 static unsigned int dmabuf_id;
 
@@ -596,6 +604,15 @@ int main(int argc, char *argv[])
 		}
 	}
 
+	if (!server_ip)
+		error(1, 0, "Missing -s argument\n");
+
+	if (!port)
+		error(1, 0, "Missing -p argument\n");
+
+	if (!ifname)
+		error(1, 0, "Missing -f argument\n");
+
 	ifindex = if_nametoindex(ifname);
 
 	for (; optind < argc; optind++)
-- 
2.47.0

[PATCH net-next v6 06/12] selftests: ncdevmem: Switch to AF_INET6

[Stanislav Fomichev]

Use dualstack socket to support both v4 and v6. v4-mapped-v6 address
can be used to do v4.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 85 +++++++++++++++++---------
 1 file changed, 57 insertions(+), 28 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index 037fb6e97d29..8e4a0fe74bb1 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -242,13 +242,22 @@ static int configure_channels(unsigned int rx, unsigned int tx)
 	return run_command("sudo ethtool -L %s rx %u tx %u", ifname, rx, tx);
 }
 
-static int configure_flow_steering(void)
+static int configure_flow_steering(struct sockaddr_in6 *server_sin)
 {
-	return run_command("sudo ethtool -N %s flow-type tcp4 %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
+	const char *server_addr = server_ip;
+	const char *type = "tcp6";
+
+	if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) {
+		type = "tcp4";
+		server_addr = strrchr(server_ip, ':') + 1;
+	}
+
+	return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2",
 			   ifname,
+			   type,
 			   client_ip ? "src-ip" : "",
 			   client_ip ?: "",
-			   server_ip,
+			   server_addr,
 			   client_ip ? "src-port" : "",
 			   client_ip ? port : "",
 			   port, start_queue);
@@ -299,13 +308,43 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd,
 	return -1;
 }
 
+static int enable_reuseaddr(int fd)
+{
+	int opt = 1;
+	int ret;
+
+	ret = setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &opt, sizeof(opt));
+	if (ret)
+		return -errno;
+
+	ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
+	if (ret)
+		return -errno;
+
+	return 0;
+}
+
+static int parse_address(const char *str, int port, struct sockaddr_in6 *sin6)
+{
+	int ret;
+
+	sin6->sin6_family = AF_INET6;
+	sin6->sin6_port = htons(port);
+
+	ret = inet_pton(sin6->sin6_family, str, &sin6->sin6_addr);
+	if (ret < 0)
+		return -1;
+
+	return 0;
+}
+
 int do_server(struct memory_buffer *mem)
 {
 	char ctrl_data[sizeof(int) * 20000];
 	struct netdev_queue_id *queues;
 	size_t non_page_aligned_frags = 0;
-	struct sockaddr_in client_addr;
-	struct sockaddr_in server_sin;
+	struct sockaddr_in6 client_addr;
+	struct sockaddr_in6 server_sin;
 	size_t page_aligned_frags = 0;
 	size_t total_received = 0;
 	socklen_t client_addr_len;
@@ -317,9 +356,12 @@ int do_server(struct memory_buffer *mem)
 	int socket_fd;
 	int client_fd;
 	size_t i = 0;
-	int opt = 1;
 	int ret;
 
+	ret = parse_address(server_ip, atoi(port), &server_sin);
+	if (ret < 0)
+		error(1, 0, "parse server address");
+
 	if (reset_flow_steering())
 		error(1, 0, "Failed to reset flow steering\n");
 
@@ -328,7 +370,7 @@ int do_server(struct memory_buffer *mem)
 		error(1, 0, "Failed to configure rss\n");
 
 	/* Flow steer our devmem flows to start_queue */
-	if (configure_flow_steering())
+	if (configure_flow_steering(&server_sin))
 		error(1, 0, "Failed to configure flow steering\n");
 
 	sleep(1);
@@ -349,29 +391,16 @@ int do_server(struct memory_buffer *mem)
 	if (!tmp_mem)
 		error(1, ENOMEM, "malloc failed");
 
-	server_sin.sin_family = AF_INET;
-	server_sin.sin_port = htons(atoi(port));
-
-	ret = inet_pton(server_sin.sin_family, server_ip, &server_sin.sin_addr);
-	if (ret < 0)
-		error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX);
-
-	socket_fd = socket(server_sin.sin_family, SOCK_STREAM, 0);
+	socket_fd = socket(AF_INET6, SOCK_STREAM, 0);
 	if (socket_fd < 0)
 		error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX);
 
-	ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEPORT, &opt,
-			 sizeof(opt));
-	if (ret)
-		error(1, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
-
-	ret = setsockopt(socket_fd, SOL_SOCKET, SO_REUSEADDR, &opt,
-			 sizeof(opt));
+	ret = enable_reuseaddr(socket_fd);
 	if (ret)
-		error(1, errno, "%s: [FAIL, set sock opt]\n", TEST_PREFIX);
+		error(1, errno, "%s: [FAIL, reuseaddr]\n", TEST_PREFIX);
 
 	fprintf(stderr, "binding to address %s:%d\n", server_ip,
-		ntohs(server_sin.sin_port));
+		ntohs(server_sin.sin6_port));
 
 	ret = bind(socket_fd, &server_sin, sizeof(server_sin));
 	if (ret)
@@ -383,16 +412,16 @@ int do_server(struct memory_buffer *mem)
 
 	client_addr_len = sizeof(client_addr);
 
-	inet_ntop(server_sin.sin_family, &server_sin.sin_addr, buffer,
+	inet_ntop(AF_INET6, &server_sin.sin6_addr, buffer,
 		  sizeof(buffer));
 	fprintf(stderr, "Waiting or connection on %s:%d\n", buffer,
-		ntohs(server_sin.sin_port));
+		ntohs(server_sin.sin6_port));
 	client_fd = accept(socket_fd, &client_addr, &client_addr_len);
 
-	inet_ntop(client_addr.sin_family, &client_addr.sin_addr, buffer,
+	inet_ntop(AF_INET6, &client_addr.sin6_addr, buffer,
 		  sizeof(buffer));
 	fprintf(stderr, "Got connection from %s:%d\n", buffer,
-		ntohs(client_addr.sin_port));
+		ntohs(client_addr.sin6_port));
 
 	while (1) {
 		struct iovec iov = { .iov_base = iobuf,
-- 
2.47.0

[PATCH net-next v6 07/12] selftests: ncdevmem: Properly reset flow steering

[Stanislav Fomichev]

ntuple off/on might be not enough to do it on all NICs.
Add a bunch of shell crap to explicitly remove the rules.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index 8e4a0fe74bb1..697771c1f9fa 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -217,13 +217,18 @@ void validate_buffer(void *line, size_t size)
 
 static int reset_flow_steering(void)
 {
-	int ret = 0;
-
-	ret = run_command("sudo ethtool -K %s ntuple off >&2", ifname);
-	if (ret)
-		return ret;
-
-	return run_command("sudo ethtool -K %s ntuple on >&2", ifname);
+	/* Depending on the NIC, toggling ntuple off and on might not
+	 * be allowed. Additionally, attempting to delete existing filters
+	 * will fail if no filters are present. Therefore, do not enforce
+	 * the exit status.
+	 */
+
+	run_command("sudo ethtool -K %s ntuple off >&2", ifname);
+	run_command("sudo ethtool -K %s ntuple on >&2", ifname);
+	run_command(
+		"sudo ethtool -n %s | grep 'Filter:' | awk '{print $2}' | xargs -n1 ethtool -N %s delete >&2",
+		ifname, ifname);
+	return 0;
 }
 
 static int configure_headersplit(bool on)
-- 
2.47.0

[PATCH net-next v6 08/12] selftests: ncdevmem: Use YNL to enable TCP header split

[Stanislav Fomichev]

In the next patch the hard-coded queue numbers are gonna be removed.
So introduce some initial support for ethtool YNL and use
it to enable header split.

Also, tcp-data-split requires latest ethtool which is unlikely
to be present in the distros right now.

(ideally, we should not shell out to ethtool at all).

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/Makefile   |  2 +-
 tools/testing/selftests/net/ncdevmem.c | 57 +++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 26a4883a65c9..759b1d2dc8b4 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -111,7 +111,7 @@ TEST_INCLUDES := forwarding/lib.sh
 include ../lib.mk
 
 # YNL build
-YNL_GENS := netdev
+YNL_GENS := ethtool netdev
 include ynl.mk
 
 $(OUTPUT)/epoll_busy_poll: LDLIBS += -lcap
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index 697771c1f9fa..e1faad46548b 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -55,10 +55,12 @@
 #include <linux/netlink.h>
 #include <linux/genetlink.h>
 #include <linux/netdev.h>
+#include <linux/ethtool_netlink.h>
 #include <time.h>
 #include <net/if.h>
 
 #include "netdev-user.h"
+#include "ethtool-user.h"
 #include <ynl.h>
 
 #define PAGE_SHIFT 12
@@ -231,10 +233,58 @@ static int reset_flow_steering(void)
 	return 0;
 }
 
+static const char *tcp_data_split_str(int val)
+{
+	switch (val) {
+	case 0:
+		return "off";
+	case 1:
+		return "auto";
+	case 2:
+		return "on";
+	default:
+		return "?";
+	}
+}
+
 static int configure_headersplit(bool on)
 {
-	return run_command("sudo ethtool -G %s tcp-data-split %s >&2", ifname,
-			   on ? "on" : "off");
+	struct ethtool_rings_get_req *get_req;
+	struct ethtool_rings_get_rsp *get_rsp;
+	struct ethtool_rings_set_req *req;
+	struct ynl_error yerr;
+	struct ynl_sock *ys;
+	int ret;
+
+	ys = ynl_sock_create(&ynl_ethtool_family, &yerr);
+	if (!ys) {
+		fprintf(stderr, "YNL: %s\n", yerr.msg);
+		return -1;
+	}
+
+	req = ethtool_rings_set_req_alloc();
+	ethtool_rings_set_req_set_header_dev_index(req, ifindex);
+	/* 0 - off, 1 - auto, 2 - on */
+	ethtool_rings_set_req_set_tcp_data_split(req, on ? 2 : 0);
+	ret = ethtool_rings_set(ys, req);
+	if (ret < 0)
+		fprintf(stderr, "YNL failed: %s\n", ys->err.msg);
+	ethtool_rings_set_req_free(req);
+
+	if (ret == 0) {
+		get_req = ethtool_rings_get_req_alloc();
+		ethtool_rings_get_req_set_header_dev_index(get_req, ifindex);
+		get_rsp = ethtool_rings_get(ys, get_req);
+		ethtool_rings_get_req_free(get_req);
+		if (get_rsp)
+			fprintf(stderr, "TCP header split: %s\n",
+				tcp_data_split_str(get_rsp->tcp_data_split));
+		ethtool_rings_get_rsp_free(get_rsp);
+	}
+
+	ynl_sock_destroy(ys);
+
+	return ret;
 }
 
 static int configure_rss(void)
@@ -370,6 +420,9 @@ int do_server(struct memory_buffer *mem)
 	if (reset_flow_steering())
 		error(1, 0, "Failed to reset flow steering\n");
 
+	if (configure_headersplit(1))
+		error(1, 0, "Failed to enable TCP header split\n");
+
 	/* Configure RSS to divert all traffic from our devmem queues */
 	if (configure_rss())
 		error(1, 0, "Failed to configure rss\n");
-- 
2.47.0

[PATCH net-next v6 09/12] selftests: ncdevmem: Remove hard-coded queue numbers

[Stanislav Fomichev]

Use single last queue of the device and probe it dynamically.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 40 ++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index e1faad46548b..fe4d81ef1ca5 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -75,8 +75,8 @@ static char *server_ip;
 static char *client_ip;
 static char *port;
 static size_t do_validation;
-static int start_queue = 8;
-static int num_queues = 8;
+static int start_queue = -1;
+static int num_queues = 1;
 static char *ifname;
 static unsigned int ifindex;
 static unsigned int dmabuf_id;
@@ -208,6 +208,33 @@ void validate_buffer(void *line, size_t size)
 	fprintf(stdout, "Validated buffer\n");
 }
 
+static int rxq_num(int ifindex)
+{
+	struct ethtool_channels_get_req *req;
+	struct ethtool_channels_get_rsp *rsp;
+	struct ynl_error yerr;
+	struct ynl_sock *ys;
+	int num = -1;
+
+	ys = ynl_sock_create(&ynl_ethtool_family, &yerr);
+	if (!ys) {
+		fprintf(stderr, "YNL: %s\n", yerr.msg);
+		return -1;
+	}
+
+	req = ethtool_channels_get_req_alloc();
+	ethtool_channels_get_req_set_header_dev_index(req, ifindex);
+	rsp = ethtool_channels_get(ys, req);
+	if (rsp)
+		num = rsp->rx_count + rsp->combined_count;
+	ethtool_channels_get_req_free(req);
+	ethtool_channels_get_rsp_free(rsp);
+
+	ynl_sock_destroy(ys);
+
+	return num;
+}
+
 #define run_command(cmd, ...)                                           \
 	({                                                              \
 		char command[256];                                      \
@@ -702,6 +729,15 @@ int main(int argc, char *argv[])
 
 	ifindex = if_nametoindex(ifname);
 
+	if (start_queue < 0) {
+		start_queue = rxq_num(ifindex) - 1;
+
+		if (start_queue < 0)
+			error(1, 0, "couldn't detect number of queues\n");
+
+		fprintf(stderr, "using queues %d..%d\n", start_queue, start_queue + num_queues);
+	}
+
 	for (; optind < argc; optind++)
 		fprintf(stderr, "extra arguments: %s\n", argv[optind]);
 
-- 
2.47.0

[PATCH net-next v6 10/12] selftests: ncdevmem: Run selftest when none of the -s or -c has been provided

[Stanislav Fomichev]

This will be used as a 'probe' mode in the selftest to check whether
the device supports the devmem or not. Use hard-coded queue layout
(two last queues) and prevent user from passing custom -q and/or -t.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/net/ncdevmem.c | 42 ++++++++++++++++++++------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c
index fe4d81ef1ca5..07a91516103a 100644
--- a/tools/testing/selftests/net/ncdevmem.c
+++ b/tools/testing/selftests/net/ncdevmem.c
@@ -76,7 +76,7 @@ static char *client_ip;
 static char *port;
 static size_t do_validation;
 static int start_queue = -1;
-static int num_queues = 1;
+static int num_queues = -1;
 static char *ifname;
 static unsigned int ifindex;
 static unsigned int dmabuf_id;
@@ -718,19 +718,31 @@ int main(int argc, char *argv[])
 		}
 	}
 
-	if (!server_ip)
-		error(1, 0, "Missing -s argument\n");
-
-	if (!port)
-		error(1, 0, "Missing -p argument\n");
-
 	if (!ifname)
 		error(1, 0, "Missing -f argument\n");
 
 	ifindex = if_nametoindex(ifname);
 
-	if (start_queue < 0) {
-		start_queue = rxq_num(ifindex) - 1;
+	if (!server_ip && !client_ip) {
+		if (start_queue < 0 && num_queues < 0) {
+			num_queues = rxq_num(ifindex);
+			if (num_queues < 0)
+				error(1, 0, "couldn't detect number of queues\n");
+			/* make sure can bind to multiple queues */
+			start_queue = num_queues / 2;
+			num_queues /= 2;
+		}
+
+		if (start_queue < 0 || num_queues < 0)
+			error(1, 0, "Both -t and -q are required\n");
+
+		run_devmem_tests();
+		return 0;
+	}
+
+	if (start_queue < 0 && num_queues < 0) {
+		num_queues = 1;
+		start_queue = rxq_num(ifindex) - num_queues;
 
 		if (start_queue < 0)
 			error(1, 0, "couldn't detect number of queues\n");
@@ -741,7 +753,17 @@ int main(int argc, char *argv[])
 	for (; optind < argc; optind++)
 		fprintf(stderr, "extra arguments: %s\n", argv[optind]);
 
-	run_devmem_tests();
+	if (start_queue < 0)
+		error(1, 0, "Missing -t argument\n");
+
+	if (num_queues < 0)
+		error(1, 0, "Missing -q argument\n");
+
+	if (!server_ip)
+		error(1, 0, "Missing -s argument\n");
+
+	if (!port)
+		error(1, 0, "Missing -p argument\n");
 
 	mem = provider->alloc(getpagesize() * NUM_PAGES);
 	ret = is_server ? do_server(mem) : 1;
-- 
2.47.0

[PATCH net-next v6 11/12] selftests: ncdevmem: Move ncdevmem under drivers/net/hw

[Stanislav Fomichev]

This is where all the tests that depend on the HW functionality live in
and this is where the automated test is gonna be added in the next
patch.

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 tools/testing/selftests/drivers/net/hw/.gitignore         | 1 +
 tools/testing/selftests/drivers/net/hw/Makefile           | 8 ++++++++
 .../testing/selftests/{net => drivers/net/hw}/ncdevmem.c  | 0
 tools/testing/selftests/net/.gitignore                    | 1 -
 tools/testing/selftests/net/Makefile                      | 8 --------
 5 files changed, 9 insertions(+), 9 deletions(-)
 create mode 100644 tools/testing/selftests/drivers/net/hw/.gitignore
 rename tools/testing/selftests/{net => drivers/net/hw}/ncdevmem.c (100%)

diff --git a/tools/testing/selftests/drivers/net/hw/.gitignore b/tools/testing/selftests/drivers/net/hw/.gitignore
new file mode 100644
index 000000000000..e9fe6ede681a
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/.gitignore
@@ -0,0 +1 @@
+ncdevmem
diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index c9f2f48fc30f..182348f4bd40 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -26,4 +26,12 @@ TEST_INCLUDES := \
 	../../../net/forwarding/tc_common.sh \
 	#
 
+# YNL files, must be before "include ..lib.mk"
+YNL_GEN_FILES := ncdevmem
+TEST_GEN_FILES += $(YNL_GEN_FILES)
+
 include ../../../lib.mk
+
+# YNL build
+YNL_GENS := ethtool netdev
+include ../../../net/ynl.mk
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
similarity index 100%
rename from tools/testing/selftests/net/ncdevmem.c
rename to tools/testing/selftests/drivers/net/hw/ncdevmem.c
diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index 217d8b7a7365..a78debbd1fe7 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -18,7 +18,6 @@ ipv6_flowlabel_mgr
 log.txt
 msg_oob
 msg_zerocopy
-ncdevmem
 nettest
 psock_fanout
 psock_snd
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 759b1d2dc8b4..22a5d6a7c3f3 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -97,10 +97,6 @@ TEST_PROGS += fq_band_pktlimit.sh
 TEST_PROGS += vlan_hw_filter.sh
 TEST_PROGS += bpf_offload.py
 
-# YNL files, must be before "include ..lib.mk"
-YNL_GEN_FILES := ncdevmem
-TEST_GEN_FILES += $(YNL_GEN_FILES)
-
 TEST_FILES := settings
 TEST_FILES += in_netns.sh lib.sh net_helper.sh setup_loopback.sh setup_veth.sh
 
@@ -110,10 +106,6 @@ TEST_INCLUDES := forwarding/lib.sh
 
 include ../lib.mk
 
-# YNL build
-YNL_GENS := ethtool netdev
-include ynl.mk
-
 $(OUTPUT)/epoll_busy_poll: LDLIBS += -lcap
 $(OUTPUT)/reuseport_bpf_numa: LDLIBS += -lnuma
 $(OUTPUT)/tcp_mmap: LDLIBS += -lpthread -lcrypto
-- 
2.47.0

[PATCH net-next v6 12/12] selftests: ncdevmem: Add automated test

[Stanislav Fomichev]

Only RX side for now and small message to test the setup.
In the future, we can extend it to TX side and to testing
both sides with a couple of megs of data.

  make \
  	-C tools/testing/selftests \
  	TARGETS="drivers/hw/net" \
  	install INSTALL_PATH=~/tmp/ksft

  scp ~/tmp/ksft ${HOST}:
  scp ~/tmp/ksft ${PEER}:

  cfg+="NETIF=${DEV}\n"
  cfg+="LOCAL_V6=${HOST_IP}\n"
  cfg+="REMOTE_V6=${PEER_IP}\n"
  cfg+="REMOTE_TYPE=ssh\n"
  cfg+="REMOTE_ARGS=root@${PEER}\n"

  echo -e "$cfg" | ssh root@${HOST} "cat > ksft/drivers/net/net.config"
  ssh root@${HOST} "cd ksft && ./run_kselftest.sh -t drivers/net:devmem.py"

Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 .../testing/selftests/drivers/net/hw/Makefile |  1 +
 .../selftests/drivers/net/hw/devmem.py        | 45 +++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100755 tools/testing/selftests/drivers/net/hw/devmem.py

diff --git a/tools/testing/selftests/drivers/net/hw/Makefile b/tools/testing/selftests/drivers/net/hw/Makefile
index 182348f4bd40..1c6a77480923 100644
--- a/tools/testing/selftests/drivers/net/hw/Makefile
+++ b/tools/testing/selftests/drivers/net/hw/Makefile
@@ -3,6 +3,7 @@
 TEST_PROGS = \
 	csum.py \
 	devlink_port_split.py \
+	devmem.py \
 	ethtool.sh \
 	ethtool_extended_state.sh \
 	ethtool_mm.sh \
diff --git a/tools/testing/selftests/drivers/net/hw/devmem.py b/tools/testing/selftests/drivers/net/hw/devmem.py
new file mode 100755
index 000000000000..1416c31ff81e
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/devmem.py
@@ -0,0 +1,45 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+
+from lib.py import ksft_run, ksft_exit
+from lib.py import ksft_eq, KsftSkipEx
+from lib.py import NetDrvEpEnv
+from lib.py import bkg, cmd, rand_port, wait_port_listen
+from lib.py import ksft_disruptive
+
+
+def require_devmem(cfg):
+    if not hasattr(cfg, "_devmem_probed"):
+        port = rand_port()
+        probe_command = f"./ncdevmem -f {cfg.ifname}"
+        cfg._devmem_supported = cmd(probe_command, fail=False, shell=True).ret == 0
+        cfg._devmem_probed = True
+
+    if not cfg._devmem_supported:
+        raise KsftSkipEx("Test requires devmem support")
+
+
+@ksft_disruptive
+def check_rx(cfg) -> None:
+    cfg.require_v6()
+    require_devmem(cfg)
+
+    port = rand_port()
+    listen_cmd = f"./ncdevmem -l -f {cfg.ifname} -s {cfg.v6} -p {port}"
+
+    with bkg(listen_cmd) as nc:
+        wait_port_listen(port)
+        cmd(f"echo -e \"hello\\nworld\"| nc {cfg.v6} {port}", host=cfg.remote, shell=True)
+
+    ksft_eq(nc.stdout.strip(), "hello\nworld")
+
+
+def main() -> None:
+    with NetDrvEpEnv(__file__) as cfg:
+        ksft_run([check_rx],
+                 args=(cfg, ))
+    ksft_exit()
+
+
+if __name__ == "__main__":
+    main()
-- 
2.47.0

Re: [PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[Mina Almasry]

On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> The goal of the series is to simplify and make it possible to use
> ncdevmem in an automated way from the ksft python wrapper.
>
> ncdevmem is slowly mutated into a state where it uses stdout
> to print the payload and the python wrapper is added to
> make sure the arrived payload matches the expected one.
>
> v6:
> - fix compilation issue in 'Unify error handling' patch (Jakub)
>

Since I saw a compilation failures on a couple of iterations I
cherry-picked this locally and tested compilation. I'm seeing this:

sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
TARGETS=ncdevmem 2>&1
make: Entering directory
'/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
  CC       ncdevmem
In file included from ncdevmem.c:63:
/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43:
warning: ‘enum ethtool_header_flags’ declared inside parameter list
will not be visible outside of this definition or declaration
   23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value);
      |                                           ^~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41:
warning: ‘enum ethtool_module_fw_flash_status’ declared inside
parameter list will not be visible outside of this definition or
declaration
   25 | ethtool_module_fw_flash_status_str(enum
ethtool_module_fw_flash_status value);
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45:
error: field ‘status’ has incomplete type
 6766 |         enum ethtool_module_fw_flash_status status;
      |                                             ^~~~~~
ncdevmem.c: In function ‘do_server’:
ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known
  517 |                 struct dmabuf_token token;
      |                                     ^~~~~
ncdevmem.c:542:47: error: ‘SCM_DEVMEM_DMABUF’ undeclared (first use in
this function)
  542 |                             (cm->cmsg_type != SCM_DEVMEM_DMABUF &&
      |                                               ^~~~~~~~~~~~~~~~~
ncdevmem.c:542:47: note: each undeclared identifier is reported only
once for each function it appears in
ncdevmem.c:543:47: error: ‘SCM_DEVMEM_LINEAR’ undeclared (first use in
this function)
  543 |                              cm->cmsg_type != SCM_DEVMEM_LINEAR)) {
      |                                               ^~~~~~~~~~~~~~~~~
ncdevmem.c:557:52: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  557 |                                         dmabuf_cmsg->frag_size);
      |                                                    ^~
ncdevmem.c:562:56: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  562 |                         token.token_start = dmabuf_cmsg->frag_token;
      |                                                        ^~
ncdevmem.c:566:42: error: ‘SO_DEVMEM_DONTNEED’ undeclared (first use
in this function)
  566 |                                          SO_DEVMEM_DONTNEED, &token,
      |                                          ^~~~~~~~~~~~~~~~~~
ncdevmem.c:573:56: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  573 |                         token.token_start = dmabuf_cmsg->frag_token;
      |                                                        ^~
ncdevmem.c:576:54: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  576 |                         total_received += dmabuf_cmsg->frag_size;
      |                                                      ^~
ncdevmem.c:579:44: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  579 |                                 dmabuf_cmsg->frag_offset >> PAGE_SHIFT,
      |                                            ^~
ncdevmem.c:580:44: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  580 |                                 dmabuf_cmsg->frag_offset %
getpagesize(),
      |                                            ^~
ncdevmem.c:581:44: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  581 |                                 dmabuf_cmsg->frag_offset,
      |                                            ^~
ncdevmem.c:582:44: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  582 |                                 dmabuf_cmsg->frag_size,
dmabuf_cmsg->frag_token,
      |                                            ^~
ncdevmem.c:582:68: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  582 |                                 dmabuf_cmsg->frag_size,
dmabuf_cmsg->frag_token,
      |                                                                    ^~
ncdevmem.c:583:60: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  583 |                                 total_received, dmabuf_cmsg->dmabuf_id);
      |                                                            ^~
ncdevmem.c:585:40: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  585 |                         if (dmabuf_cmsg->dmabuf_id != dmabuf_id)
      |                                        ^~
ncdevmem.c:589:40: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  589 |                         if (dmabuf_cmsg->frag_size % getpagesize())
      |                                        ^~
ncdevmem.c:595:65: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  595 |
dmabuf_cmsg->frag_offset,
      |                                                                 ^~
ncdevmem.c:596:65: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  596 |
dmabuf_cmsg->frag_size);
      |                                                                 ^~
ncdevmem.c:601:60: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  601 |
dmabuf_cmsg->frag_offset,
      |                                                            ^~
ncdevmem.c:602:52: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  602 |                                         dmabuf_cmsg->frag_size);
      |                                                    ^~
ncdevmem.c:604:73: error: invalid use of undefined type ‘struct dmabuf_cmsg’
  604 |                                 print_nonzero_bytes(tmp_mem,
dmabuf_cmsg->frag_size);
      |
         ^~
make: *** [../../../lib.mk:222:
/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw/ncdevmem]
Error 1
make: Leaving directory
'/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'

The errors are still there even without the CFLAGS="-static". Can you
take a look before merge?

Re: [PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[Stanislav Fomichev]

On 10/30, Mina Almasry wrote:
> On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> >
> > The goal of the series is to simplify and make it possible to use
> > ncdevmem in an automated way from the ksft python wrapper.
> >
> > ncdevmem is slowly mutated into a state where it uses stdout
> > to print the payload and the python wrapper is added to
> > make sure the arrived payload matches the expected one.
> >
> > v6:
> > - fix compilation issue in 'Unify error handling' patch (Jakub)
> >
> 
> Since I saw a compilation failures on a couple of iterations I
> cherry-picked this locally and tested compilation. I'm seeing this:

Are you cherry picking the whole series or just this patch? It looks
too broken.

> sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
> TARGETS=ncdevmem 2>&1
> make: Entering directory
> '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
>   CC       ncdevmem
> In file included from ncdevmem.c:63:
> /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43:
> warning: ‘enum ethtool_header_flags’ declared inside parameter list
> will not be visible outside of this definition or declaration
>    23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value);
>       |                                           ^~~~~~~~~~~~~~~~~~~~
> /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41:
> warning: ‘enum ethtool_module_fw_flash_status’ declared inside
> parameter list will not be visible outside of this definition or
> declaration
>    25 | ethtool_module_fw_flash_status_str(enum
> ethtool_module_fw_flash_status value);
>       |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45:
> error: field ‘status’ has incomplete type
>  6766 |         enum ethtool_module_fw_flash_status status;
>       |                                             ^~~~~~

This has been fixed via '#include <linux/ethtool_netlink.h>'

> ncdevmem.c: In function ‘do_server’:
> ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known
>   517 |                 struct dmabuf_token token;

And this, and the rest, don't make sense at all?

I'll double check on my side.

Re: [PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[Mina Almasry]

On Wed, Oct 30, 2024 at 8:13 AM Stanislav Fomichev <stfomichev@gmail.com> wrote:
>
> On 10/30, Mina Almasry wrote:
> > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > >
> > > The goal of the series is to simplify and make it possible to use
> > > ncdevmem in an automated way from the ksft python wrapper.
> > >
> > > ncdevmem is slowly mutated into a state where it uses stdout
> > > to print the payload and the python wrapper is added to
> > > make sure the arrived payload matches the expected one.
> > >
> > > v6:
> > > - fix compilation issue in 'Unify error handling' patch (Jakub)
> > >
> >
> > Since I saw a compilation failures on a couple of iterations I
> > cherry-picked this locally and tested compilation. I'm seeing this:
>
> Are you cherry picking the whole series or just this patch? It looks
> too broken.
>
> > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
> > TARGETS=ncdevmem 2>&1
> > make: Entering directory
> > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> >   CC       ncdevmem
> > In file included from ncdevmem.c:63:
> > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43:
> > warning: ‘enum ethtool_header_flags’ declared inside parameter list
> > will not be visible outside of this definition or declaration
> >    23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value);
> >       |                                           ^~~~~~~~~~~~~~~~~~~~
> > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41:
> > warning: ‘enum ethtool_module_fw_flash_status’ declared inside
> > parameter list will not be visible outside of this definition or
> > declaration
> >    25 | ethtool_module_fw_flash_status_str(enum
> > ethtool_module_fw_flash_status value);
> >       |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45:
> > error: field ‘status’ has incomplete type
> >  6766 |         enum ethtool_module_fw_flash_status status;
> >       |                                             ^~~~~~
>
> This has been fixed via '#include <linux/ethtool_netlink.h>'
>
> > ncdevmem.c: In function ‘do_server’:
> > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known
> >   517 |                 struct dmabuf_token token;
>
> And this, and the rest, don't make sense at all?
>
> I'll double check on my side.

Oh, whoops, I forgot to headers_install first. This works for me:

➜  cos-kernel git:(tcpdevmem-fixes-1) ✗ sudo make headers_install &&
sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
TARGETS=ncdevmem 2>&1
  INSTALL ./usr/include
make: Entering directory
'/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
make: Nothing to be done for 'all'.
make: Leaving directory
'/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
➜  cos-kernel git:(tcpdevmem-fixes-1) ✗ find . -iname ncdevmem
./tools/testing/selftests/drivers/net/hw/ncdevmem

Sorry for the noise :D

-- 
Thanks,
Mina

Re: [PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[Stanislav Fomichev]

On 10/30, Mina Almasry wrote:
> On Wed, Oct 30, 2024 at 8:13 AM Stanislav Fomichev <stfomichev@gmail.com> wrote:
> >
> > On 10/30, Mina Almasry wrote:
> > > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > > >
> > > > The goal of the series is to simplify and make it possible to use
> > > > ncdevmem in an automated way from the ksft python wrapper.
> > > >
> > > > ncdevmem is slowly mutated into a state where it uses stdout
> > > > to print the payload and the python wrapper is added to
> > > > make sure the arrived payload matches the expected one.
> > > >
> > > > v6:
> > > > - fix compilation issue in 'Unify error handling' patch (Jakub)
> > > >
> > >
> > > Since I saw a compilation failures on a couple of iterations I
> > > cherry-picked this locally and tested compilation. I'm seeing this:
> >
> > Are you cherry picking the whole series or just this patch? It looks
> > too broken.
> >
> > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
> > > TARGETS=ncdevmem 2>&1
> > > make: Entering directory
> > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> > >   CC       ncdevmem
> > > In file included from ncdevmem.c:63:
> > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43:
> > > warning: ‘enum ethtool_header_flags’ declared inside parameter list
> > > will not be visible outside of this definition or declaration
> > >    23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value);
> > >       |                                           ^~~~~~~~~~~~~~~~~~~~
> > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41:
> > > warning: ‘enum ethtool_module_fw_flash_status’ declared inside
> > > parameter list will not be visible outside of this definition or
> > > declaration
> > >    25 | ethtool_module_fw_flash_status_str(enum
> > > ethtool_module_fw_flash_status value);
> > >       |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45:
> > > error: field ‘status’ has incomplete type
> > >  6766 |         enum ethtool_module_fw_flash_status status;
> > >       |                                             ^~~~~~
> >
> > This has been fixed via '#include <linux/ethtool_netlink.h>'
> >
> > > ncdevmem.c: In function ‘do_server’:
> > > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known
> > >   517 |                 struct dmabuf_token token;
> >
> > And this, and the rest, don't make sense at all?
> >
> > I'll double check on my side.
> 
> Oh, whoops, I forgot to headers_install first. This works for me:
> 
> ➜  cos-kernel git:(tcpdevmem-fixes-1) ✗ sudo make headers_install &&
> sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
> TARGETS=ncdevmem 2>&1
>   INSTALL ./usr/include
> make: Entering directory
> '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> make: Nothing to be done for 'all'.
> make: Leaving directory
> '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> ➜  cos-kernel git:(tcpdevmem-fixes-1) ✗ find . -iname ncdevmem
> ./tools/testing/selftests/drivers/net/hw/ncdevmem
> 
> Sorry for the noise :D

Whew, thanks and no worries!

Re: [PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[Mina Almasry]

On Wed, Oct 30, 2024 at 8:37 AM Stanislav Fomichev <stfomichev@gmail.com> wrote:
>
> On 10/30, Mina Almasry wrote:
> > On Wed, Oct 30, 2024 at 8:13 AM Stanislav Fomichev <stfomichev@gmail.com> wrote:
> > >
> > > On 10/30, Mina Almasry wrote:
> > > > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > > > >
> > > > > The goal of the series is to simplify and make it possible to use
> > > > > ncdevmem in an automated way from the ksft python wrapper.
> > > > >
> > > > > ncdevmem is slowly mutated into a state where it uses stdout
> > > > > to print the payload and the python wrapper is added to
> > > > > make sure the arrived payload matches the expected one.
> > > > >
> > > > > v6:
> > > > > - fix compilation issue in 'Unify error handling' patch (Jakub)
> > > > >
> > > >
> > > > Since I saw a compilation failures on a couple of iterations I
> > > > cherry-picked this locally and tested compilation. I'm seeing this:
> > >
> > > Are you cherry picking the whole series or just this patch? It looks
> > > too broken.
> > >
> > > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
> > > > TARGETS=ncdevmem 2>&1
> > > > make: Entering directory
> > > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> > > >   CC       ncdevmem
> > > > In file included from ncdevmem.c:63:
> > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43:
> > > > warning: ‘enum ethtool_header_flags’ declared inside parameter list
> > > > will not be visible outside of this definition or declaration
> > > >    23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value);
> > > >       |                                           ^~~~~~~~~~~~~~~~~~~~
> > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41:
> > > > warning: ‘enum ethtool_module_fw_flash_status’ declared inside
> > > > parameter list will not be visible outside of this definition or
> > > > declaration
> > > >    25 | ethtool_module_fw_flash_status_str(enum
> > > > ethtool_module_fw_flash_status value);
> > > >       |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45:
> > > > error: field ‘status’ has incomplete type
> > > >  6766 |         enum ethtool_module_fw_flash_status status;
> > > >       |                                             ^~~~~~
> > >
> > > This has been fixed via '#include <linux/ethtool_netlink.h>'
> > >
> > > > ncdevmem.c: In function ‘do_server’:
> > > > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known
> > > >   517 |                 struct dmabuf_token token;
> > >
> > > And this, and the rest, don't make sense at all?
> > >
> > > I'll double check on my side.
> >
> > Oh, whoops, I forgot to headers_install first. This works for me:
> >
> > ➜  cos-kernel git:(tcpdevmem-fixes-1) ✗ sudo make headers_install &&
> > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
> > TARGETS=ncdevmem 2>&1
> >   INSTALL ./usr/include
> > make: Entering directory
> > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> > make: Nothing to be done for 'all'.
> > make: Leaving directory
> > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> > ➜  cos-kernel git:(tcpdevmem-fixes-1) ✗ find . -iname ncdevmem
> > ./tools/testing/selftests/drivers/net/hw/ncdevmem
> >
> > Sorry for the noise :D
>
> Whew, thanks and no worries!

Sorry, 2 issues testing this series:

1. ipv4 addresses seem broken, or maybe i'm using them wrong.

Client command:
yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | tr \\n \\0 | head -c
1G | nc 192.168.1.4 5224 -p 5224

Server command and logs:
mina-1 /home/almasrymina # ./ncdevmem -s 192.168.1.4 -c 192.168.1.5 -l
-p 5224 -v 7 -f eth1
here: ynl.c:887:ynl_req_trampoline
using queues 15..16
Running: sudo ethtool -K eth1 ntuple off >&2
Running: sudo ethtool -K eth1 ntuple on >&2
Running: sudo ethtool -n eth1 | grep 'Filter:' | awk '{print $2}' |
xargs -n1 ethtool -N eth1 delete >&2
ethtool: bad command line argument(s)
For more information run ethtool -h
here: ynl.c:887:ynl_req_trampoline
TCP header split: on
Running: sudo ethtool -X eth1 equal 15 >&2
Running: sudo ethtool -N eth1 flow-type tcp6 src-ip 192.168.1.5 dst-ip
192.168.1.4 src-port 5224 dst-port 5224 queue 15 >&2
Invalid src-ip value[192.168.1.5]
ethtool: bad command line argument(s)
For more information run ethtool -h
./ncdevmem: Failed to configure flow steering

The ethtool command to configure flow steering is not working for me.
It thinks it's in v6 mode, when the ip address is a v4 address.
(notice `-s 192.168.1.4 -c 192.168.1.5`). flow-type should be tcp in
this case.

Reverting patch 9e2da4faeccf ("Revert "selftests: ncdevmem: Switch to
AF_INET6"") resolves this issue. Leading to the second issue:

2. Validation is now broken:

Client command:
yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | tr \\n \\0 | head -c
1G | nc 192.168.1.4 5224 -p 5224

Server command and logs: mina-1 /home/almasrymina # ./ncdevmem -s
192.168.1.4 -c 192.168.1.5 -l -p 5224 -v 7 -f eth1
here: ynl.c:887:ynl_req_trampoline
using queues 15..16
Running: sudo ethtool -K eth1 ntuple off >&2
Running: sudo ethtool -K eth1 ntuple on >&2
Running: sudo ethtool -n eth1 | grep 'Filter:' | awk '{print $2}' |
xargs -n1 ethtool -N eth1 delete >&2
ethtool: bad command line argument(s)
For more information run ethtool -h
here: ynl.c:887:ynl_req_trampoline
TCP header split: on
Running: sudo ethtool -X eth1 equal 15 >&2
Running: sudo ethtool -N eth1 flow-type tcp4 src-ip 192.168.1.5 dst-ip
192.168.1.4 src-port 5224 dst-port 5224 queue 15 >&2
Added rule with ID 19999
here: ynl.c:887:ynl_req_trampoline
got dmabuf id=1
binding to address 192.168.1.4:5224
Waiting or connection on 192.168.1.4:5224
Got connection from 192.168.1.5:5224
recvmsg ret=8192
received frag_page=15997, in_page_offset=0, frag_offset=65523712,
frag_size=4096, token=1, total_received=4096, dmabuf_id=1
Failed validation: expected=1, actual=0, index=0
Failed validation: expected=2, actual=0, index=1
Failed validation: expected=3, actual=0, index=2
Failed validation: expected=4, actual=0, index=3
Failed validation: expected=5, actual=0, index=4
Failed validation: expected=6, actual=0, index=5
Failed validation: expected=1, actual=0, index=7
Failed validation: expected=2, actual=0, index=8
Failed validation: expected=3, actual=0, index=9
Failed validation: expected=4, actual=0, index=10
Failed validation: expected=5, actual=0, index=11
Failed validation: expected=6, actual=0, index=12
Failed validation: expected=1, actual=0, index=14
Failed validation: expected=2, actual=0, index=15
Failed validation: expected=3, actual=0, index=16
Failed validation: expected=4, actual=0, index=17
Failed validation: expected=5, actual=0, index=18
Failed validation: expected=6, actual=0, index=19
Failed validation: expected=1, actual=0, index=21
Failed validation: expected=2, actual=0, index=22
Failed validation: expected=3, actual=0, index=23
./ncdevmem: validation failed.

I haven't debugged issue #2 yet, but both need to be resolved before
merge. I'm happy to provide more details if you can't repro. My setup:

mina-1 /home/almasrymina # cat /boot/config-6.12.0-rc4  | grep -i ipv6
CONFIG_IPV6=y
mina-1 /home/almasrymina # cat /proc/sys/net/ipv6/bindv6only
0

-- 
Thanks,
Mina

Re: [PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[Mina Almasry]

On Thu, Oct 31, 2024 at 9:45 AM Mina Almasry <almasrymina@google.com> wrote:
>
...
>
> Sorry, 2 issues testing this series:
>
...
>
> 2. Validation is now broken:
>

Validation is re-fixed with this diff:

diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c
b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
index 692c189bb5cc..494ae66d8abf 100644
--- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c
+++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
@@ -568,8 +568,7 @@ int do_server(struct memory_buffer *mem)

                        if (do_validation)
                                validate_buffer(
-                                       ((unsigned char *)tmp_mem) +
-                                               dmabuf_cmsg->frag_offset,
+                                       ((unsigned char *)tmp_mem),
                                        dmabuf_cmsg->frag_size);
                        else
                                print_nonzero_bytes(tmp_mem,
dmabuf_cmsg->frag_size);

Since memcpy_from_device copies to the beginning of tmp_mem, then the
beginning tmp_mem should be passed to the validation.

-- 
Thanks,
Mina

Re: [PATCH net-next v6 00/12] selftests: ncdevmem: Add ncdevmem to ksft

[Stanislav Fomichev]

On 10/31, Mina Almasry wrote:
> On Wed, Oct 30, 2024 at 8:37 AM Stanislav Fomichev <stfomichev@gmail.com> wrote:
> >
> > On 10/30, Mina Almasry wrote:
> > > On Wed, Oct 30, 2024 at 8:13 AM Stanislav Fomichev <stfomichev@gmail.com> wrote:
> > > >
> > > > On 10/30, Mina Almasry wrote:
> > > > > On Wed, Oct 30, 2024 at 7:27 AM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > > > > >
> > > > > > The goal of the series is to simplify and make it possible to use
> > > > > > ncdevmem in an automated way from the ksft python wrapper.
> > > > > >
> > > > > > ncdevmem is slowly mutated into a state where it uses stdout
> > > > > > to print the payload and the python wrapper is added to
> > > > > > make sure the arrived payload matches the expected one.
> > > > > >
> > > > > > v6:
> > > > > > - fix compilation issue in 'Unify error handling' patch (Jakub)
> > > > > >
> > > > >
> > > > > Since I saw a compilation failures on a couple of iterations I
> > > > > cherry-picked this locally and tested compilation. I'm seeing this:
> > > >
> > > > Are you cherry picking the whole series or just this patch? It looks
> > > > too broken.
> > > >
> > > > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
> > > > > TARGETS=ncdevmem 2>&1
> > > > > make: Entering directory
> > > > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> > > > >   CC       ncdevmem
> > > > > In file included from ncdevmem.c:63:
> > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:23:43:
> > > > > warning: ‘enum ethtool_header_flags’ declared inside parameter list
> > > > > will not be visible outside of this definition or declaration
> > > > >    23 | const char *ethtool_header_flags_str(enum ethtool_header_flags value);
> > > > >       |                                           ^~~~~~~~~~~~~~~~~~~~
> > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:25:41:
> > > > > warning: ‘enum ethtool_module_fw_flash_status’ declared inside
> > > > > parameter list will not be visible outside of this definition or
> > > > > declaration
> > > > >    25 | ethtool_module_fw_flash_status_str(enum
> > > > > ethtool_module_fw_flash_status value);
> > > > >       |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > > /usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/../../../tools/net/ynl/generated/ethtool-user.h:6766:45:
> > > > > error: field ‘status’ has incomplete type
> > > > >  6766 |         enum ethtool_module_fw_flash_status status;
> > > > >       |                                             ^~~~~~
> > > >
> > > > This has been fixed via '#include <linux/ethtool_netlink.h>'
> > > >
> > > > > ncdevmem.c: In function ‘do_server’:
> > > > > ncdevmem.c:517:37: error: storage size of ‘token’ isn’t known
> > > > >   517 |                 struct dmabuf_token token;
> > > >
> > > > And this, and the rest, don't make sense at all?
> > > >
> > > > I'll double check on my side.
> > >
> > > Oh, whoops, I forgot to headers_install first. This works for me:
> > >
> > > ➜  cos-kernel git:(tcpdevmem-fixes-1) ✗ sudo make headers_install &&
> > > sudo CFLAGS="-static" make -C ./tools/testing/selftests/drivers/net/hw
> > > TARGETS=ncdevmem 2>&1
> > >   INSTALL ./usr/include
> > > make: Entering directory
> > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> > > make: Nothing to be done for 'all'.
> > > make: Leaving directory
> > > '/usr/local/google/home/almasrymina/cos-kernel/tools/testing/selftests/drivers/net/hw'
> > > ➜  cos-kernel git:(tcpdevmem-fixes-1) ✗ find . -iname ncdevmem
> > > ./tools/testing/selftests/drivers/net/hw/ncdevmem
> > >
> > > Sorry for the noise :D
> >
> > Whew, thanks and no worries!
> 
> Sorry, 2 issues testing this series:

Thank you for testing!


> 1. ipv4 addresses seem broken, or maybe i'm using them wrong.
> 
> Client command:
> yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | tr \\n \\0 | head -c
> 1G | nc 192.168.1.4 5224 -p 5224
> 
> Server command and logs:
> mina-1 /home/almasrymina # ./ncdevmem -s 192.168.1.4 -c 192.168.1.5 -l
> -p 5224 -v 7 -f eth1
> here: ynl.c:887:ynl_req_trampoline
> using queues 15..16
> Running: sudo ethtool -K eth1 ntuple off >&2
> Running: sudo ethtool -K eth1 ntuple on >&2
> Running: sudo ethtool -n eth1 | grep 'Filter:' | awk '{print $2}' |
> xargs -n1 ethtool -N eth1 delete >&2
> ethtool: bad command line argument(s)
> For more information run ethtool -h
> here: ynl.c:887:ynl_req_trampoline
> TCP header split: on
> Running: sudo ethtool -X eth1 equal 15 >&2
> Running: sudo ethtool -N eth1 flow-type tcp6 src-ip 192.168.1.5 dst-ip
> 192.168.1.4 src-port 5224 dst-port 5224 queue 15 >&2
> Invalid src-ip value[192.168.1.5]
> ethtool: bad command line argument(s)
> For more information run ethtool -h
> ./ncdevmem: Failed to configure flow steering
> 
> The ethtool command to configure flow steering is not working for me.
> It thinks it's in v6 mode, when the ip address is a v4 address.
> (notice `-s 192.168.1.4 -c 192.168.1.5`). flow-type should be tcp in
> this case.
> 
> Reverting patch 9e2da4faeccf ("Revert "selftests: ncdevmem: Switch to
> AF_INET6"") resolves this issue. Leading to the second issue:

For IPv4, you have to use IPv4-mapped-IPv6, so your invocation needs to be:

./ncdevmem -s ::ffff:192.168.1.4 -c ::ffff:192.168.1.5 ...

Can you try that? I actually never tested that with non-ffff-prefixed
addresses, maybe that needs some error handling or something. Will
double-check on my side.


> 2. Validation is now broken:
> 
> Client command:
> yes $(echo -e \\x01\\x02\\x03\\x04\\x05\\x06) | tr \\n \\0 | head -c
> 1G | nc 192.168.1.4 5224 -p 5224
> 
> Server command and logs: mina-1 /home/almasrymina # ./ncdevmem -s
> 192.168.1.4 -c 192.168.1.5 -l -p 5224 -v 7 -f eth1
> here: ynl.c:887:ynl_req_trampoline
> using queues 15..16
> Running: sudo ethtool -K eth1 ntuple off >&2
> Running: sudo ethtool -K eth1 ntuple on >&2
> Running: sudo ethtool -n eth1 | grep 'Filter:' | awk '{print $2}' |
> xargs -n1 ethtool -N eth1 delete >&2
> ethtool: bad command line argument(s)
> For more information run ethtool -h
> here: ynl.c:887:ynl_req_trampoline
> TCP header split: on
> Running: sudo ethtool -X eth1 equal 15 >&2
> Running: sudo ethtool -N eth1 flow-type tcp4 src-ip 192.168.1.5 dst-ip
> 192.168.1.4 src-port 5224 dst-port 5224 queue 15 >&2
> Added rule with ID 19999
> here: ynl.c:887:ynl_req_trampoline
> got dmabuf id=1
> binding to address 192.168.1.4:5224
> Waiting or connection on 192.168.1.4:5224
> Got connection from 192.168.1.5:5224
> recvmsg ret=8192
> received frag_page=15997, in_page_offset=0, frag_offset=65523712,
> frag_size=4096, token=1, total_received=4096, dmabuf_id=1
> Failed validation: expected=1, actual=0, index=0
> Failed validation: expected=2, actual=0, index=1
> Failed validation: expected=3, actual=0, index=2
> Failed validation: expected=4, actual=0, index=3
> Failed validation: expected=5, actual=0, index=4
> Failed validation: expected=6, actual=0, index=5
> Failed validation: expected=1, actual=0, index=7
> Failed validation: expected=2, actual=0, index=8
> Failed validation: expected=3, actual=0, index=9
> Failed validation: expected=4, actual=0, index=10
> Failed validation: expected=5, actual=0, index=11
> Failed validation: expected=6, actual=0, index=12
> Failed validation: expected=1, actual=0, index=14
> Failed validation: expected=2, actual=0, index=15
> Failed validation: expected=3, actual=0, index=16
> Failed validation: expected=4, actual=0, index=17
> Failed validation: expected=5, actual=0, index=18
> Failed validation: expected=6, actual=0, index=19
> Failed validation: expected=1, actual=0, index=21
> Failed validation: expected=2, actual=0, index=22
> Failed validation: expected=3, actual=0, index=23
> ./ncdevmem: validation failed.
> 
> I haven't debugged issue #2 yet, but both need to be resolved before
> merge. I'm happy to provide more details if you can't repro. My setup:
> 
> mina-1 /home/almasrymina # cat /boot/config-6.12.0-rc4  | grep -i ipv6
> CONFIG_IPV6=y
> mina-1 /home/almasrymina # cat /proc/sys/net/ipv6/bindv6only
> 0

I see you've fixed it out already in a follow up, will pull in the fix!

---
pw-bot: cr