netdev - Re: [PATCH net-next v2 09/12] selftests: ncdevmem: Remove hard-coded queue numbers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHS8izNK+DiQUUkkvnPQvBRJiQ32WRO0Crg=nvOW9vn_4kCE+Q@mail.gmail.com>
Date: Thu, 3 Oct 2024 00:14:25 -0700
From: Mina Almasry <almasrymina@...gle.com>
To: Stanislav Fomichev <sdf@...ichev.me>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com
Subject: Re: [PATCH net-next v2 09/12] selftests: ncdevmem: Remove hard-coded
 queue numbers

On Mon, Sep 30, 2024 at 10:18 AM Stanislav Fomichev <sdf@...ichev.me> wrote:
>
> Use single last queue of the device and probe it dynamically.
>

Sorry I know there was a pending discussion in the last iteration that
I didn't respond to. Been a rough week with me out sick a bit.

For this, the issue I see is that by default only 1 queue binding will
be tested, but I feel like test coverage for the multiple queues case
by default is very nice because I actually ran into some issues making
multi-queue binding work.

Can we change this so that, by default, it binds to the last rxq_num/2
queues of the device?

> Cc: Mina Almasry <almasrymina@...gle.com>
> Signed-off-by: Stanislav Fomichev <sdf@...ichev.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 a1fa818c8229..900a661a61af 100644
> --- a/tools/testing/selftests/net/ncdevmem.c
> +++ b/tools/testing/selftests/net/ncdevmem.c
> @@ -48,8 +48,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;
> @@ -198,6 +198,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];                                      \
> @@ -672,6 +699,15 @@ int main(int argc, char *argv[])
>
>         ifindex = if_nametoindex(ifname);
>
> +       if (start_queue < 0) {
> +               start_queue = rxq_num(ifindex) - 1;

I think the only changes needed are:

start_queue = rxq_num(ifindex) / 2;
num_queues = rxq_num(ifindex) - start_queue;

(I may have an off-by-1 error somewhere with this math).


-- 
Thanks,
Mina