diff mbox series

[bpf-next,v1,3/3] selftests/bpf: add test for bpf_for_each_map_elem() with different maps

Message ID 20240405025536.18113-4-lulie@linux.alibaba.com (mailing list archive)
State Accepted
Commit fecb1597cc11a23f32faa90d70a199533871686a
Delegated to: BPF
Headers show
Series bpf: allow bpf_for_each_map_elem() helper with different input maps | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-43 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-44 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-46 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-45 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-47 success Logs for x86_64-llvm-18 / veristat
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 949 this patch: 949
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: linux-kselftest@vger.kernel.org
netdev/build_clang success Errors and warnings before: 955 this patch: 955
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 961 this patch: 961
netdev/checkpatch warning CHECK: Alignment should match open parenthesis WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 81 exceeds 80 columns WARNING: line length of 82 exceeds 80 columns WARNING: line length of 91 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-16 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-18 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-42 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-41 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test (test_maps, false, 360) / test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-40 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-PR success PR summary

Commit Message

Philo Lu April 5, 2024, 2:55 a.m. UTC 
A test is added for bpf_for_each_map_elem() with either an arraymap or a
hashmap.
$ tools/testing/selftests/bpf/test_progs -t for_each
 #93/1    for_each/hash_map:OK
 #93/2    for_each/array_map:OK
 #93/3    for_each/write_map_key:OK
 #93/4    for_each/multi_maps:OK
 #93      for_each:OK
Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
---
 .../selftests/bpf/prog_tests/for_each.c       | 62 +++++++++++++++++++
 .../selftests/bpf/progs/for_each_multi_maps.c | 49 +++++++++++++++
 2 files changed, 111 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/for_each_multi_maps.c

Comments

Yonghong Song April 5, 2024, 4:39 p.m. UTC | #1 
On 4/4/24 7:55 PM, Philo Lu wrote:
> A test is added for bpf_for_each_map_elem() with either an arraymap or a
> hashmap.
> $ tools/testing/selftests/bpf/test_progs -t for_each
>   #93/1    for_each/hash_map:OK
>   #93/2    for_each/array_map:OK
>   #93/3    for_each/write_map_key:OK
>   #93/4    for_each/multi_maps:OK
>   #93      for_each:OK
> Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Philo Lu <lulie@linux.alibaba.com>

