Make ncdevmem clean up after itself. While at it make sure it sets HDS threshold to 0 automatically.
Jakub Kicinski (4): selftests: drv-net: ncdevmem: remove use of error() selftests: drv-net: ncdevmem: save IDs of flow rules we added selftests: drv-net: ncdevmem: restore old channel config selftests: drv-net: ncdevmem: configure and restore HDS threshold
.../selftests/drivers/net/hw/ncdevmem.c | 773 +++++++++++++----- 1 file changed, 566 insertions(+), 207 deletions(-)
Using error() makes it impossible for callers to unwind their changes. Replace error() calls with proper error handling.
Signed-off-by: Jakub Kicinski kuba@kernel.org --- .../selftests/drivers/net/hw/ncdevmem.c | 528 ++++++++++++------ 1 file changed, 364 insertions(+), 164 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index 71961a7688e6..e75adfed33ac 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -115,6 +115,21 @@ struct memory_provider { size_t off, int n); };
+static void pr_err(const char *fmt, ...) +{ + va_list args; + + fprintf(stderr, "%s: ", TEST_PREFIX); + + va_start(args, fmt); + vfprintf(stderr, fmt, args); + va_end(args); + + if (errno != 0) + fprintf(stderr, ": %s", strerror(errno)); + fprintf(stderr, "\n"); +} + static struct memory_buffer *udmabuf_alloc(size_t size) { struct udmabuf_create create; @@ -123,27 +138,33 @@ static struct memory_buffer *udmabuf_alloc(size_t size)
ctx = malloc(sizeof(*ctx)); if (!ctx) - error(1, ENOMEM, "malloc failed"); + return NULL;
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); + if (ctx->devfd < 0) { + pr_err("[skip,no-udmabuf: Unable to access DMA buffer device file]"); + goto err_free_ctx; + }
ctx->memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING); - if (ctx->memfd < 0) - error(1, errno, "%s: [skip,no-memfd]\n", TEST_PREFIX); + if (ctx->memfd < 0) { + pr_err("[skip,no-memfd]"); + goto err_close_dev; + }
ret = fcntl(ctx->memfd, F_ADD_SEALS, F_SEAL_SHRINK); - if (ret < 0) - error(1, errno, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX); + if (ret < 0) { + pr_err("[skip,fcntl-add-seals]"); + goto err_close_memfd; + }
ret = ftruncate(ctx->memfd, size); - if (ret == -1) - error(1, errno, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX); + if (ret == -1) { + pr_err("[FAIL,memfd-truncate]"); + goto err_close_memfd; + }
memset(&create, 0, sizeof(create));
@@ -151,15 +172,29 @@ static struct memory_buffer *udmabuf_alloc(size_t size) 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); + if (ctx->fd < 0) { + pr_err("[FAIL, create udmabuf]"); + goto err_close_fd; + }
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); + if (ctx->buf_mem == MAP_FAILED) { + pr_err("[FAIL, map udmabuf]"); + goto err_close_fd; + }
return ctx; + +err_close_fd: + close(ctx->fd); +err_close_memfd: + close(ctx->memfd); +err_close_dev: + close(ctx->devfd); +err_free_ctx: + free(ctx); + return NULL; }
static void udmabuf_free(struct memory_buffer *ctx) @@ -217,7 +252,7 @@ static void print_nonzero_bytes(void *ptr, size_t size) putchar(p[i]); }
-void validate_buffer(void *line, size_t size) +int validate_buffer(void *line, size_t size) { static unsigned char seed = 1; unsigned char *ptr = line; @@ -232,8 +267,10 @@ void validate_buffer(void *line, size_t size) "Failed validation: expected=%u, actual=%u, index=%lu\n", expected, ptr[i], i); errors++; - if (errors > 20) - error(1, 0, "validation failed."); + if (errors > 20) { + pr_err("validation failed"); + return -1; + } } seed++; if (seed == do_validation) @@ -241,6 +278,7 @@ void validate_buffer(void *line, size_t size) }
fprintf(stdout, "Validated buffer\n"); + return 0; }
static int rxq_num(int ifindex) @@ -279,7 +317,7 @@ static int rxq_num(int ifindex) system(command); \ })
-static int reset_flow_steering(void) +static void reset_flow_steering(void) { /* Depending on the NIC, toggling ntuple off and on might not * be allowed. Additionally, attempting to delete existing filters @@ -292,7 +330,6 @@ static int reset_flow_steering(void) run_command( "ethtool -n %s | grep 'Filter:' | awk '{print $2}' | xargs -n1 ethtool -N %s delete >&2", ifname, ifname); - return 0; }
static const char *tcp_data_split_str(int val) @@ -354,6 +391,11 @@ static int configure_rss(void) return run_command("ethtool -X %s equal %d >&2", ifname, start_queue); }
+static void reset_rss(void) +{ + run_command("ethtool -X %s default >&2", ifname, start_queue); +} + static int configure_channels(unsigned int rx, unsigned int tx) { struct ethtool_channels_get_req *gchan; @@ -479,6 +521,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd,
*ys = ynl_sock_create(&ynl_netdev_family, &yerr); if (!*ys) { + netdev_queue_id_free(queues); fprintf(stderr, "YNL: %s\n", yerr.msg); return -1; } @@ -557,18 +600,24 @@ static int bind_tx_queue(unsigned int ifindex, unsigned int dmabuf_fd, return -1; }
-static void enable_reuseaddr(int fd) +static int enable_reuseaddr(int fd) { int opt = 1; int ret;
ret = setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &opt, sizeof(opt)); - if (ret) - error(1, errno, "%s: [FAIL, SO_REUSEPORT]\n", TEST_PREFIX); + if (ret) { + pr_err("SO_REUSEPORT failed"); + return -1; + }
ret = setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)); - if (ret) - error(1, errno, "%s: [FAIL, SO_REUSEADDR]\n", TEST_PREFIX); + if (ret) { + pr_err("SO_REUSEADDR failed"); + return -1; + } + + return 0; }
static int parse_address(const char *str, int port, struct sockaddr_in6 *sin6) @@ -622,54 +671,70 @@ static int do_server(struct memory_buffer *mem) char *tmp_mem = NULL; struct ynl_sock *ys; char iobuf[819200]; + int ret, err = -1; char buffer[256]; int socket_fd; int client_fd; - int ret;
ret = parse_address(server_ip, atoi(port), &server_sin); - if (ret < 0) - error(1, 0, "parse server address"); + if (ret < 0) { + pr_err("parse server address"); + return -1; + }
- if (reset_flow_steering()) - error(1, 0, "Failed to reset flow steering\n"); + reset_flow_steering();
- if (configure_headersplit(1)) - error(1, 0, "Failed to enable TCP header split\n"); + if (configure_headersplit(1)) { + pr_err("Failed to enable TCP header split"); + return -1; + }
/* Configure RSS to divert all traffic from our devmem queues */ - if (configure_rss()) - error(1, 0, "Failed to configure rss\n"); + if (configure_rss()) { + pr_err("Failed to configure rss"); + goto err_reset_headersplit; + }
/* Flow steer our devmem flows to start_queue */ - if (configure_flow_steering(&server_sin)) - error(1, 0, "Failed to configure flow steering\n"); + if (configure_flow_steering(&server_sin)) { + pr_err("Failed to configure flow steering"); + goto err_reset_rss; + }
sleep(1);
- if (bind_rx_queue(ifindex, mem->fd, create_queues(), num_queues, &ys)) - error(1, 0, "Failed to bind\n"); + if (bind_rx_queue(ifindex, mem->fd, create_queues(), num_queues, &ys)) { + pr_err("Failed to bind"); + goto err_reset_flow_steering; + }
tmp_mem = malloc(mem->size); if (!tmp_mem) - error(1, ENOMEM, "malloc failed"); + goto err_unbind;
socket_fd = socket(AF_INET6, SOCK_STREAM, 0); - if (socket_fd < 0) - error(1, errno, "%s: [FAIL, create socket]\n", TEST_PREFIX); + if (socket_fd < 0) { + pr_err("Failed to create socket"); + goto err_free_tmp; + }
- enable_reuseaddr(socket_fd); + if (enable_reuseaddr(socket_fd)) + goto err_close_socket;
fprintf(stderr, "binding to address %s:%d\n", server_ip, ntohs(server_sin.sin6_port));
ret = bind(socket_fd, &server_sin, sizeof(server_sin)); - if (ret) - error(1, errno, "%s: [FAIL, bind]\n", TEST_PREFIX); + if (ret) { + pr_err("Failed to bind"); + goto err_close_socket; + }
ret = listen(socket_fd, 1); - if (ret) - error(1, errno, "%s: [FAIL, listen]\n", TEST_PREFIX); + if (ret) { + pr_err("Failed to listen"); + goto err_close_socket; + }
client_addr_len = sizeof(client_addr);
@@ -678,6 +743,10 @@ static int do_server(struct memory_buffer *mem) fprintf(stderr, "Waiting or connection on %s:%d\n", buffer, ntohs(server_sin.sin6_port)); client_fd = accept(socket_fd, &client_addr, &client_addr_len); + if (client_fd < 0) { + pr_err("Failed to accept"); + goto err_close_socket; + }
inet_ntop(AF_INET6, &client_addr.sin6_addr, buffer, sizeof(buffer)); @@ -708,7 +777,8 @@ static int do_server(struct memory_buffer *mem) continue; } if (ret == 0) { - fprintf(stderr, "client exited\n"); + errno = 0; + pr_err("client exited"); goto cleanup; }
@@ -746,9 +816,10 @@ static int do_server(struct memory_buffer *mem) dmabuf_cmsg->frag_size, dmabuf_cmsg->frag_token, total_received, dmabuf_cmsg->dmabuf_id);
- if (dmabuf_cmsg->dmabuf_id != dmabuf_id) - error(1, 0, - "received on wrong dmabuf_id: flow steering error\n"); + if (dmabuf_cmsg->dmabuf_id != dmabuf_id) { + pr_err("received on wrong dmabuf_id: flow steering error"); + goto err_close_client; + }
if (dmabuf_cmsg->frag_size % getpagesize()) non_page_aligned_frags++; @@ -759,22 +830,27 @@ static int do_server(struct memory_buffer *mem) dmabuf_cmsg->frag_offset, dmabuf_cmsg->frag_size);
- if (do_validation) - validate_buffer(tmp_mem, - dmabuf_cmsg->frag_size); - else + if (do_validation) { + if (validate_buffer(tmp_mem, + dmabuf_cmsg->frag_size)) + goto err_close_client; + } else { print_nonzero_bytes(tmp_mem, dmabuf_cmsg->frag_size); + }
ret = setsockopt(client_fd, SOL_SOCKET, SO_DEVMEM_DONTNEED, &token, sizeof(token)); - if (ret != 1) - error(1, 0, - "SO_DEVMEM_DONTNEED not enough tokens"); + if (ret != 1) { + pr_err("SO_DEVMEM_DONTNEED not enough tokens"); + goto err_close_client; + } + } + if (!is_devmem) { + pr_err("flow steering error"); + goto err_close_client; } - if (!is_devmem) - error(1, 0, "flow steering error\n");
fprintf(stderr, "total_received=%lu\n", total_received); } @@ -785,54 +861,112 @@ static int do_server(struct memory_buffer *mem) page_aligned_frags, non_page_aligned_frags);
cleanup: + err = 0;
- free(tmp_mem); +err_close_client: close(client_fd); +err_close_socket: close(socket_fd); +err_free_tmp: + free(tmp_mem); +err_unbind: ynl_sock_destroy(ys); - - return 0; +err_reset_flow_steering: + reset_flow_steering(); +err_reset_rss: + reset_rss(); +err_reset_headersplit: + configure_headersplit(0); + return err; }
-void run_devmem_tests(void) +int run_devmem_tests(void) { + struct netdev_queue_id *queues; struct memory_buffer *mem; struct ynl_sock *ys; + int err = -1;
mem = provider->alloc(getpagesize() * NUM_PAGES); + if (!mem) { + pr_err("Failed to allocate memory buffer"); + return -1; + }
/* Configure RSS to divert all traffic from our devmem queues */ - if (configure_rss()) - error(1, 0, "rss error\n"); + if (configure_rss()) { + pr_err("rss error"); + goto err_free_mem; + }
- if (configure_headersplit(1)) - error(1, 0, "Failed to configure header split\n"); + if (configure_headersplit(1)) { + pr_err("Failed to configure header split"); + goto err_reset_rss; + }
- if (!bind_rx_queue(ifindex, mem->fd, - calloc(num_queues, sizeof(struct netdev_queue_id)), - num_queues, &ys)) - error(1, 0, "Binding empty queues array should have failed\n"); + queues = netdev_queue_id_alloc(num_queues); + if (!queues) { + pr_err("Failed to allocate empty queues array"); + goto err_reset_headersplit; + }
- if (configure_headersplit(0)) - error(1, 0, "Failed to configure header split\n"); + if (!bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) { + pr_err("Binding empty queues array should have failed"); + goto err_unbind; + }
- if (!bind_rx_queue(ifindex, mem->fd, create_queues(), num_queues, &ys)) - error(1, 0, "Configure dmabuf with header split off should have failed\n"); + if (configure_headersplit(0)) { + pr_err("Failed to configure header split"); + goto err_reset_rss; + }
- if (configure_headersplit(1)) - error(1, 0, "Failed to configure header split\n"); + queues = create_queues(); + if (!queues) { + pr_err("Failed to create queues"); + goto err_reset_rss; + }
- if (bind_rx_queue(ifindex, mem->fd, create_queues(), num_queues, &ys)) - error(1, 0, "Failed to bind\n"); + if (!bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) { + pr_err("Configure dmabuf with header split off should have failed"); + goto err_unbind; + } + + if (configure_headersplit(1)) { + pr_err("Failed to configure header split"); + goto err_reset_rss; + } + + queues = create_queues(); + if (!queues) { + pr_err("Failed to create queues"); + goto err_reset_headersplit; + } + + if (bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) { + pr_err("Failed to bind"); + goto err_reset_headersplit; + }
/* Deactivating a bound queue should not be legal */ - if (!configure_channels(num_queues, num_queues)) - error(1, 0, "Deactivating a bound queue should be illegal.\n"); + if (!configure_channels(num_queues, num_queues)) { + pr_err("Deactivating a bound queue should be illegal"); + goto err_reset_channels; + }
- /* Closing the netlink socket does an implicit unbind */ + err = 0; + goto err_unbind; + +err_reset_channels: + /* TODO */ +err_unbind: ynl_sock_destroy(ys); - +err_reset_headersplit: + configure_headersplit(0); +err_reset_rss: + reset_rss(); +err_free_mem: provider->free(mem); + return err; }
static uint64_t gettimeofday_ms(void) @@ -852,13 +986,15 @@ static int do_poll(int fd) pfd.fd = fd;
ret = poll(&pfd, 1, waittime_ms); - if (ret == -1) - error(1, errno, "poll"); + if (ret == -1) { + pr_err("poll"); + return -1; + }
return ret && (pfd.revents & POLLERR); }
-static void wait_compl(int fd) +static int wait_compl(int fd) { int64_t tstop = gettimeofday_ms() + waittime_ms; char control[CMSG_SPACE(100)] = {}; @@ -872,18 +1008,23 @@ static void wait_compl(int fd) msg.msg_controllen = sizeof(control);
while (gettimeofday_ms() < tstop) { - if (!do_poll(fd)) + ret = do_poll(fd); + if (ret < 0) + return ret; + if (!ret) continue;
ret = recvmsg(fd, &msg, MSG_ERRQUEUE); if (ret < 0) { if (errno == EAGAIN) continue; - error(1, errno, "recvmsg(MSG_ERRQUEUE)"); - return; + pr_err("recvmsg(MSG_ERRQUEUE)"); + return -1; + } + if (msg.msg_flags & MSG_CTRUNC) { + pr_err("MSG_CTRUNC"); + return -1; } - if (msg.msg_flags & MSG_CTRUNC) - error(1, 0, "MSG_CTRUNC\n");
for (cm = CMSG_FIRSTHDR(&msg); cm; cm = CMSG_NXTHDR(&msg, cm)) { if (cm->cmsg_level != SOL_IP && @@ -897,20 +1038,25 @@ static void wait_compl(int fd) continue;
serr = (void *)CMSG_DATA(cm); - if (serr->ee_origin != SO_EE_ORIGIN_ZEROCOPY) - error(1, 0, "wrong origin %u", serr->ee_origin); - if (serr->ee_errno != 0) - error(1, 0, "wrong errno %d", serr->ee_errno); + if (serr->ee_origin != SO_EE_ORIGIN_ZEROCOPY) { + pr_err("wrong origin %u", serr->ee_origin); + return -1; + } + if (serr->ee_errno != 0) { + pr_err("wrong errno %d", serr->ee_errno); + return -1; + }
hi = serr->ee_data; lo = serr->ee_info;
fprintf(stderr, "tx complete [%d,%d]\n", lo, hi); - return; + return 0; } }
- error(1, 0, "did not receive tx completion"); + pr_err("did not receive tx completion"); + return -1; }
static int do_client(struct memory_buffer *mem) @@ -924,50 +1070,69 @@ static int do_client(struct memory_buffer *mem) ssize_t line_size = 0; struct cmsghdr *cmsg; char *line = NULL; + int ret, err = -1; size_t len = 0; int socket_fd; __u32 ddmabuf; int opt = 1; - int ret;
ret = parse_address(server_ip, atoi(port), &server_sin); - if (ret < 0) - error(1, 0, "parse server address"); - - socket_fd = socket(AF_INET6, SOCK_STREAM, 0); - if (socket_fd < 0) - error(1, socket_fd, "create socket"); - - enable_reuseaddr(socket_fd); - - ret = setsockopt(socket_fd, SOL_SOCKET, SO_BINDTODEVICE, ifname, - strlen(ifname) + 1); - if (ret) - error(1, errno, "bindtodevice"); - - if (bind_tx_queue(ifindex, mem->fd, &ys)) - error(1, 0, "Failed to bind\n"); + if (ret < 0) { + pr_err("parse server address"); + return -1; + }
if (client_ip) { ret = parse_address(client_ip, atoi(port), &client_sin); - if (ret < 0) - error(1, 0, "parse client address"); + if (ret < 0) { + pr_err("parse client address"); + return ret; + } + }
+ socket_fd = socket(AF_INET6, SOCK_STREAM, 0); + if (socket_fd < 0) { + pr_err("create socket"); + return -1; + } + + if (enable_reuseaddr(socket_fd)) + goto err_close_socket; + + ret = setsockopt(socket_fd, SOL_SOCKET, SO_BINDTODEVICE, ifname, + strlen(ifname) + 1); + if (ret) { + pr_err("bindtodevice"); + goto err_close_socket; + } + + if (bind_tx_queue(ifindex, mem->fd, &ys)) { + pr_err("Failed to bind"); + goto err_close_socket; + } + + if (client_ip) { ret = bind(socket_fd, &client_sin, sizeof(client_sin)); - if (ret) - error(1, errno, "bind"); + if (ret) { + pr_err("bind"); + goto err_unbind; + } }
ret = setsockopt(socket_fd, SOL_SOCKET, SO_ZEROCOPY, &opt, sizeof(opt)); - if (ret) - error(1, errno, "set sock opt"); + if (ret) { + pr_err("set sock opt"); + goto err_unbind; + }
fprintf(stderr, "Connect to %s %d (via %s)\n", server_ip, ntohs(server_sin.sin6_port), ifname);
ret = connect(socket_fd, &server_sin, sizeof(server_sin)); - if (ret) - error(1, errno, "connect"); + if (ret) { + pr_err("connect"); + goto err_unbind; + }
while (1) { free(line); @@ -980,10 +1145,11 @@ static int do_client(struct memory_buffer *mem) if (max_chunk) { msg.msg_iovlen = (line_size + max_chunk - 1) / max_chunk; - if (msg.msg_iovlen > MAX_IOV) - error(1, 0, - "can't partition %zd bytes into maximum of %d chunks", - line_size, MAX_IOV); + if (msg.msg_iovlen > MAX_IOV) { + pr_err("can't partition %zd bytes into maximum of %d chunks", + line_size, MAX_IOV); + goto err_free_line; + }
for (int i = 0; i < msg.msg_iovlen; i++) { iov[i].iov_base = (void *)(i * max_chunk); @@ -1014,34 +1180,40 @@ static int do_client(struct memory_buffer *mem) *((__u32 *)CMSG_DATA(cmsg)) = ddmabuf;
ret = sendmsg(socket_fd, &msg, MSG_ZEROCOPY); - if (ret < 0) - error(1, errno, "Failed sendmsg"); + if (ret < 0) { + pr_err("Failed sendmsg"); + goto err_free_line; + }
fprintf(stderr, "sendmsg_ret=%d\n", ret);
- if (ret != line_size) - error(1, errno, "Did not send all bytes %d vs %zd", ret, - line_size); + if (ret != line_size) { + pr_err("Did not send all bytes %d vs %zd", ret, line_size); + goto err_free_line; + }
- wait_compl(socket_fd); + if (wait_compl(socket_fd)) + goto err_free_line; }
fprintf(stderr, "%s: tx ok\n", TEST_PREFIX);
+ err = 0; + +err_free_line: free(line); +err_unbind: + ynl_sock_destroy(ys); +err_close_socket: close(socket_fd); - - if (ys) - ynl_sock_destroy(ys); - - return 0; + return err; }
int main(int argc, char *argv[]) { struct memory_buffer *mem; int is_server = 0, opt; - int ret; + int ret, err = 1;
while ((opt = getopt(argc, argv, "ls:c:p:v:q:t:f:z:")) != -1) { switch (opt) { @@ -1078,8 +1250,10 @@ int main(int argc, char *argv[]) } }
- if (!ifname) - error(1, 0, "Missing -f argument\n"); + if (!ifname) { + pr_err("Missing -f argument"); + return 1; + }
ifindex = if_nametoindex(ifname);
@@ -1088,33 +1262,41 @@ int main(int argc, char *argv[]) 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"); - if (num_queues < 2) - error(1, 0, - "number of device queues is too low\n"); + if (num_queues < 0) { + pr_err("couldn't detect number of queues"); + return 1; + } + if (num_queues < 2) { + pr_err("number of device queues is too low"); + return 1; + } /* 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"); + if (start_queue < 0 || num_queues < 0) { + pr_err("Both -t and -q are required"); + return 1; + }
- run_devmem_tests(); - return 0; + return run_devmem_tests(); }
if (start_queue < 0 && num_queues < 0) { num_queues = rxq_num(ifindex); - if (num_queues < 2) - error(1, 0, "number of device queues is too low\n"); + if (num_queues < 2) { + pr_err("number of device queues is too low"); + return 1; + }
num_queues = 1; start_queue = rxq_num(ifindex) - num_queues;
- if (start_queue < 0) - error(1, 0, "couldn't detect number of queues\n"); + if (start_queue < 0) { + pr_err("couldn't detect number of queues"); + return 1; + }
fprintf(stderr, "using queues %d..%d\n", start_queue, start_queue + num_queues); } @@ -1122,21 +1304,39 @@ int main(int argc, char *argv[]) for (; optind < argc; optind++) fprintf(stderr, "extra arguments: %s\n", argv[optind]);
- if (start_queue < 0) - error(1, 0, "Missing -t argument\n"); + if (start_queue < 0) { + pr_err("Missing -t argument"); + return 1; + }
- if (num_queues < 0) - error(1, 0, "Missing -q argument\n"); + if (num_queues < 0) { + pr_err("Missing -q argument"); + return 1; + }
- if (!server_ip) - error(1, 0, "Missing -s argument\n"); + if (!server_ip) { + pr_err("Missing -s argument"); + return 1; + }
- if (!port) - error(1, 0, "Missing -p argument\n"); + if (!port) { + pr_err("Missing -p argument"); + return 1; + }
mem = provider->alloc(getpagesize() * NUM_PAGES); - ret = is_server ? do_server(mem) : do_client(mem); - provider->free(mem); + if (!mem) { + pr_err("Failed to allocate memory buffer"); + return 1; + }
- return ret; + ret = is_server ? do_server(mem) : do_client(mem); + if (ret) + goto err_free_mem; + + err = 0; + +err_free_mem: + provider->free(mem); + return err; }
On 08/22, Jakub Kicinski wrote:
Using error() makes it impossible for callers to unwind their changes. Replace error() calls with proper error handling.
Signed-off-by: Jakub Kicinski kuba@kernel.org
Acked-by: Stanislav Fomichev sdf@fomichev.me
.../selftests/drivers/net/hw/ncdevmem.c | 528 ++++++++++++------ 1 file changed, 364 insertions(+), 164 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index 71961a7688e6..e75adfed33ac 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -115,6 +115,21 @@ struct memory_provider { size_t off, int n); }; +static void pr_err(const char *fmt, ...) +{
- va_list args;
- fprintf(stderr, "%s: ", TEST_PREFIX);
- va_start(args, fmt);
- vfprintf(stderr, fmt, args);
- va_end(args);
- if (errno != 0)
fprintf(stderr, ": %s", strerror(errno));- fprintf(stderr, "\n");
+}
static struct memory_buffer *udmabuf_alloc(size_t size) { struct udmabuf_create create; @@ -123,27 +138,33 @@ static struct memory_buffer *udmabuf_alloc(size_t size) ctx = malloc(sizeof(*ctx)); if (!ctx)
error(1, ENOMEM, "malloc failed");
return NULL;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);
- if (ctx->devfd < 0) {
pr_err("[skip,no-udmabuf: Unable to access DMA buffer device file]");goto err_free_ctx;- }
ctx->memfd = memfd_create("udmabuf-test", MFD_ALLOW_SEALING);
- if (ctx->memfd < 0)
error(1, errno, "%s: [skip,no-memfd]\n", TEST_PREFIX);
- if (ctx->memfd < 0) {
pr_err("[skip,no-memfd]");goto err_close_dev;- }
ret = fcntl(ctx->memfd, F_ADD_SEALS, F_SEAL_SHRINK);
- if (ret < 0)
error(1, errno, "%s: [skip,fcntl-add-seals]\n", TEST_PREFIX);
- if (ret < 0) {
pr_err("[skip,fcntl-add-seals]");goto err_close_memfd;- }
ret = ftruncate(ctx->memfd, size);
- if (ret == -1)
error(1, errno, "%s: [FAIL,memfd-truncate]\n", TEST_PREFIX);
- if (ret == -1) {
pr_err("[FAIL,memfd-truncate]");goto err_close_memfd;- }
memset(&create, 0, sizeof(create)); @@ -151,15 +172,29 @@ static struct memory_buffer *udmabuf_alloc(size_t size) 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);
- if (ctx->fd < 0) {
pr_err("[FAIL, create udmabuf]");goto err_close_fd;- }
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);
- if (ctx->buf_mem == MAP_FAILED) {
pr_err("[FAIL, map udmabuf]");goto err_close_fd;- }
return ctx;
+err_close_fd:
- close(ctx->fd);
+err_close_memfd:
- close(ctx->memfd);
+err_close_dev:
- close(ctx->devfd);
+err_free_ctx:
- free(ctx);
- return NULL;
} static void udmabuf_free(struct memory_buffer *ctx) @@ -217,7 +252,7 @@ static void print_nonzero_bytes(void *ptr, size_t size) putchar(p[i]); } -void validate_buffer(void *line, size_t size) +int validate_buffer(void *line, size_t size) { static unsigned char seed = 1; unsigned char *ptr = line; @@ -232,8 +267,10 @@ void validate_buffer(void *line, size_t size) "Failed validation: expected=%u, actual=%u, index=%lu\n", expected, ptr[i], i); errors++;
if (errors > 20)error(1, 0, "validation failed.");
if (errors > 20) {pr_err("validation failed");return -1;}@@ -241,6 +278,7 @@ void validate_buffer(void *line, size_t size) } fprintf(stdout, "Validated buffer\n");
- return 0;
} static int rxq_num(int ifindex) @@ -279,7 +317,7 @@ static int rxq_num(int ifindex) system(command); \ }) -static int reset_flow_steering(void) +static void reset_flow_steering(void) { /* Depending on the NIC, toggling ntuple off and on might not * be allowed. Additionally, attempting to delete existing filters @@ -292,7 +330,6 @@ static int reset_flow_steering(void) run_command( "ethtool -n %s | grep 'Filter:' | awk '{print $2}' | xargs -n1 ethtool -N %s delete >&2", ifname, ifname);
- return 0;
} static const char *tcp_data_split_str(int val) @@ -354,6 +391,11 @@ static int configure_rss(void) return run_command("ethtool -X %s equal %d >&2", ifname, start_queue); } +static void reset_rss(void) +{
- run_command("ethtool -X %s default >&2", ifname, start_queue);
+}
static int configure_channels(unsigned int rx, unsigned int tx) { struct ethtool_channels_get_req *gchan; @@ -479,6 +521,7 @@ static int bind_rx_queue(unsigned int ifindex, unsigned int dmabuf_fd, *ys = ynl_sock_create(&ynl_netdev_family, &yerr); if (!*ys) {
netdev_queue_id_free(queues);
Funny how you spotted this.. Ownership of these is complicated with ynl :-(
In prep for more selective resetting of ntuple filters try to save the rule IDs to a table.
Signed-off-by: Jakub Kicinski kuba@kernel.org --- .../selftests/drivers/net/hw/ncdevmem.c | 141 +++++++++++++----- 1 file changed, 106 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index e75adfed33ac..8d9d579834b1 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -39,6 +39,7 @@ #define __EXPORTED_HEADERS__
#include <linux/uio.h> +#include <stdarg.h> #include <stdio.h> #include <stdlib.h> #include <unistd.h> @@ -97,6 +98,10 @@ static unsigned int dmabuf_id; static uint32_t tx_dmabuf_id; static int waittime_ms = 500;
+/* System state loaded by current_config_load() */ +#define MAX_FLOWS 8 +static int ntuple_ids[MAX_FLOWS] = { -1, -1, -1, -1, -1, -1, -1, -1, }; + struct memory_buffer { int fd; size_t size; @@ -281,6 +286,85 @@ int validate_buffer(void *line, size_t size) return 0; }
+static int +__run_command(char *out, size_t outlen, const char *cmd, va_list args) +{ + char command[256]; + FILE *fp; + + vsnprintf(command, sizeof(command), cmd, args); + + fprintf(stderr, "Running: %s\n", command); + fp = popen(command, "r"); + if (!fp) + return -1; + if (out) { + size_t len; + + if (!fgets(out, outlen, fp)) + return -1; + + /* Remove trailing newline if present */ + len = strlen(out); + if (len && out[len - 1] == '\n') + out[len - 1] = '\0'; + } + return pclose(fp); +} + +static int run_command(const char *cmd, ...) +{ + va_list args; + int ret; + + va_start(args, cmd); + ret = __run_command(NULL, 0, cmd, args); + va_end(args); + + return ret; +} + +static int ethtool_add_flow(const char *format, ...) +{ + char local_output[256], cmd[256]; + const char *id_start; + int flow_idx, ret; + char *endptr; + long flow_id; + va_list args; + + for (flow_idx = 0; flow_idx < MAX_FLOWS; flow_idx++) + if (ntuple_ids[flow_idx] == -1) + break; + if (flow_idx == MAX_FLOWS) { + fprintf(stderr, "Error: too many flows\n"); + return -1; + } + + snprintf(cmd, sizeof(cmd), "ethtool -N %s %s", ifname, format); + + va_start(args, format); + ret = __run_command(local_output, sizeof(local_output), cmd, args); + va_end(args); + + if (ret != 0) + return ret; + + /* Extract the ID from the output */ + id_start = strstr(local_output, "Added rule with ID "); + if (!id_start) + return -1; + id_start += strlen("Added rule with ID "); + + flow_id = strtol(id_start, &endptr, 10); + if (endptr == id_start || flow_id < 0 || flow_id > INT_MAX) + return -1; + + fprintf(stderr, "Added flow rule with ID %ld\n", flow_id); + ntuple_ids[flow_idx] = flow_id; + return flow_id; +} + static int rxq_num(int ifindex) { struct ethtool_channels_get_req *req; @@ -308,28 +392,17 @@ static int rxq_num(int ifindex) return num; }
-#define run_command(cmd, ...) \ - ({ \ - char command[256]; \ - memset(command, 0, sizeof(command)); \ - snprintf(command, sizeof(command), cmd, ##__VA_ARGS__); \ - fprintf(stderr, "Running: %s\n", command); \ - system(command); \ - }) - static void reset_flow_steering(void) { - /* 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. - */ + int i;
- run_command("ethtool -K %s ntuple off >&2", ifname); - run_command("ethtool -K %s ntuple on >&2", ifname); - run_command( - "ethtool -n %s | grep 'Filter:' | awk '{print $2}' | xargs -n1 ethtool -N %s delete >&2", - ifname, ifname); + for (i = 0; i < MAX_FLOWS; i++) { + if (ntuple_ids[i] == -1) + continue; + run_command("ethtool -N %s delete %d", + ifname, ntuple_ids[i]); + ntuple_ids[i] = -1; + } }
static const char *tcp_data_split_str(int val) @@ -480,6 +553,7 @@ static int configure_flow_steering(struct sockaddr_in6 *server_sin) const char *type = "tcp6"; const char *server_addr; char buf[40]; + int flow_id;
inet_ntop(AF_INET6, &server_sin->sin6_addr, buf, sizeof(buf)); server_addr = buf; @@ -490,23 +564,22 @@ static int configure_flow_steering(struct sockaddr_in6 *server_sin) }
/* Try configure 5-tuple */ - if (run_command("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_addr, - client_ip ? "src-port" : "", - client_ip ? port : "", - port, start_queue)) + flow_id = ethtool_add_flow("flow-type %s %s %s dst-ip %s %s %s dst-port %s queue %d", + type, + client_ip ? "src-ip" : "", + client_ip ?: "", + server_addr, + client_ip ? "src-port" : "", + client_ip ? port : "", + port, start_queue); + if (flow_id < 0) { /* If that fails, try configure 3-tuple */ - if (run_command("ethtool -N %s flow-type %s dst-ip %s dst-port %s queue %d >&2", - ifname, - type, - server_addr, - port, start_queue)) + flow_id = ethtool_add_flow("flow-type %s dst-ip %s dst-port %s queue %d", + type, server_addr, port, start_queue); + if (flow_id < 0) /* If that fails, return error */ return -1; + }
return 0; } @@ -682,8 +755,6 @@ static int do_server(struct memory_buffer *mem) return -1; }
- reset_flow_steering(); - if (configure_headersplit(1)) { pr_err("Failed to enable TCP header split"); return -1;
On 08/22, Jakub Kicinski wrote:
In prep for more selective resetting of ntuple filters try to save the rule IDs to a table.
Signed-off-by: Jakub Kicinski kuba@kernel.org
This seems ok to unblock nipa cleanup, but at this point why not copy-paste proper ioctl from kperf? https://github.com/facebookexperimental/kperf/blob/main/devmem.c#L119 (seems less complicated than popen)
Acked-by: Stanislav Fomichev sdf@fomichev.me
On Fri, 22 Aug 2025 14:27:31 -0700 Stanislav Fomichev wrote:
On 08/22, Jakub Kicinski wrote:
In prep for more selective resetting of ntuple filters try to save the rule IDs to a table.
Signed-off-by: Jakub Kicinski kuba@kernel.org
This seems ok to unblock nipa cleanup, but at this point why not copy-paste proper ioctl from kperf? https://github.com/facebookexperimental/kperf/blob/main/devmem.c#L119 (seems less complicated than popen)
Yeah, IDK. I thought fixing the run_command() to be able to capture output is more useful long term.
In case changing channel count with provider bound succeeds unexpectedly - make sure we return to original settings.
Signed-off-by: Jakub Kicinski kuba@kernel.org --- .../selftests/drivers/net/hw/ncdevmem.c | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index 8d9d579834b1..580b4459a840 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -469,7 +469,7 @@ static void reset_rss(void) run_command("ethtool -X %s default >&2", ifname, start_queue); }
-static int configure_channels(unsigned int rx, unsigned int tx) +static int check_changing_channels(unsigned int rx, unsigned int tx) { struct ethtool_channels_get_req *gchan; struct ethtool_channels_set_req *schan; @@ -525,20 +525,32 @@ static int configure_channels(unsigned int rx, unsigned int tx) ethtool_channels_set_req_set_tx_count(schan, tx - rx); }
- ret = ethtool_channels_set(ys, schan); - if (ret) - fprintf(stderr, "YNL set channels: %s\n", ys->err.msg); } else if (chan->_present.rx_count) { ethtool_channels_set_req_set_rx_count(schan, rx); ethtool_channels_set_req_set_tx_count(schan, tx); - - ret = ethtool_channels_set(ys, schan); - if (ret) - fprintf(stderr, "YNL set channels: %s\n", ys->err.msg); } else { fprintf(stderr, "Error: device has neither combined nor rx channels\n"); ret = -1; + goto exit_free_schan; } + + ret = ethtool_channels_set(ys, schan); + if (ret) { + fprintf(stderr, "YNL set channels: %s\n", ys->err.msg); + } else { + /* We were expecting a failure, go back to previous settings */ + ethtool_channels_set_req_set_combined_count(schan, + chan->combined_count); + ethtool_channels_set_req_set_rx_count(schan, chan->rx_count); + ethtool_channels_set_req_set_tx_count(schan, chan->tx_count); + + ret = ethtool_channels_set(ys, schan); + if (ret) + fprintf(stderr, "YNL un-setting channels: %s\n", + ys->err.msg); + } + +exit_free_schan: ethtool_channels_set_req_free(schan); exit_free_chan: ethtool_channels_get_rsp_free(chan); @@ -1019,16 +1031,14 @@ int run_devmem_tests(void) }
/* Deactivating a bound queue should not be legal */ - if (!configure_channels(num_queues, num_queues)) { + if (!check_changing_channels(num_queues, num_queues)) { pr_err("Deactivating a bound queue should be illegal"); - goto err_reset_channels; + goto err_unbind; }
err = 0; goto err_unbind;
-err_reset_channels: - /* TODO */ err_unbind: ynl_sock_destroy(ys); err_reset_headersplit:
On 08/22, Jakub Kicinski wrote:
In case changing channel count with provider bound succeeds unexpectedly - make sure we return to original settings.
Acked-by: Stanislav Fomichev sdf@fomichev.me
Signed-off-by: Jakub Kicinski kuba@kernel.org
.../selftests/drivers/net/hw/ncdevmem.c | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index 8d9d579834b1..580b4459a840 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -469,7 +469,7 @@ static void reset_rss(void) run_command("ethtool -X %s default >&2", ifname, start_queue); } -static int configure_channels(unsigned int rx, unsigned int tx) +static int check_changing_channels(unsigned int rx, unsigned int tx)
Good call on renaming!
Improve HDS handling. Make sure we set threshold to 0. Restore the previous settings before exiting.
Signed-off-by: Jakub Kicinski kuba@kernel.org --- .../selftests/drivers/net/hw/ncdevmem.c | 96 +++++++++++++++++-- 1 file changed, 87 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index 580b4459a840..aa2904d1a3e1 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -419,6 +419,60 @@ static const char *tcp_data_split_str(int val) } }
+static struct ethtool_rings_get_rsp *get_ring_config(void) +{ + struct ethtool_rings_get_req *get_req; + struct ethtool_rings_get_rsp *get_rsp; + struct ynl_error yerr; + struct ynl_sock *ys; + + ys = ynl_sock_create(&ynl_ethtool_family, &yerr); + if (!ys) { + fprintf(stderr, "YNL: %s\n", yerr.msg); + return NULL; + } + + 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); + + ynl_sock_destroy(ys); + + return get_rsp; +} + +static void restore_ring_config(const struct ethtool_rings_get_rsp *config) +{ + struct ethtool_rings_set_req *req; + struct ynl_error yerr; + struct ynl_sock *ys; + int ret; + + if (!config) + return; + + ys = ynl_sock_create(&ynl_ethtool_family, &yerr); + if (!ys) { + fprintf(stderr, "YNL: %s\n", yerr.msg); + return; + } + + req = ethtool_rings_set_req_alloc(); + ethtool_rings_set_req_set_header_dev_index(req, ifindex); + ethtool_rings_set_req_set_tcp_data_split(req, + ETHTOOL_TCP_DATA_SPLIT_UNKNOWN); + if (config->_present.hds_thresh) + ethtool_rings_set_req_set_hds_thresh(req, config->hds_thresh); + + ret = ethtool_rings_set(ys, req); + if (ret < 0) + fprintf(stderr, "YNL failed: %s\n", ys->err.msg); + ethtool_rings_set_req_free(req); + + ynl_sock_destroy(ys); +} + static int configure_headersplit(bool on) { struct ethtool_rings_get_req *get_req; @@ -436,8 +490,14 @@ static int configure_headersplit(bool on)
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); + if (on) { + ethtool_rings_set_req_set_tcp_data_split(req, + ETHTOOL_TCP_DATA_SPLIT_ENABLED); + ethtool_rings_set_req_set_hds_thresh(req, 0); + } else { + ethtool_rings_set_req_set_tcp_data_split(req, + ETHTOOL_TCP_DATA_SPLIT_UNKNOWN); + } ret = ethtool_rings_set(ys, req); if (ret < 0) fprintf(stderr, "YNL failed: %s\n", ys->err.msg); @@ -745,6 +805,7 @@ static struct netdev_queue_id *create_queues(void)
static int do_server(struct memory_buffer *mem) { + struct ethtool_rings_get_rsp *ring_config; char ctrl_data[sizeof(int) * 20000]; size_t non_page_aligned_frags = 0; struct sockaddr_in6 client_addr; @@ -767,9 +828,15 @@ static int do_server(struct memory_buffer *mem) return -1; }
+ ring_config = get_ring_config(); + if (!ring_config) { + pr_err("Failed to get current ring configuration"); + return -1; + } + if (configure_headersplit(1)) { pr_err("Failed to enable TCP header split"); - return -1; + goto err_free_ring_config; }
/* Configure RSS to divert all traffic from our devmem queues */ @@ -959,12 +1026,15 @@ static int do_server(struct memory_buffer *mem) err_reset_rss: reset_rss(); err_reset_headersplit: - configure_headersplit(0); + restore_ring_config(ring_config); +err_free_ring_config: + ethtool_rings_get_rsp_free(ring_config); return err; }
int run_devmem_tests(void) { + struct ethtool_rings_get_rsp *ring_config; struct netdev_queue_id *queues; struct memory_buffer *mem; struct ynl_sock *ys; @@ -976,10 +1046,16 @@ int run_devmem_tests(void) return -1; }
+ ring_config = get_ring_config(); + if (!ring_config) { + pr_err("Failed to get current ring configuration"); + goto err_free_mem; + } + /* Configure RSS to divert all traffic from our devmem queues */ if (configure_rss()) { pr_err("rss error"); - goto err_free_mem; + goto err_free_ring_config; }
if (configure_headersplit(1)) { @@ -1000,13 +1076,13 @@ int run_devmem_tests(void)
if (configure_headersplit(0)) { pr_err("Failed to configure header split"); - goto err_reset_rss; + goto err_reset_headersplit; }
queues = create_queues(); if (!queues) { pr_err("Failed to create queues"); - goto err_reset_rss; + goto err_reset_headersplit; }
if (!bind_rx_queue(ifindex, mem->fd, queues, num_queues, &ys)) { @@ -1016,7 +1092,7 @@ int run_devmem_tests(void)
if (configure_headersplit(1)) { pr_err("Failed to configure header split"); - goto err_reset_rss; + goto err_reset_headersplit; }
queues = create_queues(); @@ -1042,9 +1118,11 @@ int run_devmem_tests(void) err_unbind: ynl_sock_destroy(ys); err_reset_headersplit: - configure_headersplit(0); + restore_ring_config(ring_config); err_reset_rss: reset_rss(); +err_free_ring_config: + ethtool_rings_get_rsp_free(ring_config); err_free_mem: provider->free(mem); return err;
On 08/22, Jakub Kicinski wrote:
Improve HDS handling. Make sure we set threshold to 0. Restore the previous settings before exiting.
Signed-off-by: Jakub Kicinski kuba@kernel.org
.../selftests/drivers/net/hw/ncdevmem.c | 96 +++++++++++++++++-- 1 file changed, 87 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c index 580b4459a840..aa2904d1a3e1 100644 --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c @@ -419,6 +419,60 @@ static const char *tcp_data_split_str(int val) } } +static struct ethtool_rings_get_rsp *get_ring_config(void) +{
- struct ethtool_rings_get_req *get_req;
- struct ethtool_rings_get_rsp *get_rsp;
- struct ynl_error yerr;
- struct ynl_sock *ys;
- ys = ynl_sock_create(&ynl_ethtool_family, &yerr);
- if (!ys) {
fprintf(stderr, "YNL: %s\n", yerr.msg);return NULL;- }
- 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);
- ynl_sock_destroy(ys);
- return get_rsp;
+}
+static void restore_ring_config(const struct ethtool_rings_get_rsp *config) +{
- struct ethtool_rings_set_req *req;
- struct ynl_error yerr;
- struct ynl_sock *ys;
- int ret;
- if (!config)
return;- ys = ynl_sock_create(&ynl_ethtool_family, &yerr);
- if (!ys) {
fprintf(stderr, "YNL: %s\n", yerr.msg);return;- }
- req = ethtool_rings_set_req_alloc();
- ethtool_rings_set_req_set_header_dev_index(req, ifindex);
- ethtool_rings_set_req_set_tcp_data_split(req,
ETHTOOL_TCP_DATA_SPLIT_UNKNOWN);- if (config->_present.hds_thresh)
ethtool_rings_set_req_set_hds_thresh(req, config->hds_thresh);- ret = ethtool_rings_set(ys, req);
- if (ret < 0)
fprintf(stderr, "YNL failed: %s\n", ys->err.msg);- ethtool_rings_set_req_free(req);
- ynl_sock_destroy(ys);
+}
static int configure_headersplit(bool on) { struct ethtool_rings_get_req *get_req; @@ -436,8 +490,14 @@ static int configure_headersplit(bool on) 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);
- if (on) {
ethtool_rings_set_req_set_tcp_data_split(req,ETHTOOL_TCP_DATA_SPLIT_ENABLED);ethtool_rings_set_req_set_hds_thresh(req, 0);
From talking to Taehee awhile ago it seemed like unconditionally setting 0 will fail on the devices that don't support it. Is it not the case anymore with ethnl_set_rings_validate?
On Fri, 22 Aug 2025 15:26:37 -0700 Stanislav Fomichev wrote:
@@ -436,8 +490,14 @@ static int configure_headersplit(bool on) 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);
- if (on) {
ethtool_rings_set_req_set_tcp_data_split(req,ETHTOOL_TCP_DATA_SPLIT_ENABLED);ethtool_rings_set_req_set_hds_thresh(req, 0);From talking to Taehee awhile ago it seemed like unconditionally setting 0 will fail on the devices that don't support it. Is it not the case anymore with ethnl_set_rings_validate?
Good point! ring params are validated against the supported attr flags. We can pass in config here and only set hds_thresh if max was reported. That should mean driver supports the setting.
linux-kselftest-mirror@lists.linaro.org
-
Jakub Kicinski -
Stanislav Fomichev