[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aYQBsTfF4PrJcHNl@devvm11784.nha0.facebook.com>
Date: Wed, 4 Feb 2026 18:34:25 -0800
From: Bobby Eshleman <bobbyeshleman@...il.com>
To: David Wei <dw@...idwei.uk>
Cc: Jakub Kicinski <kuba@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, 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 Wed, Feb 04, 2026 at 06:08:58PM -0800, Bobby Eshleman wrote:
> On Mon, Feb 02, 2026 at 07:53:34AM +0900, David Wei wrote:
> > 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.
>
> The ingress qdisc is the attachment point. I think we need
> CONFIG_NET_SCH_INGRESS here too?
Sorry for the config stuff I'm replying about, I ran into needing these
too for this:
CONFIG_NET_CLS_BPF=y
CONFIG_BPF_SYSCALL=y
Powered by blists - more mailing lists