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
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();
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; }
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);
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,
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++)
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,
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)
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");
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]);
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;
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
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()
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?
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.
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
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!
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
On Thu, Oct 31, 2024 at 9:45 AM Mina Almasry almasrymina@google.com wrote:
...
Sorry, 2 issues testing this series:
...
- 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.
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!
- 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.
- 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
linux-kselftest-mirror@lists.linaro.org