diff mbox series

[v2,bpf-next,4/5] bpf: Update iterators.lskel.h.

Message ID 20220208191306.6136-5-alexei.starovoitov@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series bpf: Light skeleton for the kernel. | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR fail PR summary
netdev/tree_selection success Clearly marked for bpf-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 7 maintainers not CCed: kpsingh@kernel.org john.fastabend@gmail.com kafai@fb.com songliubraving@fb.com ast@kernel.org yhs@fb.com netdev@vger.kernel.org
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: Avoid line continuations in quoted strings WARNING: line length of 89 exceeds 80 columns WARNING: line length of 97 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next fail VM_Test

Commit Message

Alexei Starovoitov Feb. 8, 2022, 7:13 p.m. UTC 
From: Alexei Starovoitov <ast@kernel.org>

Light skeleton and skel_internal.h have changed.
Update iterators.lskel.h.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 .../bpf/preload/iterators/iterators.lskel.h   | 28 +++++++------------
 1 file changed, 10 insertions(+), 18 deletions(-)

Comments

Yonghong Song Feb. 9, 2022, 12:27 a.m. UTC | #1 
On 2/8/22 11:13 AM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> Light skeleton and skel_internal.h have changed.
> Update iterators.lskel.h.
> 
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: Yonghong Song <yhs@fb.com>
Andrii Nakryiko Feb. 9, 2022, 4:40 a.m. UTC | #2 
On Tue, Feb 8, 2022 at 11:13 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> Light skeleton and skel_internal.h have changed.
> Update iterators.lskel.h.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  .../bpf/preload/iterators/iterators.lskel.h   | 28 +++++++------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/bpf/preload/iterators/iterators.lskel.h b/kernel/bpf/preload/iterators/iterators.lskel.h
> index d90562d672d2..3e45237f59f4 100644
> --- a/kernel/bpf/preload/iterators/iterators.lskel.h
> +++ b/kernel/bpf/preload/iterators/iterators.lskel.h
> @@ -3,8 +3,6 @@
>  #ifndef __ITERATORS_BPF_SKEL_H__
>  #define __ITERATORS_BPF_SKEL_H__
>
> -#include <stdlib.h>
> -#include <bpf/bpf.h>
>  #include <bpf/skel_internal.h>
>
>  struct iterators_bpf {
> @@ -70,31 +68,25 @@ iterators_bpf__destroy(struct iterators_bpf *skel)
>         iterators_bpf__detach(skel);
>         skel_closenz(skel->progs.dump_bpf_map.prog_fd);
>         skel_closenz(skel->progs.dump_bpf_prog.prog_fd);
> -       munmap(skel->rodata, 4096);
> +       skel_free_map_data(skel->rodata, skel->maps.rodata.initial_value, 4096);
>         skel_closenz(skel->maps.rodata.map_fd);
> -       free(skel);
> +       skel_free(skel);
>  }
>  static inline struct iterators_bpf *
>  iterators_bpf__open(void)
>  {
>         struct iterators_bpf *skel;
>
> -       skel = calloc(sizeof(*skel), 1);
> +       skel = skel_alloc(sizeof(*skel));
>         if (!skel)
>                 goto cleanup;
>         skel->ctx.sz = (void *)&skel->links - (void *)skel;
> -       skel->rodata =
> -               mmap(NULL, 4096, PROT_READ | PROT_WRITE,
> -                    MAP_SHARED | MAP_ANONYMOUS, -1, 0);
> -       if (skel->rodata == (void *) -1)
> -               goto cleanup;

previously if mmap() failed you'd go to cleanup, but now skel->rodata
will remain NULL. Are you concerned about this?


> -       memcpy(skel->rodata, (void *)"\
> +       skel->rodata = skel_prep_map_data((void *)"\
>  \x20\x20\x69\x64\x20\x6e\x61\x6d\x65\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\
>  \x20\x20\x20\x6d\x61\x78\x5f\x65\x6e\x74\x72\x69\x65\x73\x0a\0\x25\x34\x75\x20\
>  \x25\x2d\x31\x36\x73\x25\x36\x64\x0a\0\x20\x20\x69\x64\x20\x6e\x61\x6d\x65\x20\
>  \x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x61\x74\x74\x61\x63\x68\x65\
> -\x64\x0a\0\x25\x34\x75\x20\x25\x2d\x31\x36\x73\x20\x25\x73\x20\x25\x73\x0a\0", 98);
> -       skel->maps.rodata.initial_value = (__u64)(long)skel->rodata;
> +\x64\x0a\0\x25\x34\x75\x20\x25\x2d\x31\x36\x73\x20\x25\x73\x20\x25\x73\x0a\0", 4096, 98);
>         return skel;
>  cleanup:
>         iterators_bpf__destroy(skel);
> @@ -343,11 +335,11 @@ iterators_bpf__load(struct iterators_bpf *skel)
>  \0\0\x18\x62\0\0\0\0\0\0\0\0\0\0\x30\x0e\0\0\xb7\x03\0\0\x1c\0\0\0\x85\0\0\0\
>  \xa6\0\0\0\xbf\x07\0\0\0\0\0\0\xc5\x07\xd4\xff\0\0\0\0\x63\x7a\x78\xff\0\0\0\0\
>  \x61\xa0\x78\xff\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\x80\x0e\0\0\x63\x01\0\0\0\
> -\0\0\0\x61\x60\x20\0\0\0\0\0\x15\0\x03\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\
> +\0\0\0\x61\x60\x1c\0\0\0\0\0\x15\0\x03\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\
>  \x5c\x0e\0\0\x63\x01\0\0\0\0\0\0\xb7\x01\0\0\0\0\0\0\x18\x62\0\0\0\0\0\0\0\0\0\
>  \0\x50\x0e\0\0\xb7\x03\0\0\x48\0\0\0\x85\0\0\0\xa6\0\0\0\xbf\x07\0\0\0\0\0\0\
>  \xc5\x07\xc3\xff\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x63\x71\0\0\0\0\0\
> -\0\x79\x63\x18\0\0\0\0\0\x15\x03\x04\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\x98\
> +\0\x79\x63\x20\0\0\0\0\0\x15\x03\x04\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\x98\
>  \x0e\0\0\xb7\x02\0\0\x62\0\0\0\x85\0\0\0\x94\0\0\0\x18\x62\0\0\0\0\0\0\0\0\0\0\
>  \0\0\0\0\x61\x20\0\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\x08\x0f\0\0\x63\x01\0\
>  \0\0\0\0\0\x18\x60\0\0\0\0\0\0\0\0\0\0\0\x0f\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\
> @@ -401,12 +393,12 @@ iterators_bpf__load(struct iterators_bpf *skel)
>  \x28\0\0\0\0\0\x61\xa0\x84\xff\0\0\0\0\x63\x06\x2c\0\0\0\0\0\x18\x61\0\0\0\0\0\
>  \0\0\0\0\0\0\0\0\0\x61\x10\0\0\0\0\0\0\x63\x06\x18\0\0\0\0\0\xb7\0\0\0\0\0\0\0\
>  \x95\0\0\0\0\0\0\0";
> +       skel->maps.rodata.initial_value = skel_prep_init_value((void **)&skel->rodata, 4096, 98);
>         err = bpf_load_and_run(&opts);
>         if (err < 0)
>                 return err;
> -       skel->rodata =
> -               mmap(skel->rodata, 4096, PROT_READ, MAP_SHARED | MAP_FIXED,
> -                       skel->maps.rodata.map_fd, 0);
> +       skel->rodata = skel_finalize_map_data(&skel->maps.rodata.initial_value,
> +                       4096, PROT_READ, skel->maps.rodata.map_fd);

here seems like both before and now, on error, nothing happens. For
kernel mode it matches skeleton behavior (rodata will be NULL), but
for user-space code you'll have (void *)-1, which is probably not
great.

>         return 0;
>  }


>
> --
> 2.30.2
>
Alexei Starovoitov Feb. 9, 2022, 5:05 a.m. UTC | #3 
On Tue, Feb 8, 2022 at 8:40 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
> > -       skel->rodata =
> > -               mmap(skel->rodata, 4096, PROT_READ, MAP_SHARED | MAP_FIXED,
> > -                       skel->maps.rodata.map_fd, 0);
> > +       skel->rodata = skel_finalize_map_data(&skel->maps.rodata.initial_value,
> > +                       4096, PROT_READ, skel->maps.rodata.map_fd);
>
> here seems like both before and now, on error, nothing happens. For
> kernel mode it matches skeleton behavior (rodata will be NULL), but
> for user-space code you'll have (void *)-1, which is probably not
> great.

Yeah, not a regression, but let's add the checks while at it.
diff mbox series

Patch

diff --git a/kernel/bpf/preload/iterators/iterators.lskel.h b/kernel/bpf/preload/iterators/iterators.lskel.h
index d90562d672d2..3e45237f59f4 100644
--- a/kernel/bpf/preload/iterators/iterators.lskel.h
+++ b/kernel/bpf/preload/iterators/iterators.lskel.h
@@ -3,8 +3,6 @@ 
 #ifndef __ITERATORS_BPF_SKEL_H__
 #define __ITERATORS_BPF_SKEL_H__
 
-#include <stdlib.h>
-#include <bpf/bpf.h>
 #include <bpf/skel_internal.h>
 
 struct iterators_bpf {
@@ -70,31 +68,25 @@  iterators_bpf__destroy(struct iterators_bpf *skel)
 	iterators_bpf__detach(skel);
 	skel_closenz(skel->progs.dump_bpf_map.prog_fd);
 	skel_closenz(skel->progs.dump_bpf_prog.prog_fd);
-	munmap(skel->rodata, 4096);
+	skel_free_map_data(skel->rodata, skel->maps.rodata.initial_value, 4096);
 	skel_closenz(skel->maps.rodata.map_fd);
-	free(skel);
+	skel_free(skel);
 }
 static inline struct iterators_bpf *
 iterators_bpf__open(void)
 {
 	struct iterators_bpf *skel;
 
-	skel = calloc(sizeof(*skel), 1);
+	skel = skel_alloc(sizeof(*skel));
 	if (!skel)
 		goto cleanup;
 	skel->ctx.sz = (void *)&skel->links - (void *)skel;
-	skel->rodata =
-		mmap(NULL, 4096, PROT_READ | PROT_WRITE,
-		     MAP_SHARED | MAP_ANONYMOUS, -1, 0);
-	if (skel->rodata == (void *) -1)
-		goto cleanup;
-	memcpy(skel->rodata, (void *)"\
+	skel->rodata = skel_prep_map_data((void *)"\
 \x20\x20\x69\x64\x20\x6e\x61\x6d\x65\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\
 \x20\x20\x20\x6d\x61\x78\x5f\x65\x6e\x74\x72\x69\x65\x73\x0a\0\x25\x34\x75\x20\
 \x25\x2d\x31\x36\x73\x25\x36\x64\x0a\0\x20\x20\x69\x64\x20\x6e\x61\x6d\x65\x20\
 \x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x20\x61\x74\x74\x61\x63\x68\x65\
-\x64\x0a\0\x25\x34\x75\x20\x25\x2d\x31\x36\x73\x20\x25\x73\x20\x25\x73\x0a\0", 98);
-	skel->maps.rodata.initial_value = (__u64)(long)skel->rodata;
+\x64\x0a\0\x25\x34\x75\x20\x25\x2d\x31\x36\x73\x20\x25\x73\x20\x25\x73\x0a\0", 4096, 98);
 	return skel;
 cleanup:
 	iterators_bpf__destroy(skel);
@@ -343,11 +335,11 @@  iterators_bpf__load(struct iterators_bpf *skel)
 \0\0\x18\x62\0\0\0\0\0\0\0\0\0\0\x30\x0e\0\0\xb7\x03\0\0\x1c\0\0\0\x85\0\0\0\
 \xa6\0\0\0\xbf\x07\0\0\0\0\0\0\xc5\x07\xd4\xff\0\0\0\0\x63\x7a\x78\xff\0\0\0\0\
 \x61\xa0\x78\xff\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\x80\x0e\0\0\x63\x01\0\0\0\
-\0\0\0\x61\x60\x20\0\0\0\0\0\x15\0\x03\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\
+\0\0\0\x61\x60\x1c\0\0\0\0\0\x15\0\x03\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\
 \x5c\x0e\0\0\x63\x01\0\0\0\0\0\0\xb7\x01\0\0\0\0\0\0\x18\x62\0\0\0\0\0\0\0\0\0\
 \0\x50\x0e\0\0\xb7\x03\0\0\x48\0\0\0\x85\0\0\0\xa6\0\0\0\xbf\x07\0\0\0\0\0\0\
 \xc5\x07\xc3\xff\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\0\0\0\0\x63\x71\0\0\0\0\0\
-\0\x79\x63\x18\0\0\0\0\0\x15\x03\x04\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\x98\
+\0\x79\x63\x20\0\0\0\0\0\x15\x03\x04\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\x98\
 \x0e\0\0\xb7\x02\0\0\x62\0\0\0\x85\0\0\0\x94\0\0\0\x18\x62\0\0\0\0\0\0\0\0\0\0\
 \0\0\0\0\x61\x20\0\0\0\0\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\x08\x0f\0\0\x63\x01\0\
 \0\0\0\0\0\x18\x60\0\0\0\0\0\0\0\0\0\0\0\x0f\0\0\x18\x61\0\0\0\0\0\0\0\0\0\0\
@@ -401,12 +393,12 @@  iterators_bpf__load(struct iterators_bpf *skel)
 \x28\0\0\0\0\0\x61\xa0\x84\xff\0\0\0\0\x63\x06\x2c\0\0\0\0\0\x18\x61\0\0\0\0\0\
 \0\0\0\0\0\0\0\0\0\x61\x10\0\0\0\0\0\0\x63\x06\x18\0\0\0\0\0\xb7\0\0\0\0\0\0\0\
 \x95\0\0\0\0\0\0\0";
+	skel->maps.rodata.initial_value = skel_prep_init_value((void **)&skel->rodata, 4096, 98);
 	err = bpf_load_and_run(&opts);
 	if (err < 0)
 		return err;
-	skel->rodata =
-		mmap(skel->rodata, 4096, PROT_READ, MAP_SHARED | MAP_FIXED,
-			skel->maps.rodata.map_fd, 0);
+	skel->rodata = skel_finalize_map_data(&skel->maps.rodata.initial_value,
+			4096, PROT_READ, skel->maps.rodata.map_fd);
 	return 0;
 }