netdev - Re: [PATCH net-next 4/4] selftests: drv-net: rss_ctx: add tests for RSS configuration and contexts

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 for Android: free password hash cracker in your pocket

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

Date: Mon, 24 Jun 2024 13:55:34 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: Jakub Kicinski <kuba@...nel.org>, davem@...emloft.net
Cc: netdev@...r.kernel.org, edumazet@...gle.com, pabeni@...hat.com,
 willemdebruijn.kernel@...il.com
Subject: Re: [PATCH net-next 4/4] selftests: drv-net: rss_ctx: add tests for
 RSS configuration and contexts

On 21/06/2024 00:29, Jakub Kicinski wrote:
> Add tests focusing on indirection table configuration and
> creating extra RSS contexts in drivers which support it.
> 
>   $ ./drivers/net/hw/rss_ctx.py
>   KTAP version 1
>   1..6
>   ok 1 rss_ctx.test_rss_key_indir
>   ok 2 rss_ctx.test_rss_context
>   ok 3 rss_ctx.test_rss_context4
>   # Increasing queue count 44 -> 66
>   # Failed to create context 32, trying to test what we got
>   ok 4 rss_ctx.test_rss_context32 # SKIP Tested only 31 contexts, wanted 32
>   ok 5 rss_ctx.test_rss_context_overlap
>   ok 6 rss_ctx.test_rss_context_overlap2
>   # Totals: pass:5 fail:0 xfail:0 xpass:0 skip:1 error:0
> 
> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> ---
...
> +def test_rss_context(cfg, ctx_cnt=1):
> +    """
> +    Test separating traffic into RSS contexts.
> +    The queues will be allocated 2 for each context:
> +     ctx0  ctx1  ctx2  ctx3
> +    [0 1] [2 3] [4 5] [6 7] ...
> +    """
> +
> +    requested_ctx_cnt = ctx_cnt
> +
> +    # Try to allocate more queues when necessary
> +    qcnt = len(_get_rx_cnts(cfg))
> +    if qcnt >= 2 + 2 * ctx_cnt:
> +        qcnt = None
> +    else:
> +        try:
> +            ksft_pr(f"Increasing queue count {qcnt} -> {2 + 2 * ctx_cnt}")
> +            ethtool(f"-L {cfg.ifname} combined {2 + 2 * ctx_cnt}")
> +        except:
> +            raise KsftSkipEx("Not enough queues for the test")
> +
> +    ntuple = []
> +    ctx_id = []
> +    ports = []
> +    try:
> +        # Use queues 0 and 1 for normal traffic
> +        ethtool(f"-X {cfg.ifname} equal 2")
> +
> +        for i in range(ctx_cnt):
> +            try:
> +                ctx_id.append(ethtool_create(cfg, "-X", "context new"))
> +            except CmdExitFailure:
> +                # try to carry on and skip at the end
> +                if i == 0:
> +                    raise
> +                ksft_pr(f"Failed to create context {i + 1}, trying to test what we got")
> +                ctx_cnt = i
> +                break
> +
> +            ethtool(f"-X {cfg.ifname} context {ctx_id[i]} start {2 + i * 2} equal 2")

Is it worth also testing the single command
    f"ethtool -X {cfg.ifname} context new start {2 + i * 2} equal 2"
 as that will exercise the kernel & driver slightly differently to
 first creating a context and then configuring it?

...

> diff --git a/tools/testing/selftests/net/lib/py/ksft.py b/tools/testing/selftests/net/lib/py/ksft.py
> index 4769b4eb1ea1..91648c5baf40 100644
> --- a/tools/testing/selftests/net/lib/py/ksft.py
> +++ b/tools/testing/selftests/net/lib/py/ksft.py
> @@ -57,6 +57,11 @@ KSFT_RESULT_ALL = True
>          _fail("Check failed", a, "<", b, comment)
>  
>  
> +def ksft_lt(a, b, comment=""):
> +    if a > b:
> +        _fail("Check failed", a, ">", b, comment)

AFAICT this implements 'le' (less-or-equal), not 'lt' (less than) as
 the name implies.

Apart from that these tests LGTM as far as they go.  One thing that I
 notice *isn't* tested here, that I generally make a point of testing,
 is: add a bunch of contexts (and ntuple filters), remove some of
 them, then run your traffic and make sure that the ones you left
 intact still work (and that the deleted ones are actually gone).
Also wonder if it's worth adding tests for 'ethtool -x ... context N'?
 You have it for context 0 in test_rss_key_indir(), but on custom
 contexts it can exercise different code in the kernel.

-ed