public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check
@ 2024-09-09  7:13 Tao Chen
  2024-09-09  7:13 ` [v2 PATCH bpf-next 1/2] bpf: Check percpu map value size first Tao Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Tao Chen @ 2024-09-09  7:13 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Song Liu, Hou Tao, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev
  Cc: bpf, linux-kernel, Tao Chen

Check percpu map value size first and add the test case in selftest.

Change list:
- v1 -> v2:
    - round up map value size with 8 bytes in patch 1
    - add selftest case in patch 2

Tao Chen (2):
  bpf: Check percpu map value size first
  bpf/selftests: Check errno when percpu map value size exceeds

 kernel/bpf/arraymap.c                         |  3 ++
 kernel/bpf/hashtab.c                          |  3 ++
 .../selftests/bpf/prog_tests/map_init.c       | 32 +++++++++++++++++++
 .../selftests/bpf/progs/test_map_init.c       |  6 ++++
 4 files changed, 44 insertions(+)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [v2 PATCH bpf-next 1/2] bpf: Check percpu map value size first
  2024-09-09  7:13 [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check Tao Chen
@ 2024-09-09  7:13 ` Tao Chen
  2024-09-09 20:17   ` Andrii Nakryiko
  2024-09-09  7:13 ` [v2 PATCH bpf-next 2/2] bpf/selftests: Check errno when percpu map value size exceeds Tao Chen
  2024-09-09  9:17 ` [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check Jiri Olsa
  2 siblings, 1 reply; 8+ messages in thread
From: Tao Chen @ 2024-09-09  7:13 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Song Liu, Hou Tao, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev
  Cc: bpf, linux-kernel, Tao Chen, jinke han

Percpu map is often used, but the map value size limit often ignored,
like issue: https://github.com/iovisor/bcc/issues/2519. Actually,
percpu map value size is bound by PCPU_MIN_UNIT_SIZE, so we
can check the value size whether it exceeds PCPU_MIN_UNIT_SIZE first,
like percpu map of local_storage. Maybe the error message seems clearer
compared with "cannot allocate memory".

Signed-off-by: Tao Chen <chen.dylane@gmail.com>
Signed-off-by: jinke han <jinkehan@didiglobal.com>
---
 kernel/bpf/arraymap.c | 3 +++
 kernel/bpf/hashtab.c  | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index a43e62e2a8bb..79660e3fca4c 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -73,6 +73,9 @@ int array_map_alloc_check(union bpf_attr *attr)
 	/* avoid overflow on round_up(map->value_size) */
 	if (attr->value_size > INT_MAX)
 		return -E2BIG;
+	/* percpu map value size is bound by PCPU_MIN_UNIT_SIZE */
+	if (percpu && round_up(attr->value_size, 8) > PCPU_MIN_UNIT_SIZE)
+		return -E2BIG;
 
 	return 0;
 }
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 45c7195b65ba..b14b87463ee0 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -462,6 +462,9 @@ static int htab_map_alloc_check(union bpf_attr *attr)
 		 * kmalloc-able later in htab_map_update_elem()
 		 */
 		return -E2BIG;
+	/* percpu map value size is bound by PCPU_MIN_UNIT_SIZE */
+	if (percpu && round_up(attr->value_size, 8) > PCPU_MIN_UNIT_SIZE)
+		return -E2BIG;
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [v2 PATCH bpf-next 2/2] bpf/selftests: Check errno when percpu map value size exceeds
  2024-09-09  7:13 [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check Tao Chen
  2024-09-09  7:13 ` [v2 PATCH bpf-next 1/2] bpf: Check percpu map value size first Tao Chen