Acked-by: Yonghong Song <yonghong.song@linux.dev>
Andrii Nakryiko April 5, 2024, 5:35 p.m. UTC | #2 
On Thu, Apr 4, 2024 at 7:55 PM Philo Lu <lulie@linux.alibaba.com> wrote:
>
> A test is added for bpf_for_each_map_elem() with either an arraymap or a
> hashmap.
> $ tools/testing/selftests/bpf/test_progs -t for_each
>  #93/1    for_each/hash_map:OK
>  #93/2    for_each/array_map:OK
>  #93/3    for_each/write_map_key:OK
>  #93/4    for_each/multi_maps:OK
>  #93      for_each:OK
> Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
> ---
>  .../selftests/bpf/prog_tests/for_each.c       | 62 +++++++++++++++++++
>  .../selftests/bpf/progs/for_each_multi_maps.c | 49 +++++++++++++++
>  2 files changed, 111 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/progs/for_each_multi_maps.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/for_each.c b/tools/testing/selftests/bpf/prog_tests/for_each.c
> index 8963f8a549f2..09f6487f58b9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/for_each.c
> +++ b/tools/testing/selftests/bpf/prog_tests/for_each.c
> @@ -5,6 +5,7 @@
>  #include "for_each_hash_map_elem.skel.h"
>  #include "for_each_array_map_elem.skel.h"
>  #include "for_each_map_elem_write_key.skel.h"
> +#include "for_each_multi_maps.skel.h"
>
>  static unsigned int duration;
>
> @@ -143,6 +144,65 @@ static void test_write_map_key(void)
>                 for_each_map_elem_write_key__destroy(skel);
>  }
>
> +static void test_multi_maps(void)
> +{
> +       struct for_each_multi_maps *skel;
> +       __u64 val, array_total, hash_total;
> +       __u32 key, max_entries;
> +       int i, err;
> +
> +       LIBBPF_OPTS(bpf_test_run_opts, topts,
> +               .data_in = &pkt_v4,
> +               .data_size_in = sizeof(pkt_v4),
> +               .repeat = 1,
> +       );
> +
> +       skel = for_each_multi_maps__open_and_load();
> +       if (!ASSERT_OK_PTR(skel, "for_each_multi_maps__open_and_load"))
> +               return;
> +
> +       array_total = 0;
> +       max_entries = bpf_map__max_entries(skel->maps.arraymap);
> +       for (i = 0; i < max_entries; i++) {
> +               key = i;
> +               val = i + 1;
> +               array_total += val;
> +               err = bpf_map__update_elem(skel->maps.arraymap, &key, sizeof(key),
> +                                          &val, sizeof(val), BPF_ANY);
> +               if (!ASSERT_OK(err, "array_map_update"))
> +                       goto out;
> +       }
> +
> +       hash_total = 0;
> +       max_entries = bpf_map__max_entries(skel->maps.hashmap);
> +       for (i = 0; i < max_entries; i++) {
> +               key = i + 100;
> +               val = i + 1;
> +               hash_total += val;
> +               err = bpf_map__update_elem(skel->maps.hashmap, &key, sizeof(key),
> +                                          &val, sizeof(val), BPF_ANY);
> +               if (!ASSERT_OK(err, "hash_map_update"))
> +                       goto out;
> +       }
> +
> +       skel->bss->data_output = 0;
> +       skel->bss->use_array = 1;
> +       err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
> +       ASSERT_OK(err, "bpf_prog_test_run_opts");
> +       ASSERT_OK(topts.retval, "retval");
> +       ASSERT_EQ(skel->bss->data_output, array_total, "array output");
> +
> +       skel->bss->data_output = 0;
> +       skel->bss->use_array = 0;
> +       err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
> +       ASSERT_OK(err, "bpf_prog_test_run_opts");
> +       ASSERT_OK(topts.retval, "retval");
> +       ASSERT_EQ(skel->bss->data_output, hash_total, "hash output");
> +
> +out:
> +       for_each_multi_maps__destroy(skel);
> +}
> +
>  void test_for_each(void)
>  {
>         if (test__start_subtest("hash_map"))
> @@ -151,4 +211,6 @@ void test_for_each(void)
>                 test_array_map();
>         if (test__start_subtest("write_map_key"))
>                 test_write_map_key();
> +       if (test__start_subtest("multi_maps"))
> +               test_multi_maps();
>  }
> diff --git a/tools/testing/selftests/bpf/progs/for_each_multi_maps.c b/tools/testing/selftests/bpf/progs/for_each_multi_maps.c
> new file mode 100644
> index 000000000000..ff0bed7d4459
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/for_each_multi_maps.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +
> +char _license[] SEC("license") = "GPL";
> +
> +struct {
> +       __uint(type, BPF_MAP_TYPE_ARRAY);
> +       __uint(max_entries, 3);
> +       __type(key, __u32);
> +       __type(value, __u64);
> +} arraymap SEC(".maps");
> +
> +struct {
> +       __uint(type, BPF_MAP_TYPE_HASH);
> +       __uint(max_entries, 5);
> +       __type(key, __u32);
> +       __type(value, __u64);
> +} hashmap SEC(".maps");
> +
> +struct callback_ctx {
> +       int output;
> +};
> +
> +u32 data_output = 0;
> +int use_array = 0;
> +
> +static __u64
> +check_map_elem(struct bpf_map *map, __u32 *key, __u64 *val,
> +              struct callback_ctx *data)
> +{
> +       data->output += *val;
> +       return 0;
> +}
> +
> +SEC("tc")
> +int test_pkt_access(struct __sk_buff *skb)
> +{
> +       struct callback_ctx data;
> +
> +       data.output = 0;
> +       if (use_array)
> +               bpf_for_each_map_elem(&arraymap, check_map_elem, &data, 0);
> +       else
> +               bpf_for_each_map_elem(&hashmap, check_map_elem, &data, 0);

you are relying on the compiler to combine bpf_for_each_map_elem calls
to one call instruction and just passing different map pointers,
right? It would be best to code this in BPF assembly so that this
doesn't rely on compiler decisions and optimizations.

> +       data_output = data.output;
> +
> +       return 0;
> +}
> --
> 2.32.0.3.g01195cf9f
>
Alexei Starovoitov April 5, 2024, 5:41 p.m. UTC | #3 
On Fri, Apr 5, 2024 at 10:36 AM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Apr 4, 2024 at 7:55 PM Philo Lu <lulie@linux.alibaba.com> wrote:
> >
> > A test is added for bpf_for_each_map_elem() with either an arraymap or a
> > hashmap.
> > $ tools/testing/selftests/bpf/test_progs -t for_each
> >  #93/1    for_each/hash_map:OK
> >  #93/2    for_each/array_map:OK
> >  #93/3    for_each/write_map_key:OK
> >  #93/4    for_each/multi_maps:OK
> >  #93      for_each:OK
> > Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED
> >
> > Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
> > ---
> >  .../selftests/bpf/prog_tests/for_each.c       | 62 +++++++++++++++++++
> >  .../selftests/bpf/progs/for_each_multi_maps.c | 49 +++++++++++++++
> >  2 files changed, 111 insertions(+)
> >  create mode 100644 tools/testing/selftests/bpf/progs/for_each_multi_maps.c
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/for_each.c b/tools/testing/selftests/bpf/prog_tests/for_each.c
> > index 8963f8a549f2..09f6487f58b9 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/for_each.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/for_each.c
> > @@ -5,6 +5,7 @@
> >  #include "for_each_hash_map_elem.skel.h"
> >  #include "for_each_array_map_elem.skel.h"
> >  #include "for_each_map_elem_write_key.skel.h"
> > +#include "for_each_multi_maps.skel.h"
> >
> >  static unsigned int duration;
> >
> > @@ -143,6 +144,65 @@ static void test_write_map_key(void)
> >                 for_each_map_elem_write_key__destroy(skel);
> >  }
> >
> > +static void test_multi_maps(void)
> > +{
> > +       struct for_each_multi_maps *skel;
> > +       __u64 val, array_total, hash_total;
> > +       __u32 key, max_entries;
> > +       int i, err;
> > +
> > +       LIBBPF_OPTS(bpf_test_run_opts, topts,
> > +               .data_in = &pkt_v4,
> > +               .data_size_in = sizeof(pkt_v4),
> > +               .repeat = 1,
> > +       );
> > +
> > +       skel = for_each_multi_maps__open_and_load();
> > +       if (!ASSERT_OK_PTR(skel, "for_each_multi_maps__open_and_load"))
> > +               return;
> > +
> > +       array_total = 0;
> > +       max_entries = bpf_map__max_entries(skel->maps.arraymap);
> > +       for (i = 0; i < max_entries; i++) {
> > +               key = i;
> > +               val = i + 1;
> > +               array_total += val;
> > +               err = bpf_map__update_elem(skel->maps.arraymap, &key, sizeof(key),
> > +                                          &val, sizeof(val), BPF_ANY);
> > +               if (!ASSERT_OK(err, "array_map_update"))
> > +                       goto out;
> > +       }
> > +
> > +       hash_total = 0;
> > +       max_entries = bpf_map__max_entries(skel->maps.hashmap);
> > +       for (i = 0; i < max_entries; i++) {
> > +               key = i + 100;
> > +               val = i + 1;
> > +               hash_total += val;
> > +               err = bpf_map__update_elem(skel->maps.hashmap, &key, sizeof(key),
> > +                                          &val, sizeof(val), BPF_ANY);
> > +               if (!ASSERT_OK(err, "hash_map_update"))
> > +                       goto out;
> > +       }
> > +
> > +       skel->bss->data_output = 0;
> > +       skel->bss->use_array = 1;
> > +       err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
> > +       ASSERT_OK(err, "bpf_prog_test_run_opts");
> > +       ASSERT_OK(topts.retval, "retval");
> > +       ASSERT_EQ(skel->bss->data_output, array_total, "array output");
> > +
> > +       skel->bss->data_output = 0;
> > +       skel->bss->use_array = 0;
> > +       err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
> > +       ASSERT_OK(err, "bpf_prog_test_run_opts");
> > +       ASSERT_OK(topts.retval, "retval");
> > +       ASSERT_EQ(skel->bss->data_output, hash_total, "hash output");
> > +
> > +out:
> > +       for_each_multi_maps__destroy(skel);
> > +}
> > +
> >  void test_for_each(void)
> >  {
> >         if (test__start_subtest("hash_map"))
> > @@ -151,4 +211,6 @@ void test_for_each(void)
> >                 test_array_map();
> >         if (test__start_subtest("write_map_key"))
> >                 test_write_map_key();
> > +       if (test__start_subtest("multi_maps"))
> > +               test_multi_maps();
> >  }
> > diff --git a/tools/testing/selftests/bpf/progs/for_each_multi_maps.c b/tools/testing/selftests/bpf/progs/for_each_multi_maps.c
> > new file mode 100644
> > index 000000000000..ff0bed7d4459
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/for_each_multi_maps.c
> > @@ -0,0 +1,49 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include "vmlinux.h"
> > +#include <bpf/bpf_helpers.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +struct {
> > +       __uint(type, BPF_MAP_TYPE_ARRAY);
> > +       __uint(max_entries, 3);
> > +       __type(key, __u32);
> > +       __type(value, __u64);
> > +} arraymap SEC(".maps");
> > +
> > +struct {
> > +       __uint(type, BPF_MAP_TYPE_HASH);
> > +       __uint(max_entries, 5);
> > +       __type(key, __u32);
> > +       __type(value, __u64);
> > +} hashmap SEC(".maps");
> > +
> > +struct callback_ctx {
> > +       int output;
> > +};
> > +
> > +u32 data_output = 0;
> > +int use_array = 0;
> > +
> > +static __u64
> > +check_map_elem(struct bpf_map *map, __u32 *key, __u64 *val,
> > +              struct callback_ctx *data)
> > +{
> > +       data->output += *val;
> > +       return 0;
> > +}
> > +
> > +SEC("tc")
> > +int test_pkt_access(struct __sk_buff *skb)
> > +{
> > +       struct callback_ctx data;
> > +
> > +       data.output = 0;
> > +       if (use_array)
> > +               bpf_for_each_map_elem(&arraymap, check_map_elem, &data, 0);
> > +       else
> > +               bpf_for_each_map_elem(&hashmap, check_map_elem, &data, 0);
>
> you are relying on the compiler to combine bpf_for_each_map_elem calls
> to one call instruction and just passing different map pointers,
> right? It would be best to code this in BPF assembly so that this
> doesn't rely on compiler decisions and optimizations.

It will look sad in asm. C is fine.
Maybe a follow up with
void *map;
if (..)
   map = &array;
else
   map = &hash;

bpf_for_each_map_elem(map, ...);

but optional. imo.
Andrii Nakryiko April 5, 2024, 5:52 p.m. UTC | #4 
On Fri, Apr 5, 2024 at 10:41 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Apr 5, 2024 at 10:36 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Apr 4, 2024 at 7:55 PM Philo Lu <lulie@linux.alibaba.com> wrote:
> > >
> > > A test is added for bpf_for_each_map_elem() with either an arraymap or a
> > > hashmap.
> > > $ tools/testing/selftests/bpf/test_progs -t for_each
> > >  #93/1    for_each/hash_map:OK
> > >  #93/2    for_each/array_map:OK
> > >  #93/3    for_each/write_map_key:OK
> > >  #93/4    for_each/multi_maps:OK
> > >  #93      for_each:OK
> > > Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED
> > >
> > > Signed-off-by: Philo Lu <lulie@linux.alibaba.com>
> > > ---
> > >  .../selftests/bpf/prog_tests/for_each.c       | 62 +++++++++++++++++++
> > >  .../selftests/bpf/progs/for_each_multi_maps.c | 49 +++++++++++++++
> > >  2 files changed, 111 insertions(+)
> > >  create mode 100644 tools/testing/selftests/bpf/progs/for_each_multi_maps.c
> > >
> > > diff --git a/tools/testing/selftests/bpf/prog_tests/for_each.c b/tools/testing/selftests/bpf/prog_tests/for_each.c
> > > index 8963f8a549f2..09f6487f58b9 100644
> > > --- a/tools/testing/selftests/bpf/prog_tests/for_each.c
> > > +++ b/tools/testing/selftests/bpf/prog_tests/for_each.c
> > > @@ -5,6 +5,7 @@
> > >  #include "for_each_hash_map_elem.skel.h"
> > >  #include "for_each_array_map_elem.skel.h"
> > >  #include "for_each_map_elem_write_key.skel.h"
> > > +#include "for_each_multi_maps.skel.h"
> > >
> > >  static unsigned int duration;
> > >
> > > @@ -143,6 +144,65 @@ static void test_write_map_key(void)
> > >                 for_each_map_elem_write_key__destroy(skel);
> > >  }
> > >
> > > +static void test_multi_maps(void)
> > > +{
> > > +       struct for_each_multi_maps *skel;
> > > +       __u64 val, array_total, hash_total;
> > > +       __u32 key, max_entries;
> > > +       int i, err;
> > > +
> > > +       LIBBPF_OPTS(bpf_test_run_opts, topts,
> > > +               .data_in = &pkt_v4,
> > > +               .data_size_in = sizeof(pkt_v4),
> > > +               .repeat = 1,
> > > +       );
> > > +
> > > +       skel = for_each_multi_maps__open_and_load();
> > > +       if (!ASSERT_OK_PTR(skel, "for_each_multi_maps__open_and_load"))
> > > +               return;
> > > +
> > > +       array_total = 0;
> > > +       max_entries = bpf_map__max_entries(skel->maps.arraymap);
> > > +       for (i = 0; i < max_entries; i++) {
> > > +               key = i;
> > > +               val = i + 1;
> > > +               array_total += val;
> > > +               err = bpf_map__update_elem(skel->maps.arraymap, &key, sizeof(key),
> > > +                                          &val, sizeof(val), BPF_ANY);
> > > +               if (!ASSERT_OK(err, "array_map_update"))
> > > +                       goto out;
> > > +       }
> > > +
> > > +       hash_total = 0;
> > > +       max_entries = bpf_map__max_entries(skel->maps.hashmap);
> > > +       for (i = 0; i < max_entries; i++) {
> > > +               key = i + 100;
> > > +               val = i + 1;
> > > +               hash_total += val;
> > > +               err = bpf_map__update_elem(skel->maps.hashmap, &key, sizeof(key),
> > > +                                          &val, sizeof(val), BPF_ANY);
> > > +               if (!ASSERT_OK(err, "hash_map_update"))
> > > +                       goto out;
> > > +       }
> > > +
> > > +       skel->bss->data_output = 0;
> > > +       skel->bss->use_array = 1;
> > > +       err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
> > > +       ASSERT_OK(err, "bpf_prog_test_run_opts");
> > > +       ASSERT_OK(topts.retval, "retval");
> > > +       ASSERT_EQ(skel->bss->data_output, array_total, "array output");
> > > +
> > > +       skel->bss->data_output = 0;
> > > +       skel->bss->use_array = 0;
> > > +       err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
> > > +       ASSERT_OK(err, "bpf_prog_test_run_opts");
> > > +       ASSERT_OK(topts.retval, "retval");
> > > +       ASSERT_EQ(skel->bss->data_output, hash_total, "hash output");
> > > +
> > > +out:
> > > +       for_each_multi_maps__destroy(skel);
> > > +}
> > > +
> > >  void test_for_each(void)
> > >  {
> > >         if (test__start_subtest("hash_map"))
> > > @@ -151,4 +211,6 @@ void test_for_each(void)
> > >                 test_array_map();
> > >         if (test__start_subtest("write_map_key"))
> > >                 test_write_map_key();
> > > +       if (test__start_subtest("multi_maps"))
> > > +               test_multi_maps();
> > >  }
> > > diff --git a/tools/testing/selftests/bpf/progs/for_each_multi_maps.c b/tools/testing/selftests/bpf/progs/for_each_multi_maps.c
> > > new file mode 100644
> > > index 000000000000..ff0bed7d4459
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/bpf/progs/for_each_multi_maps.c
> > > @@ -0,0 +1,49 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#include "vmlinux.h"
> > > +#include <bpf/bpf_helpers.h>
> > > +
> > > +char _license[] SEC("license") = "GPL";
> > > +
> > > +struct {
> > > +       __uint(type, BPF_MAP_TYPE_ARRAY);
> > > +       __uint(max_entries, 3);
> > > +       __type(key, __u32);
> > > +       __type(value, __u64);
> > > +} arraymap SEC(".maps");
> > > +
> > > +struct {
> > > +       __uint(type, BPF_MAP_TYPE_HASH);
> > > +       __uint(max_entries, 5);
> > > +       __type(key, __u32);
> > > +       __type(value, __u64);
> > > +} hashmap SEC(".maps");
> > > +
> > > +struct callback_ctx {
> > > +       int output;
> > > +};
> > > +
> > > +u32 data_output = 0;
> > > +int use_array = 0;
> > > +
> > > +static __u64
> > > +check_map_elem(struct bpf_map *map, __u32 *key, __u64 *val,
> > > +              struct callback_ctx *data)
> > > +{
> > > +       data->output += *val;
> > > +       return 0;
> > > +}
> > > +
> > > +SEC("tc")
> > > +int test_pkt_access(struct __sk_buff *skb)
> > > +{
> > > +       struct callback_ctx data;
> > > +
> > > +       data.output = 0;
> > > +       if (use_array)
> > > +               bpf_for_each_map_elem(&arraymap, check_map_elem, &data, 0);
> > > +       else
> > > +               bpf_for_each_map_elem(&hashmap, check_map_elem, &data, 0);
> >
> > you are relying on the compiler to combine bpf_for_each_map_elem calls
> > to one call instruction and just passing different map pointers,
> > right? It would be best to code this in BPF assembly so that this
> > doesn't rely on compiler decisions and optimizations.
>
> It will look sad in asm. C is fine.
> Maybe a follow up with
> void *map;
> if (..)
>    map = &array;
> else
>    map = &hash;
>

yep, this is simpler, no need to go for assembly

Though not sure why assembly would be sad, we do call subprograms by
their name (it's just symbols after all), so passing a pointer to
subprog should work just as well.

> bpf_for_each_map_elem(map, ...);
>
> but optional. imo.
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/for_each.c b/tools/testing/selftests/bpf/prog_tests/for_each.c
index 8963f8a549f2..09f6487f58b9 100644
--- a/tools/testing/selftests/bpf/prog_tests/for_each.c
+++ b/tools/testing/selftests/bpf/prog_tests/for_each.c
@@ -5,6 +5,7 @@ 
 #include "for_each_hash_map_elem.skel.h"
 #include "for_each_array_map_elem.skel.h"
 #include "for_each_map_elem_write_key.skel.h"
+#include "for_each_multi_maps.skel.h"
 
 static unsigned int duration;
 
@@ -143,6 +144,65 @@  static void test_write_map_key(void)
 		for_each_map_elem_write_key__destroy(skel);
 }
 
+static void test_multi_maps(void)
+{
+	struct for_each_multi_maps *skel;
+	__u64 val, array_total, hash_total;
+	__u32 key, max_entries;
+	int i, err;
+
+	LIBBPF_OPTS(bpf_test_run_opts, topts,
+		.data_in = &pkt_v4,
+		.data_size_in = sizeof(pkt_v4),
+		.repeat = 1,
+	);
+
+	skel = for_each_multi_maps__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "for_each_multi_maps__open_and_load"))
+		return;
+
+	array_total = 0;
+	max_entries = bpf_map__max_entries(skel->maps.arraymap);
+	for (i = 0; i < max_entries; i++) {
+		key = i;
+		val = i + 1;
+		array_total += val;
+		err = bpf_map__update_elem(skel->maps.arraymap, &key, sizeof(key),
+					   &val, sizeof(val), BPF_ANY);
+		if (!ASSERT_OK(err, "array_map_update"))
+			goto out;
+	}
+
+	hash_total = 0;
+	max_entries = bpf_map__max_entries(skel->maps.hashmap);
+	for (i = 0; i < max_entries; i++) {
+		key = i + 100;
+		val = i + 1;
+		hash_total += val;
+		err = bpf_map__update_elem(skel->maps.hashmap, &key, sizeof(key),
+					   &val, sizeof(val), BPF_ANY);
+		if (!ASSERT_OK(err, "hash_map_update"))
+			goto out;
+	}
+
+	skel->bss->data_output = 0;
+	skel->bss->use_array = 1;
+	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
+	ASSERT_OK(err, "bpf_prog_test_run_opts");
+	ASSERT_OK(topts.retval, "retval");
+	ASSERT_EQ(skel->bss->data_output, array_total, "array output");
+
+	skel->bss->data_output = 0;
+	skel->bss->use_array = 0;
+	err = bpf_prog_test_run_opts(bpf_program__fd(skel->progs.test_pkt_access), &topts);
+	ASSERT_OK(err, "bpf_prog_test_run_opts");
+	ASSERT_OK(topts.retval, "retval");
+	ASSERT_EQ(skel->bss->data_output, hash_total, "hash output");
+
+out:
+	for_each_multi_maps__destroy(skel);
+}
+
 void test_for_each(void)
 {
 	if (test__start_subtest("hash_map"))
@@ -151,4 +211,6 @@  void test_for_each(void)
 		test_array_map();
 	if (test__start_subtest("write_map_key"))
 		test_write_map_key();
+	if (test__start_subtest("multi_maps"))
+		test_multi_maps();
 }
