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