@ 2024-09-09  7:13 ` Tao Chen
  2024-09-09 20:16   ` Andrii Nakryiko
  2024-09-09  9:17 ` [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check Jiri Olsa
  2 siblings, 1 reply; 8+ messages in thread
From: Tao Chen @ 2024-09-09  7:13 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Song Liu, Hou Tao, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev
  Cc: bpf, linux-kernel, Tao Chen, jinke han

This test case checks the errno message when percpu map value size
exceeds PCPU_MIN_UNIT_SIZE.

root@debian:~# ./test_progs -t map_init
 #160/1   map_init/pcpu_map_init:OK
 #160/2   map_init/pcpu_lru_map_init:OK
 #160/3   map_init/pcpu map value size:OK
 #160     map_init:OK
Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Tao Chen <chen.dylane@gmail.com>
Signed-off-by: jinke han <jinkehan@didiglobal.com>
---
 .../selftests/bpf/prog_tests/map_init.c       | 32 +++++++++++++++++++
 .../selftests/bpf/progs/test_map_init.c       |  6 ++++
 2 files changed, 38 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
index 14a31109dd0e..7f1a6fa3679f 100644
--- a/tools/testing/selftests/bpf/prog_tests/map_init.c
+++ b/tools/testing/selftests/bpf/prog_tests/map_init.c
@@ -6,6 +6,7 @@
 
 #define TEST_VALUE 0x1234
 #define FILL_VALUE 0xdeadbeef
+#define PCPU_MIN_UNIT_SIZE 32768
 
 static int nr_cpus;
 static int duration;
@@ -118,6 +119,35 @@ static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
 
 	return 0;
 }
+/*
+ * percpu map value size is bound by PCPU_MIN_UNIT_SIZE
+ * check the errno when the value exceed PCPU_MIN_UNIT_SIZE
+ */
+static void test_pcpu_map_value_size(void)
+{
+	struct test_map_init *skel;
+	int err;
+	int value_sz = PCPU_MIN_UNIT_SIZE + 1;
+	enum bpf_map_type map_types[] = { BPF_MAP_TYPE_PERCPU_ARRAY,
+					  BPF_MAP_TYPE_PERCPU_HASH,
+					  BPF_MAP_TYPE_LRU_PERCPU_HASH };
+	for (int i = 0; i < ARRAY_SIZE(map_types); i++) {
+		skel = test_map_init__open();
+		if (!ASSERT_OK_PTR(skel, "skel_open"))
+			return;
+		err = bpf_map__set_type(skel->maps.hashmap2, map_types[i]);
+		if (!ASSERT_OK(err, "bpf_map__set_type"))
+			goto error;
+		err = bpf_map__set_value_size(skel->maps.hashmap2, value_sz);
+		if (!ASSERT_OK(err, "bpf_map__set_value_size"))
+			goto error;
+
+		err = test_map_init__load(skel);
+		ASSERT_EQ(err, -E2BIG, "skel_load");
+error:
+		test_map_init__destroy(skel);
+	}
+}
 
 /* Add key=1 elem with values set for all CPUs
  * Delete elem key=1
@@ -211,4 +241,6 @@ void test_map_init(void)
 		test_pcpu_map_init();
 	if (test__start_subtest("pcpu_lru_map_init"))
 		test_pcpu_lru_map_init();
+	if (test__start_subtest("pcpu map value size"))
+		test_pcpu_map_value_size();
 }
diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
index c89d28ead673..7a772cbf0570 100644
--- a/tools/testing/selftests/bpf/progs/test_map_init.c
+++ b/tools/testing/selftests/bpf/progs/test_map_init.c
@@ -15,6 +15,12 @@ struct {
 	__type(value, __u64);
 } hashmap1 SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 1);
+	__type(key, __u32);
+	__type(value, __u64);
+} hashmap2 SEC(".maps");
 
 SEC("tp/syscalls/sys_enter_getpgid")
 int sysenter_getpgid(const void *ctx)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check
  2024-09-09  7:13 [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check Tao Chen
  2024-09-09  7:13 ` [v2 PATCH bpf-next 1/2] bpf: Check percpu map value size first Tao Chen
  2024-09-09  7:13 ` [v2 PATCH bpf-next 2/2] bpf/selftests: Check errno when percpu map value size exceeds Tao Chen
@ 2024-09-09  9:17 ` Jiri Olsa
  2 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2024-09-09  9:17 UTC (permalink / raw)
  To: Tao Chen
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Song Liu, Hou Tao, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, bpf, linux-kernel

