| Message ID | 20210905100914.33007-2-hengqi.chen@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,1/2] libbpf: Support uniform BTF-defined key/value specification across all BPF maps | expand |
On Sun, Sep 5, 2021 at 3:09 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: > > Test BPF map creation using BTF-defined key/value. The test defines > some specialized maps by specifying BTF types for key/value and > checks those maps are correctly initialized and loaded. > > Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com> > --- > .../selftests/bpf/prog_tests/map_create.c | 87 ++++++++++++++ > .../selftests/bpf/progs/test_map_create.c | 110 ++++++++++++++++++ > 2 files changed, 197 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/map_create.c > create mode 100644 tools/testing/selftests/bpf/progs/test_map_create.c > > diff --git a/tools/testing/selftests/bpf/prog_tests/map_create.c b/tools/testing/selftests/bpf/prog_tests/map_create.c > new file mode 100644 > index 000000000000..6ca32d0dffd2 > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/map_create.c > @@ -0,0 +1,87 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* Copyright (c) 2021 Hengqi Chen */ > + > +#include <test_progs.h> > +#include "test_map_create.skel.h" > + > +void test_map_create(void) > +{ > + struct test_map_create *skel; > + int err, fd; > + > + skel = test_map_create__open(); > + if (!ASSERT_OK_PTR(skel, "test_map_create__open failed")) > + return; > + > + err = test_map_create__load(skel); If load() succeeds, all the maps will definitely be created, so all the below tests are meaningless. I think it's better to just change all the existing map definitions used throughout selftests to use key/value types, instead of key_size/value_size. That will automatically test this feature without adding an extra test. Unfortunately to really test that the logic is working, we'd need to check that libbpf doesn't emit the warning about retrying map creation w/o BTF, but I think one-time manual check (please use ./test_progs -v to see libbpf warnings during tests) should be sufficient for this. > + if (!ASSERT_OK(err, "test_map_create__load failed")) > + goto cleanup; > + > + fd = bpf_map__fd(skel->maps.map1); > + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) > + goto cleanup; > + close(fd); > + > + fd = bpf_map__fd(skel->maps.map2); > + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) > + goto cleanup; > + close(fd); [...]
On 9/9/21 12:29 PM, Andrii Nakryiko wrote: > On Sun, Sep 5, 2021 at 3:09 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: >> >> Test BPF map creation using BTF-defined key/value. The test defines >> some specialized maps by specifying BTF types for key/value and >> checks those maps are correctly initialized and loaded. >> >> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com> >> --- >> .../selftests/bpf/prog_tests/map_create.c | 87 ++++++++++++++ >> .../selftests/bpf/progs/test_map_create.c | 110 ++++++++++++++++++ >> 2 files changed, 197 insertions(+) >> create mode 100644 tools/testing/selftests/bpf/prog_tests/map_create.c >> create mode 100644 tools/testing/selftests/bpf/progs/test_map_create.c >> >> diff --git a/tools/testing/selftests/bpf/prog_tests/map_create.c b/tools/testing/selftests/bpf/prog_tests/map_create.c >> new file mode 100644 >> index 000000000000..6ca32d0dffd2 >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/prog_tests/map_create.c >> @@ -0,0 +1,87 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* Copyright (c) 2021 Hengqi Chen */ >> + >> +#include <test_progs.h> >> +#include "test_map_create.skel.h" >> + >> +void test_map_create(void) >> +{ >> + struct test_map_create *skel; >> + int err, fd; >> + >> + skel = test_map_create__open(); >> + if (!ASSERT_OK_PTR(skel, "test_map_create__open failed")) >> + return; >> + >> + err = test_map_create__load(skel); > > If load() succeeds, all the maps will definitely be created, so all > the below tests are meaningless. > > I think it's better to just change all the existing map definitions > used throughout selftests to use key/value types, instead of > key_size/value_size. That will automatically test this feature without > adding an extra test. Unfortunately to really test that the logic is > working, we'd need to check that libbpf doesn't emit the warning about > retrying map creation w/o BTF, but I think one-time manual check > (please use ./test_progs -v to see libbpf warnings during tests) > should be sufficient for this. > Hello, Andrii I updated these existing tests as you suggested, but I was unable to run the whole bpf selftests locally. Running ./test_progs -v made my system hang up, didn't find the root cause yet. Instead I just run the modified test with the following commands: sudo ./test_progs -v --name=kfree_skb,perf_event_stackmap,btf_map_in_map,pe_preserve_elems,stacktrace_map,stacktrace_build_id,xdp_bpf2bpf,select_reuseport,tcpbpf_user >> + if (!ASSERT_OK(err, "test_map_create__load failed")) >> + goto cleanup; >> + >> + fd = bpf_map__fd(skel->maps.map1); >> + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) >> + goto cleanup; >> + close(fd); >> + >> + fd = bpf_map__fd(skel->maps.map2); >> + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) >> + goto cleanup; >> + close(fd); > > [...] >
On Thu, Sep 30, 2021 at 9:05 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: > > > > On 9/9/21 12:29 PM, Andrii Nakryiko wrote: > > On Sun, Sep 5, 2021 at 3:09 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: > >> > >> Test BPF map creation using BTF-defined key/value. The test defines > >> some specialized maps by specifying BTF types for key/value and > >> checks those maps are correctly initialized and loaded. > >> > >> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com> > >> --- > >> .../selftests/bpf/prog_tests/map_create.c | 87 ++++++++++++++ > >> .../selftests/bpf/progs/test_map_create.c | 110 ++++++++++++++++++ > >> 2 files changed, 197 insertions(+) > >> create mode 100644 tools/testing/selftests/bpf/prog_tests/map_create.c > >> create mode 100644 tools/testing/selftests/bpf/progs/test_map_create.c > >> > >> diff --git a/tools/testing/selftests/bpf/prog_tests/map_create.c b/tools/testing/selftests/bpf/prog_tests/map_create.c > >> new file mode 100644 > >> index 000000000000..6ca32d0dffd2 > >> --- /dev/null > >> +++ b/tools/testing/selftests/bpf/prog_tests/map_create.c > >> @@ -0,0 +1,87 @@ > >> +/* SPDX-License-Identifier: GPL-2.0 */ > >> +/* Copyright (c) 2021 Hengqi Chen */ > >> + > >> +#include <test_progs.h> > >> +#include "test_map_create.skel.h" > >> + > >> +void test_map_create(void) > >> +{ > >> + struct test_map_create *skel; > >> + int err, fd; > >> + > >> + skel = test_map_create__open(); > >> + if (!ASSERT_OK_PTR(skel, "test_map_create__open failed")) > >> + return; > >> + > >> + err = test_map_create__load(skel); > > > > If load() succeeds, all the maps will definitely be created, so all > > the below tests are meaningless. > > > > I think it's better to just change all the existing map definitions > > used throughout selftests to use key/value types, instead of > > key_size/value_size. That will automatically test this feature without > > adding an extra test. Unfortunately to really test that the logic is > > working, we'd need to check that libbpf doesn't emit the warning about > > retrying map creation w/o BTF, but I think one-time manual check > > (please use ./test_progs -v to see libbpf warnings during tests) > > should be sufficient for this. > > > > Hello, Andrii > > I updated these existing tests as you suggested, > but I was unable to run the whole bpf selftests locally. > > Running ./test_progs -v made my system hang up, > didn't find the root cause yet. This is strange. Do you know at which test this happens? Do you get kernel warning/oops when this happens in dmesg? But overall, the development and testing workflow for people working on bpf/bpf-next involves building latest kernel and running selftests inside the QEMU VM for testing. We also have vmtest.sh script in selftests/bpf that automates a lot of building steps. It will build kernel, test_progs and other selftest binaries, and will spin up QEMU VM with the same image that our BPF CIs are using. You just need to have very recent/latest Clang available and similarly pahole (from dwarves package) should be up to date and available through $PATH. After that, running ./vmtest.sh will just run all ./test_progs automatically. Either way, our CI will also run your changes through the tests (except there are some intermittent issues right now, so we'll have to wait a bit for that to kick in). You can monitor [0], or the link will actually appear on each of your patches (e.g., [1]) in "Checks" section, once everything is up and running properly. [0] https://github.com/kernel-patches/bpf/pulls [1] https://patchwork.kernel.org/project/netdevbpf/patch/20210930161456.3444544-2-hengqi.chen@gmail.com/ > > Instead I just run the modified test with the following commands: > > sudo ./test_progs -v --name=kfree_skb,perf_event_stackmap,btf_map_in_map,pe_preserve_elems,stacktrace_map,stacktrace_build_id,xdp_bpf2bpf,select_reuseport,tcpbpf_user > > >> + if (!ASSERT_OK(err, "test_map_create__load failed")) > >> + goto cleanup; > >> + > >> + fd = bpf_map__fd(skel->maps.map1); > >> + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) > >> + goto cleanup; > >> + close(fd); > >> + > >> + fd = bpf_map__fd(skel->maps.map2); > >> + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) > >> + goto cleanup; > >> + close(fd); > > > > [...] > >
On 10/1/21 2:39 AM, Andrii Nakryiko wrote: > On Thu, Sep 30, 2021 at 9:05 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: >> >> >> >> On 9/9/21 12:29 PM, Andrii Nakryiko wrote: >>> On Sun, Sep 5, 2021 at 3:09 AM Hengqi Chen <hengqi.chen@gmail.com> wrote: >>>> >>>> Test BPF map creation using BTF-defined key/value. The test defines >>>> some specialized maps by specifying BTF types for key/value and >>>> checks those maps are correctly initialized and loaded. >>>> >>>> Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com> >>>> --- >>>> .../selftests/bpf/prog_tests/map_create.c | 87 ++++++++++++++ >>>> .../selftests/bpf/progs/test_map_create.c | 110 ++++++++++++++++++ >>>> 2 files changed, 197 insertions(+) >>>> create mode 100644 tools/testing/selftests/bpf/prog_tests/map_create.c >>>> create mode 100644 tools/testing/selftests/bpf/progs/test_map_create.c >>>> >>>> diff --git a/tools/testing/selftests/bpf/prog_tests/map_create.c b/tools/testing/selftests/bpf/prog_tests/map_create.c >>>> new file mode 100644 >>>> index 000000000000..6ca32d0dffd2 >>>> --- /dev/null >>>> +++ b/tools/testing/selftests/bpf/prog_tests/map_create.c >>>> @@ -0,0 +1,87 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +/* Copyright (c) 2021 Hengqi Chen */ >>>> + >>>> +#include <test_progs.h> >>>> +#include "test_map_create.skel.h" >>>> + >>>> +void test_map_create(void) >>>> +{ >>>> + struct test_map_create *skel; >>>> + int err, fd; >>>> + >>>> + skel = test_map_create__open(); >>>> + if (!ASSERT_OK_PTR(skel, "test_map_create__open failed")) >>>> + return; >>>> + >>>> + err = test_map_create__load(skel); >>> >>> If load() succeeds, all the maps will definitely be created, so all >>> the below tests are meaningless. >>> >>> I think it's better to just change all the existing map definitions >>> used throughout selftests to use key/value types, instead of >>> key_size/value_size. That will automatically test this feature without >>> adding an extra test. Unfortunately to really test that the logic is >>> working, we'd need to check that libbpf doesn't emit the warning about >>> retrying map creation w/o BTF, but I think one-time manual check >>> (please use ./test_progs -v to see libbpf warnings during tests) >>> should be sufficient for this. >>> >> >> Hello, Andrii >> >> I updated these existing tests as you suggested, >> but I was unable to run the whole bpf selftests locally. >> >> Running ./test_progs -v made my system hang up, >> didn't find the root cause yet. > > This is strange. Do you know at which test this happens? Do you get > kernel warning/oops when this happens in dmesg? > No, when this situation occurred, I just restarted my laptop. Will look into this later. > But overall, the development and testing workflow for people working > on bpf/bpf-next involves building latest kernel and running selftests > inside the QEMU VM for testing. We also have vmtest.sh script in > selftests/bpf that automates a lot of building steps. It will build > kernel, test_progs and other selftest binaries, and will spin up QEMU > VM with the same image that our BPF CIs are using. You just need to > have very recent/latest Clang available and similarly pahole (from > dwarves package) should be up to date and available through $PATH. > After that, running ./vmtest.sh will just run all ./test_progs > automatically. > This workflow info is quite useful for me. Thanks. > Either way, our CI will also run your changes through the tests > (except there are some intermittent issues right now, so we'll have to > wait a bit for that to kick in). You can monitor [0], or the link will > actually appear on each of your patches (e.g., [1]) in "Checks" > section, once everything is up and running properly. > > [0] https://github.com/kernel-patches/bpf/pulls > [1] https://patchwork.kernel.org/project/netdevbpf/patch/20210930161456.3444544-2-hengqi.chen@gmail.com/ > >> >> Instead I just run the modified test with the following commands: >> >> sudo ./test_progs -v --name=kfree_skb,perf_event_stackmap,btf_map_in_map,pe_preserve_elems,stacktrace_map,stacktrace_build_id,xdp_bpf2bpf,select_reuseport,tcpbpf_user >> >>>> + if (!ASSERT_OK(err, "test_map_create__load failed")) >>>> + goto cleanup; >>>> + >>>> + fd = bpf_map__fd(skel->maps.map1); >>>> + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) >>>> + goto cleanup; >>>> + close(fd); >>>> + >>>> + fd = bpf_map__fd(skel->maps.map2); >>>> + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) >>>> + goto cleanup; >>>> + close(fd); >>> >>> [...] >>>
diff --git a/tools/testing/selftests/bpf/prog_tests/map_create.c b/tools/testing/selftests/bpf/prog_tests/map_create.c new file mode 100644 index 000000000000..6ca32d0dffd2 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/map_create.c @@ -0,0 +1,87 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2021 Hengqi Chen */ + +#include <test_progs.h> +#include "test_map_create.skel.h" + +void test_map_create(void) +{ + struct test_map_create *skel; + int err, fd; + + skel = test_map_create__open(); + if (!ASSERT_OK_PTR(skel, "test_map_create__open failed")) + return; + + err = test_map_create__load(skel); + if (!ASSERT_OK(err, "test_map_create__load failed")) + goto cleanup; + + fd = bpf_map__fd(skel->maps.map1); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map2); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map3); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map4); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map5); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map6); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map7); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map8); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map9); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map10); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map11); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map12); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + + fd = bpf_map__fd(skel->maps.map13); + if (!ASSERT_GT(fd, 0, "bpf_map__fd failed")) + goto cleanup; + close(fd); + +cleanup: + test_map_create__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/test_map_create.c b/tools/testing/selftests/bpf/progs/test_map_create.c new file mode 100644 index 000000000000..2e9950e56b0f --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_map_create.c @@ -0,0 +1,110 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (c) 2021 Hengqi Chen */ + +#include "vmlinux.h" +#include <bpf/bpf_helpers.h> + +#define MAX_ENTRIES 8 + +struct { + __uint(type,BPF_MAP_TYPE_PERF_EVENT_ARRAY); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); +} map1 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_STACK_TRACE); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, __u64); +} map2 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_CGROUP_ARRAY); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); +} map3 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_ARRAY_OF_MAPS); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); + __array(values, struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, __u32); + }); +} map4 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_HASH_OF_MAPS); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); + __array(values, struct { + __uint(type, BPF_MAP_TYPE_ARRAY); + __uint(max_entries, 1); + __type(key, __u32); + __type(value, __u32); + }); +} map5 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_DEVMAP); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); +} map6 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_SOCKMAP); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); +} map7 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_CPUMAP); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); +} map8 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_XSKMAP); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); +} map9 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_SOCKHASH); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); +} map10 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_QUEUE); + __uint(max_entries, MAX_ENTRIES); + __type(value, int); +} map11 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_STACK); + __uint(max_entries, MAX_ENTRIES); + __type(value, int); +} map12 SEC(".maps"); + +struct { + __uint(type, BPF_MAP_TYPE_DEVMAP_HASH); + __uint(max_entries, MAX_ENTRIES); + __type(key, int); + __type(value, int); +} map13 SEC(".maps"); + +char _license[] SEC("license") = "GPL";
Test BPF map creation using BTF-defined key/value. The test defines some specialized maps by specifying BTF types for key/value and checks those maps are correctly initialized and loaded. Signed-off-by: Hengqi Chen <hengqi.chen@gmail.com> --- .../selftests/bpf/prog_tests/map_create.c | 87 ++++++++++++++ .../selftests/bpf/progs/test_map_create.c | 110 ++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/map_create.c create mode 100644 tools/testing/selftests/bpf/progs/test_map_create.c