| Message ID | 20191212164129.494329-16-andriin@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Add code-generated BPF object skeleton support | expand |
2019-12-12 08:41 UTC-0800 ~ Andrii Nakryiko <andriin@fb.com> > Add BASH completions for gen sub-command. > > Cc: Quentin Monnet <quentin.monnet@netronome.com> > Acked-by: Martin KaFai Lau <kafai@fb.com> > Signed-off-by: Andrii Nakryiko <andriin@fb.com> > --- > tools/bpf/bpftool/bash-completion/bpftool | 11 +++++++++++ > tools/bpf/bpftool/main.c | 2 +- > tools/testing/selftests/bpf/prog_tests/skeleton.c | 6 ++++-- > tools/testing/selftests/bpf/progs/test_skeleton.c | 3 ++- > 4 files changed, 18 insertions(+), 4 deletions(-) > > diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool > index 70493a6da206..986519cc58d1 100644 > --- a/tools/bpf/bpftool/bash-completion/bpftool > +++ b/tools/bpf/bpftool/bash-completion/bpftool > @@ -716,6 +716,17 @@ _bpftool() > ;; > esac > ;; > + gen) > + case $command in > + skeleton) > + _filedir > + ;; > + *) > + [[ $prev == $object ]] && \ > + COMPREPLY=( $( compgen -W 'skeleton help' -- "$cur" ) ) > + ;; > + esac > + ;; Hi Andrii, Bpftool completion looks OK to me... > cgroup) > case $command in > show|list|tree) > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c > index 758b294e8a7d..1fe91c558508 100644 > --- a/tools/bpf/bpftool/main.c > +++ b/tools/bpf/bpftool/main.c > @@ -58,7 +58,7 @@ static int do_help(int argc, char **argv) > " %s batch file FILE\n" > " %s version\n" > "\n" > - " OBJECT := { prog | map | cgroup | perf | net | feature | btf }\n" > + " OBJECT := { prog | map | cgroup | perf | net | feature | btf | gen }\n" ... but this is part of the usage message, and ideally should be added when you add the new feature to bpftool in patch 11. Not a major issue, but ... > " " HELP_SPEC_OPTIONS "\n" > "", > bin_name, bin_name, bin_name); > diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c > index d65a0203e1df..94e0300f437a 100644 > --- a/tools/testing/selftests/bpf/prog_tests/skeleton.c > +++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c > @@ -39,8 +39,10 @@ void test_skeleton(void) > CHECK(bss->out2 != 2, "res2", "got %lld != exp %d\n", bss->out2, 2); > CHECK(bss->out3 != 3, "res3", "got %d != exp %d\n", (int)bss->out3, 3); > CHECK(bss->out4 != 4, "res4", "got %lld != exp %d\n", bss->out4, 4); > - CHECK(bss->out5.a != 5, "res5", "got %d != exp %d\n", bss->out5.a, 5); > - CHECK(bss->out5.b != 6, "res6", "got %lld != exp %d\n", bss->out5.b, 6); > + CHECK(bss->handler_out5.a != 5, "res5", "got %d != exp %d\n", > + bss->handler_out5.a, 5); > + CHECK(bss->handler_out5.b != 6, "res6", "got %lld != exp %d\n", > + bss->handler_out5.b, 6); ... This and the code below does not seem to relate to bpftool completion at all. And it was not present in v1. I suspect this code was not intended to end up in this patch? > > cleanup: > test_skeleton__destroy(skel); > diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c b/tools/testing/selftests/bpf/progs/test_skeleton.c > index 303a841c4d1c..db4fd88f3ecb 100644 > --- a/tools/testing/selftests/bpf/progs/test_skeleton.c > +++ b/tools/testing/selftests/bpf/progs/test_skeleton.c > @@ -16,7 +16,6 @@ long long in4 __attribute__((aligned(64))) = 0; > struct s in5 = {}; > > long long out2 = 0; > -struct s out5 = {}; > char out3 = 0; > long long out4 = 0; > int out1 = 0; > @@ -25,6 +24,8 @@ int out1 = 0; > SEC("raw_tp/sys_enter") > int handler(const void *ctx) > { > + static volatile struct s out5; > + > out1 = in1; > out2 = in2; > out3 = in3; >
On Thu, Dec 12, 2019 at 1:12 PM Quentin Monnet <quentin.monnet@netronome.com> wrote: > > 2019-12-12 08:41 UTC-0800 ~ Andrii Nakryiko <andriin@fb.com> > > Add BASH completions for gen sub-command. > > > > Cc: Quentin Monnet <quentin.monnet@netronome.com> > > Acked-by: Martin KaFai Lau <kafai@fb.com> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com> > > --- > > tools/bpf/bpftool/bash-completion/bpftool | 11 +++++++++++ > > tools/bpf/bpftool/main.c | 2 +- > > tools/testing/selftests/bpf/prog_tests/skeleton.c | 6 ++++-- > > tools/testing/selftests/bpf/progs/test_skeleton.c | 3 ++- > > 4 files changed, 18 insertions(+), 4 deletions(-) > > > > diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool > > index 70493a6da206..986519cc58d1 100644 > > --- a/tools/bpf/bpftool/bash-completion/bpftool > > +++ b/tools/bpf/bpftool/bash-completion/bpftool > > @@ -716,6 +716,17 @@ _bpftool() > > ;; > > esac > > ;; > > + gen) > > + case $command in > > + skeleton) > > + _filedir > > + ;; > > + *) > > + [[ $prev == $object ]] && \ > > + COMPREPLY=( $( compgen -W 'skeleton help' -- "$cur" ) ) > > + ;; > > + esac > > + ;; > > Hi Andrii, > > Bpftool completion looks OK to me... > > > cgroup) > > case $command in > > show|list|tree) > > diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c > > index 758b294e8a7d..1fe91c558508 100644 > > --- a/tools/bpf/bpftool/main.c > > +++ b/tools/bpf/bpftool/main.c > > @@ -58,7 +58,7 @@ static int do_help(int argc, char **argv) > > " %s batch file FILE\n" > > " %s version\n" > > "\n" > > - " OBJECT := { prog | map | cgroup | perf | net | feature | btf }\n" > > + " OBJECT := { prog | map | cgroup | perf | net | feature | btf | gen }\n" > > ... but this is part of the usage message, and ideally should be added > when you add the new feature to bpftool in patch 11. Not a major issue, > but ... > > > " " HELP_SPEC_OPTIONS "\n" > > "", > > bin_name, bin_name, bin_name); > > diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c > > index d65a0203e1df..94e0300f437a 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/skeleton.c > > +++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c > > @@ -39,8 +39,10 @@ void test_skeleton(void) > > CHECK(bss->out2 != 2, "res2", "got %lld != exp %d\n", bss->out2, 2); > > CHECK(bss->out3 != 3, "res3", "got %d != exp %d\n", (int)bss->out3, 3); > > CHECK(bss->out4 != 4, "res4", "got %lld != exp %d\n", bss->out4, 4); > > - CHECK(bss->out5.a != 5, "res5", "got %d != exp %d\n", bss->out5.a, 5); > > - CHECK(bss->out5.b != 6, "res6", "got %lld != exp %d\n", bss->out5.b, 6); > > + CHECK(bss->handler_out5.a != 5, "res5", "got %d != exp %d\n", > > + bss->handler_out5.a, 5); > > + CHECK(bss->handler_out5.b != 6, "res6", "got %lld != exp %d\n", > > + bss->handler_out5.b, 6); > > ... This and the code below does not seem to relate to bpftool > completion at all. And it was not present in v1. I suspect this code was > not intended to end up in this patch? > Oh yeah, I screwed up amending, sorry about that. I will fix both issues, thanks. > > > > cleanup: > > test_skeleton__destroy(skel); > > diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c b/tools/testing/selftests/bpf/progs/test_skeleton.c > > index 303a841c4d1c..db4fd88f3ecb 100644 > > --- a/tools/testing/selftests/bpf/progs/test_skeleton.c > > +++ b/tools/testing/selftests/bpf/progs/test_skeleton.c > > @@ -16,7 +16,6 @@ long long in4 __attribute__((aligned(64))) = 0; > > struct s in5 = {}; > > > > long long out2 = 0; > > -struct s out5 = {}; > > char out3 = 0; > > long long out4 = 0; > > int out1 = 0; > > @@ -25,6 +24,8 @@ int out1 = 0; > > SEC("raw_tp/sys_enter") > > int handler(const void *ctx) > > { > > + static volatile struct s out5; > > + > > out1 = in1; > > out2 = in2; > > out3 = in3; > > >
diff --git a/tools/bpf/bpftool/bash-completion/bpftool b/tools/bpf/bpftool/bash-completion/bpftool index 70493a6da206..986519cc58d1 100644 --- a/tools/bpf/bpftool/bash-completion/bpftool +++ b/tools/bpf/bpftool/bash-completion/bpftool @@ -716,6 +716,17 @@ _bpftool() ;; esac ;; + gen) + case $command in + skeleton) + _filedir + ;; + *) + [[ $prev == $object ]] && \ + COMPREPLY=( $( compgen -W 'skeleton help' -- "$cur" ) ) + ;; + esac + ;; cgroup) case $command in show|list|tree) diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c index 758b294e8a7d..1fe91c558508 100644 --- a/tools/bpf/bpftool/main.c +++ b/tools/bpf/bpftool/main.c @@ -58,7 +58,7 @@ static int do_help(int argc, char **argv) " %s batch file FILE\n" " %s version\n" "\n" - " OBJECT := { prog | map | cgroup | perf | net | feature | btf }\n" + " OBJECT := { prog | map | cgroup | perf | net | feature | btf | gen }\n" " " HELP_SPEC_OPTIONS "\n" "", bin_name, bin_name, bin_name); diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c b/tools/testing/selftests/bpf/prog_tests/skeleton.c index d65a0203e1df..94e0300f437a 100644 --- a/tools/testing/selftests/bpf/prog_tests/skeleton.c +++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c @@ -39,8 +39,10 @@ void test_skeleton(void) CHECK(bss->out2 != 2, "res2", "got %lld != exp %d\n", bss->out2, 2); CHECK(bss->out3 != 3, "res3", "got %d != exp %d\n", (int)bss->out3, 3); CHECK(bss->out4 != 4, "res4", "got %lld != exp %d\n", bss->out4, 4); - CHECK(bss->out5.a != 5, "res5", "got %d != exp %d\n", bss->out5.a, 5); - CHECK(bss->out5.b != 6, "res6", "got %lld != exp %d\n", bss->out5.b, 6); + CHECK(bss->handler_out5.a != 5, "res5", "got %d != exp %d\n", + bss->handler_out5.a, 5); + CHECK(bss->handler_out5.b != 6, "res6", "got %lld != exp %d\n", + bss->handler_out5.b, 6); cleanup: test_skeleton__destroy(skel); diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c b/tools/testing/selftests/bpf/progs/test_skeleton.c index 303a841c4d1c..db4fd88f3ecb 100644 --- a/tools/testing/selftests/bpf/progs/test_skeleton.c +++ b/tools/testing/selftests/bpf/progs/test_skeleton.c @@ -16,7 +16,6 @@ long long in4 __attribute__((aligned(64))) = 0; struct s in5 = {}; long long out2 = 0; -struct s out5 = {}; char out3 = 0; long long out4 = 0; int out1 = 0; @@ -25,6 +24,8 @@ int out1 = 0; SEC("raw_tp/sys_enter") int handler(const void *ctx) { + static volatile struct s out5; + out1 = in1; out2 = in2; out3 = in3;