On Mon, Sep 09, 2024 at 03:13:44PM +0800, Tao Chen wrote:
> Check percpu map value size first and add the test case in selftest.
> 
> Change list:
> - v1 -> v2:
>     - round up map value size with 8 bytes in patch 1
>     - add selftest case in patch 2
> 
> Tao Chen (2):
>   bpf: Check percpu map value size first
>   bpf/selftests: Check errno when percpu map value size exceeds

lgtm

Acked-by: Jiri Olsa <jolsa@kernel.org>

jirka

> 
>  kernel/bpf/arraymap.c                         |  3 ++
>  kernel/bpf/hashtab.c                          |  3 ++
>  .../selftests/bpf/prog_tests/map_init.c       | 32 +++++++++++++++++++
>  .../selftests/bpf/progs/test_map_init.c       |  6 ++++
>  4 files changed, 44 insertions(+)
> 
> -- 
> 2.25.1
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v2 PATCH bpf-next 2/2] bpf/selftests: Check errno when percpu map value size exceeds
  2024-09-09  7:13 ` [v2 PATCH bpf-next 2/2] bpf/selftests: Check errno when percpu map value size exceeds Tao Chen
@ 2024-09-09 20:16   ` Andrii Nakryiko
  2024-09-10  2:40     ` Tao Chen
  0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2024-09-09 20:16 UTC (permalink / raw)
  To: Tao Chen
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Song Liu, Hou Tao, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, bpf, linux-kernel,
	jinke han

On Mon, Sep 9, 2024 at 12:14 AM Tao Chen <chen.dylane@gmail.com> wrote:
>
> This test case checks the errno message when percpu map value size
> exceeds PCPU_MIN_UNIT_SIZE.
>
> root@debian:~# ./test_progs -t map_init
>  #160/1   map_init/pcpu_map_init:OK
>  #160/2   map_init/pcpu_lru_map_init:OK
>  #160/3   map_init/pcpu map value size:OK
>  #160     map_init:OK
> Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
> Signed-off-by: jinke han <jinkehan@didiglobal.com>
> ---
>  .../selftests/bpf/prog_tests/map_init.c       | 32 +++++++++++++++++++
>  .../selftests/bpf/progs/test_map_init.c       |  6 ++++
>  2 files changed, 38 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
> index 14a31109dd0e..7f1a6fa3679f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/map_init.c
> +++ b/tools/testing/selftests/bpf/prog_tests/map_init.c
> @@ -6,6 +6,7 @@
>
>  #define TEST_VALUE 0x1234
>  #define FILL_VALUE 0xdeadbeef
> +#define PCPU_MIN_UNIT_SIZE 32768
>
>  static int nr_cpus;
>  static int duration;
> @@ -118,6 +119,35 @@ static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
>
>         return 0;
>  }
> +/*
> + * percpu map value size is bound by PCPU_MIN_UNIT_SIZE
> + * check the errno when the value exceed PCPU_MIN_UNIT_SIZE
> + */
> +static void test_pcpu_map_value_size(void)
> +{
> +       struct test_map_init *skel;
> +       int err;
> +       int value_sz = PCPU_MIN_UNIT_SIZE + 1;
> +       enum bpf_map_type map_types[] = { BPF_MAP_TYPE_PERCPU_ARRAY,
> +                                         BPF_MAP_TYPE_PERCPU_HASH,
> +                                         BPF_MAP_TYPE_LRU_PERCPU_HASH };
> +       for (int i = 0; i < ARRAY_SIZE(map_types); i++) {
> +               skel = test_map_init__open();
> +               if (!ASSERT_OK_PTR(skel, "skel_open"))
> +                       return;
> +               err = bpf_map__set_type(skel->maps.hashmap2, map_types[i]);
> +               if (!ASSERT_OK(err, "bpf_map__set_type"))
> +                       goto error;
> +               err = bpf_map__set_value_size(skel->maps.hashmap2, value_sz);
> +               if (!ASSERT_OK(err, "bpf_map__set_value_size"))
> +                       goto error;
> +
> +               err = test_map_init__load(skel);
> +               ASSERT_EQ(err, -E2BIG, "skel_load");

This is quite an overkill to test map creation. It will be much more
straightforward to just use low-level bpf_map_create() API, can you
please make use of that instead?

pw-bot: cr

> +error:
> +               test_map_init__destroy(skel);
> +       }
> +}
>
>  /* Add key=1 elem with values set for all CPUs
>   * Delete elem key=1
> @@ -211,4 +241,6 @@ void test_map_init(void)
>                 test_pcpu_map_init();
>         if (test__start_subtest("pcpu_lru_map_init"))
>                 test_pcpu_lru_map_init();
> +       if (test__start_subtest("pcpu map value size"))
> +               test_pcpu_map_value_size();
>  }
> diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
> index c89d28ead673..7a772cbf0570 100644
> --- a/tools/testing/selftests/bpf/progs/test_map_init.c
> +++ b/tools/testing/selftests/bpf/progs/test_map_init.c
> @@ -15,6 +15,12 @@ struct {
>         __type(value, __u64);
>  } hashmap1 SEC(".maps");
>
> +struct {
> +       __uint(type, BPF_MAP_TYPE_HASH);
> +       __uint(max_entries, 1);
> +       __type(key, __u32);
> +       __type(value, __u64);
> +} hashmap2 SEC(".maps");
>
>  SEC("tp/syscalls/sys_enter_getpgid")
>  int sysenter_getpgid(const void *ctx)
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v2 PATCH bpf-next 1/2] bpf: Check percpu map value size first
  2024-09-09  7:13 ` [v2 PATCH bpf-next 1/2] bpf: Check percpu map value size first Tao Chen
