Message-ID: <9037828b-c7fd-4eb1-9dd8-19ee49063361@davidwei.uk>
Date: Mon, 2 Feb 2026 07:53:34 +0900
From: David Wei <dw@...idwei.uk>
To: Jakub Kicinski <kuba@...nel.org>, Daniel Borkmann <daniel@...earbox.net>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org, davem@...emloft.net,
 razor@...ckwall.org, pabeni@...hat.com, willemb@...gle.com, sdf@...ichev.me,
 john.fastabend@...il.com, martin.lau@...nel.org, jordan@...fe.io,
 maciej.fijalkowski@...el.com, magnus.karlsson@...el.com, toke@...hat.com,
 yangzhenze@...edance.com, wangdongdong.6@...edance.com
Subject: Re: [PATCH net-next v8 15/16] selftests/net: Add env for container
 based tests

On 2026-02-01 09:38, Jakub Kicinski wrote:
> On Thu, 29 Jan 2026 23:28:29 +0100 Daniel Borkmann wrote:
>> +class NetDrvContEnv(NetDrvEpEnv):
>> +    """
>> +    Class for an environment with a netkit pair setup for forwarding traffic
>> +    between the physical interface and a network namespace.
> 
> A little diagram would be pretty awesome to have here, and perhaps copy
> it to the README file too?

Sounds good, I'll add one. I forget how much you like ASCII diagrams.

> 
>> +    """
>> +
>> +    def __init__(self, src_path, rxqueues=1, **kwargs):
>> +        super().__init__(src_path, **kwargs)
>> +
>> +        self.require_ipver("6")
>> +        local_prefix = self.env.get("LOCAL_PREFIX_V6")
>> +        if not local_prefix:
>> +            raise KsftSkipEx("LOCAL_PREFIX_V6 required")
>> +
>> +        local_prefix = local_prefix.rstrip("/64").rstrip("::").rstrip(":")
>> +        self.ipv6_prefix = f"{local_prefix}::"
>> +        self.nk_host_ipv6 = f"{local_prefix}::2:1"
>> +        self.nk_guest_ipv6 = f"{local_prefix}::2:2"
>> +
>> +        self.netns = None
>> +        self._nk_host_ifname = None
>> +        self._nk_guest_ifname = None
>> +        self._tc_attached = False
>> +        self._bpf_prog_pref = None
>> +        self._bpf_prog_id = None
>> +        self._init_ns_attached = False
> 
> I'm not a Python expert but the pre-init of variables that are set
> later on in __init__() (on all possible paths) seems like a waste of LoC

It's so that exceptions thrown -> __del__() don't complain about
undefined vars.

> 
>> +
>> +        ip(f"link add type netkit mode l2 forward peer forward numrxqueues {rxqueues}")
> 
> Would we be able to do this with YNL?
> no big deal but always nice to avoid the dependency on very latest CLI

Hmm, I tried looking into this already. My understanding is that this
can be done in C but not currently in YNL, not without adding a spec.

> 
>> +        all_links = ip("-d link show", json=True)
>> +        netkit_links = [link for link in all_links
>> +                        if link.get('linkinfo', {}).get('info_kind') == 'netkit'
>> +                        and 'UP' not in link.get('flags', [])]
> 
> And of course if you use YNL you can actually ask Netlink to
> echo back / return to you what interface it ended up creating.
> No need for the scanning..
> 
>> +        if len(netkit_links) != 2:
>> +            raise KsftSkipEx("Failed to create netkit pair")
>> +
>> +        netkit_links.sort(key=lambda x: x['ifindex'])
>> +        self._nk_host_ifname = netkit_links[1]['ifname']
>> +        self._nk_guest_ifname = netkit_links[0]['ifname']
>> +        self.nk_host_ifindex = netkit_links[1]['ifindex']
>> +        self.nk_guest_ifindex = netkit_links[0]['ifindex']
>> +
>> +        self._setup_ns()
>> +        self._attach_bpf()
> 
>> +    def _setup_ns(self):
>> +        self.netns = NetNS()
>> +        cmd("ip netns attach init 1")
>> +        self._init_ns_attached = True
>> +        ip("netns set init 0", ns=self.netns)
> 
> Hm, refactor class NetNSEnter or just use its enter method?

I did consider this but I didn't feel like it fit the very specific
purpose of NetNSEnter. I needed the init ns to have a valid nsid from
within the test netns, which I didn't think many other tests would need.

> 
>> +        ip(f"link set dev {self._nk_guest_ifname} netns {self.netns.name}")
>> +        ip(f"link set dev {self._nk_host_ifname} up")
>> +        ip(f"-6 addr add fe80::1/64 dev {self._nk_host_ifname} nodad")
>> +        ip(f"-6 route add {self.nk_guest_ipv6}/128 via fe80::2 dev {self._nk_host_ifname}")
>> +
>> +        ip("link set lo up", ns=self.netns)
>> +        ip(f"link set dev {self._nk_guest_ifname} up", ns=self.netns)
>> +        ip(f"-6 addr add fe80::2/64 dev {self._nk_guest_ifname}", ns=self.netns)
>> +        ip(f"-6 addr add {self.nk_guest_ipv6}/64 dev {self._nk_guest_ifname} nodad", ns=self.netns)
>> +        ip(f"-6 route add default via fe80::1 dev {self._nk_guest_ifname}", ns=self.netns)
>> +
>> +    def _attach_bpf(self):
>> +        bpf_obj = self.test_dir / "nk_forward.bpf.o"
>> +        if not bpf_obj.exists():
>> +            raise KsftSkipEx("BPF prog not found")
>> +
>> +        cmd(f"tc filter add dev {self.ifname} ingress bpf obj {bpf_obj} sec tc/ingress direct-action")
>> +        self._tc_attached = True
>> +
>> +        tc_info = cmd(f"tc filter show dev {self.ifname} ingress").stdout
> 
> Don't we need to also add a qdisc if there isn't one already?

Sorry, why would qdisc be needed? The bpf prog attaches to tc. I don't
know anything about qdisc, though.

> 
>> +        match = re.search(r'pref (\d+).*nk_forward\.bpf.*id (\d+)', tc_info)
> 
> I haven't checked but TC doesn't support enough JSON and YNL doesn't
> support enough TC to avoid re hacks? :( :(

Yeah the old TC tool doesn't give me this info programmatically :(

Borkmann mentioned TC is outdated/unmaintained. I did use tcx via a C
loader before, but that was removed in an earlier iteration.

> 
> 
> As I mentioned to David in person, Wei Wang also needs to use this
> class for PSP tests so would be awesome to detach these from the large
> series. I promise to prioritize the "detached" bits of these series in
> case you're worried about the MW.

I'll move the env, basic ping test out. Then keep the queue lease tests,
adding the error paths you requested, in this series.

Powered by blists - more mailing lists