All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 0/3] bpftool: Switch to new versioning scheme (align on libbpf's)
@ 2022-02-08 12:06 Quentin Monnet
  2022-02-08 12:06 ` [PATCH bpf-next v2 1/3] bpftool: Add libbpf's version number to "bpftool version" output Quentin Monnet
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Quentin Monnet @ 2022-02-08 12:06 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

Hi, this set aims at updating the way bpftool versions are numbered.
Instead of copying the version from the kernel (given that the sources for
the kernel and bpftool are shipped together), align it on libbpf's version
number, with a fixed offset (6) to avoid going backwards. Please refer to
the description of the third commit for details on the motivations.

The patchset also adds the number of the version of libbpf that was used to
compile to the output of "bpftool version". Bpftool makes such a heavy
usage of libbpf that it makes sense to indicate what version was used to
build it.

v2:
- Align on libbpf's version number instead of creating an independent
  versioning scheme.
- Use libbpf_version_string() to retrieve and display libbpf's version.
- Re-order patches (1 <-> 2).

Quentin Monnet (3):
  bpftool: Add libbpf's version number to "bpftool version" output
  libbpf: Add "libbpversion" make target to print version
  bpftool: Update versioning scheme, align on libbpf's version number

 .../bpf/bpftool/Documentation/common_options.rst  | 13 +++++++------
 tools/bpf/bpftool/Makefile                        | 15 +++++++++------
 tools/bpf/bpftool/main.c                          |  4 ++++
 tools/lib/bpf/Makefile                            |  3 +++
 4 files changed, 23 insertions(+), 12 deletions(-)

-- 
2.32.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH bpf-next v2 1/3] bpftool: Add libbpf's version number to "bpftool version" output
  2022-02-08 12:06 [PATCH bpf-next v2 0/3] bpftool: Switch to new versioning scheme (align on libbpf's) Quentin Monnet
@ 2022-02-08 12:06 ` Quentin Monnet
  2022-02-08 12:06 ` [PATCH bpf-next v2 2/3] libbpf: Add "libbpversion" make target to print version Quentin Monnet
  2022-02-08 12:06 ` [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number Quentin Monnet
  2 siblings, 0 replies; 10+ messages in thread
From: Quentin Monnet @ 2022-02-08 12:06 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

To help users check what version of libbpf is being used with bpftool,
print the number along with bpftool's own version number.

Output:

    $ ./bpftool version
    ./bpftool v5.16.0
    using libbpf v0.7
    features: libbfd, libbpf_strict, skeletons

    $ ./bpftool version --json --pretty
    {
        "version": "5.16.0",
        "libbpf_version": "0.7",
        "features": {
            "libbfd": true,
            "libbpf_strict": true,
            "skeletons": true
        }
    }

Note that libbpf_version_string() does note return the patch number.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/bpf/bpftool/Documentation/common_options.rst | 13 +++++++------
 tools/bpf/bpftool/main.c                           |  4 ++++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/common_options.rst b/tools/bpf/bpftool/Documentation/common_options.rst
index 908487b9c2ad..c88e4c6a06d2 100644
--- a/tools/bpf/bpftool/Documentation/common_options.rst
+++ b/tools/bpf/bpftool/Documentation/common_options.rst
@@ -4,12 +4,13 @@
 	  Print short help message (similar to **bpftool help**).
 
 -V, --version
-	  Print version number (similar to **bpftool version**), and optional
-	  features that were included when bpftool was compiled. Optional
-	  features include linking against libbfd to provide the disassembler
-	  for JIT-ted programs (**bpftool prog dump jited**) and usage of BPF
-	  skeletons (some features like **bpftool prog profile** or showing
-	  pids associated to BPF objects may rely on it).
+	  Print bpftool's version number (similar to **bpftool version**), the
+	  number of the version of libbpf in use, and optional features that
+	  were included when bpftool was compiled. Optional features include
+	  linking against libbfd to provide the disassembler for JIT-ted
+	  programs (**bpftool prog dump jited**) and usage of BPF skeletons
+	  (some features like **bpftool prog profile** or showing pids
+	  associated to BPF objects may rely on it).
 
 -j, --json
 	  Generate JSON output. For commands that cannot produce JSON, this
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 490f7bd54e4c..6265ac5bcf4e 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -89,6 +89,9 @@ static int do_version(int argc, char **argv)
 
 		jsonw_name(json_wtr, "version");
 		jsonw_printf(json_wtr, "\"%s\"", BPFTOOL_VERSION);
+		jsonw_name(json_wtr, "libbpf_version");
+		/* Offset by one to skip the "v" prefix */
+		jsonw_printf(json_wtr, "\"%s\"", libbpf_version_string() + 1);
 
 		jsonw_name(json_wtr, "features");
 		jsonw_start_object(json_wtr);	/* features */
@@ -102,6 +105,7 @@ static int do_version(int argc, char **argv)
 		unsigned int nb_features = 0;
 
 		printf("%s v%s\n", bin_name, BPFTOOL_VERSION);
+		printf("using libbpf %s\n", libbpf_version_string());
 		printf("features:");
 		if (has_libbfd) {
 			printf(" libbfd");
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next v2 2/3] libbpf: Add "libbpversion" make target to print version
  2022-02-08 12:06 [PATCH bpf-next v2 0/3] bpftool: Switch to new versioning scheme (align on libbpf's) Quentin Monnet
  2022-02-08 12:06 ` [PATCH bpf-next v2 1/3] bpftool: Add libbpf's version number to "bpftool version" output Quentin Monnet
@ 2022-02-08 12:06 ` Quentin Monnet
  2022-02-09  0:35   ` Andrii Nakryiko
  2022-02-08 12:06 ` [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number Quentin Monnet
  2 siblings, 1 reply; 10+ messages in thread
From: Quentin Monnet @ 2022-02-08 12:06 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

Add a target to libbpf's Makefile to print its version number, in a
similar way to what running "make kernelversion" at the root of the
repository does.

This is to avoid re-implementing the parsing of the libbpf.map file in
case some other tools want to extract the version of the libbpf sources
they are using.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
 tools/lib/bpf/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
index b8b37fe76006..91136623edf9 100644
--- a/tools/lib/bpf/Makefile
+++ b/tools/lib/bpf/Makefile
@@ -108,6 +108,9 @@ MAKEOVERRIDES=
 
 all:
 
+libbpfversion:
+	@echo $(LIBBPF_VERSION)
+
 export srctree OUTPUT CC LD CFLAGS V
 include $(srctree)/tools/build/Makefile.include
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number
  2022-02-08 12:06 [PATCH bpf-next v2 0/3] bpftool: Switch to new versioning scheme (align on libbpf's) Quentin Monnet
  2022-02-08 12:06 ` [PATCH bpf-next v2 1/3] bpftool: Add libbpf's version number to "bpftool version" output Quentin Monnet
  2022-02-08 12:06 ` [PATCH bpf-next v2 2/3] libbpf: Add "libbpversion" make target to print version Quentin Monnet
@ 2022-02-08 12:06 ` Quentin Monnet
  2022-02-09  0:39   ` Andrii Nakryiko
  2 siblings, 1 reply; 10+ messages in thread
From: Quentin Monnet @ 2022-02-08 12:06 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
  Cc: netdev, bpf, Quentin Monnet

Since the notion of versions was introduced for bpftool, it has been
following the version number of the kernel (using the version number
corresponding to the tree in which bpftool's sources are located). The
rationale was that bpftool's features are loosely tied to BPF features
in the kernel, and that we could defer versioning to the kernel
repository itself.

But this versioning scheme is confusing today, because a bpftool binary
should be able to work with both older and newer kernels, even if some
of its recent features won't be available on older systems. Furthermore,
if bpftool is ported to other systems in the future, keeping a
Linux-based version number is not a good option.

Looking at other options, we could either have a totally independent
scheme for bpftool, or we could align it on libbpf's version number
(with an offset on the major version number, to avoid going backwards).
The latter comes with a few drawbacks:

- We may want bpftool releases in-between two libbpf versions. We can
  always append pre-release numbers to distinguish versions, although
  those won't look as "official" as something with a proper release
  number. But at the same time, having bpftool with version numbers that
  look "official" hasn't really been an issue so far.

- If no new feature lands in bpftool for some time, we may move from
  e.g. 6.7.0 to 6.8.0 when libbpf levels up and have two different
  versions which are in fact the same.

- Following libbpf's versioning scheme sounds better than kernel's, but
  ultimately it doesn't make too much sense either, because even though
  bpftool uses the lib a lot, its behaviour is not that much conditioned
  by the internal evolution of the library (or by new APIs that it may
  not use).

Having an independent versioning scheme solves the above, but at the
cost of heavier maintenance. Developers will likely forget to increase
the numbers when adding features or bug fixes, and we would take the
risk of having to send occasional "catch-up" patches just to update the
version number.

Based on these considerations, this patch aligns bpftool's version
number on libbpf's. This is not a perfect solution, but 1) it's
certainly an improvement over the current scheme, 2) the issues raised
above are all minor at the moment, and 3) we can still move to an
independent scheme in the future if we realise we need it.

Given that libbpf is currently at version 0.7.0, and bpftool, before
this patch, was at 5.16, we use an offset of 6 for the major version,
bumping bpftool to 6.7.0.

It remains possible to manually override the version number by setting
BPFTOOL_VERSION when calling make.

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
---
Contrarily to the previous discussion and to what the first patch of the
set does, I chose not to use the libbpf_version_string() API from libbpf
to compute the version for bpftool. There were three reasons for that:

- I don't feel comfortable having bpftool's version number computed at
  runtime. Somehow it really feels like we should now it when we compile
  it. We link statically against libbpf today, but if we were to support
  dynamic linking in the future we may forget to update and would have
  bpftool's version changing based on the libbpf version installed
  beside it, which does not make sense.

- We cannot get the patch version for libbpf, the current API only
  returns the major and minor version numbers (we could fix it, although
  I'm not sure if desirable to expose the patch number).

- I found it less elegant to compute the version strings in the code,
  which meant malloc() and error handling just for printing a version
  number, and having a separate case for when $(BPFTOOL_VERSION) is
  defined, whereas passing a macro from the Makefile makes things
  straightforwards.
---
 tools/bpf/bpftool/Makefile | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 83369f55df61..8dd30abff3d9 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -7,14 +7,21 @@ srctree := $(patsubst %/,%,$(dir $(srctree)))
 srctree := $(patsubst %/,%,$(dir $(srctree)))
 endif
 
+BPF_DIR = $(srctree)/tools/lib/bpf
+
+# bpftool's version is libbpf's with a fixed offset for the major version.
+# This is because bpftool's version was higher than libbpf's when we adopted
+# this scheme.
+BPFTOOL_MAJOR_OFFSET := 6
+LIBBPF_VERSION := $(shell make -r --no-print-directory -sC $(BPF_DIR) libbpfversion)
+BPFTOOL_VERSION ?= $(shell lv="$(LIBBPF_VERSION)"; echo "$$((${lv%%.*} + $(BPFTOOL_MAJOR_OFFSET))).$${lv#*.}")
+
 ifeq ($(V),1)
   Q =
 else
   Q = @
 endif
 
-BPF_DIR = $(srctree)/tools/lib/bpf
-
 ifneq ($(OUTPUT),)
   _OUTPUT := $(OUTPUT)
 else
@@ -39,10 +46,6 @@ LIBBPF_BOOTSTRAP := $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
 LIBBPF_INTERNAL_HDRS := $(addprefix $(LIBBPF_HDRS_DIR)/,hashmap.h nlattr.h)
 LIBBPF_BOOTSTRAP_INTERNAL_HDRS := $(addprefix $(LIBBPF_BOOTSTRAP_HDRS_DIR)/,hashmap.h)
 
-ifeq ($(BPFTOOL_VERSION),)
-BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
-endif
-
 $(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT) $(LIBBPF_HDRS_DIR) $(LIBBPF_BOOTSTRAP_HDRS_DIR):
 	$(QUIET_MKDIR)mkdir -p $@
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v2 2/3] libbpf: Add "libbpversion" make target to print version
  2022-02-08 12:06 ` [PATCH bpf-next v2 2/3] libbpf: Add "libbpversion" make target to print version Quentin Monnet
@ 2022-02-09  0:35   ` Andrii Nakryiko
  0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2022-02-09  0:35 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Tue, Feb 8, 2022 at 4:06 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> Add a target to libbpf's Makefile to print its version number, in a
> similar way to what running "make kernelversion" at the root of the
> repository does.
>
> This is to avoid re-implementing the parsing of the libbpf.map file in
> case some other tools want to extract the version of the libbpf sources
> they are using.
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
>  tools/lib/bpf/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index b8b37fe76006..91136623edf9 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -108,6 +108,9 @@ MAKEOVERRIDES=
>
>  all:
>
> +libbpfversion:

I don't think we need it (see next patch), but if we end up keeping
it, please call it just "version". Worst case, "libbpf-version" seems
better still.

> +       @echo $(LIBBPF_VERSION)
> +
>  export srctree OUTPUT CC LD CFLAGS V
>  include $(srctree)/tools/build/Makefile.include
>
> --
> 2.32.0
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number
  2022-02-08 12:06 ` [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number Quentin Monnet
@ 2022-02-09  0:39   ` Andrii Nakryiko
  2022-02-09 12:37     ` Quentin Monnet
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2022-02-09  0:39 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Tue, Feb 8, 2022 at 4:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> Since the notion of versions was introduced for bpftool, it has been
> following the version number of the kernel (using the version number
> corresponding to the tree in which bpftool's sources are located). The
> rationale was that bpftool's features are loosely tied to BPF features
> in the kernel, and that we could defer versioning to the kernel
> repository itself.
>
> But this versioning scheme is confusing today, because a bpftool binary
> should be able to work with both older and newer kernels, even if some
> of its recent features won't be available on older systems. Furthermore,
> if bpftool is ported to other systems in the future, keeping a
> Linux-based version number is not a good option.
>
> Looking at other options, we could either have a totally independent
> scheme for bpftool, or we could align it on libbpf's version number
> (with an offset on the major version number, to avoid going backwards).
> The latter comes with a few drawbacks:
>
> - We may want bpftool releases in-between two libbpf versions. We can
>   always append pre-release numbers to distinguish versions, although
>   those won't look as "official" as something with a proper release
>   number. But at the same time, having bpftool with version numbers that
>   look "official" hasn't really been an issue so far.
>
> - If no new feature lands in bpftool for some time, we may move from
>   e.g. 6.7.0 to 6.8.0 when libbpf levels up and have two different
>   versions which are in fact the same.
>
> - Following libbpf's versioning scheme sounds better than kernel's, but
>   ultimately it doesn't make too much sense either, because even though
>   bpftool uses the lib a lot, its behaviour is not that much conditioned
>   by the internal evolution of the library (or by new APIs that it may
>   not use).
>
> Having an independent versioning scheme solves the above, but at the
> cost of heavier maintenance. Developers will likely forget to increase
> the numbers when adding features or bug fixes, and we would take the
> risk of having to send occasional "catch-up" patches just to update the
> version number.
>
> Based on these considerations, this patch aligns bpftool's version
> number on libbpf's. This is not a perfect solution, but 1) it's
> certainly an improvement over the current scheme, 2) the issues raised
> above are all minor at the moment, and 3) we can still move to an
> independent scheme in the future if we realise we need it.
>
> Given that libbpf is currently at version 0.7.0, and bpftool, before
> this patch, was at 5.16, we use an offset of 6 for the major version,
> bumping bpftool to 6.7.0.
>
> It remains possible to manually override the version number by setting
> BPFTOOL_VERSION when calling make.
>
> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> ---
> Contrarily to the previous discussion and to what the first patch of the
> set does, I chose not to use the libbpf_version_string() API from libbpf
> to compute the version for bpftool. There were three reasons for that:
>
> - I don't feel comfortable having bpftool's version number computed at
>   runtime. Somehow it really feels like we should now it when we compile

Fair, but why not use LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION to
define BPFTOOL_VERSION (unless it's overridden). Which all seems to be
doable at compilation time in C code, not in Makefile. This will work
with Github version of libbpf just as well with no extra Makefile
changes (and in general, the less stuff is done in Makefile the
better, IMO).

Version string can also be "composed" at compile time with a bit of
helper macro, see libbpf_version_string() implementation in libbpf.


>   it. We link statically against libbpf today, but if we were to support
>   dynamic linking in the future we may forget to update and would have
>   bpftool's version changing based on the libbpf version installed
>   beside it, which does not make sense.
>
> - We cannot get the patch version for libbpf, the current API only
>   returns the major and minor version numbers (we could fix it, although
>   I'm not sure if desirable to expose the patch number).
>
> - I found it less elegant to compute the version strings in the code,
>   which meant malloc() and error handling just for printing a version
>   number, and having a separate case for when $(BPFTOOL_VERSION) is
>   defined, whereas passing a macro from the Makefile makes things
>   straightforwards.
> ---
>  tools/bpf/bpftool/Makefile | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 83369f55df61..8dd30abff3d9 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -7,14 +7,21 @@ srctree := $(patsubst %/,%,$(dir $(srctree)))
>  srctree := $(patsubst %/,%,$(dir $(srctree)))
>  endif
>
> +BPF_DIR = $(srctree)/tools/lib/bpf
> +
> +# bpftool's version is libbpf's with a fixed offset for the major version.
> +# This is because bpftool's version was higher than libbpf's when we adopted
> +# this scheme.
> +BPFTOOL_MAJOR_OFFSET := 6
> +LIBBPF_VERSION := $(shell make -r --no-print-directory -sC $(BPF_DIR) libbpfversion)
> +BPFTOOL_VERSION ?= $(shell lv="$(LIBBPF_VERSION)"; echo "$$((${lv%%.*} + $(BPFTOOL_MAJOR_OFFSET))).$${lv#*.}")
> +
>  ifeq ($(V),1)
>    Q =
>  else
>    Q = @
>  endif
>
> -BPF_DIR = $(srctree)/tools/lib/bpf
> -
>  ifneq ($(OUTPUT),)
>    _OUTPUT := $(OUTPUT)
>  else
> @@ -39,10 +46,6 @@ LIBBPF_BOOTSTRAP := $(LIBBPF_BOOTSTRAP_OUTPUT)libbpf.a
>  LIBBPF_INTERNAL_HDRS := $(addprefix $(LIBBPF_HDRS_DIR)/,hashmap.h nlattr.h)
>  LIBBPF_BOOTSTRAP_INTERNAL_HDRS := $(addprefix $(LIBBPF_BOOTSTRAP_HDRS_DIR)/,hashmap.h)
>
> -ifeq ($(BPFTOOL_VERSION),)
> -BPFTOOL_VERSION := $(shell make -rR --no-print-directory -sC ../../.. kernelversion)
> -endif
> -
>  $(LIBBPF_OUTPUT) $(BOOTSTRAP_OUTPUT) $(LIBBPF_BOOTSTRAP_OUTPUT) $(LIBBPF_HDRS_DIR) $(LIBBPF_BOOTSTRAP_HDRS_DIR):
>         $(QUIET_MKDIR)mkdir -p $@
>
> --
> 2.32.0
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number
  2022-02-09  0:39   ` Andrii Nakryiko
@ 2022-02-09 12:37     ` Quentin Monnet
  2022-02-09 17:53       ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Quentin Monnet @ 2022-02-09 12:37 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

2022-02-08 16:39 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Tue, Feb 8, 2022 at 4:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> Since the notion of versions was introduced for bpftool, it has been
>> following the version number of the kernel (using the version number
>> corresponding to the tree in which bpftool's sources are located). The
>> rationale was that bpftool's features are loosely tied to BPF features
>> in the kernel, and that we could defer versioning to the kernel
>> repository itself.
>>
>> But this versioning scheme is confusing today, because a bpftool binary
>> should be able to work with both older and newer kernels, even if some
>> of its recent features won't be available on older systems. Furthermore,
>> if bpftool is ported to other systems in the future, keeping a
>> Linux-based version number is not a good option.
>>
>> Looking at other options, we could either have a totally independent
>> scheme for bpftool, or we could align it on libbpf's version number
>> (with an offset on the major version number, to avoid going backwards).
>> The latter comes with a few drawbacks:
>>
>> - We may want bpftool releases in-between two libbpf versions. We can
>>   always append pre-release numbers to distinguish versions, although
>>   those won't look as "official" as something with a proper release
>>   number. But at the same time, having bpftool with version numbers that
>>   look "official" hasn't really been an issue so far.
>>
>> - If no new feature lands in bpftool for some time, we may move from
>>   e.g. 6.7.0 to 6.8.0 when libbpf levels up and have two different
>>   versions which are in fact the same.
>>
>> - Following libbpf's versioning scheme sounds better than kernel's, but
>>   ultimately it doesn't make too much sense either, because even though
>>   bpftool uses the lib a lot, its behaviour is not that much conditioned
>>   by the internal evolution of the library (or by new APIs that it may
>>   not use).
>>
>> Having an independent versioning scheme solves the above, but at the
>> cost of heavier maintenance. Developers will likely forget to increase
>> the numbers when adding features or bug fixes, and we would take the
>> risk of having to send occasional "catch-up" patches just to update the
>> version number.
>>
>> Based on these considerations, this patch aligns bpftool's version
>> number on libbpf's. This is not a perfect solution, but 1) it's
>> certainly an improvement over the current scheme, 2) the issues raised
>> above are all minor at the moment, and 3) we can still move to an
>> independent scheme in the future if we realise we need it.
>>
>> Given that libbpf is currently at version 0.7.0, and bpftool, before
>> this patch, was at 5.16, we use an offset of 6 for the major version,
>> bumping bpftool to 6.7.0.
>>
>> It remains possible to manually override the version number by setting
>> BPFTOOL_VERSION when calling make.
>>
>> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
>> ---
>> Contrarily to the previous discussion and to what the first patch of the
>> set does, I chose not to use the libbpf_version_string() API from libbpf
>> to compute the version for bpftool. There were three reasons for that:
>>
>> - I don't feel comfortable having bpftool's version number computed at
>>   runtime. Somehow it really feels like we should now it when we compile
> 
> Fair, but why not use LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION to
> define BPFTOOL_VERSION (unless it's overridden).

I forgot the macros were exposed, which is silly, because I was the one
to help expose them in the first place. Anyway.

> Which all seems to be
> doable at compilation time in C code, not in Makefile. This will work
> with Github version of libbpf just as well with no extra Makefile
> changes (and in general, the less stuff is done in Makefile the
> better, IMO).
> 
> Version string can also be "composed" at compile time with a bit of
> helper macro, see libbpf_version_string() implementation in libbpf.

Sounds good, I can do that.

This won't give me the patch number, though, only major and minor
version. We could add an additional LIBBPF_PATCH_VERSION to libbpf.
Although thinking about it, I'm not sure we need a patch version for
bpftool at the moment, because changes in libbpf's patch number is
unlikely to reflect any change in bpftool, so it makes little sense to
copy it. So I'm considering just leaving it at 0 in bpftool, and having
updates on major/minor numbers only when libbpf releases a major/minor
version. If we do want bugfix releases, it will still be possible to
overwrite the version number with BPFTOOL_VERSION anyway. What do you think?

Quentin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number
  2022-02-09 12:37     ` Quentin Monnet
@ 2022-02-09 17:53       ` Andrii Nakryiko
  2022-02-09 19:15         ` Quentin Monnet
  0 siblings, 1 reply; 10+ messages in thread
From: Andrii Nakryiko @ 2022-02-09 17:53 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Wed, Feb 9, 2022 at 4:37 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2022-02-08 16:39 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Tue, Feb 8, 2022 at 4:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> Since the notion of versions was introduced for bpftool, it has been
> >> following the version number of the kernel (using the version number
> >> corresponding to the tree in which bpftool's sources are located). The
> >> rationale was that bpftool's features are loosely tied to BPF features
> >> in the kernel, and that we could defer versioning to the kernel
> >> repository itself.
> >>
> >> But this versioning scheme is confusing today, because a bpftool binary
> >> should be able to work with both older and newer kernels, even if some
> >> of its recent features won't be available on older systems. Furthermore,
> >> if bpftool is ported to other systems in the future, keeping a
> >> Linux-based version number is not a good option.
> >>
> >> Looking at other options, we could either have a totally independent
> >> scheme for bpftool, or we could align it on libbpf's version number
> >> (with an offset on the major version number, to avoid going backwards).
> >> The latter comes with a few drawbacks:
> >>
> >> - We may want bpftool releases in-between two libbpf versions. We can
> >>   always append pre-release numbers to distinguish versions, although
> >>   those won't look as "official" as something with a proper release
> >>   number. But at the same time, having bpftool with version numbers that
> >>   look "official" hasn't really been an issue so far.
> >>
> >> - If no new feature lands in bpftool for some time, we may move from
> >>   e.g. 6.7.0 to 6.8.0 when libbpf levels up and have two different
> >>   versions which are in fact the same.
> >>
> >> - Following libbpf's versioning scheme sounds better than kernel's, but
> >>   ultimately it doesn't make too much sense either, because even though
> >>   bpftool uses the lib a lot, its behaviour is not that much conditioned
> >>   by the internal evolution of the library (or by new APIs that it may
> >>   not use).
> >>
> >> Having an independent versioning scheme solves the above, but at the
> >> cost of heavier maintenance. Developers will likely forget to increase
> >> the numbers when adding features or bug fixes, and we would take the
> >> risk of having to send occasional "catch-up" patches just to update the
> >> version number.
> >>
> >> Based on these considerations, this patch aligns bpftool's version
> >> number on libbpf's. This is not a perfect solution, but 1) it's
> >> certainly an improvement over the current scheme, 2) the issues raised
> >> above are all minor at the moment, and 3) we can still move to an
> >> independent scheme in the future if we realise we need it.
> >>
> >> Given that libbpf is currently at version 0.7.0, and bpftool, before
> >> this patch, was at 5.16, we use an offset of 6 for the major version,
> >> bumping bpftool to 6.7.0.
> >>
> >> It remains possible to manually override the version number by setting
> >> BPFTOOL_VERSION when calling make.
> >>
> >> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> >> ---
> >> Contrarily to the previous discussion and to what the first patch of the
> >> set does, I chose not to use the libbpf_version_string() API from libbpf
> >> to compute the version for bpftool. There were three reasons for that:
> >>
> >> - I don't feel comfortable having bpftool's version number computed at
> >>   runtime. Somehow it really feels like we should now it when we compile
> >
> > Fair, but why not use LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION to
> > define BPFTOOL_VERSION (unless it's overridden).
>
> I forgot the macros were exposed, which is silly, because I was the one
> to help expose them in the first place. Anyway.
>
> > Which all seems to be
> > doable at compilation time in C code, not in Makefile. This will work
> > with Github version of libbpf just as well with no extra Makefile
> > changes (and in general, the less stuff is done in Makefile the
> > better, IMO).
> >
> > Version string can also be "composed" at compile time with a bit of
> > helper macro, see libbpf_version_string() implementation in libbpf.
>
> Sounds good, I can do that.
>
> This won't give me the patch number, though, only major and minor
> version. We could add an additional LIBBPF_PATCH_VERSION to libbpf.
> Although thinking about it, I'm not sure we need a patch version for
> bpftool at the moment, because changes in libbpf's patch number is
> unlikely to reflect any change in bpftool, so it makes little sense to
> copy it. So I'm considering just leaving it at 0 in bpftool, and having
> updates on major/minor numbers only when libbpf releases a major/minor
> version. If we do want bugfix releases, it will still be possible to
> overwrite the version number with BPFTOOL_VERSION anyway. What do you think?

So the patch version is not currently reflected in libbpf.map file. I
do patch version bumps only when we detect some small issue after
official release. It happened 2 or 3 times so far. I'm hesitant to
expose that as LIBBPF_PATCH_VERSION, because I'll need to remember to
bump it manually (and coordinating this between kernel sources and
Github is a slow nightmare). Let's not rely on patch version, too much
burden.

>
> Quentin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number
  2022-02-09 17:53       ` Andrii Nakryiko
@ 2022-02-09 19:15         ` Quentin Monnet
  2022-02-09 19:23           ` Andrii Nakryiko
  0 siblings, 1 reply; 10+ messages in thread
From: Quentin Monnet @ 2022-02-09 19:15 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

2022-02-09 09:53 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Wed, Feb 9, 2022 at 4:37 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>
>> 2022-02-08 16:39 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
>>> On Tue, Feb 8, 2022 at 4:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
>>>>
>>>> Since the notion of versions was introduced for bpftool, it has been
>>>> following the version number of the kernel (using the version number
>>>> corresponding to the tree in which bpftool's sources are located). The
>>>> rationale was that bpftool's features are loosely tied to BPF features
>>>> in the kernel, and that we could defer versioning to the kernel
>>>> repository itself.
>>>>
>>>> But this versioning scheme is confusing today, because a bpftool binary
>>>> should be able to work with both older and newer kernels, even if some
>>>> of its recent features won't be available on older systems. Furthermore,
>>>> if bpftool is ported to other systems in the future, keeping a
>>>> Linux-based version number is not a good option.
>>>>
>>>> Looking at other options, we could either have a totally independent
>>>> scheme for bpftool, or we could align it on libbpf's version number
>>>> (with an offset on the major version number, to avoid going backwards).
>>>> The latter comes with a few drawbacks:
>>>>
>>>> - We may want bpftool releases in-between two libbpf versions. We can
>>>>   always append pre-release numbers to distinguish versions, although
>>>>   those won't look as "official" as something with a proper release
>>>>   number. But at the same time, having bpftool with version numbers that
>>>>   look "official" hasn't really been an issue so far.
>>>>
>>>> - If no new feature lands in bpftool for some time, we may move from
>>>>   e.g. 6.7.0 to 6.8.0 when libbpf levels up and have two different
>>>>   versions which are in fact the same.
>>>>
>>>> - Following libbpf's versioning scheme sounds better than kernel's, but
>>>>   ultimately it doesn't make too much sense either, because even though
>>>>   bpftool uses the lib a lot, its behaviour is not that much conditioned
>>>>   by the internal evolution of the library (or by new APIs that it may
>>>>   not use).
>>>>
>>>> Having an independent versioning scheme solves the above, but at the
>>>> cost of heavier maintenance. Developers will likely forget to increase
>>>> the numbers when adding features or bug fixes, and we would take the
>>>> risk of having to send occasional "catch-up" patches just to update the
>>>> version number.
>>>>
>>>> Based on these considerations, this patch aligns bpftool's version
>>>> number on libbpf's. This is not a perfect solution, but 1) it's
>>>> certainly an improvement over the current scheme, 2) the issues raised
>>>> above are all minor at the moment, and 3) we can still move to an
>>>> independent scheme in the future if we realise we need it.
>>>>
>>>> Given that libbpf is currently at version 0.7.0, and bpftool, before
>>>> this patch, was at 5.16, we use an offset of 6 for the major version,
>>>> bumping bpftool to 6.7.0.
>>>>
>>>> It remains possible to manually override the version number by setting
>>>> BPFTOOL_VERSION when calling make.
>>>>
>>>> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
>>>> ---
>>>> Contrarily to the previous discussion and to what the first patch of the
>>>> set does, I chose not to use the libbpf_version_string() API from libbpf
>>>> to compute the version for bpftool. There were three reasons for that:
>>>>
>>>> - I don't feel comfortable having bpftool's version number computed at
>>>>   runtime. Somehow it really feels like we should now it when we compile
>>>
>>> Fair, but why not use LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION to
>>> define BPFTOOL_VERSION (unless it's overridden).
>>
>> I forgot the macros were exposed, which is silly, because I was the one
>> to help expose them in the first place. Anyway.
>>
>>> Which all seems to be
>>> doable at compilation time in C code, not in Makefile. This will work
>>> with Github version of libbpf just as well with no extra Makefile
>>> changes (and in general, the less stuff is done in Makefile the
>>> better, IMO).
>>>
>>> Version string can also be "composed" at compile time with a bit of
>>> helper macro, see libbpf_version_string() implementation in libbpf.
>>
>> Sounds good, I can do that.

... Except that you can only compose so much. The preprocessor won't
allow me to sum libbpf's major version with the offset (6) before
turning it into a string. I need to think about this a bit more.

>>
>> This won't give me the patch number, though, only major and minor
>> version. We could add an additional LIBBPF_PATCH_VERSION to libbpf.
>> Although thinking about it, I'm not sure we need a patch version for
>> bpftool at the moment, because changes in libbpf's patch number is
>> unlikely to reflect any change in bpftool, so it makes little sense to
>> copy it. So I'm considering just leaving it at 0 in bpftool, and having
>> updates on major/minor numbers only when libbpf releases a major/minor
>> version. If we do want bugfix releases, it will still be possible to
>> overwrite the version number with BPFTOOL_VERSION anyway. What do you think?
> 
> So the patch version is not currently reflected in libbpf.map file. I
> do patch version bumps only when we detect some small issue after
> official release. It happened 2 or 3 times so far. I'm hesitant to
> expose that as LIBBPF_PATCH_VERSION, because I'll need to remember to
> bump it manually (and coordinating this between kernel sources and
> Github is a slow nightmare). Let's not rely on patch version, too much
> burden.

Agreed, thanks.
Quentin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number
  2022-02-09 19:15         ` Quentin Monnet
@ 2022-02-09 19:23           ` Andrii Nakryiko
  0 siblings, 0 replies; 10+ messages in thread
From: Andrii Nakryiko @ 2022-02-09 19:23 UTC (permalink / raw)
  To: Quentin Monnet
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Networking, bpf

On Wed, Feb 9, 2022 at 11:15 AM Quentin Monnet <quentin@isovalent.com> wrote:
>
> 2022-02-09 09:53 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> > On Wed, Feb 9, 2022 at 4:37 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>
> >> 2022-02-08 16:39 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> >>> On Tue, Feb 8, 2022 at 4:07 AM Quentin Monnet <quentin@isovalent.com> wrote:
> >>>>
> >>>> Since the notion of versions was introduced for bpftool, it has been
> >>>> following the version number of the kernel (using the version number
> >>>> corresponding to the tree in which bpftool's sources are located). The
> >>>> rationale was that bpftool's features are loosely tied to BPF features
> >>>> in the kernel, and that we could defer versioning to the kernel
> >>>> repository itself.
> >>>>
> >>>> But this versioning scheme is confusing today, because a bpftool binary
> >>>> should be able to work with both older and newer kernels, even if some
> >>>> of its recent features won't be available on older systems. Furthermore,
> >>>> if bpftool is ported to other systems in the future, keeping a
> >>>> Linux-based version number is not a good option.
> >>>>
> >>>> Looking at other options, we could either have a totally independent
> >>>> scheme for bpftool, or we could align it on libbpf's version number
> >>>> (with an offset on the major version number, to avoid going backwards).
> >>>> The latter comes with a few drawbacks:
> >>>>
> >>>> - We may want bpftool releases in-between two libbpf versions. We can
> >>>>   always append pre-release numbers to distinguish versions, although
> >>>>   those won't look as "official" as something with a proper release
> >>>>   number. But at the same time, having bpftool with version numbers that
> >>>>   look "official" hasn't really been an issue so far.
> >>>>
> >>>> - If no new feature lands in bpftool for some time, we may move from
> >>>>   e.g. 6.7.0 to 6.8.0 when libbpf levels up and have two different
> >>>>   versions which are in fact the same.
> >>>>
> >>>> - Following libbpf's versioning scheme sounds better than kernel's, but
> >>>>   ultimately it doesn't make too much sense either, because even though
> >>>>   bpftool uses the lib a lot, its behaviour is not that much conditioned
> >>>>   by the internal evolution of the library (or by new APIs that it may
> >>>>   not use).
> >>>>
> >>>> Having an independent versioning scheme solves the above, but at the
> >>>> cost of heavier maintenance. Developers will likely forget to increase
> >>>> the numbers when adding features or bug fixes, and we would take the
> >>>> risk of having to send occasional "catch-up" patches just to update the
> >>>> version number.
> >>>>
> >>>> Based on these considerations, this patch aligns bpftool's version
> >>>> number on libbpf's. This is not a perfect solution, but 1) it's
> >>>> certainly an improvement over the current scheme, 2) the issues raised
> >>>> above are all minor at the moment, and 3) we can still move to an
> >>>> independent scheme in the future if we realise we need it.
> >>>>
> >>>> Given that libbpf is currently at version 0.7.0, and bpftool, before
> >>>> this patch, was at 5.16, we use an offset of 6 for the major version,
> >>>> bumping bpftool to 6.7.0.
> >>>>
> >>>> It remains possible to manually override the version number by setting
> >>>> BPFTOOL_VERSION when calling make.
> >>>>
> >>>> Signed-off-by: Quentin Monnet <quentin@isovalent.com>
> >>>> ---
> >>>> Contrarily to the previous discussion and to what the first patch of the
> >>>> set does, I chose not to use the libbpf_version_string() API from libbpf
> >>>> to compute the version for bpftool. There were three reasons for that:
> >>>>
> >>>> - I don't feel comfortable having bpftool's version number computed at
> >>>>   runtime. Somehow it really feels like we should now it when we compile
> >>>
> >>> Fair, but why not use LIBBPF_MAJOR_VERSION and LIBBPF_MINOR_VERSION to
> >>> define BPFTOOL_VERSION (unless it's overridden).
> >>
> >> I forgot the macros were exposed, which is silly, because I was the one
> >> to help expose them in the first place. Anyway.
> >>
> >>> Which all seems to be
> >>> doable at compilation time in C code, not in Makefile. This will work
> >>> with Github version of libbpf just as well with no extra Makefile
> >>> changes (and in general, the less stuff is done in Makefile the
> >>> better, IMO).
> >>>
> >>> Version string can also be "composed" at compile time with a bit of
> >>> helper macro, see libbpf_version_string() implementation in libbpf.
> >>
> >> Sounds good, I can do that.
>
> ... Except that you can only compose so much. The preprocessor won't
> allow me to sum libbpf's major version with the offset (6) before
> turning it into a string. I need to think about this a bit more.

Yeah, it sucks. Well, we can either go back to `make version` or
you'll have to do snprintf() to get string representation. 6 +
LIBBPF_MAJOR_VERSION should work in #if condition, it just doesn't
stringifies to 6, but rather "6 + 0", unfortunately.


>
> >>
> >> This won't give me the patch number, though, only major and minor
> >> version. We could add an additional LIBBPF_PATCH_VERSION to libbpf.
> >> Although thinking about it, I'm not sure we need a patch version for
> >> bpftool at the moment, because changes in libbpf's patch number is
> >> unlikely to reflect any change in bpftool, so it makes little sense to
> >> copy it. So I'm considering just leaving it at 0 in bpftool, and having
> >> updates on major/minor numbers only when libbpf releases a major/minor
> >> version. If we do want bugfix releases, it will still be possible to
> >> overwrite the version number with BPFTOOL_VERSION anyway. What do you think?
> >
> > So the patch version is not currently reflected in libbpf.map file. I
> > do patch version bumps only when we detect some small issue after
> > official release. It happened 2 or 3 times so far. I'm hesitant to
> > expose that as LIBBPF_PATCH_VERSION, because I'll need to remember to
> > bump it manually (and coordinating this between kernel sources and
> > Github is a slow nightmare). Let's not rely on patch version, too much
> > burden.
>
> Agreed, thanks.
> Quentin

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-02-09 19:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-08 12:06 [PATCH bpf-next v2 0/3] bpftool: Switch to new versioning scheme (align on libbpf's) Quentin Monnet
2022-02-08 12:06 ` [PATCH bpf-next v2 1/3] bpftool: Add libbpf's version number to "bpftool version" output Quentin Monnet
2022-02-08 12:06 ` [PATCH bpf-next v2 2/3] libbpf: Add "libbpversion" make target to print version Quentin Monnet
2022-02-09  0:35   ` Andrii Nakryiko
2022-02-08 12:06 ` [PATCH bpf-next v2 3/3] bpftool: Update versioning scheme, align on libbpf's version number Quentin Monnet
2022-02-09  0:39   ` Andrii Nakryiko
2022-02-09 12:37     ` Quentin Monnet
2022-02-09 17:53       ` Andrii Nakryiko
2022-02-09 19:15         ` Quentin Monnet
2022-02-09 19:23           ` Andrii Nakryiko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.