| Message ID | 20200909182406.3147878-6-sdf@google.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Allow storage of flexible metadata information for eBPF programs | expand |
On Wed, Sep 9, 2020 at 11:25 AM Stanislav Fomichev <sdf@google.com> wrote: > > From: YiFei Zhu <zhuyifei@google.com> > > This is a simple test to check that loading and dumping metadata > in btftool works, whether or not metadata contents are used by the > program. > > A C test is also added to make sure the skeleton code can read the > metadata values. > > Cc: YiFei Zhu <zhuyifei1999@gmail.com> > Signed-off-by: YiFei Zhu <zhuyifei@google.com> > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- It would be good to test that libbpf does bind .rodata even if BPF program doesn't use it. So for metadata_unused you can get bpf_prog_info and check that it does contain the id of .rodata? > tools/testing/selftests/bpf/Makefile | 3 +- > .../selftests/bpf/prog_tests/metadata.c | 81 ++++++++++++++++++ > .../selftests/bpf/progs/metadata_unused.c | 15 ++++ > .../selftests/bpf/progs/metadata_used.c | 15 ++++ > .../selftests/bpf/test_bpftool_metadata.sh | 82 +++++++++++++++++++ > 5 files changed, 195 insertions(+), 1 deletion(-) > create mode 100644 tools/testing/selftests/bpf/prog_tests/metadata.c > create mode 100644 tools/testing/selftests/bpf/progs/metadata_unused.c > create mode 100644 tools/testing/selftests/bpf/progs/metadata_used.c > create mode 100755 tools/testing/selftests/bpf/test_bpftool_metadata.sh > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index 65d3d9aaeb31..3c92db8a189a 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -68,7 +68,8 @@ TEST_PROGS := test_kmod.sh \ > test_tc_edt.sh \ > test_xdping.sh \ > test_bpftool_build.sh \ > - test_bpftool.sh > + test_bpftool.sh \ > + test_bpftool_metadata.sh \ > > TEST_PROGS_EXTENDED := with_addr.sh \ > with_tunnels.sh \ > diff --git a/tools/testing/selftests/bpf/prog_tests/metadata.c b/tools/testing/selftests/bpf/prog_tests/metadata.c > new file mode 100644 > index 000000000000..dea8fa86b5fb > --- /dev/null > +++ b/tools/testing/selftests/bpf/prog_tests/metadata.c > @@ -0,0 +1,81 @@ > +// SPDX-License-Identifier: GPL-2.0-only > + > +/* > + * Copyright 2020 Google LLC. > + */ > + > +#include <test_progs.h> > +#include <cgroup_helpers.h> > +#include <network_helpers.h> > + > +#include "metadata_unused.skel.h" > +#include "metadata_used.skel.h" > + > +static int duration; > + > +static void test_metadata_unused(void) > +{ > + struct metadata_unused *obj; > + int err; > + > + obj = metadata_unused__open_and_load(); > + if (CHECK(!obj, "skel-load", "errno %d", errno)) > + return; > + > + /* Assert that we can access the metadata in skel and the values are > + * what we expect. > + */ > + if (CHECK(strncmp(obj->rodata->bpf_metadata_a, "foo", > + sizeof(obj->rodata->bpf_metadata_a)), > + "bpf_metadata_a", "expected \"foo\", value differ")) > + goto close_bpf_object; > + if (CHECK(obj->rodata->bpf_metadata_b != 1, "bpf_metadata_b", > + "expected 1, got %d", obj->rodata->bpf_metadata_b)) > + goto close_bpf_object; > + > + /* Assert that binding metadata map to prog again succeeds. */ > + err = bpf_prog_bind_map(bpf_program__fd(obj->progs.prog), > + bpf_map__fd(obj->maps.rodata), NULL); > + CHECK(err, "rebind_map", "errno %d, expected 0", errno); > + > +close_bpf_object: > + metadata_unused__destroy(obj); > +} > + > +static void test_metadata_used(void) > +{ > + struct metadata_used *obj; > + int err; > + > + obj = metadata_used__open_and_load(); > + if (CHECK(!obj, "skel-load", "errno %d", errno)) > + return; > + > + /* Assert that we can access the metadata in skel and the values are > + * what we expect. > + */ > + if (CHECK(strncmp(obj->rodata->bpf_metadata_a, "bar", > + sizeof(obj->rodata->bpf_metadata_a)), > + "metadata_a", "expected \"bar\", value differ")) > + goto close_bpf_object; > + if (CHECK(obj->rodata->bpf_metadata_b != 2, "metadata_b", > + "expected 2, got %d", obj->rodata->bpf_metadata_b)) > + goto close_bpf_object; > + > + /* Assert that binding metadata map to prog again succeeds. */ > + err = bpf_prog_bind_map(bpf_program__fd(obj->progs.prog), > + bpf_map__fd(obj->maps.rodata), NULL); > + CHECK(err, "rebind_map", "errno %d, expected 0", errno); > + > +close_bpf_object: > + metadata_used__destroy(obj); > +} > + > +void test_metadata(void) > +{ > + if (test__start_subtest("unused")) > + test_metadata_unused(); > + > + if (test__start_subtest("used")) > + test_metadata_used(); > +} > diff --git a/tools/testing/selftests/bpf/progs/metadata_unused.c b/tools/testing/selftests/bpf/progs/metadata_unused.c > new file mode 100644 > index 000000000000..db5b804f6f4c > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/metadata_unused.c > @@ -0,0 +1,15 @@ > +// SPDX-License-Identifier: GPL-2.0-only > + > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > + > +const char bpf_metadata_a[] SEC(".rodata") = "foo"; > +const int bpf_metadata_b SEC(".rodata") = 1; please add volatile to ensure these are not optimized away > + > +SEC("cgroup_skb/egress") > +int prog(struct xdp_md *ctx) > +{ > + return 0; > +} > + > +char _license[] SEC("license") = "GPL"; > diff --git a/tools/testing/selftests/bpf/progs/metadata_used.c b/tools/testing/selftests/bpf/progs/metadata_used.c > new file mode 100644 > index 000000000000..0dcb1ba2f0ae > --- /dev/null > +++ b/tools/testing/selftests/bpf/progs/metadata_used.c > @@ -0,0 +1,15 @@ > +// SPDX-License-Identifier: GPL-2.0-only > + > +#include <linux/bpf.h> > +#include <bpf/bpf_helpers.h> > + > +const char bpf_metadata_a[] SEC(".rodata") = "bar"; > +const int bpf_metadata_b SEC(".rodata") = 2; same about volatile > + > +SEC("cgroup_skb/egress") > +int prog(struct xdp_md *ctx) > +{ > + return bpf_metadata_b ? 1 : 0; > +} > + > +char _license[] SEC("license") = "GPL"; [...]
On Thu, Sep 10, 2020 at 12:59 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Wed, Sep 9, 2020 at 11:25 AM Stanislav Fomichev <sdf@google.com> wrote: > > > > From: YiFei Zhu <zhuyifei@google.com> > > > > This is a simple test to check that loading and dumping metadata > > in btftool works, whether or not metadata contents are used by the > > program. > > > > A C test is also added to make sure the skeleton code can read the > > metadata values. > > > > Cc: YiFei Zhu <zhuyifei1999@gmail.com> > > Signed-off-by: YiFei Zhu <zhuyifei@google.com> > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > --- > > It would be good to test that libbpf does bind .rodata even if BPF > program doesn't use it. So for metadata_unused you can get > bpf_prog_info and check that it does contain the id of .rodata? Good idea, will add that. > > +const char bpf_metadata_a[] SEC(".rodata") = "foo"; > > +const int bpf_metadata_b SEC(".rodata") = 1; > > please add volatile to ensure these are not optimized away Ack, ty!
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 65d3d9aaeb31..3c92db8a189a 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -68,7 +68,8 @@ TEST_PROGS := test_kmod.sh \ test_tc_edt.sh \ test_xdping.sh \ test_bpftool_build.sh \ - test_bpftool.sh + test_bpftool.sh \ + test_bpftool_metadata.sh \ TEST_PROGS_EXTENDED := with_addr.sh \ with_tunnels.sh \ diff --git a/tools/testing/selftests/bpf/prog_tests/metadata.c b/tools/testing/selftests/bpf/prog_tests/metadata.c new file mode 100644 index 000000000000..dea8fa86b5fb --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/metadata.c @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0-only + +/* + * Copyright 2020 Google LLC. + */ + +#include <test_progs.h> +#include <cgroup_helpers.h> +#include <network_helpers.h> + +#include "metadata_unused.skel.h" +#include "metadata_used.skel.h" + +static int duration; + +static void test_metadata_unused(void) +{ + struct metadata_unused *obj; + int err; + + obj = metadata_unused__open_and_load(); + if (CHECK(!obj, "skel-load", "errno %d", errno)) + return; + + /* Assert that we can access the metadata in skel and the values are + * what we expect. + */ + if (CHECK(strncmp(obj->rodata->bpf_metadata_a, "foo", + sizeof(obj->rodata->bpf_metadata_a)), + "bpf_metadata_a", "expected \"foo\", value differ")) + goto close_bpf_object; + if (CHECK(obj->rodata->bpf_metadata_b != 1, "bpf_metadata_b", + "expected 1, got %d", obj->rodata->bpf_metadata_b)) + goto close_bpf_object; + + /* Assert that binding metadata map to prog again succeeds. */ + err = bpf_prog_bind_map(bpf_program__fd(obj->progs.prog), + bpf_map__fd(obj->maps.rodata), NULL); + CHECK(err, "rebind_map", "errno %d, expected 0", errno); + +close_bpf_object: + metadata_unused__destroy(obj); +} + +static void test_metadata_used(void) +{ + struct metadata_used *obj; + int err; + + obj = metadata_used__open_and_load(); + if (CHECK(!obj, "skel-load", "errno %d", errno)) + return; + + /* Assert that we can access the metadata in skel and the values are + * what we expect. + */ + if (CHECK(strncmp(obj->rodata->bpf_metadata_a, "bar", + sizeof(obj->rodata->bpf_metadata_a)), + "metadata_a", "expected \"bar\", value differ")) + goto close_bpf_object; + if (CHECK(obj->rodata->bpf_metadata_b != 2, "metadata_b", + "expected 2, got %d", obj->rodata->bpf_metadata_b)) + goto close_bpf_object; + + /* Assert that binding metadata map to prog again succeeds. */ + err = bpf_prog_bind_map(bpf_program__fd(obj->progs.prog), + bpf_map__fd(obj->maps.rodata), NULL); + CHECK(err, "rebind_map", "errno %d, expected 0", errno); + +close_bpf_object: + metadata_used__destroy(obj); +} + +void test_metadata(void) +{ + if (test__start_subtest("unused")) + test_metadata_unused(); + + if (test__start_subtest("used")) + test_metadata_used(); +} diff --git a/tools/testing/selftests/bpf/progs/metadata_unused.c b/tools/testing/selftests/bpf/progs/metadata_unused.c new file mode 100644 index 000000000000..db5b804f6f4c --- /dev/null +++ b/tools/testing/selftests/bpf/progs/metadata_unused.c @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +const char bpf_metadata_a[] SEC(".rodata") = "foo"; +const int bpf_metadata_b SEC(".rodata") = 1; + +SEC("cgroup_skb/egress") +int prog(struct xdp_md *ctx) +{ + return 0; +} + +char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/progs/metadata_used.c b/tools/testing/selftests/bpf/progs/metadata_used.c new file mode 100644 index 000000000000..0dcb1ba2f0ae --- /dev/null +++ b/tools/testing/selftests/bpf/progs/metadata_used.c @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <linux/bpf.h> +#include <bpf/bpf_helpers.h> + +const char bpf_metadata_a[] SEC(".rodata") = "bar"; +const int bpf_metadata_b SEC(".rodata") = 2; + +SEC("cgroup_skb/egress") +int prog(struct xdp_md *ctx) +{ + return bpf_metadata_b ? 1 : 0; +} + +char _license[] SEC("license") = "GPL"; diff --git a/tools/testing/selftests/bpf/test_bpftool_metadata.sh b/tools/testing/selftests/bpf/test_bpftool_metadata.sh new file mode 100755 index 000000000000..1bf81b49457a --- /dev/null +++ b/tools/testing/selftests/bpf/test_bpftool_metadata.sh @@ -0,0 +1,82 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 + +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + +TESTNAME=bpftool_metadata +BPF_FS=$(awk '$3 == "bpf" {print $2; exit}' /proc/mounts) +BPF_DIR=$BPF_FS/test_$TESTNAME + +_cleanup() +{ + set +e + rm -rf $BPF_DIR 2> /dev/null +} + +cleanup_skip() +{ + echo "selftests: $TESTNAME [SKIP]" + _cleanup + + exit $ksft_skip +} + +cleanup() +{ + if [ "$?" = 0 ]; then + echo "selftests: $TESTNAME [PASS]" + else + echo "selftests: $TESTNAME [FAILED]" + fi + _cleanup +} + +if [ $(id -u) -ne 0 ]; then + echo "selftests: $TESTNAME [SKIP] Need root privileges" + exit $ksft_skip +fi + +if [ -z "$BPF_FS" ]; then + echo "selftests: $TESTNAME [SKIP] Could not run test without bpffs mounted" + exit $ksft_skip +fi + +if ! bpftool version > /dev/null 2>&1; then + echo "selftests: $TESTNAME [SKIP] Could not run test without bpftool" + exit $ksft_skip +fi + +set -e + +trap cleanup_skip EXIT + +mkdir $BPF_DIR + +trap cleanup EXIT + +bpftool prog load metadata_unused.o $BPF_DIR/unused + +METADATA_PLAIN="$(bpftool prog)" +echo "$METADATA_PLAIN" | grep 'a = "foo"' > /dev/null +echo "$METADATA_PLAIN" | grep 'b = 1' > /dev/null + +bpftool prog --json | grep '"metadata":{"a":"foo","b":1}' > /dev/null + +bpftool map | grep 'metadata.rodata' > /dev/null + +rm $BPF_DIR/unused + +bpftool prog load metadata_used.o $BPF_DIR/used + +METADATA_PLAIN="$(bpftool prog)" +echo "$METADATA_PLAIN" | grep 'a = "bar"' > /dev/null +echo "$METADATA_PLAIN" | grep 'b = 2' > /dev/null + +bpftool prog --json | grep '"metadata":{"a":"bar","b":2}' > /dev/null + +bpftool map | grep 'metadata.rodata' > /dev/null + +rm $BPF_DIR/used + +exit 0