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.