@ 2024-09-09 20:17   ` Andrii Nakryiko
  2024-09-10  2:35     ` Tao Chen
  0 siblings, 1 reply; 8+ messages in thread
From: Andrii Nakryiko @ 2024-09-09 20:17 UTC (permalink / raw)
  To: Tao Chen
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Song Liu, Hou Tao, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, bpf, linux-kernel,
	jinke han

On Mon, Sep 9, 2024 at 12:14 AM Tao Chen <chen.dylane@gmail.com> wrote:
>
> Percpu map is often used, but the map value size limit often ignored,
> like issue: https://github.com/iovisor/bcc/issues/2519. Actually,
> percpu map value size is bound by PCPU_MIN_UNIT_SIZE, so we
> can check the value size whether it exceeds PCPU_MIN_UNIT_SIZE first,
> like percpu map of local_storage. Maybe the error message seems clearer
> compared with "cannot allocate memory".
>
> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
> Signed-off-by: jinke han <jinkehan@didiglobal.com>

names in SOB should be capitalized

the check is useful, so:

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

> ---
>  kernel/bpf/arraymap.c | 3 +++
>  kernel/bpf/hashtab.c  | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index a43e62e2a8bb..79660e3fca4c 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -73,6 +73,9 @@ int array_map_alloc_check(union bpf_attr *attr)
>         /* avoid overflow on round_up(map->value_size) */
>         if (attr->value_size > INT_MAX)
>                 return -E2BIG;
> +       /* percpu map value size is bound by PCPU_MIN_UNIT_SIZE */
> +       if (percpu && round_up(attr->value_size, 8) > PCPU_MIN_UNIT_SIZE)
> +               return -E2BIG;
>
>         return 0;
>  }
> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 45c7195b65ba..b14b87463ee0 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
> @@ -462,6 +462,9 @@ static int htab_map_alloc_check(union bpf_attr *attr)
>                  * kmalloc-able later in htab_map_update_elem()
>                  */
>                 return -E2BIG;
> +       /* percpu map value size is bound by PCPU_MIN_UNIT_SIZE */
> +       if (percpu && round_up(attr->value_size, 8) > PCPU_MIN_UNIT_SIZE)
> +               return -E2BIG;
>
>         return 0;
>  }
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v2 PATCH bpf-next 1/2] bpf: Check percpu map value size first
  2024-09-09 20:17   ` Andrii Nakryiko
@ 2024-09-10  2:35     ` Tao Chen
  0 siblings, 0 replies; 8+ messages in thread
From: Tao Chen @ 2024-09-10  2:35 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Song Liu, Hou Tao, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, bpf, linux-kernel,
	jinke han

