Re: [PATCH net-next v3 7/8] selftests: drv-net: add primary_rx_redirect support to NetDrvContEnv

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Hardcoded little-endian byte order for BPF map values breaks tests on big-endian architectures.
- [Low] BPF program fails to account for IPv6 extension headers, potentially misrouting ICMPv6 packets.
- [Low] Unsafe dictionary access when parsing `tc filter show` output can lead to a `KeyError`.
--

commit 431500c080b55b59605689dc687523876de45d23
Author: Bobby Eshleman <bobbyeshleman@meta.com>

selftests: drv-net: add primary_rx_redirect support to NetDrvContEnv

This commit adds a BPF program and Python harness support to redirect RX
traffic from the host netkit interface to the physical NIC. It skips ICMPv6
packets so neighbor discovery continues to work locally. It also extracts map
ID lookups into a reusable helper.

> diff --git a/tools/testing/selftests/drivers/net/hw/nk_primary_rx_redirect.bpf.c b/tools/testing/selftests/drivers/net/hw/nk_primary_rx_redirect.bpf.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/hw/nk_primary_rx_redirect.bpf.c

[ ... ]

> +	ip6h = data + sizeof(struct ethhdr);
> +	if ((void *)(ip6h + 1) > data_end)
> +		return TC_ACT_OK;
> +
> +	if (ip6h->nexthdr == IPPROTO_ICMPV6)
> +		return TC_ACT_OK;

Will this check miss ICMPv6 packets if the IPv6 packet contains extension
headers?

For example, Multicast Listener Discovery packets use the Hop-by-Hop Options
header where nexthdr is 0. For these packets, ip6h->nexthdr won't equal
IPPROTO_ICMPV6 here, causing them to be incorrectly redirected to the
physical NIC instead of being processed by the host netkit interface.

> +
> +	return bpf_redirect_neigh(phys_ifindex, NULL, 0, 0);
> +}

[ ... ]

> diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
> index 409b419222454..af8e1de8ed7b1 100644
> --- a/tools/testing/selftests/drivers/net/lib/py/env.py
> +++ b/tools/testing/selftests/drivers/net/lib/py/env.py

[ ... ]

> +    def _attach_primary_rx_redirect_bpf(self):
> +        """Attach BPF redirect program on the primary netkit ingress."""

[ ... ]

> +        filters = json.loads(
> +            cmd(f"tc -j filter show dev {self._nk_host_ifname} ingress").stdout)
> +        redirect_prog_id = None
> +        for bpf in filters:
> +            if 'options' not in bpf:
> +                continue
> +            if bpf['options']['bpf_name'].startswith('nk_primary_rx_redirect'):
> +                redirect_prog_id = bpf['options']['prog']['id']
> +                break

Could this throw a KeyError if a non-BPF filter is present on the interface?

Other filter types, such as u32 or matchall, might have an options dictionary
but lack the bpf_name key. Using .get('bpf_name', '') might be a safer way
to check the prefix.

> +        if redirect_prog_id is None:
> +            raise Exception("Failed to get primary RX redirect BPF prog ID")
> +
> +        bss_map_id = self._find_bss_map_id(redirect_prog_id)
> +        phys_ifindex_bytes = self.ifindex.to_bytes(4, byteorder='little')

Will hardcoding the byteorder to little break this test on big-endian
architectures?

BPF programs access memory using host-native byte order. Writing a
little-endian value via bpftool on a big-endian system like s390x might
cause the BPF program to read an incorrect ifindex, leading to a failure
in the bpf_redirect_neigh() call.

> +        value_hex = ' '.join(f'{b:02x}' for b in phys_ifindex_bytes)
> +        bpftool(f"map update id {bss_map_id} key hex 00 00 00 00 value hex {value_hex}")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507-tcp-dm-netkit-v3-0-52821445867c@meta.com?part=7