On Wed, 03 Apr 2024 12:50:35 +0100
Luca Boccassi <bl...@debian.org> wrote:
> On Tue, 2024-04-02 at 10:12 -0700, Stephen Hemminger wrote:
> > There were multiple issues in the RSS queue support in the TAP
> > driver. This required extensive rework of the BPF support.
> >
> > Change the BPF loading to use bpftool to
> > create a skeleton header file, and load with libbpf.
> > The BPF is always compiled from source so less chance that
> > source and instructions diverge. Also resolves issue where
> > libbpf and source get out of sync. The program
> > is only loaded once, so if multiple rules are created
> > only one BPF program is loaded in kernel.
> >
> > The new BPF program only needs a single action.
> > No need for action and re-classification step.
> >
> > It alsow fixes the missing bits from the original.
> > - supports setting RSS key per flow
> > - level of hash can be L3 or L3/L4.
> >
> > Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
> > ---
> > drivers/net/tap/bpf/meson.build | 14 +-
> > drivers/net/tap/meson.build | 29 +--
> > drivers/net/tap/rte_eth_tap.c | 2 +
> > drivers/net/tap/rte_eth_tap.h | 9 +-
> > drivers/net/tap/tap_flow.c | 410 +++++++-------------------------
> > drivers/net/tap/tap_flow.h | 16 +-
> > drivers/net/tap/tap_rss.h | 10 +-
> > drivers/net/tap/tap_tcmsgs.h | 4 +-
> > 8 files changed, 127 insertions(+), 367 deletions(-)
> >
> > diff --git a/drivers/net/tap/bpf/meson.build
> > b/drivers/net/tap/bpf/meson.build
> > index f2c03a19fd..3f3c4e6602 100644
> > --- a/drivers/net/tap/bpf/meson.build
> > +++ b/drivers/net/tap/bpf/meson.build
> > @@ -3,15 +3,24 @@
> >
> > enable_tap_rss = false
> >
> > +# Loading BPF requires libbpf
> > libbpf = dependency('libbpf', required: false, method: 'pkg-config')
> > if not libbpf.found()
> > message('net/tap: no RSS support missing libbpf')
> > subdir_done()
> > endif
> >
> > +# Making skeleton needs bpftool
> > # Debian install this in /usr/sbin which is not in $PATH
> > -bpftool = find_program('bpftool', '/usr/sbin/bpftool', required: false,
> > version: '>= 5.6.0')
> > -if not bpftool.found()
> > +bpftool_supports_skel = false
> > +bpftool = find_program('bpftool', '/usr/sbin/bpftool', required: false)
> > +if bpftool.found()
> > + # Some Ubuntu has non-functional bpftool
> > + bpftool_supports_skel = run_command(bpftool, 'gen', 'help',
> > + check:false).returncode() == 0
> > +endif
>
> Using bpftool to generate the header at build time is a bit icky,
> because it will look at sysfs on the build system, which is from the
> running kernel. But a build system's kernel might be some ancient LTS,
> and even be a completely different kconfig/build/distro from the actual
> runtime one.
I wish there was a better way to get bpf compiled to an array to load.
The other alternative is to embed the object but then the ickiness of decoding
ELF headers ends up in the driver.
This method seems to be the BPF way now.
>
> We have ran in the same problem in systemd recently, and the solution
> is to have distros publish the vmlinux.h together with the kernel
> image/headers, that way we can rely on the fact that by build-depending
> on the right kernel package we get exactly the generated vmlinux.h that
> we want. This has already happened in Centos, Debian, Fedora and Arch,
> and I am trying to get Ubuntu onboard too.
Not sure how much it matters for the TAP BPF bits. Unlike other kernel BPF
this program only really depends on layout of skb, and is not using system
call hooks. So vmlinux.h is not referenced directly. But if layout of skb
changes, then things would break.
> The annoying thing is that every distro packages differently, so the
> path needs to be configurable with a meson option.
>
> Feel free to pilfer the systemd meson glue:
>
> https://github.com/systemd/systemd/pull/26826/commits/d917079e7e320aa281fc4ad6f8073b0814b9cb13
>
> It's of course file to go to the runtime kernel if no vmlinux.h is
> specified, as a fallback, which is going to be useful for developers
> machines.
>