在 2024/9/10 04:17, Andrii Nakryiko 写道:
> On Mon, Sep 9, 2024 at 12:14 AM Tao Chen <chen.dylane@gmail.com> wrote:
>>
>> Percpu map is often used, but the map value size limit often ignored,
>> like issue: https://github.com/iovisor/bcc/issues/2519. Actually,
>> percpu map value size is bound by PCPU_MIN_UNIT_SIZE, so we
>> can check the value size whether it exceeds PCPU_MIN_UNIT_SIZE first,
>> like percpu map of local_storage. Maybe the error message seems clearer
>> compared with "cannot allocate memory".
>>
>> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
>> Signed-off-by: jinke han <jinkehan@didiglobal.com>
> 
> names in SOB should be capitalized

Hi Andrii,thank you for your attention, i will change it.

> 
> the check is useful, so:
> 
> Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
>> ---
>>   kernel/bpf/arraymap.c | 3 +++
>>   kernel/bpf/hashtab.c  | 3 +++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>> index a43e62e2a8bb..79660e3fca4c 100644
>> --- a/kernel/bpf/arraymap.c
>> +++ b/kernel/bpf/arraymap.c
>> @@ -73,6 +73,9 @@ int array_map_alloc_check(union bpf_attr *attr)
>>          /* avoid overflow on round_up(map->value_size) */
>>          if (attr->value_size > INT_MAX)
>>                  return -E2BIG;
>> +       /* percpu map value size is bound by PCPU_MIN_UNIT_SIZE */
>> +       if (percpu && round_up(attr->value_size, 8) > PCPU_MIN_UNIT_SIZE)
>> +               return -E2BIG;
>>
>>          return 0;
>>   }
>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>> index 45c7195b65ba..b14b87463ee0 100644
>> --- a/kernel/bpf/hashtab.c
>> +++ b/kernel/bpf/hashtab.c
>> @@ -462,6 +462,9 @@ static int htab_map_alloc_check(union bpf_attr *attr)
>>                   * kmalloc-able later in htab_map_update_elem()
>>                   */
>>                  return -E2BIG;
>> +       /* percpu map value size is bound by PCPU_MIN_UNIT_SIZE */
>> +       if (percpu && round_up(attr->value_size, 8) > PCPU_MIN_UNIT_SIZE)
>> +               return -E2BIG;
>>
>>          return 0;
>>   }
>> --
>> 2.25.1
>>


-- 
Best Regards
Dylane Chen

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v2 PATCH bpf-next 2/2] bpf/selftests: Check errno when percpu map value size exceeds
  2024-09-09 20:16   ` Andrii Nakryiko
@ 2024-09-10  2:40     ` Tao Chen
  0 siblings, 0 replies; 8+ messages in thread
From: Tao Chen @ 2024-09-10  2:40 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Song Liu, Hou Tao, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, bpf, linux-kernel,
	jinke han

