netdev - Re: [PATCH net-next v4 5/5] selftests: drv-net-hw: Add a test for symmetric RSS hash

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: <20250220180351.7e278ec9@kernel.org>
Date: Thu, 20 Feb 2025 18:03:51 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Gal Pressman <gal@...dia.com>
Cc: "David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>, Eric
 Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Simon
 Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>, Tony Nguyen
 <anthony.l.nguyen@...el.com>, Przemek Kitszel
 <przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "Tariq
 Toukan" <tariqt@...dia.com>, Edward Cree <ecree.xilinx@...il.com>, Ahmed
 Zaki <ahmed.zaki@...el.com>, <linux-doc@...r.kernel.org>, Nimrod Oren
 <noren@...dia.com>
Subject: Re: [PATCH net-next v4 5/5] selftests: drv-net-hw: Add a test for
 symmetric RSS hash

On Thu, 20 Feb 2025 13:34:35 +0200 Gal Pressman wrote:
> +def _get_rand_port(remote):
> +    for _ in range(1000):
> +        port = rand_port()
> +        try:
> +            check_port_available_remote(port, remote)
> +            return port
> +        except:
> +            continue
> +
> +    raise Exception("Can't find any free unprivileged port")

TCP and UDP port spaces are separate, I think your checking if the
ports are available on TCP here, and then use them for UDP below.

We don't really care about the 100% success, I don't think we should 
be checking the ports. Pick two ports, send a A<>B packet, send a B<>A
packet, if either fails to connect or doesn't arrive just ignore.
As long as we can get ~10? successful pairs in 100? ties it's good.

> +def traffic(cfg, local_port, remote_port, ipver):
> +    af_inet = socket.AF_INET if ipver == "4" else socket.AF_INET6
> +    sock = socket.socket(af_inet, socket.SOCK_DGRAM)
> +    sock.bind(('', local_port))
> +    sock.connect((cfg.remote_addr_v[ipver], remote_port))
> +    tgt = f"{ipver}:[{cfg.addr_v[ipver]}]:{local_port},sourceport={remote_port}"
> +    cmd("echo a | socat - UDP" + tgt, host=cfg.remote)
> +    sock.recvmsg(100)

Could you use fd_read_timeout():
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/tools/testing/selftests/net/lib/py/utils.py#n20

In case the packet got lost?

> +    return sock.getsockopt(socket.SOL_SOCKET, socket.SO_INCOMING_CPU)
> +
> +
> +def test_rss_input_xfrm(cfg, ipver):
> +    """
> +    Test symmetric input_xfrm.
> +    If symmetric RSS hash is configured, send traffic twice, swapping the
> +    src/dst UDP ports, and verify that the same queue is receiving the traffic
> +    in both cases (IPs are constant).
> +    """
> +
> +    input_xfrm = cfg.ethnl.rss_get(
> +        {'header': {'dev-name': cfg.ifname}}).get('input_xfrm')
> +
> +    # Check for symmetric xor/or-xor
> +    if input_xfrm and (input_xfrm == 1 or input_xfrm == 2):
> +        cpus = set()
> +        for _ in range(8):
> +            port1 = _get_rand_port(cfg.remote)
> +            port2 = _get_rand_port(cfg.remote)
> +            cpu1 = traffic(cfg, port1, port2, ipver)
> +            cpu2 = traffic(cfg, port2, port1, ipver)
> +            cpus.update([cpu1, cpu2])
> +
> +            ksft_eq(
> +                cpu1, cpu2, comment=f"Received traffic on different cpus ({cpu1} != {cpu2}) with ports ({port1 = }, {port2 = }) while symmetric hash is configured")

the cpu1 cpu2 values will already be printed by the helper, no need 
to format them in

> +
> +        ksft_ge(len(cpus), 2, comment=f"Received traffic on less than two cpus")
> +    else:
> +        raise KsftSkipEx("Symmetric RSS hash not requested")

Flip the condition, raise the exception right after the if, then the
rest of the code doesn't have to be indented?

I'd also add a:

	if len(cpus) == 1:
		raise KsftSkipEx(f"Only one CPU seen traffic: {cpus}")
-- 
pw-bot: cr