[PATCH 0/2] iproute2: ss: clarify build warnings when building with libbpf 0.5.0

[PATCH 0/2] iproute2: ss: clarify build warnings when building with libbpf 0.5.0

[Stefan Mätje]

Hi,
when building current iproute2 source on Ubuntu 22.04 with libbpf0
0.5.0 installed, I stumbled over the warning "libbpf version 0.5 or 
later is required, ...". This prompted me to look closer having the
version 0.5.0 installed which should suppress this warning.
The warning lured me into the impression that building without
warning should be possible using libbpf 0.5.0.

I found out that this warning came from ss.c where a conditional
compile path depends on LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION.
Newer libbpf versions define these in libbpf_version.h but the library
version 0.5.0 and earlier on Ubuntu and Debian don't package this header.
The version 0.7.0 on Debian packages the header libbpf_version.h.

Therefore these defines were undefined during the build and prompted
the output of the warning message. I derived these version defines
from the library version in the configure script and provided them
via CFLAGS. This is the first patch.

Now building ss.c against the libbpf 0.5.0 with ENABLE_BPF_SKSTORAGE_SUPPORT
enabled, triggered compilation errors. The function btf_dump__new is
used there with a calling convention that was introduced with libbpf
version 0.6.0. Therefore ENABLE_BPF_SKSTORAGE_SUPPORT shall only be
enabled for libbpf versions >= 0.6.0.

Best regards,
    Stefan Mätje

Stefan Mätje (2):
  configure: provide surrogates for possibly missing libbpf_version.h
  ss: fix libbpf version check for ENABLE_BPF_SKSTORAGE_SUPPORT

 configure | 6 ++++++
 misc/ss.c | 6 +++---
 2 files changed, 9 insertions(+), 3 deletions(-)


base-commit: 354d8a36885172b6e27ca65ff85c2c51e740fda0
-- 
2.34.1

[PATCH 1/2] configure: provide surrogates for possibly missing libbpf_version.h

[Stefan Mätje]

Old libbpf library versions (< 0.7.x) may not have the libbpf_version.h
header packaged. This header would provide LIBBPF_MAJOR_VERSION and
LIBBPF_MINOR_VERSION which are then missing to control conditional
compilation in some source files.

Provide surrogates for these defines via CFLAGS that are derived from
the LIBBPF_VERSION determined with $(${PKG_CONFIG} libbpf --modversion).

Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
---
 configure | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/configure b/configure
index 928048b3..7437db4f 100755
--- a/configure
+++ b/configure
@@ -315,6 +315,12 @@ check_libbpf()
         echo "HAVE_LIBBPF:=y" >> $CONFIG
         echo 'CFLAGS += -DHAVE_LIBBPF ' $LIBBPF_CFLAGS >> $CONFIG
         echo "CFLAGS += -DLIBBPF_VERSION=\\\"$LIBBPF_VERSION\\\"" >> $CONFIG
+	LIBBPF_MAJOR=$(IFS="."; set $LIBBPF_VERSION; echo $1)
+	LIBBPF_MINOR=$(IFS="."; set $LIBBPF_VERSION; echo $2)
+	if [ "$LIBBPF_MAJOR" -eq 0 -a "$LIBBPF_MINOR" -lt 7 ]; then
+	    # Newer libbpf versions provide these defines in the bpf/libbpf_version.h header.
+            echo "CFLAGS += -DLIBBPF_MAJOR_VERSION=$LIBBPF_MAJOR -DLIBBPF_MINOR_VERSION=$LIBBPF_MINOR" >> $CONFIG
+	fi
         echo 'LDLIBS += ' $LIBBPF_LDLIBS >> $CONFIG
 
         if [ -z "$LIBBPF_DIR" ]; then
-- 
2.34.1

[PATCH 2/2] ss: fix libbpf version check for ENABLE_BPF_SKSTORAGE_SUPPORT

