Re: [PATCH v3 bpf-next] libbpf: add bpf object kern_version attribute setter

Andrii Nakryiko wrote:

On Mon, Mar 22, 2021 at 10:31 PM John Fastabend
[off-list ref] wrote:

quoted

Rafael David Tinoco wrote:

quoted

Unfortunately some distros don't have their kernel version defined
accurately in <linux/version.h> due to different long term support
reasons.

It is important to have a way to override the bpf kern_version
attribute during runtime: some old kernels might still check for
kern_version attribute during bpf_prog_load().

Signed-off-by: Rafael David Tinoco <redacted>
---
 tools/lib/bpf/libbpf.c   | 10 ++++++++++
 tools/lib/bpf/libbpf.h   |  1 +
 tools/lib/bpf/libbpf.map |  1 +
 3 files changed, 12 insertions(+)

Hi Andrii and Rafael,

Did you consider making kernel version an attribute of the load
API, bpf_prog_load_xattr()? This feels slightly more natural
to me, to tell the API the kernel you need at load time.

Um... kern_version is already part of bpf_load_program_attr, used by
bpf_load_program_xattr. What am I missing? But you can't use that with
bpf_object APIs.

Aha I mistyped. It looks like I have a patch floating around on my
stack to add it to bpf_object_load_attr.

quoted

Although, I don't use the skeleton pieces so maybe it would be
awkward for that usage.

Yes, low-level APIs are separate. This is for cases where you have
struct bpf_program abstractions, which are loaded by
bpf_object__load(). We could set it at per-program level, but they
should be all the same, so bpf_object__set_kversion() makes more sense
and is more convenient to use. And there is already a getter for that,
so it complements that nicely.

+1

quoted

Sorry, missed v1,v2 so didn't reply sooner.

quoted

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 058b643cbcb1..3ac3d8dced7f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c

@@ -8269,6 +8269,16 @@ int bpf_object__btf_fd(const struct bpf_object *obj)
      return obj->btf ? btf__fd(obj->btf) : -1;
 }

+int bpf_object__set_kversion(struct bpf_object *obj, __u32 kern_version)
+{
+     if (obj->loaded)
+             return -EINVAL;
+
+     obj->kern_version = kern_version;
+
+     return 0;
+}
+

Having a test to read uname and feed it into libbpf using
above to be sure we don't break this in the future would be
nice.

kern_version has been ignored by kernel for a long time. So there is
no way to test this in selftests/bpf. We could use libbpf CI's old
kernel setup to validate, but I don't think it's worth it. It's
extremely unlikely this will ever change or break (and it's a legacy
stuff we move away from anyways, so it's born sort of obsolete).

+1

For the patch, thanks for the details Andrii, thanks for the patch
Rafael it will be useful here.

Acked-by: John Fastabend <john.fastabend@gmail.com>