All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
@ 2021-01-09  2:36 Vamsi Kodavanty
  2021-01-11 14:13 ` Daniel Borkmann
  2021-01-11 22:02 ` Andrii Nakryiko
  0 siblings, 2 replies; 9+ messages in thread
From: Vamsi Kodavanty @ 2021-01-09  2:36 UTC (permalink / raw)
  To: bpf, andrii.nakryiko

Andrii,
     I have made the following changes as discussed to add an option to the `open_opts`
to take in the BTF.
     Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews. 
If anyone has any pointers to a webpage where this is described I can go through it. But, below are
the proposed changes.

Best Regards,
Vamsi.

---
 src/libbpf.c | 56 +++++++++++++++++++++++++++++++++++++---------------
 src/libbpf.h |  4 +++-
 2 files changed, 43 insertions(+), 17 deletions(-)

diff --git a/src/libbpf.c b/src/libbpf.c
index 6ae748f..35d7254 100644
--- a/src/libbpf.c
+++ b/src/libbpf.c
@@ -2538,9 +2538,12 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
 	struct bpf_program *prog;
 	int i;
 
-	/* CO-RE relocations need kernel BTF */
+	/* CO-RE relocations need kernel BTF or an override BTF.
+	 * If override BTF present CO-RE can use it.
+	 */
 	if (obj->btf_ext && obj->btf_ext->core_relo_info.len)
-		return true;
+		if (!obj->btf_vmlinux_override)
+			return true;
 
 	/* Support for typed ksyms needs kernel BTF */
 	for (i = 0; i < obj->nr_extern; i++) {
@@ -2561,6 +2564,27 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
 	return false;
 }
 
+static int bpf_object__load_override_btf(struct bpf_object *obj,
+										 const char *targ_btf_path)
+{
+	/* Could have been be set via `bpf_object_open_opts` */
+	if (obj->btf_vmlinux_override)
+		return 0;
+
+	if (!targ_btf_path)
+		return 0;
+
+	obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
+	if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
+		int err = PTR_ERR(obj->btf_vmlinux_override);
+		obj->btf_vmlinux_override = NULL;
+		pr_warn("failed to parse target BTF: %d\n", err);
+		return err;
+	}
+
+	return 0;
+}
+
 static int bpf_object__load_vmlinux_btf(struct bpf_object *obj, bool force)
 {
 	int err;
@@ -6031,7 +6055,7 @@ patch_insn:
 }
 
 static int
-bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
+bpf_object__relocate_core(struct bpf_object *obj)
 {
 	const struct btf_ext_info_sec *sec;
 	const struct bpf_core_relo *rec;
@@ -6045,15 +6069,6 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
 	if (obj->btf_ext->core_relo_info.len == 0)
 		return 0;
 
-	if (targ_btf_path) {
-		obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
-		if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
-			err = PTR_ERR(obj->btf_vmlinux_override);
-			pr_warn("failed to parse target BTF: %d\n", err);
-			return err;
-		}
-	}
-
 	cand_cache = hashmap__new(bpf_core_hash_fn, bpf_core_equal_fn, NULL);
 	if (IS_ERR(cand_cache)) {
 		err = PTR_ERR(cand_cache);
@@ -6556,14 +6571,14 @@ bpf_object__relocate_calls(struct bpf_object *obj, struct bpf_program *prog)
 }
 
 static int
-bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
+bpf_object__relocate(struct bpf_object *obj)
 {
 	struct bpf_program *prog;
 	size_t i;
 	int err;
 
 	if (obj->btf_ext) {
-		err = bpf_object__relocate_core(obj, targ_btf_path);
+		err = bpf_object__relocate_core(obj);
 		if (err) {
 			pr_warn("failed to perform CO-RE relocations: %d\n",
 				err);
@@ -7088,7 +7103,7 @@ static struct bpf_object *
 __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
 		   const struct bpf_object_open_opts *opts)
 {
-	const char *obj_name, *kconfig;
+	const char *obj_name, *kconfig, *core_btf_path;
 	struct bpf_program *prog;
 	struct bpf_object *obj;
 	char tmp_name[64];
@@ -7126,6 +7141,14 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
 			return ERR_PTR(-ENOMEM);
 	}
 
+	core_btf_path = OPTS_GET(opts, core_btf_path, NULL);
+	if (core_btf_path) {
+		pr_debug("parse btf '%s' for CO-RE relocations\n", core_btf_path);
+		obj->btf_vmlinux_override = btf__parse(core_btf_path, NULL);
+		if (IS_ERR_OR_NULL(obj->btf_vmlinux_override))
+			pr_warn("can't parse btf at '%s'\n", core_btf_path);
+	}
+
 	err = bpf_object__elf_init(obj);
 	err = err ? : bpf_object__check_endianness(obj);
 	err = err ? : bpf_object__elf_collect(obj);
@@ -7481,13 +7504,14 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
 	}
 
 	err = bpf_object__probe_loading(obj);
+	err = err ? : bpf_object__load_override_btf(obj, attr->target_btf_path);
 	err = err ? : bpf_object__load_vmlinux_btf(obj, false);
 	err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
 	err = err ? : bpf_object__sanitize_and_load_btf(obj);
 	err = err ? : bpf_object__sanitize_maps(obj);
 	err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
 	err = err ? : bpf_object__create_maps(obj);
-	err = err ? : bpf_object__relocate(obj, attr->target_btf_path);
+	err = err ? : bpf_object__relocate(obj);
 	err = err ? : bpf_object__load_progs(obj, attr->log_level);
 
 	/* clean up module BTFs */
diff --git a/src/libbpf.h b/src/libbpf.h
index 3c35eb4..40c4ee9 100644
--- a/src/libbpf.h
+++ b/src/libbpf.h
@@ -93,8 +93,10 @@ struct bpf_object_open_opts {
 	 * system Kconfig for CONFIG_xxx externs.
 	 */
 	const char *kconfig;
+	/* Path to ELF file with BTF section to be used for relocations. */
+	const char *core_btf_path;
 };
-#define bpf_object_open_opts__last_field kconfig
+#define bpf_object_open_opts__last_field core_btf_path
 
 LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
 LIBBPF_API struct bpf_object *
-- 
2.23.3


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

• * Re: [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
    2021-01-09  2:36 [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program Vamsi Kodavanty
  @ 2021-01-11 14:13 ` Daniel Borkmann
    2021-01-11 19:07   ` Vamsi Kodavanty
    2021-01-11 22:02 ` Andrii Nakryiko
    1 sibling, 1 reply; 9+ messages in thread
  From: Daniel Borkmann @ 2021-01-11 14:13 UTC (permalink / raw)
    To: Vamsi Kodavanty, bpf, andrii.nakryiko
  
  On 1/9/21 3:36 AM, Vamsi Kodavanty wrote:
  [...]
  >       Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews.
  > If anyone has any pointers to a webpage where this is described I can go through it. But, below are
  > the proposed changes.
  
  For submitting patches there is an official write-up here [0]. An example of a commit message
  can be found here [1]. Please make sure to add your own Signed-off-by before officially submitting
  the patch. If you are stuck somewhere please let us know so we can help.
  
  Cheers,
  Daniel
  
     [0] https://www.kernel.org/doc/html/latest/process/submitting-patches.html
     [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/patch/?id=e22d7f05e445165e58feddb4e40cc9c0f94453bc
  
  ^ permalink raw reply	[flat|nested] 9+ messages in thread

• • * Re: [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
      2021-01-11 14:13 ` Daniel Borkmann
    @ 2021-01-11 19:07   ` Vamsi Kodavanty
      0 siblings, 0 replies; 9+ messages in thread
    From: Vamsi Kodavanty @ 2021-01-11 19:07 UTC (permalink / raw)
      To: Daniel Borkmann; +Cc: bpf, Andrii Nakryiko
    
    Thank you so much!. Will follow protocol and get back here.
    
    Best Regards
    Vamsi.
    
    On Mon, Jan 11, 2021 at 6:14 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
    >
    > On 1/9/21 3:36 AM, Vamsi Kodavanty wrote:
    > [...]
    > >       Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews.
    > > If anyone has any pointers to a webpage where this is described I can go through it. But, below are
    > > the proposed changes.
    >
    > For submitting patches there is an official write-up here [0]. An example of a commit message
    > can be found here [1]. Please make sure to add your own Signed-off-by before officially submitting
    > the patch. If you are stuck somewhere please let us know so we can help.
    >
    > Cheers,
    > Daniel
    >
    >    [0] https://www.kernel.org/doc/html/latest/process/submitting-patches.html
    >    [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/patch/?id=e22d7f05e445165e58feddb4e40cc9c0f94453bc
    
    ^ permalink raw reply	[flat|nested] 9+ messages in thread

• * Re: [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
    2021-01-09  2:36 [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program Vamsi Kodavanty
    2021-01-11 14:13 ` Daniel Borkmann
  @ 2021-01-11 22:02 ` Andrii Nakryiko
    2021-01-12  3:33   ` Vamsi Kodavanty
    1 sibling, 1 reply; 9+ messages in thread
  From: Andrii Nakryiko @ 2021-01-11 22:02 UTC (permalink / raw)
    To: Vamsi Kodavanty; +Cc: bpf
  
  On Fri, Jan 8, 2021 at 6:36 PM Vamsi Kodavanty <vamsi@araalinetworks.com> wrote:
  >
  > Andrii,
  >      I have made the following changes as discussed to add an option to the `open_opts`
  > to take in the BTF.
  >      Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews.
  > If anyone has any pointers to a webpage where this is described I can go through it. But, below are
  > the proposed changes.
  >
  
  Daniel already gave you pointers. Also make sure you add [PATCH
  bpf-next] prefix to email subject to identify the patch is for
  bpf-next kernel tree.
  And with all changes like this we should also add selftests,
  exercising new features. Please take a look at
  tools/testing/selftests/bpf. I think updating
  test_progs/test_core_reloc.c in there to use this instead of
  bpf_object__load_xattr() might be enough of the testing.
  
  > Best Regards,
  > Vamsi.
  >
  > ---
  >  src/libbpf.c | 56 +++++++++++++++++++++++++++++++++++++---------------
  >  src/libbpf.h |  4 +++-
  >  2 files changed, 43 insertions(+), 17 deletions(-)
  >
  > diff --git a/src/libbpf.c b/src/libbpf.c
  > index 6ae748f..35d7254 100644
  > --- a/src/libbpf.c
  > +++ b/src/libbpf.c
  > @@ -2538,9 +2538,12 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
  >         struct bpf_program *prog;
  >         int i;
  >
  > -       /* CO-RE relocations need kernel BTF */
  > +       /* CO-RE relocations need kernel BTF or an override BTF.
  > +        * If override BTF present CO-RE can use it.
  
  nit: "CO-RE relocations need kernel BTF, unless custom BTF override is
  specified"
  
  > +        */
  >         if (obj->btf_ext && obj->btf_ext->core_relo_info.len)
  > -               return true;
  > +               if (!obj->btf_vmlinux_override)
  
  please combine this into a single if
  
  > +                       return true;
  >
  >         /* Support for typed ksyms needs kernel BTF */
  >         for (i = 0; i < obj->nr_extern; i++) {
  > @@ -2561,6 +2564,27 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
  >         return false;
  >  }
  >
  > +static int bpf_object__load_override_btf(struct bpf_object *obj,
  > +                                                                                const char *targ_btf_path)
  
  formatting is off? scripts/checkpatch.pl -f <path to file> will report
  issues like this
  
  > +{
  > +       /* Could have been be set via `bpf_object_open_opts` */
  > +       if (obj->btf_vmlinux_override)
  > +               return 0;
  
  see below, let's make sure we load btf_vmlinux_override in one place
  (and cleanup somewhere close)
  
  > +
  > +       if (!targ_btf_path)
  > +               return 0;
  > +
  > +       obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
  > +       if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
  > +               int err = PTR_ERR(obj->btf_vmlinux_override);
  > +               obj->btf_vmlinux_override = NULL;
  > +               pr_warn("failed to parse target BTF: %d\n", err);
  > +               return err;
  > +       }
  > +
  > +       return 0;
  > +}
  > +
  >  static int bpf_object__load_vmlinux_btf(struct bpf_object *obj, bool force)
  >  {
  >         int err;
  > @@ -6031,7 +6055,7 @@ patch_insn:
  >  }
  >
  >  static int
  > -bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
  > +bpf_object__relocate_core(struct bpf_object *obj)
  >  {
  >         const struct btf_ext_info_sec *sec;
  >         const struct bpf_core_relo *rec;
  > @@ -6045,15 +6069,6 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
  >         if (obj->btf_ext->core_relo_info.len == 0)
  >                 return 0;
  >
  > -       if (targ_btf_path) {
  > -               obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
  > -               if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
  > -                       err = PTR_ERR(obj->btf_vmlinux_override);
  > -                       pr_warn("failed to parse target BTF: %d\n", err);
  > -                       return err;
  > -               }
  > -       }
  > -
  
  given we are moving out btf_vmlinux_override loading from
  bpf_object__relocate_core, we need to move out its destruction and
  cleanup to the same function that does BTF parsing. That will keep the
  logic simpler.
  
  >         cand_cache = hashmap__new(bpf_core_hash_fn, bpf_core_equal_fn, NULL);
  >         if (IS_ERR(cand_cache)) {
  >                 err = PTR_ERR(cand_cache);
  > @@ -6556,14 +6571,14 @@ bpf_object__relocate_calls(struct bpf_object *obj, struct bpf_program *prog)
  >  }
  >
  >  static int
  > -bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
  > +bpf_object__relocate(struct bpf_object *obj)
  >  {
  >         struct bpf_program *prog;
  >         size_t i;
  >         int err;
  >
  >         if (obj->btf_ext) {
  > -               err = bpf_object__relocate_core(obj, targ_btf_path);
  > +               err = bpf_object__relocate_core(obj);
  >                 if (err) {
  >                         pr_warn("failed to perform CO-RE relocations: %d\n",
  >                                 err);
  > @@ -7088,7 +7103,7 @@ static struct bpf_object *
  >  __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
  >                    const struct bpf_object_open_opts *opts)
  >  {
  > -       const char *obj_name, *kconfig;
  > +       const char *obj_name, *kconfig, *core_btf_path;
  >         struct bpf_program *prog;
  >         struct bpf_object *obj;
  >         char tmp_name[64];
  > @@ -7126,6 +7141,14 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
  >                         return ERR_PTR(-ENOMEM);
  >         }
  >
  > +       core_btf_path = OPTS_GET(opts, core_btf_path, NULL);
  > +       if (core_btf_path) {
  > +               pr_debug("parse btf '%s' for CO-RE relocations\n", core_btf_path);
  
  Move this right after btf__parse(), so you can report success or
  failure in one log statement; see how libbpf_find_kernel_btf() does
  it. Please use similar wording (just "target BTF" instead of "kernel
  BTF" to distinguish them).
  
  > +               obj->btf_vmlinux_override = btf__parse(core_btf_path, NULL);
  
  This BTF is not needed until the load phase, so let's not attempt to
  load it on open() unnecessarily. Just remember the path and defer till
  bpf_object__load() time.
  
  > +               if (IS_ERR_OR_NULL(obj->btf_vmlinux_override))
  > +                       pr_warn("can't parse btf at '%s'\n", core_btf_path);
  
  if BTF can't be loaded, load should fail
  
  > +       }
  > +
  >         err = bpf_object__elf_init(obj);
  >         err = err ? : bpf_object__check_endianness(obj);
  >         err = err ? : bpf_object__elf_collect(obj);
  > @@ -7481,13 +7504,14 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
  >         }
  >
  >         err = bpf_object__probe_loading(obj);
  > +       err = err ? : bpf_object__load_override_btf(obj, attr->target_btf_path);
  >         err = err ? : bpf_object__load_vmlinux_btf(obj, false);
  >         err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
  >         err = err ? : bpf_object__sanitize_and_load_btf(obj);
  >         err = err ? : bpf_object__sanitize_maps(obj);
  >         err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
  >         err = err ? : bpf_object__create_maps(obj);
  > -       err = err ? : bpf_object__relocate(obj, attr->target_btf_path);
  
  For backwards compatibility, we need to still respect
  attr->target_btf_path. I'd say let attr->target_btf_path override
  opts->core_btf_path, if specified. Later on we'll probably just
  deprecate bpf_object__load_xattr() and target_btf_path will go away.
  
  > +       err = err ? : bpf_object__relocate(obj);
  >         err = err ? : bpf_object__load_progs(obj, attr->log_level);
  >
  >         /* clean up module BTFs */
  > diff --git a/src/libbpf.h b/src/libbpf.h
  > index 3c35eb4..40c4ee9 100644
  > --- a/src/libbpf.h
  > +++ b/src/libbpf.h
  > @@ -93,8 +93,10 @@ struct bpf_object_open_opts {
  >          * system Kconfig for CONFIG_xxx externs.
  >          */
  >         const char *kconfig;
  > +       /* Path to ELF file with BTF section to be used for relocations. */
  
  Given you use btf__parse() when opening this BTF, it handles both ELF
  and raw BTF data. So let's reword this comment to mention BTF in
  general.
  
  > +       const char *core_btf_path;
  >  };
  > -#define bpf_object_open_opts__last_field kconfig
  > +#define bpf_object_open_opts__last_field core_btf_path
  >
  >  LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
  >  LIBBPF_API struct bpf_object *
  > --
  > 2.23.3
  >
  
  ^ permalink raw reply	[flat|nested] 9+ messages in thread

• • * Re: [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
      2021-01-11 22:02 ` Andrii Nakryiko
    @ 2021-01-12  3:33   ` Vamsi Kodavanty
      2021-01-13 22:16     ` Vamsi Kodavanty
      0 siblings, 1 reply; 9+ messages in thread
    From: Vamsi Kodavanty @ 2021-01-12  3:33 UTC (permalink / raw)
      To: Andrii Nakryiko; +Cc: bpf
    
    Andrii,
       Thank you for the detailed review. I will address them as well as
    the self tests. And will send out a new patch addressing them and
    conforming to style/expectations.
    
    Cheers
    Vamsi.
    
    On Mon, Jan 11, 2021 at 2:02 PM Andrii Nakryiko
    <andrii.nakryiko@gmail.com> wrote:
    >
    > On Fri, Jan 8, 2021 at 6:36 PM Vamsi Kodavanty <vamsi@araalinetworks.com> wrote:
    > >
    > > Andrii,
    > >      I have made the following changes as discussed to add an option to the `open_opts`
    > > to take in the BTF.
    > >      Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews.
    > > If anyone has any pointers to a webpage where this is described I can go through it. But, below are
    > > the proposed changes.
    > >
    >
    > Daniel already gave you pointers. Also make sure you add [PATCH
    > bpf-next] prefix to email subject to identify the patch is for
    > bpf-next kernel tree.
    > And with all changes like this we should also add selftests,
    > exercising new features. Please take a look at
    > tools/testing/selftests/bpf. I think updating
    > test_progs/test_core_reloc.c in there to use this instead of
    > bpf_object__load_xattr() might be enough of the testing.
    >
    > > Best Regards,
    > > Vamsi.
    > >
    > > ---
    > >  src/libbpf.c | 56 +++++++++++++++++++++++++++++++++++++---------------
    > >  src/libbpf.h |  4 +++-
    > >  2 files changed, 43 insertions(+), 17 deletions(-)
    > >
    > > diff --git a/src/libbpf.c b/src/libbpf.c
    > > index 6ae748f..35d7254 100644
    > > --- a/src/libbpf.c
    > > +++ b/src/libbpf.c
    > > @@ -2538,9 +2538,12 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
    > >         struct bpf_program *prog;
    > >         int i;
    > >
    > > -       /* CO-RE relocations need kernel BTF */
    > > +       /* CO-RE relocations need kernel BTF or an override BTF.
    > > +        * If override BTF present CO-RE can use it.
    >
    > nit: "CO-RE relocations need kernel BTF, unless custom BTF override is
    > specified"
    >
    > > +        */
    > >         if (obj->btf_ext && obj->btf_ext->core_relo_info.len)
    > > -               return true;
    > > +               if (!obj->btf_vmlinux_override)
    >
    > please combine this into a single if
    >
    > > +                       return true;
    > >
    > >         /* Support for typed ksyms needs kernel BTF */
    > >         for (i = 0; i < obj->nr_extern; i++) {
    > > @@ -2561,6 +2564,27 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
    > >         return false;
    > >  }
    > >
    > > +static int bpf_object__load_override_btf(struct bpf_object *obj,
    > > +                                                                                const char *targ_btf_path)
    >
    > formatting is off? scripts/checkpatch.pl -f <path to file> will report
    > issues like this
    >
    > > +{
    > > +       /* Could have been be set via `bpf_object_open_opts` */
    > > +       if (obj->btf_vmlinux_override)
    > > +               return 0;
    >
    > see below, let's make sure we load btf_vmlinux_override in one place
    > (and cleanup somewhere close)
    >
    > > +
    > > +       if (!targ_btf_path)
    > > +               return 0;
    > > +
    > > +       obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
    > > +       if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
    > > +               int err = PTR_ERR(obj->btf_vmlinux_override);
    > > +               obj->btf_vmlinux_override = NULL;
    > > +               pr_warn("failed to parse target BTF: %d\n", err);
    > > +               return err;
    > > +       }
    > > +
    > > +       return 0;
    > > +}
    > > +
    > >  static int bpf_object__load_vmlinux_btf(struct bpf_object *obj, bool force)
    > >  {
    > >         int err;
    > > @@ -6031,7 +6055,7 @@ patch_insn:
    > >  }
    > >
    > >  static int
    > > -bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
    > > +bpf_object__relocate_core(struct bpf_object *obj)
    > >  {
    > >         const struct btf_ext_info_sec *sec;
    > >         const struct bpf_core_relo *rec;
    > > @@ -6045,15 +6069,6 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
    > >         if (obj->btf_ext->core_relo_info.len == 0)
    > >                 return 0;
    > >
    > > -       if (targ_btf_path) {
    > > -               obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
    > > -               if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
    > > -                       err = PTR_ERR(obj->btf_vmlinux_override);
    > > -                       pr_warn("failed to parse target BTF: %d\n", err);
    > > -                       return err;
    > > -               }
    > > -       }
    > > -
    >
    > given we are moving out btf_vmlinux_override loading from
    > bpf_object__relocate_core, we need to move out its destruction and
    > cleanup to the same function that does BTF parsing. That will keep the
    > logic simpler.
    >
    > >         cand_cache = hashmap__new(bpf_core_hash_fn, bpf_core_equal_fn, NULL);
    > >         if (IS_ERR(cand_cache)) {
    > >                 err = PTR_ERR(cand_cache);
    > > @@ -6556,14 +6571,14 @@ bpf_object__relocate_calls(struct bpf_object *obj, struct bpf_program *prog)
    > >  }
    > >
    > >  static int
    > > -bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
    > > +bpf_object__relocate(struct bpf_object *obj)
    > >  {
    > >         struct bpf_program *prog;
    > >         size_t i;
    > >         int err;
    > >
    > >         if (obj->btf_ext) {
    > > -               err = bpf_object__relocate_core(obj, targ_btf_path);
    > > +               err = bpf_object__relocate_core(obj);
    > >                 if (err) {
    > >                         pr_warn("failed to perform CO-RE relocations: %d\n",
    > >                                 err);
    > > @@ -7088,7 +7103,7 @@ static struct bpf_object *
    > >  __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
    > >                    const struct bpf_object_open_opts *opts)
    > >  {
    > > -       const char *obj_name, *kconfig;
    > > +       const char *obj_name, *kconfig, *core_btf_path;
    > >         struct bpf_program *prog;
    > >         struct bpf_object *obj;
    > >         char tmp_name[64];
    > > @@ -7126,6 +7141,14 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
    > >                         return ERR_PTR(-ENOMEM);
    > >         }
    > >
    > > +       core_btf_path = OPTS_GET(opts, core_btf_path, NULL);
    > > +       if (core_btf_path) {
    > > +               pr_debug("parse btf '%s' for CO-RE relocations\n", core_btf_path);
    >
    > Move this right after btf__parse(), so you can report success or
    > failure in one log statement; see how libbpf_find_kernel_btf() does
    > it. Please use similar wording (just "target BTF" instead of "kernel
    > BTF" to distinguish them).
    >
    > > +               obj->btf_vmlinux_override = btf__parse(core_btf_path, NULL);
    >
    > This BTF is not needed until the load phase, so let's not attempt to
    > load it on open() unnecessarily. Just remember the path and defer till
    > bpf_object__load() time.
    >
    > > +               if (IS_ERR_OR_NULL(obj->btf_vmlinux_override))
    > > +                       pr_warn("can't parse btf at '%s'\n", core_btf_path);
    >
    > if BTF can't be loaded, load should fail
    >
    > > +       }
    > > +
    > >         err = bpf_object__elf_init(obj);
    > >         err = err ? : bpf_object__check_endianness(obj);
    > >         err = err ? : bpf_object__elf_collect(obj);
    > > @@ -7481,13 +7504,14 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
    > >         }
    > >
    > >         err = bpf_object__probe_loading(obj);
    > > +       err = err ? : bpf_object__load_override_btf(obj, attr->target_btf_path);
    > >         err = err ? : bpf_object__load_vmlinux_btf(obj, false);
    > >         err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
    > >         err = err ? : bpf_object__sanitize_and_load_btf(obj);
    > >         err = err ? : bpf_object__sanitize_maps(obj);
    > >         err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
    > >         err = err ? : bpf_object__create_maps(obj);
    > > -       err = err ? : bpf_object__relocate(obj, attr->target_btf_path);
    >
    > For backwards compatibility, we need to still respect
    > attr->target_btf_path. I'd say let attr->target_btf_path override
    > opts->core_btf_path, if specified. Later on we'll probably just
    > deprecate bpf_object__load_xattr() and target_btf_path will go away.
    >
    > > +       err = err ? : bpf_object__relocate(obj);
    > >         err = err ? : bpf_object__load_progs(obj, attr->log_level);
    > >
    > >         /* clean up module BTFs */
    > > diff --git a/src/libbpf.h b/src/libbpf.h
    > > index 3c35eb4..40c4ee9 100644
    > > --- a/src/libbpf.h
    > > +++ b/src/libbpf.h
    > > @@ -93,8 +93,10 @@ struct bpf_object_open_opts {
    > >          * system Kconfig for CONFIG_xxx externs.
    > >          */
    > >         const char *kconfig;
    > > +       /* Path to ELF file with BTF section to be used for relocations. */
    >
    > Given you use btf__parse() when opening this BTF, it handles both ELF
    > and raw BTF data. So let's reword this comment to mention BTF in
    > general.
    >
    > > +       const char *core_btf_path;
    > >  };
    > > -#define bpf_object_open_opts__last_field kconfig
    > > +#define bpf_object_open_opts__last_field core_btf_path
    > >
    > >  LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
    > >  LIBBPF_API struct bpf_object *
    > > --
    > > 2.23.3
    > >
    
    ^ permalink raw reply	[flat|nested] 9+ messages in thread

  • • * Re: [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
        2021-01-12  3:33   ` Vamsi Kodavanty
      @ 2021-01-13 22:16     ` Vamsi Kodavanty
        2021-01-14  3:21       ` Andrii Nakryiko
        0 siblings, 1 reply; 9+ messages in thread
      From: Vamsi Kodavanty @ 2021-01-13 22:16 UTC (permalink / raw)
        To: Andrii Nakryiko; +Cc: bpf
      
      On Mon, Jan 11, 2021 at 7:33 PM Vamsi Kodavanty
      <vamsi@araalinetworks.com> wrote:
      >
      > Andrii,
      >    Thank you for the detailed review. I will address them as well as
      > the self tests. And will send out a new patch addressing them and
      > conforming to style/expectations.
      >
      > Cheers
      > Vamsi.
      >
      Andrii,
            I understand the `bpf` repository being a mirror of the
      `bpf-next` tools/lib/bpf. Do the patches
      to `bpf` go back into `bpf-next`. I see there is a script for
      `bpf-next` to `bpf`syncs.
            I ask because the `btf_vmlinux_override` changes only exist in
      the `bpf` repo. So, I make my
      changes in `bpf`?. In that case what happens to the `selftests` which
      are in `bpf-next`. And they
      won't have any idea of the new open option 'core_btf_path` that is
      being introduced.
      
      Thanks again. Hopefully this is my last question before I come back to
      you with a proper patch.
      
      Cheers
      Vamsi.
      
      > On Mon, Jan 11, 2021 at 2:02 PM Andrii Nakryiko
      > <andrii.nakryiko@gmail.com> wrote:
      > >
      > > On Fri, Jan 8, 2021 at 6:36 PM Vamsi Kodavanty <vamsi@araalinetworks.com> wrote:
      > > >
      > > > Andrii,
      > > >      I have made the following changes as discussed to add an option to the `open_opts`
      > > > to take in the BTF.
      > > >      Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews.
      > > > If anyone has any pointers to a webpage where this is described I can go through it. But, below are
      > > > the proposed changes.
      > > >
      > >
      > > Daniel already gave you pointers. Also make sure you add [PATCH
      > > bpf-next] prefix to email subject to identify the patch is for
      > > bpf-next kernel tree.
      > > And with all changes like this we should also add selftests,
      > > exercising new features. Please take a look at
      > > tools/testing/selftests/bpf. I think updating
      > > test_progs/test_core_reloc.c in there to use this instead of
      > > bpf_object__load_xattr() might be enough of the testing.
      > >
      > > > Best Regards,
      > > > Vamsi.
      > > >
      > > > ---
      > > >  src/libbpf.c | 56 +++++++++++++++++++++++++++++++++++++---------------
      > > >  src/libbpf.h |  4 +++-
      > > >  2 files changed, 43 insertions(+), 17 deletions(-)
      > > >
      > > > diff --git a/src/libbpf.c b/src/libbpf.c
      > > > index 6ae748f..35d7254 100644
      > > > --- a/src/libbpf.c
      > > > +++ b/src/libbpf.c
      > > > @@ -2538,9 +2538,12 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
      > > >         struct bpf_program *prog;
      > > >         int i;
      > > >
      > > > -       /* CO-RE relocations need kernel BTF */
      > > > +       /* CO-RE relocations need kernel BTF or an override BTF.
      > > > +        * If override BTF present CO-RE can use it.
      > >
      > > nit: "CO-RE relocations need kernel BTF, unless custom BTF override is
      > > specified"
      > >
      > > > +        */
      > > >         if (obj->btf_ext && obj->btf_ext->core_relo_info.len)
      > > > -               return true;
      > > > +               if (!obj->btf_vmlinux_override)
      > >
      > > please combine this into a single if
      > >
      > > > +                       return true;
      > > >
      > > >         /* Support for typed ksyms needs kernel BTF */
      > > >         for (i = 0; i < obj->nr_extern; i++) {
      > > > @@ -2561,6 +2564,27 @@ static bool obj_needs_vmlinux_btf(const struct bpf_object *obj)
      > > >         return false;
      > > >  }
      > > >
      > > > +static int bpf_object__load_override_btf(struct bpf_object *obj,
      > > > +                                                                                const char *targ_btf_path)
      > >
      > > formatting is off? scripts/checkpatch.pl -f <path to file> will report
      > > issues like this
      > >
      > > > +{
      > > > +       /* Could have been be set via `bpf_object_open_opts` */
      > > > +       if (obj->btf_vmlinux_override)
      > > > +               return 0;
      > >
      > > see below, let's make sure we load btf_vmlinux_override in one place
      > > (and cleanup somewhere close)
      > >
      > > > +
      > > > +       if (!targ_btf_path)
      > > > +               return 0;
      > > > +
      > > > +       obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
      > > > +       if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
      > > > +               int err = PTR_ERR(obj->btf_vmlinux_override);
      > > > +               obj->btf_vmlinux_override = NULL;
      > > > +               pr_warn("failed to parse target BTF: %d\n", err);
      > > > +               return err;
      > > > +       }
      > > > +
      > > > +       return 0;
      > > > +}
      > > > +
      > > >  static int bpf_object__load_vmlinux_btf(struct bpf_object *obj, bool force)
      > > >  {
      > > >         int err;
      > > > @@ -6031,7 +6055,7 @@ patch_insn:
      > > >  }
      > > >
      > > >  static int
      > > > -bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
      > > > +bpf_object__relocate_core(struct bpf_object *obj)
      > > >  {
      > > >         const struct btf_ext_info_sec *sec;
      > > >         const struct bpf_core_relo *rec;
      > > > @@ -6045,15 +6069,6 @@ bpf_object__relocate_core(struct bpf_object *obj, const char *targ_btf_path)
      > > >         if (obj->btf_ext->core_relo_info.len == 0)
      > > >                 return 0;
      > > >
      > > > -       if (targ_btf_path) {
      > > > -               obj->btf_vmlinux_override = btf__parse(targ_btf_path, NULL);
      > > > -               if (IS_ERR_OR_NULL(obj->btf_vmlinux_override)) {
      > > > -                       err = PTR_ERR(obj->btf_vmlinux_override);
      > > > -                       pr_warn("failed to parse target BTF: %d\n", err);
      > > > -                       return err;
      > > > -               }
      > > > -       }
      > > > -
      > >
      > > given we are moving out btf_vmlinux_override loading from
      > > bpf_object__relocate_core, we need to move out its destruction and
      > > cleanup to the same function that does BTF parsing. That will keep the
      > > logic simpler.
      > >
      > > >         cand_cache = hashmap__new(bpf_core_hash_fn, bpf_core_equal_fn, NULL);
      > > >         if (IS_ERR(cand_cache)) {
      > > >                 err = PTR_ERR(cand_cache);
      > > > @@ -6556,14 +6571,14 @@ bpf_object__relocate_calls(struct bpf_object *obj, struct bpf_program *prog)
      > > >  }
      > > >
      > > >  static int
      > > > -bpf_object__relocate(struct bpf_object *obj, const char *targ_btf_path)
      > > > +bpf_object__relocate(struct bpf_object *obj)
      > > >  {
      > > >         struct bpf_program *prog;
      > > >         size_t i;
      > > >         int err;
      > > >
      > > >         if (obj->btf_ext) {
      > > > -               err = bpf_object__relocate_core(obj, targ_btf_path);
      > > > +               err = bpf_object__relocate_core(obj);
      > > >                 if (err) {
      > > >                         pr_warn("failed to perform CO-RE relocations: %d\n",
      > > >                                 err);
      > > > @@ -7088,7 +7103,7 @@ static struct bpf_object *
      > > >  __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
      > > >                    const struct bpf_object_open_opts *opts)
      > > >  {
      > > > -       const char *obj_name, *kconfig;
      > > > +       const char *obj_name, *kconfig, *core_btf_path;
      > > >         struct bpf_program *prog;
      > > >         struct bpf_object *obj;
      > > >         char tmp_name[64];
      > > > @@ -7126,6 +7141,14 @@ __bpf_object__open(const char *path, const void *obj_buf, size_t obj_buf_sz,
      > > >                         return ERR_PTR(-ENOMEM);
      > > >         }
      > > >
      > > > +       core_btf_path = OPTS_GET(opts, core_btf_path, NULL);
      > > > +       if (core_btf_path) {
      > > > +               pr_debug("parse btf '%s' for CO-RE relocations\n", core_btf_path);
      > >
      > > Move this right after btf__parse(), so you can report success or
      > > failure in one log statement; see how libbpf_find_kernel_btf() does
      > > it. Please use similar wording (just "target BTF" instead of "kernel
      > > BTF" to distinguish them).
      > >
      > > > +               obj->btf_vmlinux_override = btf__parse(core_btf_path, NULL);
      > >
      > > This BTF is not needed until the load phase, so let's not attempt to
      > > load it on open() unnecessarily. Just remember the path and defer till
      > > bpf_object__load() time.
      > >
      > > > +               if (IS_ERR_OR_NULL(obj->btf_vmlinux_override))
      > > > +                       pr_warn("can't parse btf at '%s'\n", core_btf_path);
      > >
      > > if BTF can't be loaded, load should fail
      > >
      > > > +       }
      > > > +
      > > >         err = bpf_object__elf_init(obj);
      > > >         err = err ? : bpf_object__check_endianness(obj);
      > > >         err = err ? : bpf_object__elf_collect(obj);
      > > > @@ -7481,13 +7504,14 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
      > > >         }
      > > >
      > > >         err = bpf_object__probe_loading(obj);
      > > > +       err = err ? : bpf_object__load_override_btf(obj, attr->target_btf_path);
      > > >         err = err ? : bpf_object__load_vmlinux_btf(obj, false);
      > > >         err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
      > > >         err = err ? : bpf_object__sanitize_and_load_btf(obj);
      > > >         err = err ? : bpf_object__sanitize_maps(obj);
      > > >         err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
      > > >         err = err ? : bpf_object__create_maps(obj);
      > > > -       err = err ? : bpf_object__relocate(obj, attr->target_btf_path);
      > >
      > > For backwards compatibility, we need to still respect
      > > attr->target_btf_path. I'd say let attr->target_btf_path override
      > > opts->core_btf_path, if specified. Later on we'll probably just
      > > deprecate bpf_object__load_xattr() and target_btf_path will go away.
      > >
      > > > +       err = err ? : bpf_object__relocate(obj);
      > > >         err = err ? : bpf_object__load_progs(obj, attr->log_level);
      > > >
      > > >         /* clean up module BTFs */
      > > > diff --git a/src/libbpf.h b/src/libbpf.h
      > > > index 3c35eb4..40c4ee9 100644
      > > > --- a/src/libbpf.h
      > > > +++ b/src/libbpf.h
      > > > @@ -93,8 +93,10 @@ struct bpf_object_open_opts {
      > > >          * system Kconfig for CONFIG_xxx externs.
      > > >          */
      > > >         const char *kconfig;
      > > > +       /* Path to ELF file with BTF section to be used for relocations. */
      > >
      > > Given you use btf__parse() when opening this BTF, it handles both ELF
      > > and raw BTF data. So let's reword this comment to mention BTF in
      > > general.
      > >
      > > > +       const char *core_btf_path;
      > > >  };
      > > > -#define bpf_object_open_opts__last_field kconfig
      > > > +#define bpf_object_open_opts__last_field core_btf_path
      > > >
      > > >  LIBBPF_API struct bpf_object *bpf_object__open(const char *path);
      > > >  LIBBPF_API struct bpf_object *
      > > > --
      > > > 2.23.3
      > > >
      
      ^ permalink raw reply	[flat|nested] 9+ messages in thread

    • • * Re: [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
          2021-01-13 22:16     ` Vamsi Kodavanty
        @ 2021-01-14  3:21       ` Andrii Nakryiko
          2021-01-14  3:52         ` Vamsi Kodavanty
          0 siblings, 1 reply; 9+ messages in thread
        From: Andrii Nakryiko @ 2021-01-14  3:21 UTC (permalink / raw)
          To: Vamsi Kodavanty; +Cc: bpf
        
        On Wed, Jan 13, 2021 at 2:17 PM Vamsi Kodavanty
        <vamsi@araalinetworks.com> wrote:
        >
        > On Mon, Jan 11, 2021 at 7:33 PM Vamsi Kodavanty
        > <vamsi@araalinetworks.com> wrote:
        > >
        > > Andrii,
        > >    Thank you for the detailed review. I will address them as well as
        > > the self tests. And will send out a new patch addressing them and
        > > conforming to style/expectations.
        > >
        > > Cheers
        > > Vamsi.
        > >
        > Andrii,
        >       I understand the `bpf` repository being a mirror of the
        > `bpf-next` tools/lib/bpf. Do the patches
        > to `bpf` go back into `bpf-next`. I see there is a script for
        > `bpf-next` to `bpf`syncs.
        >       I ask because the `btf_vmlinux_override` changes only exist in
        > the `bpf` repo. So, I make my
        > changes in `bpf`?. In that case what happens to the `selftests` which
        > are in `bpf-next`. And they
        > won't have any idea of the new open option 'core_btf_path` that is
        > being introduced.
        >
        
        There are two Linux upstream repositories to which BPF and libbpf
        patches are applied: bpf ([0]) and bpf-next ([1]). Fixes usually go
        into bpf, while all the new features go into bpf-next. They are
        periodically merged and thus converge.
        
        Then, specifically for libbpf, there is a Github mirror ([2]), which
        is synced by me periodically from bpf-next and bpf trees. This Github
        repo is what is considered to be "canonical" libbpf repo for the
        purposes of building libbpf packages and consuming libbpf in user
        applications. You shouldn't concern yourself with this one when
        submitting patches, because it's a derivative of upstream
        repositories.
        
        What is confusing to me, though, is that all three of them have code
        with btf_vmlinux_override, so I'm curious which "bpf" repository did
        you find that doesn't yet have btf_vmlinux_override?
        
          [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
          [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
          [2] https://github.com/libbpf/libbpf
        
        > Thanks again. Hopefully this is my last question before I come back to
        > you with a proper patch.
        >
        > Cheers
        > Vamsi.
        >
        > > On Mon, Jan 11, 2021 at 2:02 PM Andrii Nakryiko
        > > <andrii.nakryiko@gmail.com> wrote:
        > > >
        > > > On Fri, Jan 8, 2021 at 6:36 PM Vamsi Kodavanty <vamsi@araalinetworks.com> wrote:
        > > > >
        > > > > Andrii,
        > > > >      I have made the following changes as discussed to add an option to the `open_opts`
        > > > > to take in the BTF.
        > > > >      Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews.
        > > > > If anyone has any pointers to a webpage where this is described I can go through it. But, below are
        > > > > the proposed changes.
        > > > >
        > > >
        > > > Daniel already gave you pointers. Also make sure you add [PATCH
        > > > bpf-next] prefix to email subject to identify the patch is for
        > > > bpf-next kernel tree.
        > > > And with all changes like this we should also add selftests,
        > > > exercising new features. Please take a look at
        > > > tools/testing/selftests/bpf. I think updating
        > > > test_progs/test_core_reloc.c in there to use this instead of
        > > > bpf_object__load_xattr() might be enough of the testing.
        > > >
        > > > > Best Regards,
        > > > > Vamsi.
        > > > >
        > > > > ---
        > > > >  src/libbpf.c | 56 +++++++++++++++++++++++++++++++++++++---------------
        > > > >  src/libbpf.h |  4 +++-
        > > > >  2 files changed, 43 insertions(+), 17 deletions(-)
        > > > >
        
        [...]
        
        ^ permalink raw reply	[flat|nested] 9+ messages in thread

      • • * Re: [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
            2021-01-14  3:21       ` Andrii Nakryiko
          @ 2021-01-14  3:52         ` Vamsi Kodavanty
            2021-03-03  4:11           ` Andrii Nakryiko
            0 siblings, 1 reply; 9+ messages in thread
          From: Vamsi Kodavanty @ 2021-01-14  3:52 UTC (permalink / raw)
            To: Andrii Nakryiko; +Cc: bpf
          
          On Wed, Jan 13, 2021 at 7:21 PM Andrii Nakryiko
          <andrii.nakryiko@gmail.com> wrote:
          >
          > On Wed, Jan 13, 2021 at 2:17 PM Vamsi Kodavanty
          > <vamsi@araalinetworks.com> wrote:
          > >
          > > On Mon, Jan 11, 2021 at 7:33 PM Vamsi Kodavanty
          > > <vamsi@araalinetworks.com> wrote:
          > > >
          > > > Andrii,
          > > >    Thank you for the detailed review. I will address them as well as
          > > > the self tests. And will send out a new patch addressing them and
          > > > conforming to style/expectations.
          > > >
          > > > Cheers
          > > > Vamsi.
          > > >
          > > Andrii,
          > >       I understand the `bpf` repository being a mirror of the
          > > `bpf-next` tools/lib/bpf. Do the patches
          > > to `bpf` go back into `bpf-next`. I see there is a script for
          > > `bpf-next` to `bpf`syncs.
          > >       I ask because the `btf_vmlinux_override` changes only exist in
          > > the `bpf` repo. So, I make my
          > > changes in `bpf`?. In that case what happens to the `selftests` which
          > > are in `bpf-next`. And they
          > > won't have any idea of the new open option 'core_btf_path` that is
          > > being introduced.
          > >
          >
          > There are two Linux upstream repositories to which BPF and libbpf
          > patches are applied: bpf ([0]) and bpf-next ([1]). Fixes usually go
          > into bpf, while all the new features go into bpf-next. They are
          > periodically merged and thus converge.
          >
          > Then, specifically for libbpf, there is a Github mirror ([2]), which
          > is synced by me periodically from bpf-next and bpf trees. This Github
          > repo is what is considered to be "canonical" libbpf repo for the
          > purposes of building libbpf packages and consuming libbpf in user
          > applications. You shouldn't concern yourself with this one when
          > submitting patches, because it's a derivative of upstream
          > repositories.
          >
          > What is confusing to me, though, is that all three of them have code
          > with btf_vmlinux_override, so I'm curious which "bpf" repository did
          > you find that doesn't yet have btf_vmlinux_override?
          >
          >   [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
          >   [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
          >   [2] https://github.com/libbpf/libbpf
          >
          
          Thank you again. I was looking at [1]. I cloned the repo today morning and
          noticed the absence. I just did a 'git pull' and it seems to have the
          `btf_vmlinux_override` now. So, I will use `bpf-next`. Thank you for the
          repo links. Also, my earlier diffs were incorrectly using the `libbpf` repo.
          
          Regards
          Vamsi.
          
          > > Thanks again. Hopefully this is my last question before I come back to
          > > you with a proper patch.
          > >
          > > Cheers
          > > Vamsi.
          > >
          > > > On Mon, Jan 11, 2021 at 2:02 PM Andrii Nakryiko
          > > > <andrii.nakryiko@gmail.com> wrote:
          > > > >
          > > > > On Fri, Jan 8, 2021 at 6:36 PM Vamsi Kodavanty <vamsi@araalinetworks.com> wrote:
          > > > > >
          > > > > > Andrii,
          > > > > >      I have made the following changes as discussed to add an option to the `open_opts`
          > > > > > to take in the BTF.
          > > > > >      Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews.
          > > > > > If anyone has any pointers to a webpage where this is described I can go through it. But, below are
          > > > > > the proposed changes.
          > > > > >
          > > > >
          > > > > Daniel already gave you pointers. Also make sure you add [PATCH
          > > > > bpf-next] prefix to email subject to identify the patch is for
          > > > > bpf-next kernel tree.
          > > > > And with all changes like this we should also add selftests,
          > > > > exercising new features. Please take a look at
          > > > > tools/testing/selftests/bpf. I think updating
          > > > > test_progs/test_core_reloc.c in there to use this instead of
          > > > > bpf_object__load_xattr() might be enough of the testing.
          > > > >
          > > > > > Best Regards,
          > > > > > Vamsi.
          > > > > >
          > > > > > ---
          > > > > >  src/libbpf.c | 56 +++++++++++++++++++++++++++++++++++++---------------
          > > > > >  src/libbpf.h |  4 +++-
          > > > > >  2 files changed, 43 insertions(+), 17 deletions(-)
          > > > > >
          >
          > [...]
          
          ^ permalink raw reply	[flat|nested] 9+ messages in thread

        • • * Re: [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program
              2021-01-14  3:52         ` Vamsi Kodavanty
            @ 2021-03-03  4:11           ` Andrii Nakryiko
              0 siblings, 0 replies; 9+ messages in thread
            From: Andrii Nakryiko @ 2021-03-03  4:11 UTC (permalink / raw)
              To: Vamsi Kodavanty; +Cc: bpf
            
            On Wed, Jan 13, 2021 at 7:52 PM Vamsi Kodavanty
            <vamsi@araalinetworks.com> wrote:
            >
            > On Wed, Jan 13, 2021 at 7:21 PM Andrii Nakryiko
            > <andrii.nakryiko@gmail.com> wrote:
            > >
            > > On Wed, Jan 13, 2021 at 2:17 PM Vamsi Kodavanty
            > > <vamsi@araalinetworks.com> wrote:
            > > >
            > > > On Mon, Jan 11, 2021 at 7:33 PM Vamsi Kodavanty
            > > > <vamsi@araalinetworks.com> wrote:
            > > > >
            > > > > Andrii,
            > > > >    Thank you for the detailed review. I will address them as well as
            > > > > the self tests. And will send out a new patch addressing them and
            > > > > conforming to style/expectations.
            > > > >
            > > > > Cheers
            > > > > Vamsi.
            > > > >
            > > > Andrii,
            > > >       I understand the `bpf` repository being a mirror of the
            > > > `bpf-next` tools/lib/bpf. Do the patches
            > > > to `bpf` go back into `bpf-next`. I see there is a script for
            > > > `bpf-next` to `bpf`syncs.
            > > >       I ask because the `btf_vmlinux_override` changes only exist in
            > > > the `bpf` repo. So, I make my
            > > > changes in `bpf`?. In that case what happens to the `selftests` which
            > > > are in `bpf-next`. And they
            > > > won't have any idea of the new open option 'core_btf_path` that is
            > > > being introduced.
            > > >
            > >
            > > There are two Linux upstream repositories to which BPF and libbpf
            > > patches are applied: bpf ([0]) and bpf-next ([1]). Fixes usually go
            > > into bpf, while all the new features go into bpf-next. They are
            > > periodically merged and thus converge.
            > >
            > > Then, specifically for libbpf, there is a Github mirror ([2]), which
            > > is synced by me periodically from bpf-next and bpf trees. This Github
            > > repo is what is considered to be "canonical" libbpf repo for the
            > > purposes of building libbpf packages and consuming libbpf in user
            > > applications. You shouldn't concern yourself with this one when
            > > submitting patches, because it's a derivative of upstream
            > > repositories.
            > >
            > > What is confusing to me, though, is that all three of them have code
            > > with btf_vmlinux_override, so I'm curious which "bpf" repository did
            > > you find that doesn't yet have btf_vmlinux_override?
            > >
            > >   [0] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
            > >   [1] https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
            > >   [2] https://github.com/libbpf/libbpf
            > >
            >
            > Thank you again. I was looking at [1]. I cloned the repo today morning and
            > noticed the absence. I just did a 'git pull' and it seems to have the
            > `btf_vmlinux_override` now. So, I will use `bpf-next`. Thank you for the
            > repo links. Also, my earlier diffs were incorrectly using the `libbpf` repo.
            >
            
            Hi Vamsi,
            
            Did you get a chance to finish this patch by any chance? It seems like
            the request to support the ability to provide a custom vmlinux BTF
            (usually to get a chance to use BPF CO-RE on older kernels) comes up
            quite often recently, so it would be good to have this landed. Please
            let me know if you can finish this up. It's ok if not, but please let
            us know. Thanks!
            
            > Regards
            > Vamsi.
            >
            > > > Thanks again. Hopefully this is my last question before I come back to
            > > > you with a proper patch.
            > > >
            > > > Cheers
            > > > Vamsi.
            > > >
            > > > > On Mon, Jan 11, 2021 at 2:02 PM Andrii Nakryiko
            > > > > <andrii.nakryiko@gmail.com> wrote:
            > > > > >
            > > > > > On Fri, Jan 8, 2021 at 6:36 PM Vamsi Kodavanty <vamsi@araalinetworks.com> wrote:
            > > > > > >
            > > > > > > Andrii,
            > > > > > >      I have made the following changes as discussed to add an option to the `open_opts`
            > > > > > > to take in the BTF.
            > > > > > >      Please do take a look. Also, I am not sure what the procedure is for submitting patches/reviews.
            > > > > > > If anyone has any pointers to a webpage where this is described I can go through it. But, below are
            > > > > > > the proposed changes.
            > > > > > >
            > > > > >
            > > > > > Daniel already gave you pointers. Also make sure you add [PATCH
            > > > > > bpf-next] prefix to email subject to identify the patch is for
            > > > > > bpf-next kernel tree.
            > > > > > And with all changes like this we should also add selftests,
            > > > > > exercising new features. Please take a look at
            > > > > > tools/testing/selftests/bpf. I think updating
            > > > > > test_progs/test_core_reloc.c in there to use this instead of
            > > > > > bpf_object__load_xattr() might be enough of the testing.
            > > > > >
            > > > > > > Best Regards,
            > > > > > > Vamsi.
            > > > > > >
            > > > > > > ---
            > > > > > >  src/libbpf.c | 56 +++++++++++++++++++++++++++++++++++++---------------
            > > > > > >  src/libbpf.h |  4 +++-
            > > > > > >  2 files changed, 43 insertions(+), 17 deletions(-)
            > > > > > >
            > >
            > > [...]
            
            ^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-03 22:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-09  2:36 [PATCH bpf v1] Add `core_btf_path` to `bpf_object_open_opts` to pass BTF path from skeleton program Vamsi Kodavanty
2021-01-11 14:13 ` Daniel Borkmann
2021-01-11 19:07   ` Vamsi Kodavanty
2021-01-11 22:02 ` Andrii Nakryiko
2021-01-12  3:33   ` Vamsi Kodavanty
2021-01-13 22:16     ` Vamsi Kodavanty
2021-01-14  3:21       ` Andrii Nakryiko
2021-01-14  3:52         ` Vamsi Kodavanty
2021-03-03  4:11           ` 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.