On Mon, Mar 22, 2021 at 10:31 PM John Fastabend
<john.fastabend@xxxxxxxxx> wrote:
>
> Rafael David Tinoco wrote:
> > 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 <rafaeldtinoco@xxxxxxxxxx>
> > ---
> > 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.
>
> 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.
>
> Sorry, missed v1,v2 so didn't reply sooner.
>
> > 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).
>
> > int bpf_object__set_priv(struct bpf_object *obj, void *priv,
> > bpf_object_clear_priv_t clear_priv)
> > {
> > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> > index a1a424b9b8ff..cf9bc6f1f925 100644
> > --- a/tools/lib/bpf/libbpf.h
> > +++ b/tools/lib/bpf/libbpf.h
> > @@ -143,6 +143,7 @@ LIBBPF_API int bpf_object__unload(struct bpf_object *obj);
> >
> > LIBBPF_API const char *bpf_object__name(const struct bpf_object *obj);
> > LIBBPF_API unsigned int bpf_object__kversion(const struct bpf_object *obj);
> > +LIBBPF_API int bpf_object__set_kversion(struct bpf_object *obj, __u32 kern_version);
> >
> > struct btf;
> > LIBBPF_API struct btf *bpf_object__btf(const struct bpf_object *obj);
> > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > index 279ae861f568..f5990f7208ce 100644
> > --- a/tools/lib/bpf/libbpf.map
> > +++ b/tools/lib/bpf/libbpf.map
> > @@ -359,4 +359,5 @@ LIBBPF_0.4.0 {
> > bpf_linker__finalize;
> > bpf_linker__free;
> > bpf_linker__new;
> > + bpf_object__set_kversion;
> > } LIBBPF_0.3.0;
> > --
> > 2.27.0
> >
