bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: John Fastabend <john.fastabend@gmail.com>
Cc: Andrii Nakryiko <andriin@fb.com>, bpf <bpf@vger.kernel.org>,
	Networking <netdev@vger.kernel.org>,
	Alexei Starovoitov <ast@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Kernel Team <kernel-team@fb.com>
Subject: Re: [PATCH bpf-next 1/2] libbpf: stop enforcing kern_version, populate it for users
Date: Fri, 4 Oct 2019 07:32:50 -0700	[thread overview]
Message-ID: <CAEf4BzbP=k72O2UXA=Om+Gv1Laj+Ya4QaTNKy7AVkMze6GqLEw@mail.gmail.com> (raw)
In-Reply-To: <5d97519e9e7f3_4e6d2b183260e5bcbf@john-XPS-13-9370.notmuch>

On Fri, Oct 4, 2019 at 7:05 AM John Fastabend <john.fastabend@gmail.com> wrote:
>
> Andrii Nakryiko wrote:
> > Kernel version enforcement for kprobes/kretprobes was removed from
> > 5.0 kernel in 6c4fc209fcf9 ("bpf: remove useless version check for prog load").
> > Since then, BPF programs were specifying SEC("version") just to please
> > libbpf. We should stop enforcing this in libbpf, if even kernel doesn't
> > care. Furthermore, libbpf now will pre-populate current kernel version
> > of the host system, in case we are still running on old kernel.
> >
> > This patch also removes __bpf_object__open_xattr from libbpf.h, as
> > nothing in libbpf is relying on having it in that header. That function
> > was never exported as LIBBPF_API and even name suggests its internal
> > version. So this should be safe to remove, as it doesn't break ABI.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> > ---
> >  tools/lib/bpf/libbpf.c                        | 79 ++++++-------------
> >  tools/lib/bpf/libbpf.h                        |  2 -
> >  .../selftests/bpf/progs/test_attach_probe.c   |  1 -
> >  .../bpf/progs/test_get_stack_rawtp.c          |  1 -
> >  .../selftests/bpf/progs/test_perf_buffer.c    |  1 -
> >  .../selftests/bpf/progs/test_stacktrace_map.c |  1 -
> >  6 files changed, 22 insertions(+), 63 deletions(-)
>
> [...]
>
> >  static struct bpf_object *
> > -__bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz,
> > -                bool needs_kver, int flags)
> > +__bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
> > +                int flags)
> >  {
> >       struct bpf_object *obj;
> >       int err;
> > @@ -3617,7 +3585,6 @@ __bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz,
> >       CHECK_ERR(bpf_object__probe_caps(obj), err, out);
> >       CHECK_ERR(bpf_object__elf_collect(obj, flags), err, out);
>
> If we are not going to validate the section should we also skip collect'ing it?

Well, if user supplied version, we will parse and use it to override
out prepopulated one, so in that sense we do have validation.

But I think it's fine just to drop it altogether. Will do in v3.

>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e0276520171b..22a458cd602c 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -1567,12 +1567,6 @@ static int bpf_object__elf_collect(struct bpf_object *obj, int flags)
>                                                        data->d_size);
>                         if (err)
>                                 return err;
> -               } else if (strcmp(name, "version") == 0) {
> -                       err = bpf_object__init_kversion(obj,
> -                                                       data->d_buf,
> -                                                       data->d_size);
> -                       if (err)
> -                               return err;
>                 } else if (strcmp(name, "maps") == 0) {
>                         obj->efile.maps_shndx = idx;
>                 } else if (strcmp(name, MAPS_ELF_SEC) == 0) {
>
>
> >       CHECK_ERR(bpf_object__collect_reloc(obj), err, out);
> > -     CHECK_ERR(bpf_object__validate(obj, needs_kver), err, out);
> >
> >       bpf_object__elf_finish(obj);
> >       return obj;
> > @@ -3626,8 +3593,8 @@ __bpf_object__open(const char *path, void *obj_buf, size_t obj_buf_sz,
> >       return ERR_PTR(err);
> >  }

  reply	other threads:[~2019-10-04 14:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-04  3:00 [PATCH bpf-next 0/2] Add new-style bpf_object__open APIs Andrii Nakryiko
2019-10-04  3:00 ` [PATCH bpf-next 1/2] libbpf: stop enforcing kern_version, populate it for users Andrii Nakryiko
2019-10-04 14:05   ` John Fastabend
2019-10-04 14:32     ` Andrii Nakryiko [this message]
2019-10-04 14:36       ` Alexei Starovoitov
2019-10-04 14:55         ` John Fastabend
2019-10-04 15:07         ` Andrii Nakryiko
2019-10-04 15:59           ` John Fastabend
2019-10-18 14:49             ` John Fastabend
2019-10-04  3:00 ` [PATCH bpf-next 2/2] libbpf: add bpf_object__open_{file,mem} w/ extensible opts Andrii Nakryiko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEf4BzbP=k72O2UXA=Om+Gv1Laj+Ya4QaTNKy7AVkMze6GqLEw@mail.gmail.com' \
    --to=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=john.fastabend@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH bpf-next 1/2] libbpf: stop enforcing kern_version, populate it for users' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).