* [PATCH bpf-next 0/3] "map_extra" and bloom filter fixups
@ 2021-10-29 17:01 Joanne Koong
2021-10-29 17:01 ` [PATCH bpf-next 1/3] bpf: Bloom filter map naming fixups Joanne Koong
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Joanne Koong @ 2021-10-29 17:01 UTC (permalink / raw)
To: bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team, Joanne Koong
There are 3 patches in this patchset:
1/3 - Bloom filter naming fixups (kernel/bpf/bloom_filter.c)
2/3 - Add alignment padding for map_extra, rearrange fields in
bpf_map struct to consolidate holes
3/3 - Bloom filter tests (prog_tests/bloom_filter_map):
Add test for successful userspace calls, some refactoring to
use bpf_create_map instead of bpf_create_map_xattr
Joanne Koong (3):
bpf: Bloom filter map naming fixups
bpf: Add alignment padding for "map_extra" + consolidate holes
selftests/bpf: Add bloom map success test for userspace calls
include/linux/bpf.h | 6 +--
include/uapi/linux/bpf.h | 1 +
kernel/bpf/bloom_filter.c | 49 +++++++++--------
tools/include/uapi/linux/bpf.h | 1 +
.../bpf/prog_tests/bloom_filter_map.c | 53 ++++++++++++-------
5 files changed, 64 insertions(+), 46 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next 1/3] bpf: Bloom filter map naming fixups
2021-10-29 17:01 [PATCH bpf-next 0/3] "map_extra" and bloom filter fixups Joanne Koong
@ 2021-10-29 17:01 ` Joanne Koong
2021-10-29 21:28 ` Yonghong Song
2021-10-29 17:01 ` [PATCH bpf-next 2/3] bpf: Add alignment padding for "map_extra" + consolidate holes Joanne Koong
2021-10-29 17:01 ` [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls Joanne Koong
2 siblings, 1 reply; 9+ messages in thread
From: Joanne Koong @ 2021-10-29 17:01 UTC (permalink / raw)
To: bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team, Joanne Koong
This patch has two changes in the kernel bloom filter map
implementation:
1) Change the names of map-ops functions to include the
"bloom_map" prefix.
As Martin pointed out on a previous patchset, having generic
map-ops names may be confusing in tracing and in perf-report.
2) Drop the "& 0xF" when getting nr_hash_funcs, since we
already ascertain that no other bits in map_extra beyond the
first 4 bits can be set.
Signed-off-by: Joanne Koong <joannekoong@fb.com>
---
kernel/bpf/bloom_filter.c | 49 +++++++++++++++++++++------------------
1 file changed, 26 insertions(+), 23 deletions(-)
diff --git a/kernel/bpf/bloom_filter.c b/kernel/bpf/bloom_filter.c
index 7c50232b7571..073c2f2cab8b 100644
--- a/kernel/bpf/bloom_filter.c
+++ b/kernel/bpf/bloom_filter.c
@@ -40,7 +40,7 @@ static u32 hash(struct bpf_bloom_filter *bloom, void *value,
return h & bloom->bitset_mask;
}
-static int peek_elem(struct bpf_map *map, void *value)
+static int bloom_map_peek_elem(struct bpf_map *map, void *value)
{
struct bpf_bloom_filter *bloom =
container_of(map, struct bpf_bloom_filter, map);
@@ -55,7 +55,7 @@ static int peek_elem(struct bpf_map *map, void *value)
return 0;
}
-static int push_elem(struct bpf_map *map, void *value, u64 flags)
+static int bloom_map_push_elem(struct bpf_map *map, void *value, u64 flags)
{
struct bpf_bloom_filter *bloom =
container_of(map, struct bpf_bloom_filter, map);
@@ -72,12 +72,12 @@ static int push_elem(struct bpf_map *map, void *value, u64 flags)
return 0;
}
-static int pop_elem(struct bpf_map *map, void *value)
+static int bloom_map_pop_elem(struct bpf_map *map, void *value)
{
return -EOPNOTSUPP;
}
-static struct bpf_map *map_alloc(union bpf_attr *attr)
+static struct bpf_map *bloom_map_alloc(union bpf_attr *attr)
{
u32 bitset_bytes, bitset_mask, nr_hash_funcs, nr_bits;
int numa_node = bpf_map_attr_numa_node(attr);
@@ -90,11 +90,13 @@ static struct bpf_map *map_alloc(union bpf_attr *attr)
attr->max_entries == 0 ||
attr->map_flags & ~BLOOM_CREATE_FLAG_MASK ||
!bpf_map_flags_access_ok(attr->map_flags) ||
+ /* The lower 4 bits of map_extra (0xF) specify the number
+ * of hash functions
+ */
(attr->map_extra & ~0xF))
return ERR_PTR(-EINVAL);
- /* The lower 4 bits of map_extra specify the number of hash functions */
- nr_hash_funcs = attr->map_extra & 0xF;
+ nr_hash_funcs = attr->map_extra;
if (nr_hash_funcs == 0)
/* Default to using 5 hash functions if unspecified */
nr_hash_funcs = 5;
@@ -150,7 +152,7 @@ static struct bpf_map *map_alloc(union bpf_attr *attr)
return &bloom->map;
}
-static void map_free(struct bpf_map *map)
+static void bloom_map_free(struct bpf_map *map)
{
struct bpf_bloom_filter *bloom =
container_of(map, struct bpf_bloom_filter, map);
@@ -158,38 +160,39 @@ static void map_free(struct bpf_map *map)
bpf_map_area_free(bloom);
}
-static void *lookup_elem(struct bpf_map *map, void *key)
+static void *bloom_map_lookup_elem(struct bpf_map *map, void *key)
{
/* The eBPF program should use map_peek_elem instead */
return ERR_PTR(-EINVAL);
}
-static int update_elem(struct bpf_map *map, void *key,
- void *value, u64 flags)
+static int bloom_map_update_elem(struct bpf_map *map, void *key,
+ void *value, u64 flags)
{
/* The eBPF program should use map_push_elem instead */
return -EINVAL;
}
-static int check_btf(const struct bpf_map *map, const struct btf *btf,
- const struct btf_type *key_type,
- const struct btf_type *value_type)
+static int bloom_map_check_btf(const struct bpf_map *map,
+ const struct btf *btf,
+ const struct btf_type *key_type,
+ const struct btf_type *value_type)
{
/* Bloom filter maps are keyless */
return btf_type_is_void(key_type) ? 0 : -EINVAL;
}
-static int bpf_bloom_btf_id;
+static int bpf_bloom_map_btf_id;
const struct bpf_map_ops bloom_filter_map_ops = {
.map_meta_equal = bpf_map_meta_equal,
- .map_alloc = map_alloc,
- .map_free = map_free,
- .map_push_elem = push_elem,
- .map_peek_elem = peek_elem,
- .map_pop_elem = pop_elem,
- .map_lookup_elem = lookup_elem,
- .map_update_elem = update_elem,
- .map_check_btf = check_btf,
+ .map_alloc = bloom_map_alloc,
+ .map_free = bloom_map_free,
+ .map_push_elem = bloom_map_push_elem,
+ .map_peek_elem = bloom_map_peek_elem,
+ .map_pop_elem = bloom_map_pop_elem,
+ .map_lookup_elem = bloom_map_lookup_elem,
+ .map_update_elem = bloom_map_update_elem,
+ .map_check_btf = bloom_map_check_btf,
.map_btf_name = "bpf_bloom_filter",
- .map_btf_id = &bpf_bloom_btf_id,
+ .map_btf_id = &bpf_bloom_map_btf_id,
};
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next 2/3] bpf: Add alignment padding for "map_extra" + consolidate holes
2021-10-29 17:01 [PATCH bpf-next 0/3] "map_extra" and bloom filter fixups Joanne Koong
2021-10-29 17:01 ` [PATCH bpf-next 1/3] bpf: Bloom filter map naming fixups Joanne Koong
@ 2021-10-29 17:01 ` Joanne Koong
2021-10-29 21:29 ` Yonghong Song
2021-10-29 17:01 ` [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls Joanne Koong
2 siblings, 1 reply; 9+ messages in thread
From: Joanne Koong @ 2021-10-29 17:01 UTC (permalink / raw)
To: bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team, Joanne Koong
This patch makes 2 changes regarding alignment padding
for the "map_extra" field.
1) In the kernel header, "map_extra" and "btf_value_type_id"
are rearranged to consolidate the hole.
Before:
struct bpf_map {
...
u32 max_entries; /* 36 4 */
u32 map_flags; /* 40 4 */
/* XXX 4 bytes hole, try to pack */
u64 map_extra; /* 48 8 */
int spin_lock_off; /* 56 4 */
int timer_off; /* 60 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
u32 id; /* 64 4 */
int numa_node; /* 68 4 */
...
bool frozen; /* 117 1 */
/* XXX 10 bytes hole, try to pack */
/* --- cacheline 2 boundary (128 bytes) --- */
...
struct work_struct work; /* 144 72 */
/* --- cacheline 3 boundary (192 bytes) was 24 bytes ago --- */
struct mutex freeze_mutex; /* 216 144 */
/* --- cacheline 5 boundary (320 bytes) was 40 bytes ago --- */
u64 writecnt; /* 360 8 */
/* size: 384, cachelines: 6, members: 26 */
/* sum members: 354, holes: 2, sum holes: 14 */
/* padding: 16 */
/* forced alignments: 2, forced holes: 1, sum forced holes: 10 */
} __attribute__((__aligned__(64)));
After:
struct bpf_map {
...
u32 max_entries; /* 36 4 */
u64 map_extra; /* 40 8 */
u32 map_flags; /* 48 4 */
int spin_lock_off; /* 52 4 */
int timer_off; /* 56 4 */
u32 id; /* 60 4 */
/* --- cacheline 1 boundary (64 bytes) --- */
int numa_node; /* 64 4 */
...
bool frozen /* 113 1 */
/* XXX 14 bytes hole, try to pack */
/* --- cacheline 2 boundary (128 bytes) --- */
...
struct work_struct work; /* 144 72 */
/* --- cacheline 3 boundary (192 bytes) was 24 bytes ago --- */
struct mutex freeze_mutex; /* 216 144 */
/* --- cacheline 5 boundary (320 bytes) was 40 bytes ago --- */
u64 writecnt; /* 360 8 */
/* size: 384, cachelines: 6, members: 26 */
/* sum members: 354, holes: 1, sum holes: 14 */
/* padding: 16 */
/* forced alignments: 2, forced holes: 1, sum forced holes: 14 */
} __attribute__((__aligned__(64)));
2) Add alignment padding to the bpf_map_info struct
More details can be found in commit 36f9814a494a ("bpf: fix uapi hole
for 32 bit compat applications")
Signed-off-by: Joanne Koong <joannekoong@fb.com>
---
include/linux/bpf.h | 6 +++---
include/uapi/linux/bpf.h | 1 +
tools/include/uapi/linux/bpf.h | 1 +
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 6deebf8bf78f..d695b2ef67d5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -168,23 +168,23 @@ struct bpf_map {
u32 key_size;
u32 value_size;
u32 max_entries;
- u32 map_flags;
u64 map_extra; /* any per-map-type extra fields */
+ u32 map_flags;
int spin_lock_off; /* >=0 valid offset, <0 error */
int timer_off; /* >=0 valid offset, <0 error */
u32 id;
int numa_node;
u32 btf_key_type_id;
u32 btf_value_type_id;
+ u32 btf_vmlinux_value_type_id;
struct btf *btf;
#ifdef CONFIG_MEMCG_KMEM
struct mem_cgroup *memcg;
#endif
char name[BPF_OBJ_NAME_LEN];
- u32 btf_vmlinux_value_type_id;
bool bypass_spec_v1;
bool frozen; /* write-once; write-protected by freeze_mutex */
- /* 22 bytes hole */
+ /* 14 bytes hole */
/* The 3rd and 4th cacheline with misc members to avoid false sharing
* particularly with refcounting.
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bd0c9f0487f6..ba5af15e25f5 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5662,6 +5662,7 @@ struct bpf_map_info {
__u32 btf_id;
__u32 btf_key_type_id;
__u32 btf_value_type_id;
+ __u32 :32; /* alignment pad */
__u64 map_extra;
} __attribute__((aligned(8)));
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bd0c9f0487f6..ba5af15e25f5 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5662,6 +5662,7 @@ struct bpf_map_info {
__u32 btf_id;
__u32 btf_key_type_id;
__u32 btf_value_type_id;
+ __u32 :32; /* alignment pad */
__u64 map_extra;
} __attribute__((aligned(8)));
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls
2021-10-29 17:01 [PATCH bpf-next 0/3] "map_extra" and bloom filter fixups Joanne Koong
2021-10-29 17:01 ` [PATCH bpf-next 1/3] bpf: Bloom filter map naming fixups Joanne Koong
2021-10-29 17:01 ` [PATCH bpf-next 2/3] bpf: Add alignment padding for "map_extra" + consolidate holes Joanne Koong
@ 2021-10-29 17:01 ` Joanne Koong
2021-10-29 22:04 ` Yonghong Song
2 siblings, 1 reply; 9+ messages in thread
From: Joanne Koong @ 2021-10-29 17:01 UTC (permalink / raw)
To: bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team, Joanne Koong
This patch has two changes:
1) Adds a new function "test_success_cases" to test
successfully creating + adding + looking up a value
in a bloom filter map from the userspace side.
2) Use bpf_create_map instead of bpf_create_map_xattr in
the "test_fail_cases" to make the code look cleaner.
Signed-off-by: Joanne Koong <joannekoong@fb.com>
---
.../bpf/prog_tests/bloom_filter_map.c | 53 ++++++++++++-------
1 file changed, 33 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
index 9aa3fbed918b..dbc0035e43e5 100644
--- a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
@@ -7,44 +7,32 @@
static void test_fail_cases(void)
{
- struct bpf_create_map_attr xattr = {
- .name = "bloom_filter_map",
- .map_type = BPF_MAP_TYPE_BLOOM_FILTER,
- .max_entries = 100,
- .value_size = 11,
- };
__u32 value;
int fd, err;
/* Invalid key size */
- xattr.key_size = 4;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 4, sizeof(value), 100, 0);
if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid key size"))
close(fd);
- xattr.key_size = 0;
/* Invalid value size */
- xattr.value_size = 0;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, 0, 100, 0);
if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid value size 0"))
close(fd);
- xattr.value_size = 11;
/* Invalid max entries size */
- xattr.max_entries = 0;
- fd = bpf_create_map_xattr(&xattr);
- if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid max entries size"))
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 0, 0);
+ if (!ASSERT_LT(fd, 0,
+ "bpf_create_map bloom filter invalid max entries size"))
close(fd);
- xattr.max_entries = 100;
/* Bloom filter maps do not support BPF_F_NO_PREALLOC */
- xattr.map_flags = BPF_F_NO_PREALLOC;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 100,
+ BPF_F_NO_PREALLOC);
if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid flags"))
close(fd);
- xattr.map_flags = 0;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 100, 0);
if (!ASSERT_GE(fd, 0, "bpf_create_map bloom filter"))
return;
@@ -67,6 +55,30 @@ static void test_fail_cases(void)
close(fd);
}
+static void test_success_cases(void)
+{
+ char value[11];
+ int fd, err;
+
+ /* Create a map */
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 100,
+ BPF_F_ZERO_SEED | BPF_F_NUMA_NODE);
+ if (!ASSERT_GE(fd, 0, "bpf_create_map bloom filter success case"))
+ return;
+
+ /* Add a value to the bloom filter */
+ err = bpf_map_update_elem(fd, NULL, &value, 0);
+ if (!ASSERT_OK(err, "bpf_map_update_elem bloom filter success case"))
+ goto done;
+
+ /* Lookup a value in the bloom filter */
+ err = bpf_map_lookup_elem(fd, NULL, &value);
+ ASSERT_OK(err, "bpf_map_update_elem bloom filter success case");
+
+done:
+ close(fd);
+}
+
static void check_bloom(struct bloom_filter_map *skel)
{
struct bpf_link *link;
@@ -190,6 +202,7 @@ void test_bloom_filter_map(void)
int err;
test_fail_cases();
+ test_success_cases();
err = setup_progs(&skel, &rand_vals, &nr_rand_vals);
if (err)
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 1/3] bpf: Bloom filter map naming fixups
2021-10-29 17:01 ` [PATCH bpf-next 1/3] bpf: Bloom filter map naming fixups Joanne Koong
@ 2021-10-29 21:28 ` Yonghong Song
0 siblings, 0 replies; 9+ messages in thread
From: Yonghong Song @ 2021-10-29 21:28 UTC (permalink / raw)
To: Joanne Koong, bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team
On 10/29/21 10:01 AM, Joanne Koong wrote:
> This patch has two changes in the kernel bloom filter map
> implementation:
>
> 1) Change the names of map-ops functions to include the
> "bloom_map" prefix.
>
> As Martin pointed out on a previous patchset, having generic
> map-ops names may be confusing in tracing and in perf-report.
>
> 2) Drop the "& 0xF" when getting nr_hash_funcs, since we
> already ascertain that no other bits in map_extra beyond the
> first 4 bits can be set.
>
> Signed-off-by: Joanne Koong <joannekoong@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: Add alignment padding for "map_extra" + consolidate holes
2021-10-29 17:01 ` [PATCH bpf-next 2/3] bpf: Add alignment padding for "map_extra" + consolidate holes Joanne Koong
@ 2021-10-29 21:29 ` Yonghong Song
0 siblings, 0 replies; 9+ messages in thread
From: Yonghong Song @ 2021-10-29 21:29 UTC (permalink / raw)
To: Joanne Koong, bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team
On 10/29/21 10:01 AM, Joanne Koong wrote:
> This patch makes 2 changes regarding alignment padding
> for the "map_extra" field.
>
> 1) In the kernel header, "map_extra" and "btf_value_type_id"
> are rearranged to consolidate the hole.
>
> Before:
> struct bpf_map {
> ...
> u32 max_entries; /* 36 4 */
> u32 map_flags; /* 40 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> u64 map_extra; /* 48 8 */
> int spin_lock_off; /* 56 4 */
> int timer_off; /* 60 4 */
> /* --- cacheline 1 boundary (64 bytes) --- */
> u32 id; /* 64 4 */
> int numa_node; /* 68 4 */
> ...
> bool frozen; /* 117 1 */
>
> /* XXX 10 bytes hole, try to pack */
>
> /* --- cacheline 2 boundary (128 bytes) --- */
> ...
> struct work_struct work; /* 144 72 */
>
> /* --- cacheline 3 boundary (192 bytes) was 24 bytes ago --- */
> struct mutex freeze_mutex; /* 216 144 */
>
> /* --- cacheline 5 boundary (320 bytes) was 40 bytes ago --- */
> u64 writecnt; /* 360 8 */
>
> /* size: 384, cachelines: 6, members: 26 */
> /* sum members: 354, holes: 2, sum holes: 14 */
> /* padding: 16 */
> /* forced alignments: 2, forced holes: 1, sum forced holes: 10 */
>
> } __attribute__((__aligned__(64)));
>
> After:
> struct bpf_map {
> ...
> u32 max_entries; /* 36 4 */
> u64 map_extra; /* 40 8 */
> u32 map_flags; /* 48 4 */
> int spin_lock_off; /* 52 4 */
> int timer_off; /* 56 4 */
> u32 id; /* 60 4 */
>
> /* --- cacheline 1 boundary (64 bytes) --- */
> int numa_node; /* 64 4 */
> ...
> bool frozen /* 113 1 */
>
> /* XXX 14 bytes hole, try to pack */
>
> /* --- cacheline 2 boundary (128 bytes) --- */
> ...
> struct work_struct work; /* 144 72 */
>
> /* --- cacheline 3 boundary (192 bytes) was 24 bytes ago --- */
> struct mutex freeze_mutex; /* 216 144 */
>
> /* --- cacheline 5 boundary (320 bytes) was 40 bytes ago --- */
> u64 writecnt; /* 360 8 */
>
> /* size: 384, cachelines: 6, members: 26 */
> /* sum members: 354, holes: 1, sum holes: 14 */
> /* padding: 16 */
> /* forced alignments: 2, forced holes: 1, sum forced holes: 14 */
>
> } __attribute__((__aligned__(64)));
>
> 2) Add alignment padding to the bpf_map_info struct
> More details can be found in commit 36f9814a494a ("bpf: fix uapi hole
> for 32 bit compat applications")
>
> Signed-off-by: Joanne Koong <joannekoong@fb.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls
2021-10-29 17:01 ` [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls Joanne Koong
@ 2021-10-29 22:04 ` Yonghong Song
2021-10-29 22:30 ` Joanne Koong
0 siblings, 1 reply; 9+ messages in thread
From: Yonghong Song @ 2021-10-29 22:04 UTC (permalink / raw)
To: Joanne Koong, bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team
On 10/29/21 10:01 AM, Joanne Koong wrote:
> This patch has two changes:
> 1) Adds a new function "test_success_cases" to test
> successfully creating + adding + looking up a value
> in a bloom filter map from the userspace side.
>
> 2) Use bpf_create_map instead of bpf_create_map_xattr in
> the "test_fail_cases" to make the code look cleaner.
>
> Signed-off-by: Joanne Koong <joannekoong@fb.com>
LGTM with one minor comment below.
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> .../bpf/prog_tests/bloom_filter_map.c | 53 ++++++++++++-------
> 1 file changed, 33 insertions(+), 20 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> index 9aa3fbed918b..dbc0035e43e5 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
> @@ -7,44 +7,32 @@
>
> static void test_fail_cases(void)
> {
> - struct bpf_create_map_attr xattr = {
> - .name = "bloom_filter_map",
> - .map_type = BPF_MAP_TYPE_BLOOM_FILTER,
> - .max_entries = 100,
> - .value_size = 11,
> - };
> __u32 value;
> int fd, err;
>
> /* Invalid key size */
> - xattr.key_size = 4;
> - fd = bpf_create_map_xattr(&xattr);
> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 4, sizeof(value), 100, 0);
> if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid key size"))
> close(fd);
> - xattr.key_size = 0;
>
> /* Invalid value size */
> - xattr.value_size = 0;
> - fd = bpf_create_map_xattr(&xattr);
> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, 0, 100, 0);
> if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid value size 0"))
> close(fd);
> - xattr.value_size = 11;
>
> /* Invalid max entries size */
> - xattr.max_entries = 0;
> - fd = bpf_create_map_xattr(&xattr);
> - if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid max entries size"))
> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 0, 0);
> + if (!ASSERT_LT(fd, 0,
> + "bpf_create_map bloom filter invalid max entries size"))
It is OK to have "bpf_create_map ..." in the same line as ASSERT_LT
for better readability and consistent with other ASSERT_LT. It is over
80 but less than 100 char's per line.
> close(fd);
> - xattr.max_entries = 100;
>
> /* Bloom filter maps do not support BPF_F_NO_PREALLOC */
> - xattr.map_flags = BPF_F_NO_PREALLOC;
> - fd = bpf_create_map_xattr(&xattr);
> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 100,
> + BPF_F_NO_PREALLOC);
> if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid flags"))
> close(fd);
> - xattr.map_flags = 0;
>
> - fd = bpf_create_map_xattr(&xattr);
> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 100, 0);
> if (!ASSERT_GE(fd, 0, "bpf_create_map bloom filter"))
> return;
>
> @@ -67,6 +55,30 @@ static void test_fail_cases(void)
> close(fd);
> }
[...]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls
2021-10-29 22:04 ` Yonghong Song
@ 2021-10-29 22:30 ` Joanne Koong
0 siblings, 0 replies; 9+ messages in thread
From: Joanne Koong @ 2021-10-29 22:30 UTC (permalink / raw)
To: Yonghong Song, bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team
On 10/29/21 3:04 PM, Yonghong Song wrote:
> On 10/29/21 10:01 AM, Joanne Koong wrote:
>> This patch has two changes:
>> 1) Adds a new function "test_success_cases" to test
>> successfully creating + adding + looking up a value
>> in a bloom filter map from the userspace side.
>>
>> 2) Use bpf_create_map instead of bpf_create_map_xattr in
>> the "test_fail_cases" to make the code look cleaner.
>>
>> Signed-off-by: Joanne Koong <joannekoong@fb.com>
>
> LGTM with one minor comment below.
>
> Acked-by: Yonghong Song <yhs@fb.com>
>
>> ---
>> .../bpf/prog_tests/bloom_filter_map.c | 53 ++++++++++++-------
>> 1 file changed, 33 insertions(+), 20 deletions(-)
>>
>> diff --git
>> a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
>> b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
>> index 9aa3fbed918b..dbc0035e43e5 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
>> @@ -7,44 +7,32 @@
>> static void test_fail_cases(void)
>> {
>> - struct bpf_create_map_attr xattr = {
>> - .name = "bloom_filter_map",
>> - .map_type = BPF_MAP_TYPE_BLOOM_FILTER,
>> - .max_entries = 100,
>> - .value_size = 11,
>> - };
>> __u32 value;
>> int fd, err;
>> /* Invalid key size */
>> - xattr.key_size = 4;
>> - fd = bpf_create_map_xattr(&xattr);
>> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 4, sizeof(value),
>> 100, 0);
>> if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid key
>> size"))
>> close(fd);
>> - xattr.key_size = 0;
>> /* Invalid value size */
>> - xattr.value_size = 0;
>> - fd = bpf_create_map_xattr(&xattr);
>> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, 0, 100, 0);
>> if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid
>> value size 0"))
>> close(fd);
>> - xattr.value_size = 11;
>> /* Invalid max entries size */
>> - xattr.max_entries = 0;
>> - fd = bpf_create_map_xattr(&xattr);
>> - if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid max
>> entries size"))
>> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value),
>> 0, 0);
>> + if (!ASSERT_LT(fd, 0,
>> + "bpf_create_map bloom filter invalid max entries size"))
>
> It is OK to have "bpf_create_map ..." in the same line as ASSERT_LT
> for better readability and consistent with other ASSERT_LT. It is over
> 80 but less than 100 char's per line.
>
Great, I will send out v2 of this patchset where this line break is
removed.
Thanks for reviewing the patchset!
>> close(fd);
>> - xattr.max_entries = 100;
>> /* Bloom filter maps do not support BPF_F_NO_PREALLOC */
>> - xattr.map_flags = BPF_F_NO_PREALLOC;
>> - fd = bpf_create_map_xattr(&xattr);
>> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value),
>> 100,
>> + BPF_F_NO_PREALLOC);
>> if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid
>> flags"))
>> close(fd);
>> - xattr.map_flags = 0;
>> - fd = bpf_create_map_xattr(&xattr);
>> + fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value),
>> 100, 0);
>> if (!ASSERT_GE(fd, 0, "bpf_create_map bloom filter"))
>> return;
>> @@ -67,6 +55,30 @@ static void test_fail_cases(void)
>> close(fd);
>> }
> [...]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls
2021-10-29 22:49 [PATCH bpf-next 0/3] "map_extra" and bloom filter fixups Joanne Koong
@ 2021-10-29 22:49 ` Joanne Koong
0 siblings, 0 replies; 9+ messages in thread
From: Joanne Koong @ 2021-10-29 22:49 UTC (permalink / raw)
To: bpf; +Cc: andrii, ast, daniel, kafai, Kernel-team, yhs, Joanne Koong
This patch has two changes:
1) Adds a new function "test_success_cases" to test
successfully creating + adding + looking up a value
in a bloom filter map from the userspace side.
2) Use bpf_create_map instead of bpf_create_map_xattr in
the "test_fail_cases" and test_inner_map to make the
code look cleaner.
Acked-by: Yonghong Song <yhs@fb.com>
Signed-off-by: Joanne Koong <joannekoong@fb.com>
---
.../bpf/prog_tests/bloom_filter_map.c | 59 +++++++++++--------
1 file changed, 33 insertions(+), 26 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
index 9aa3fbed918b..be73e3de6668 100644
--- a/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
+++ b/tools/testing/selftests/bpf/prog_tests/bloom_filter_map.c
@@ -7,44 +7,31 @@
static void test_fail_cases(void)
{
- struct bpf_create_map_attr xattr = {
- .name = "bloom_filter_map",
- .map_type = BPF_MAP_TYPE_BLOOM_FILTER,
- .max_entries = 100,
- .value_size = 11,
- };
__u32 value;
int fd, err;
/* Invalid key size */
- xattr.key_size = 4;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 4, sizeof(value), 100, 0);
if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid key size"))
close(fd);
- xattr.key_size = 0;
/* Invalid value size */
- xattr.value_size = 0;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, 0, 100, 0);
if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid value size 0"))
close(fd);
- xattr.value_size = 11;
/* Invalid max entries size */
- xattr.max_entries = 0;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 0, 0);
if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid max entries size"))
close(fd);
- xattr.max_entries = 100;
/* Bloom filter maps do not support BPF_F_NO_PREALLOC */
- xattr.map_flags = BPF_F_NO_PREALLOC;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 100,
+ BPF_F_NO_PREALLOC);
if (!ASSERT_LT(fd, 0, "bpf_create_map bloom filter invalid flags"))
close(fd);
- xattr.map_flags = 0;
- fd = bpf_create_map_xattr(&xattr);
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 100, 0);
if (!ASSERT_GE(fd, 0, "bpf_create_map bloom filter"))
return;
@@ -67,6 +54,30 @@ static void test_fail_cases(void)
close(fd);
}
+static void test_success_cases(void)
+{
+ char value[11];
+ int fd, err;
+
+ /* Create a map */
+ fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(value), 100,
+ BPF_F_ZERO_SEED | BPF_F_NUMA_NODE);
+ if (!ASSERT_GE(fd, 0, "bpf_create_map bloom filter success case"))
+ return;
+
+ /* Add a value to the bloom filter */
+ err = bpf_map_update_elem(fd, NULL, &value, 0);
+ if (!ASSERT_OK(err, "bpf_map_update_elem bloom filter success case"))
+ goto done;
+
+ /* Lookup a value in the bloom filter */
+ err = bpf_map_lookup_elem(fd, NULL, &value);
+ ASSERT_OK(err, "bpf_map_update_elem bloom filter success case");
+
+done:
+ close(fd);
+}
+
static void check_bloom(struct bloom_filter_map *skel)
{
struct bpf_link *link;
@@ -86,16 +97,11 @@ static void test_inner_map(struct bloom_filter_map *skel, const __u32 *rand_vals
__u32 nr_rand_vals)
{
int outer_map_fd, inner_map_fd, err, i, key = 0;
- struct bpf_create_map_attr xattr = {
- .name = "bloom_filter_inner_map",
- .map_type = BPF_MAP_TYPE_BLOOM_FILTER,
- .value_size = sizeof(__u32),
- .max_entries = nr_rand_vals,
- };
struct bpf_link *link;
/* Create a bloom filter map that will be used as the inner map */
- inner_map_fd = bpf_create_map_xattr(&xattr);
+ inner_map_fd = bpf_create_map(BPF_MAP_TYPE_BLOOM_FILTER, 0, sizeof(*rand_vals),
+ nr_rand_vals, 0);
if (!ASSERT_GE(inner_map_fd, 0, "bpf_create_map bloom filter inner map"))
return;
@@ -190,6 +196,7 @@ void test_bloom_filter_map(void)
int err;
test_fail_cases();
+ test_success_cases();
err = setup_progs(&skel, &rand_vals, &nr_rand_vals);
if (err)
--
2.30.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-10-29 22:49 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29 17:01 [PATCH bpf-next 0/3] "map_extra" and bloom filter fixups Joanne Koong
2021-10-29 17:01 ` [PATCH bpf-next 1/3] bpf: Bloom filter map naming fixups Joanne Koong
2021-10-29 21:28 ` Yonghong Song
2021-10-29 17:01 ` [PATCH bpf-next 2/3] bpf: Add alignment padding for "map_extra" + consolidate holes Joanne Koong
2021-10-29 21:29 ` Yonghong Song
2021-10-29 17:01 ` [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls Joanne Koong
2021-10-29 22:04 ` Yonghong Song
2021-10-29 22:30 ` Joanne Koong
2021-10-29 22:49 [PATCH bpf-next 0/3] "map_extra" and bloom filter fixups Joanne Koong
2021-10-29 22:49 ` [PATCH bpf-next 3/3] selftests/bpf: Add bloom map success test for userspace calls Joanne Koong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).