在 2024/9/10 04:16, Andrii Nakryiko 写道:
> On Mon, Sep 9, 2024 at 12:14 AM Tao Chen <chen.dylane@gmail.com> wrote:
>>
>> This test case checks the errno message when percpu map value size
>> exceeds PCPU_MIN_UNIT_SIZE.
>>
>> root@debian:~# ./test_progs -t map_init
>>   #160/1   map_init/pcpu_map_init:OK
>>   #160/2   map_init/pcpu_lru_map_init:OK
>>   #160/3   map_init/pcpu map value size:OK
>>   #160     map_init:OK
>> Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED
>>
>> Signed-off-by: Tao Chen <chen.dylane@gmail.com>
>> Signed-off-by: jinke han <jinkehan@didiglobal.com>
>> ---
>>   .../selftests/bpf/prog_tests/map_init.c       | 32 +++++++++++++++++++
>>   .../selftests/bpf/progs/test_map_init.c       |  6 ++++
>>   2 files changed, 38 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/map_init.c b/tools/testing/selftests/bpf/prog_tests/map_init.c
>> index 14a31109dd0e..7f1a6fa3679f 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/map_init.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/map_init.c
>> @@ -6,6 +6,7 @@
>>
>>   #define TEST_VALUE 0x1234
>>   #define FILL_VALUE 0xdeadbeef
>> +#define PCPU_MIN_UNIT_SIZE 32768
>>
>>   static int nr_cpus;
>>   static int duration;
>> @@ -118,6 +119,35 @@ static int check_values_one_cpu(pcpu_map_value_t *value, map_value_t expected)
>>
>>          return 0;
>>   }
>> +/*
>> + * percpu map value size is bound by PCPU_MIN_UNIT_SIZE
>> + * check the errno when the value exceed PCPU_MIN_UNIT_SIZE
>> + */
>> +static void test_pcpu_map_value_size(void)
>> +{
>> +       struct test_map_init *skel;
>> +       int err;
>> +       int value_sz = PCPU_MIN_UNIT_SIZE + 1;
>> +       enum bpf_map_type map_types[] = { BPF_MAP_TYPE_PERCPU_ARRAY,
>> +                                         BPF_MAP_TYPE_PERCPU_HASH,
>> +                                         BPF_MAP_TYPE_LRU_PERCPU_HASH };
>> +       for (int i = 0; i < ARRAY_SIZE(map_types); i++) {
>> +               skel = test_map_init__open();
>> +               if (!ASSERT_OK_PTR(skel, "skel_open"))
>> +                       return;
>> +               err = bpf_map__set_type(skel->maps.hashmap2, map_types[i]);
>> +               if (!ASSERT_OK(err, "bpf_map__set_type"))
>> +                       goto error;
>> +               err = bpf_map__set_value_size(skel->maps.hashmap2, value_sz);
>> +               if (!ASSERT_OK(err, "bpf_map__set_value_size"))
>> +                       goto error;
>> +
>> +               err = test_map_init__load(skel);
>> +               ASSERT_EQ(err, -E2BIG, "skel_load");
> 
> This is quite an overkill to test map creation. It will be much more
> straightforward to just use low-level bpf_map_create() API, can you
> please make use of that instead?
> 
> pw-bot: cr
> 

Ok, i will use the bpf_map_create API in v3.

>> +error:
>> +               test_map_init__destroy(skel);
>> +       }
>> +}
>>
>>   /* Add key=1 elem with values set for all CPUs
>>    * Delete elem key=1
>> @@ -211,4 +241,6 @@ void test_map_init(void)
>>                  test_pcpu_map_init();
>>          if (test__start_subtest("pcpu_lru_map_init"))
>>                  test_pcpu_lru_map_init();
>> +       if (test__start_subtest("pcpu map value size"))
>> +               test_pcpu_map_value_size();
>>   }
>> diff --git a/tools/testing/selftests/bpf/progs/test_map_init.c b/tools/testing/selftests/bpf/progs/test_map_init.c
>> index c89d28ead673..7a772cbf0570 100644
>> --- a/tools/testing/selftests/bpf/progs/test_map_init.c
>> +++ b/tools/testing/selftests/bpf/progs/test_map_init.c
>> @@ -15,6 +15,12 @@ struct {
>>          __type(value, __u64);
>>   } hashmap1 SEC(".maps");
>>
>> +struct {
>> +       __uint(type, BPF_MAP_TYPE_HASH);
>> +       __uint(max_entries, 1);
>> +       __type(key, __u32);
>> +       __type(value, __u64);
>> +} hashmap2 SEC(".maps");
>>
>>   SEC("tp/syscalls/sys_enter_getpgid")
>>   int sysenter_getpgid(const void *ctx)
>> --
>> 2.25.1
>>


-- 
Best Regards
Dylane Chen

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-09-10  2:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-09  7:13 [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check Tao Chen
2024-09-09  7:13 ` [v2 PATCH bpf-next 1/2] bpf: Check percpu map value size first Tao Chen
2024-09-09 20:17   ` Andrii Nakryiko
2024-09-10  2:35     ` Tao Chen
2024-09-09  7:13 ` [v2 PATCH bpf-next 2/2] bpf/selftests: Check errno when percpu map value size exceeds Tao Chen
2024-09-09 20:16   ` Andrii Nakryiko
2024-09-10  2:40     ` Tao Chen
2024-09-09  9:17 ` [v2 PATCH bpf-next 0/2] bpf: Add percpu map value size check Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox