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] [day] [month] [year] [list]

Message-ID: <a3dde0e4-aca0-464e-a93f-8721b0919e69@nvidia.com>
Date: Sun, 23 Feb 2025 14:43:41 +0200
From: Gal Pressman <gal@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>
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 21/02/2025 4:03, Jakub Kicinski wrote:
> 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.

Ack.

> 
>> +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?

Yes.

> 
>> +    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?

Ack.

> 
> I'd also add a:
> 
> 	if len(cpus) == 1:
> 		raise KsftSkipEx(f"Only one CPU seen traffic: {cpus}")

It's covered by the less than two CPUs check, I added a print of cpus to
the existing check.