| Message ID | 20241104181430.228682-1-sdf@fomichev.me |
|---|---|
| Headers | show |
| Series | selftests: ncdevmem: Add ncdevmem to ksft | expand |
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>
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 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>
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>
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>
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.
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