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.
v7: - fix validation (Mina) - add support for working with non ::ffff-prefixed addresses (Mina)
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 | 786 ++++++++++++++++++ tools/testing/selftests/net/.gitignore | 1 - tools/testing/selftests/net/Makefile | 8 - tools/testing/selftests/net/ncdevmem.c | 570 ------------- 7 files changed, 841 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();
On Mon, Nov 04, 2024 at 10:14:19AM -0800, Stanislav Fomichev wrote:
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(-)
Reviewed-by: Joe Damato jdamato@fastly.com
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 | 203 +++++++++++++++---------- 1 file changed, 119 insertions(+), 84 deletions(-)
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c index 9245d3f158dd..3e7ef2eedd60 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,16 @@ 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) + - dmabuf_cmsg->frag_offset, - dmabuf_cmsg->frag_size); + validate_buffer(tmp_mem, + 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 +486,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 +497,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 +512,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 +525,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 +538,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 +547,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 +596,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; }
On Mon, Nov 04, 2024 at 10:14:20AM -0800, Stanislav Fomichev wrote:
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 | 203 +++++++++++++++---------- 1 file changed, 119 insertions(+), 84 deletions(-)
Reviewed-by: Joe Damato jdamato@fastly.com
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 3e7ef2eedd60..4733d1a0aab5 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);
On Mon, Nov 04, 2024 at 10:14:21AM -0800, Stanislav Fomichev wrote:
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 3e7ef2eedd60..4733d1a0aab5 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);
A minor nit (definitely not worth re-sending for this on its own): it might be helpful to add which of the sockopts failed to the error message REUSEADDR or REUSEPORT.
Reviewed-by: Joe Damato jdamato@fastly.com
On 11/04, Joe Damato wrote:
On Mon, Nov 04, 2024 at 10:14:21AM -0800, Stanislav Fomichev wrote:
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 3e7ef2eedd60..4733d1a0aab5 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);
A minor nit (definitely not worth re-sending for this on its own): it might be helpful to add which of the sockopts failed to the error message REUSEADDR or REUSEPORT.
Reviewed-by: Joe Damato jdamato@fastly.com
Thank you for the review!
I later move these two under enable_reuseaddr and make it even less debuggable :-( Let me maybe keep the calls to error() inside the enable_reuseaddr.
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 4733d1a0aab5..faa9dce121c7 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,
On Mon, Nov 04, 2024 at 10:14:22AM -0800, Stanislav Fomichev wrote:
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(-)
Reviewed-by: Joe Damato jdamato@fastly.com
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 faa9dce121c7..0feeca56c049 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;
@@ -595,6 +603,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++)
On Mon, Nov 04, 2024 at 10:14:23AM -0800, Stanislav Fomichev wrote:
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(-)
Reviewed-by: Joe Damato jdamato@fastly.com
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 | 99 ++++++++++++++++++-------- 1 file changed, 71 insertions(+), 28 deletions(-)
diff --git a/tools/testing/selftests/net/ncdevmem.c b/tools/testing/selftests/net/ncdevmem.c index 0feeca56c049..c4897b2bdc7c 100644 --- a/tools/testing/selftests/net/ncdevmem.c +++ b/tools/testing/selftests/net/ncdevmem.c @@ -242,13 +242,26 @@ 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 *type = "tcp6"; + const char *server_addr; + char buf[256]; + + inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf)); + server_addr = buf; + + if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) { + type = "tcp4"; + server_addr = strrchr(server_addr, ':') + 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 +312,53 @@ 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 != 1) { + /* fallback to plain IPv4 */ + ret = inet_pton(AF_INET, str, &sin6->sin6_addr.s6_addr32[3]); + if (ret != 1) + return -1; + + /* add ::ffff prefix */ + sin6->sin6_addr.s6_addr32[0] = 0; + sin6->sin6_addr.s6_addr32[1] = 0; + sin6->sin6_addr.s6_addr16[4] = 0; + sin6->sin6_addr.s6_addr16[5] = 0xffff; + } + + 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 +370,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 +384,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 +405,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 +426,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,
On Mon, Nov 04, 2024 at 10:14:24AM -0800, Stanislav Fomichev wrote:
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 | 99 ++++++++++++++++++-------- 1 file changed, 71 insertions(+), 28 deletions(-)
Reviewed-by: Joe Damato jdamato@fastly.com
On Mon, 4 Nov 2024 10:14:24 -0800 Stanislav Fomichev wrote:
-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 *type = "tcp6";
- const char *server_addr;
- char buf[256];
- inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf));
- server_addr = buf;
- if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) {
type = "tcp4";
server_addr = strrchr(server_addr, ':') + 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);
nit: I think this generate a truncation warning, not sure if it's easy to fix:
ncdevmem.c:259:28: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size between 209 and 215 [-Wformat-truncation=] 259 | return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
maybe make buf smaller? 🤔️
On 11/04, Jakub Kicinski wrote:
On Mon, 4 Nov 2024 10:14:24 -0800 Stanislav Fomichev wrote:
-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 *type = "tcp6";
- const char *server_addr;
- char buf[256];
- inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf));
- server_addr = buf;
- if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) {
type = "tcp4";
server_addr = strrchr(server_addr, ':') + 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);
nit: I think this generate a truncation warning, not sure if it's easy to fix:
ncdevmem.c:259:28: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size between 209 and 215 [-Wformat-truncation=] 259 | return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
maybe make buf smaller? 🤔️
Are you having some extra -W arguments to make it trigger on the build bot? Wonder why I don't see the warning locally.. (will try to pass -Wformat-truncation)
On 11/04, Stanislav Fomichev wrote:
On 11/04, Jakub Kicinski wrote:
On Mon, 4 Nov 2024 10:14:24 -0800 Stanislav Fomichev wrote:
-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 *type = "tcp6";
- const char *server_addr;
- char buf[256];
- inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf));
- server_addr = buf;
- if (IN6_IS_ADDR_V4MAPPED(&server_sin->sin6_addr)) {
type = "tcp4";
server_addr = strrchr(server_addr, ':') + 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);
nit: I think this generate a truncation warning, not sure if it's easy to fix:
ncdevmem.c:259:28: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size between 209 and 215 [-Wformat-truncation=] 259 | return run_command("sudo ethtool -N %s flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d >&2", | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
maybe make buf smaller? 🤔️
Are you having some extra -W arguments to make it trigger on the build bot? Wonder why I don't see the warning locally.. (will try to pass -Wformat-truncation)
It is indeed -Wformat-truncation but only gcc complains about it, not clang. Setting buf size to 40 calms it down.
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 c4897b2bdc7c..8062d26fbce0 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)
On Mon, Nov 04, 2024 at 10:14:25AM -0800, Stanislav Fomichev wrote:
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(-)
Reviewed-by: Joe Damato jdamato@fastly.com
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 8062d26fbce0..be89735d6408 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) @@ -384,6 +434,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");
On Mon, Nov 04, 2024 at 10:14:26AM -0800, Stanislav Fomichev wrote:
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(-)
Reviewed-by: Joe Damato jdamato@fastly.com
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 be89735d6408..044198ce02a7 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]; \ @@ -715,6 +742,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]);
On Mon, Nov 04, 2024 at 10:14:27AM -0800, Stanislav Fomichev wrote:
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(-)
Reviewed-by: Joe Damato jdamato@fastly.com
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 044198ce02a7..270a77206f65 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; @@ -731,19 +731,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"); @@ -754,7 +766,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;
On Mon, Nov 04, 2024 at 10:14:28AM -0800, Stanislav Fomichev wrote:
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 044198ce02a7..270a77206f65 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; @@ -731,19 +731,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;
Sorry for the beginner question :) -- is it possible that rxq_num ever returns 1 and thus start_queue = 0, num_queues = 0
}
if (start_queue < 0 || num_queues < 0)
error(1, 0, "Both -t and -q are required\n");
And then isn't caught here because this only checks < 0 (instead of num_queues <= 0) ?
On 11/04, Joe Damato wrote:
On Mon, Nov 04, 2024 at 10:14:28AM -0800, Stanislav Fomichev wrote:
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 044198ce02a7..270a77206f65 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; @@ -731,19 +731,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;
Sorry for the beginner question :) -- is it possible that rxq_num ever returns 1 and thus start_queue = 0, num_queues = 0
}
if (start_queue < 0 || num_queues < 0)
error(1, 0, "Both -t and -q are required\n");
And then isn't caught here because this only checks < 0 (instead of num_queues <= 0) ?
In theory it's possible to configure a netdev with a single queue. I can add an extra 'num_queues == 1' check just in case...
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 Mon, Nov 04, 2024 at 10:14:30AM -0800, Stanislav Fomichev wrote:
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)
FWIW, in the v3 of the series I submit, Jakub asked me to replace nc with socat due to issues with nc [1].
Your usage of nc seems pretty basic though, so maybe it's fine?
[1]: https://lore.kernel.org/netdev/20241101063426.2e1423a8@kernel.org/
On Mon, 4 Nov 2024 16:15:10 -0800 Joe Damato wrote:
- 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)
FWIW, in the v3 of the series I submit, Jakub asked me to replace nc with socat due to issues with nc [1].
Your usage of nc seems pretty basic though, so maybe it's fine?
Good catch, let's not use nc. I seem to recall it's also funny about address family selection. Also may be useful to add a helper for sending simple strings from remote to avoid having this problem again.
On 11/04, Joe Damato wrote:
On Mon, Nov 04, 2024 at 10:14:30AM -0800, Stanislav Fomichev wrote:
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)
FWIW, in the v3 of the series I submit, Jakub asked me to replace nc with socat due to issues with nc [1].
Your usage of nc seems pretty basic though, so maybe it's fine?
Since I'm doing a respin anyway will try to move to socat as well to keep it uniform, thanks!
linux-kselftest-mirror@lists.linaro.org