[Stefan Mätje]

This patch fixes a problem with the libbpf version comparison to decide
if ENABLE_BPF_SKSTORAGE_SUPPORT could be enabled.

- The code enabled by ENABLE_BPF_SKSTORAGE_SUPPORT uses the function
  btf_dump__new with an API that was introduced in libbpf 0.6.0. So
  check now against libbpf version to be >= 0.6.x instead of 0.5.x.

- This code still depends on the necessity to have LIBBPF_MAJOR_VERSION
  and LIBBPF_MINOR_VERSION defined, even if libbpf_version.h is not
  present in the library development package. This was ensured with
  the previous patch for the configure script.

Fixes: e3ecf048 ("ss: pretty-print BPF socket-local storage")
Signed-off-by: Stefan Mätje <stefan.maetje@esd.eu>
---
 misc/ss.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 620f4c8f..aef1a714 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -53,7 +53,7 @@
 #include <linux/mptcp.h>
 
 #ifdef HAVE_LIBBPF
-/* If libbpf is new enough (0.5+), support for pretty-printing BPF socket-local
+/* If libbpf is new enough (0.6+), support for pretty-printing BPF socket-local
  * storage is enabled, otherwise we emit a warning and disable it.
  * ENABLE_BPF_SKSTORAGE_SUPPORT is only used to gate the socket-local storage
  * feature, so this wouldn't prevent any feature relying on HAVE_LIBBPF to be
@@ -66,8 +66,8 @@
 #include <bpf/libbpf.h>
 #include <linux/btf.h>
 
-#if (LIBBPF_MAJOR_VERSION == 0) && (LIBBPF_MINOR_VERSION < 5)
-#warning "libbpf version 0.5 or later is required, disabling BPF socket-local storage support"
+#if ((LIBBPF_MAJOR_VERSION == 0) && (LIBBPF_MINOR_VERSION < 6))
+#warning "libbpf version 0.6 or later is required, disabling BPF socket-local storage support"
 #undef ENABLE_BPF_SKSTORAGE_SUPPORT
 #endif
 #endif
-- 
2.34.1

Re: [PATCH 0/2] iproute2: ss: clarify build warnings when building with libbpf 0.5.0

[Stephen Hemminger]

On Mon, 12 Aug 2024 00:31:33 +0200
Stefan Mätje <stefan.maetje@esd.eu> wrote:

> Hi,
> when building current iproute2 source on Ubuntu 22.04 with libbpf0
> 0.5.0 installed, I stumbled over the warning "libbpf version 0.5 or 
> later is required, ...". This prompted me to look closer having the
> version 0.5.0 installed which should suppress this warning.
> The warning lured me into the impression that building without
> warning should be possible using libbpf 0.5.0.

Why is using new iproute2 on 2 year old distro going to add
anything here? Especially when BPF has under gone breaking API changes
over the recent past.

> 
> I found out that this warning came from ss.c where a conditional
> compile path depends on LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION.
> Newer libbpf versions define these in libbpf_version.h but the library
> version 0.5.0 and earlier on Ubuntu and Debian don't package this header.
> The version 0.7.0 on Debian packages the header libbpf_version.h.
> 
> Therefore these defines were undefined during the build and prompted
> the output of the warning message. I derived these version defines
> from the library version in the configure script and provided them
> via CFLAGS. This is the first patch.
> 
> Now building ss.c against the libbpf 0.5.0 with ENABLE_BPF_SKSTORAGE_SUPPORT
> enabled, triggered compilation errors. The function btf_dump__new is
> used there with a calling convention that was introduced with libbpf
> version 0.6.0. Therefore ENABLE_BPF_SKSTORAGE_SUPPORT shall only be
> enabled for libbpf versions >= 0.6.0.

Might be better just to drop support for old libbpf and also
the legacy mode. Having multiple versions means there is more code
that doesn't get covered by tests.

Also, configure shell script is getting to be so messy, it is time for a redo.
Maybe give up on make and go to meson?

Re: [PATCH 0/2] iproute2: ss: clarify build warnings when building with libbpf 0.5.0

[Stefan Mätje]

Am Sonntag, dem 11.08.2024 um 16:25 -0700 schrieb Stephen Hemminger:
> On Mon, 12 Aug 2024 00:31:33 +0200
> Stefan Mätje <stefan.maetje@esd.eu> wrote:
> 
> > Hi,
> > when building current iproute2 source on Ubuntu 22.04 with libbpf0
> > 0.5.0 installed, I stumbled over the warning "libbpf version 0.5 or 
> > later is required, ...". This prompted me to look closer having the
> > version 0.5.0 installed which should suppress this warning.
> > The warning lured me into the impression that building without
> > warning should be possible using libbpf 0.5.0.
> 
> Why is using new iproute2 on 2 year old distro going to add
> anything here? Especially when BPF has under gone breaking API changes
> over the recent past.

I'm sorry I didn't make my intentions clear. Ubuntu 22.04 is a LTS version
and will therefore not go away soon. And its unfortunate that thay packaged
this old libbpf version.

The aim of my patches was to bring the fact to your attention that the 
source implicitely promises to work with that version (by issueing the 
warning "libbpf version 0.5 or later is required") which is wrong.

I only wanted to save other people's time who also may try to figure out
why the warning is issued even on a 0.5.0 version that fulfills the
announced minimum version requirement.


> > I found out that this warning came from ss.c where a conditional
> > compile path depends on LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION.
> > Newer libbpf versions define these in libbpf_version.h but the library
> > version 0.5.0 and earlier on Ubuntu and Debian don't package this header.
> > The version 0.7.0 on Debian packages the header libbpf_version.h.
> > 
> > Therefore these defines were undefined during the build and prompted
> > the output of the warning message. I derived these version defines
> > from the library version in the configure script and provided them
> > via CFLAGS. This is the first patch.
> > 
> > Now building ss.c against the libbpf 0.5.0 with ENABLE_BPF_SKSTORAGE_SUPPORT
> > enabled, triggered compilation errors. The function btf_dump__new is
> > used there with a calling convention that was introduced with libbpf
> > version 0.6.0. Therefore ENABLE_BPF_SKSTORAGE_SUPPORT shall only be
> > enabled for libbpf versions >= 0.6.0.
> 
> Might be better just to drop support for old libbpf and also
> the legacy mode. Having multiple versions means there is more code
> that doesn't get covered by tests.

Increasing the minimum required version to 0.7.0 would be fine for me. Then
the configure script patch can be dropped and the second patch only needs
to have the minimum LIBBPF_MINOR_VERSION limit increased to 7.

I provided the configure script patch only for the case that somebody wanted
to configure the build based on the exact version which the API changed 
and I had the patch already in place anyway.

> Also, configure shell script is getting to be so messy, it is time for a redo.
> Maybe give up on make and go to meson?

Re: [PATCH 0/2] iproute2: ss: clarify build warnings when building with libbpf 0.5.0

[patchwork-bot+netdevbpf]

Hello:

This series was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Mon, 12 Aug 2024 00:31:33 +0200 you wrote:
> Hi,
> when building current iproute2 source on Ubuntu 22.04 with libbpf0
> 0.5.0 installed, I stumbled over the warning "libbpf version 0.5 or
> later is required, ...". This prompted me to look closer having the
> version 0.5.0 installed which should suppress this warning.
> The warning lured me into the impression that building without
> warning should be possible using libbpf 0.5.0.
> 
> [...]

Here is the summary with links:
  - [1/2] configure: provide surrogates for possibly missing libbpf_version.h
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=0ddadc93e54f
  - [2/2] ss: fix libbpf version check for ENABLE_BPF_SKSTORAGE_SUPPORT
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=e9096586e070

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html