On Mon, Nov 2, 2020 at 7:47 AM David Ahern [off-list ref] wrote:
On 10/29/20 9:11 AM, Hangbin Liu wrote:
quoted
This series converts iproute2 to use libbpf for loading and attaching
BPF programs when it is available. This means that iproute2 will
correctly process BTF information and support the new-style BTF-defined
maps, while keeping compatibility with the old internal map definition
syntax.
This is achieved by checking for libbpf at './configure' time, and using
it if available. By default the system libbpf will be used, but static
linking against a custom libbpf version can be achieved by passing
LIBBPF_DIR to configure. FORCE_LIBBPF can be set to force configure to
abort if no suitable libbpf is found (useful for automatic packaging
that wants to enforce the dependency).
The old iproute2 bpf code is kept and will be used if no suitable libbpf
is available. When using libbpf, wrapper code ensures that iproute2 will
still understand the old map definition format, including populating
map-in-map and tail call maps before load.
The examples in bpf/examples are kept, and a separate set of examples
are added with BTF-based map definitions for those examples where this
is possible (libbpf doesn't currently support declaratively populating
tail call maps).
At last, Thanks a lot for Toke's help on this patch set.
In regards to comments from v2 of the series:
iproute2 is a stable, production package that requires minimal support
from external libraries. The external packages it does require are also
stable with few to no relevant changes.
bpf and libbpf on the other hand are under active development and
rapidly changing month over month. The git submodule approach has its
conveniences for rapid development but is inappropriate for a package
like iproute2 and will not be considered.
It's ok to not consider that, really. I'm trying to understand what's
so bad about the submodule approach, not convince you (anymore) to use
libbpf through submodule. And the submodule is not for rapid
development, it's mainly for guaranteed libbpf features and version,
and simplicity of iproute2 code when using libbpf.
But I don't think I got a real answer as to what's the exact reason
against the submodule. Like what "inappropriate" even means in this
case? Jesper's security argument so far was the only objective
criteria, as far as I can tell.
To explicitly state what I think should be obvious to any experienced
Linux user, iproute2 code should always compile and work *without
functionality loss* on LTS versions N and N-1 of well known OS’es with
LTS releases (e.g., Debian, Ubuntu, RHEL). Meaning iproute2 will compile
and work with the external dependencies as they exist in that OS version.
I love the appeal to obviousness and "experienced Linux user" ;)
But I also see that using libbpf through submodule gives iproute2
exact control over which version of libbpf is being used. And that
does not depend at all on any specific Linux distribution, its
version, LTS vs non-LTS, etc. iproute2 will just work the same across
all of them. So matches your stated goals very directly and
explicitly.
I believe there are more than enough established compatibility and
library version checks to find the middle ground to integrate new
features requiring new versions of libbpf while maintaining stability
and compatibility with older releases. The biannual releases of Ubuntu
and Fedora serve as testing grounds for integrating new features
requiring a newer version of libbpf while continuing to work with
released versions of libbpf. It appears Debian Bullseye will also fall
into this category.
Beyond just more unnecessary complexity in iproute2 library to
accommodate older libbpf versions, users basically will need to pay
closer attention not just to which version of iproute2 they have, but
also which version of libbpf is installed on their system. Which is
ok, but an unnecessary burden, IMO. By controlling the libbpf version
through the submodule, it would be simple to say: "iproute2 vX uses
libbpf vY with features Z1, Z2, Z3". Then the user would just know
what to expect from iproute2 and its BPF support. And iproute2 code
base won't have to do as much feature detection and condition
compilation tricks.
That's what I don't understand, why settle for the lowest common
denominator of libbpf versions across a wide range of systems, when
you can take control and develop against a well-known version of
libbpf. I get security upgrades angle (even if I don't rank it higher
than simplicity). But I don't get the ideal behind a blanket statement
"libbpf through submodule is inappropriate".
Finally, bpf-based features in iproute2 will only be committed once
relevant support exists in a released version of libbpf (ie., the github
version, not just commits to the in-kernel tree version). Patches can
and should be sent for review based on testing with the in-kernel tree
version of libbpf, but I will not commit them until the library has been
released.
Makes sense. And the submodule approach gives you a great deal of
control and flexibility in this case. For testing, it's easy to use
either Github or even in-kernel sources (with a bit of symlinking,
though). But for upstreaming the submodule should only reference a
released tag from Github repo. Again, everything seems to work out,
no?
Thanks for working on this, Hangbin. It is right direction in the long term.