Re: [PATCH bpf-next 3/3] libbpf: add non-mmapable data section selftest

[sdf@google.com]

On 10/17, Andrii Nakryiko wrote:
> Add non-mmapable data section to test_skeleton selftest and make sure it
> really isn't mmapable by trying to mmap() it anyways.

> Also make sure that libbpf doesn't report BPF_F_MMAPABLE flag to users.

> Additional, some more manual testing was performed that this feature
> works as intended.

> Looking at created map through bpftool shows that flags passed to kernel  
> are
> indeed zero:

>    $ bpftool map show
>    ...
>    1782: array  name .data.non_mmapa  flags 0x0
>            key 4B  value 16B  max_entries 1  memlock 4096B
>            btf_id 1169
>            pids test_progs(8311)
>    ...

> Checking BTF uploaded to kernel for this map shows that zero_key and
> zero_value are indeed marked as static, even though zero_key is actually
> original global (but STV_HIDDEN) variable:

>    $ bpftool btf dump id 1169
>    ...
>    [51] VAR 'zero_key' type_id=2, linkage=static
>    [52] VAR 'zero_value' type_id=7, linkage=static
>    ...
>    [62] DATASEC '.data.non_mmapable' size=16 vlen=2
>            type_id=51 offset=0 size=4 (VAR 'zero_key')
>            type_id=52 offset=4 size=12 (VAR 'zero_value')
>    ...

> And original BTF does have zero_key marked as linkage=global:

>    $ bpftool btf dump file test_skeleton.bpf.linked3.o
>    ...
>    [51] VAR 'zero_key' type_id=2, linkage=global
>    [52] VAR 'zero_value' type_id=7, linkage=static
>    ...
>    [62] DATASEC '.data.non_mmapable' size=16 vlen=2
>            type_id=51 offset=0 size=4 (VAR 'zero_key')
>            type_id=52 offset=4 size=12 (VAR 'zero_value')

> Bpftool didn't require any changes at all because it checks whether  
> internal
> map is mmapable already, but just to double-check generated skeleton, we
> see that .data.non_mmapable neither sets mmaped pointer nor has
> a corresponding field in the skeleton:

>    $ grep non_mmapable test_skeleton.skel.h
>                    struct bpf_map *data_non_mmapable;
>            s->maps[7].name = ".data.non_mmapable";
>            s->maps[7].map = &obj->maps.data_non_mmapable;

> But .data.read_mostly has all of those things:

>    $ grep read_mostly test_skeleton.skel.h
>                    struct bpf_map *data_read_mostly;
>            struct test_skeleton__data_read_mostly {
>                    int read_mostly_var;
>            } *data_read_mostly;
>            s->maps[6].name = ".data.read_mostly";
>            s->maps[6].map = &obj->maps.data_read_mostly;
>            s->maps[6].mmaped = (void **)&obj->data_read_mostly;
>            _Static_assert(sizeof(s->data_read_mostly->read_mostly_var) ==  
> 4, "unexpected size of 'read_mostly_var'");

> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Stanislav Fomichev <sdf@google.com>

> ---
>   .../testing/selftests/bpf/prog_tests/skeleton.c | 11 ++++++++++-
>   .../testing/selftests/bpf/progs/test_skeleton.c | 17 +++++++++++++++++
>   2 files changed, 27 insertions(+), 1 deletion(-)

> diff --git a/tools/testing/selftests/bpf/prog_tests/skeleton.c  
> b/tools/testing/selftests/bpf/prog_tests/skeleton.c
> index 99dac5292b41..bc6817aee9aa 100644
> --- a/tools/testing/selftests/bpf/prog_tests/skeleton.c
> +++ b/tools/testing/selftests/bpf/prog_tests/skeleton.c
> @@ -2,6 +2,7 @@
>   /* Copyright (c) 2019 Facebook */

>   #include <test_progs.h>
> +#include <sys/mman.h>

>   struct s {
>   	int a;
> @@ -22,7 +23,8 @@ void test_skeleton(void)
>   	struct test_skeleton__kconfig *kcfg;
>   	const void *elf_bytes;
>   	size_t elf_bytes_sz = 0;
> -	int i;
> +	void *m;
> +	int i, fd;

>   	skel = test_skeleton__open();
>   	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> @@ -124,6 +126,13 @@ void test_skeleton(void)

>   	ASSERT_EQ(bss->huge_arr[ARRAY_SIZE(bss->huge_arr) - 1],  
> 123, "huge_arr");

> +	fd = bpf_map__fd(skel->maps.data_non_mmapable);
> +	m = mmap(NULL, getpagesize(), PROT_READ, MAP_SHARED, fd, 0);
> +	if (!ASSERT_EQ(m, MAP_FAILED, "unexpected_mmap_success"))
> +		munmap(m, getpagesize());
> +
> +	ASSERT_EQ(bpf_map__map_flags(skel->maps.data_non_mmapable),  
> 0, "non_mmap_flags");
> +
>   	elf_bytes = test_skeleton__elf_bytes(&elf_bytes_sz);
>   	ASSERT_OK_PTR(elf_bytes, "elf_bytes");
>   	ASSERT_GE(elf_bytes_sz, 0, "elf_bytes_sz");
> diff --git a/tools/testing/selftests/bpf/progs/test_skeleton.c  
> b/tools/testing/selftests/bpf/progs/test_skeleton.c
> index 1a4e93f6d9df..adece9f91f58 100644
> --- a/tools/testing/selftests/bpf/progs/test_skeleton.c
> +++ b/tools/testing/selftests/bpf/progs/test_skeleton.c
> @@ -53,6 +53,20 @@ int out_mostly_var;

>   char huge_arr[16 * 1024 * 1024];

> +/* non-mmapable custom .data section */
> +
> +struct my_value { int x, y, z; };
> +
> +__hidden int zero_key SEC(".data.non_mmapable");
> +static struct my_value zero_value SEC(".data.non_mmapable");
> +
> +struct {
> +	__uint(type, BPF_MAP_TYPE_ARRAY);
> +	__type(key, int);
> +	__type(value, struct my_value);
> +	__uint(max_entries, 1);
> +} my_map SEC(".maps");
> +
>   SEC("raw_tp/sys_enter")
>   int handler(const void *ctx)
>   {
> @@ -75,6 +89,9 @@ int handler(const void *ctx)

>   	huge_arr[sizeof(huge_arr) - 1] = 123;

> +	/* make sure zero_key and zero_value are not optimized out */
> +	bpf_map_update_elem(&my_map, &zero_key, &zero_value, BPF_ANY);
> +
>   	return 0;
>   }

> --
> 2.30.2