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: <CAHS8izOJaGk5g9m-DgBJE13XDcgdXNAbbBq9MmcjH229cSVoxg@mail.gmail.com>
Date: Thu, 3 Oct 2024 12:07:12 -0700
From: Mina Almasry <almasrymina@...gle.com>
To: Stanislav Fomichev <stfomichev@...il.com>
Cc: Stanislav Fomichev <sdf@...ichev.me>, 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 Thu, Oct 3, 2024 at 10:02 AM Stanislav Fomichev <stfomichev@...il.com> wrote:
>
> On 10/03, Mina Almasry wrote:
> > 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?
>
> I'm probably missing something, but why do you think exercising this from
> the probe/selftest mode is not enough? It might be confusing for the readers
> to understand why we bind to half of the queues and flow steer into them
> when in reality there is only single tcp flow.
>
> IOW, can we keep these two modes:
> 1. server / client - use single queue
> 2. selftest / probe - use more than 1 queue by default (and I'll remove the
>    checks that enforce the number of queues for this mode to let the
>    users override)

Ah, I see. Thanks for the explanation.

My paranoia here is that we don't notice multi-queue binding
regressions because the tests are often run in data path mode and we
don't use or notice failures in the probe mode.

I will concede my paranoia is just that and this is not very likely to
happen, but also if it is confusing to bind multi-queues and then just
use one, then we could remedy that with a comment and keep the
accidental test coverage. It also makes the test simpler to always
bind the same # of queues rather than special case data and control
path tests.

But your 2 mode approach sounds fine as well. But to implement that
you need more than to remove the checks that enforce the number of
queues, right? In probe mode num_queues should be rxq_num/2, and in
server mode num_queues should be 1, yes?

-- 
Thanks,
Mina