All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.