diff --git a/tools/testing/selftests/bpf/progs/for_each_multi_maps.c b/tools/testing/selftests/bpf/progs/for_each_multi_maps.c
new file mode 100644
index 000000000000..ff0bed7d4459
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/for_each_multi_maps.c
@@ -0,0 +1,49 @@ 
+// SPDX-License-Identifier: GPL-2.0
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 3);
+	__type(key, __u32);
+	__type(value, __u64);
+} arraymap SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 5);
+	__type(key, __u32);
+	__type(value, __u64);
+} hashmap SEC(".maps");
+
+struct callback_ctx {
+	int output;
+};
+
+u32 data_output = 0;
+int use_array = 0;
+
+static __u64
+check_map_elem(struct bpf_map *map, __u32 *key, __u64 *val,
+	       struct callback_ctx *data)
+{
+	data->output += *val;
+	return 0;
+}
+
+SEC("tc")
+int test_pkt_access(struct __sk_buff *skb)
+{
+	struct callback_ctx data;
+
+	data.output = 0;
+	if (use_array)
+		bpf_for_each_map_elem(&arraymap, check_map_elem, &data, 0);
+	else
+		bpf_for_each_map_elem(&hashmap, check_map_elem, &data, 0);
+	data_output = data.output;
+
+	return 0;
+}