[PATCH bpf-next 3/3] selftests/bpf: Add tests to verify no unintended eviction when updating lru hash maps

Leon Hwang posted 3 patches 2 months, 1 week ago
[PATCH bpf-next 3/3] selftests/bpf: Add tests to verify no unintended eviction when updating lru hash maps
Posted by Leon Hwang 2 months, 1 week ago
Add two tests to verify that updating an existing element in LRU hash
maps does not cause unintended eviction of other elements.

The test creates lru_hash/lru_percpu_hash maps with max_entries slots and
populates all of them. It then updates an existing key and verifies that:
1. The update succeeds without error
2. The updated key has the new value
3. All other keys still exist with their original values

This validates the fix that prevents unnecessary LRU eviction when
updating existing elements in full LRU hash maps.

Signed-off-by: Leon Hwang <leon.hwang@linux.dev>
---
 .../selftests/bpf/prog_tests/htab_update.c    | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/htab_update.c b/tools/testing/selftests/bpf/prog_tests/htab_update.c
index d0b405eb2966..bd29a915bb05 100644
--- a/tools/testing/selftests/bpf/prog_tests/htab_update.c
+++ b/tools/testing/selftests/bpf/prog_tests/htab_update.c
@@ -143,3 +143,76 @@ void test_htab_update(void)
 	if (test__start_subtest("concurrent_update"))
 		test_concurrent_update();
 }
+
+static void test_lru_hash_map_update_elem(enum bpf_map_type map_type)
+{
+	int err, map_fd, i, key, nr_cpus, max_entries = 128;
+	u64 *values, value = 0xDEADC0DE;
+
+	nr_cpus = libbpf_num_possible_cpus();
+	if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
+		return;
+
+	values = calloc(nr_cpus, sizeof(u64));
+	if (!ASSERT_OK_PTR(values, "calloc values"))
+		return;
+	for (i = 0; i < nr_cpus; i++)
+		values[i] = value;
+
+	map_fd = bpf_map_create(map_type, "test_lru", sizeof(int), sizeof(u64), max_entries, NULL);
+	if (!ASSERT_GE(map_fd, 0, "bpf_map_create")) {
+		free(values);
+		return;
+	}
+
+	/* populate all slots */
+	for (key = 0; key < max_entries; key++) {
+		err = bpf_map_update_elem(map_fd, &key, values, 0);
+		if (!ASSERT_OK(err, "bpf_map_update_elem"))
+			goto out;
+	}
+
+	/* LRU eviction should not happen */
+
+	key = 0;
+	memset(values, 0, nr_cpus * sizeof(u64));
+	err = bpf_map_update_elem(map_fd, &key, values, 0);
+	if (!ASSERT_OK(err, "bpf_map_update_elem"))
+		goto out;
+
+	err = bpf_map_lookup_elem(map_fd, &key, values);
+	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
+		goto out;
+	if (!ASSERT_EQ(*values, 0, "bpf_map_lookup_elem value"))
+		goto out;
+
+	for (key = 1; key < max_entries; key++) {
+		err = bpf_map_lookup_elem(map_fd, &key, values);
+		if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
+			goto out;
+		if (!ASSERT_EQ(*values, value, "bpf_map_lookup_elem value"))
+			goto out;
+	}
+
+out:
+	close(map_fd);
+	free(values);
+}
+
+static void test_update_lru_hash_map(void)
+{
+	test_lru_hash_map_update_elem(BPF_MAP_TYPE_LRU_HASH);
+}
+
+static void test_update_lru_percpu_hash_map(void)
+{
+	test_lru_hash_map_update_elem(BPF_MAP_TYPE_LRU_PERCPU_HASH);
+}
+
+void test_update_lru_hash_maps(void)
+{
+	if (test__start_subtest("lru_hash"))
+		test_update_lru_hash_map();
+	if (test__start_subtest("lru_percpu_hash"))
+		test_update_lru_percpu_hash_map();
+}
-- 
2.52.0
Re: [PATCH bpf-next 3/3] selftests/bpf: Add tests to verify no unintended eviction when updating lru hash maps
Posted by bot+bpf-ci@kernel.org 2 months, 1 week ago
> diff --git a/tools/testing/selftests/bpf/prog_tests/htab_update.c b/tools/testing/selftests/bpf/prog_tests/htab_update.c
> index d0b405eb2..bd29a915b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/htab_update.c
> +++ b/tools/testing/selftests/bpf/prog_tests/htab_update.c

[ ... ]

> +static void test_lru_hash_map_update_elem(enum bpf_map_type map_type)
> +{
> +	int err, map_fd, i, key, nr_cpus, max_entries = 128;
> +	u64 *values, value = 0xDEADC0DE;
> +
> +	nr_cpus = libbpf_num_possible_cpus();
> +	if (!ASSERT_GT(nr_cpus, 0, "libbpf_num_possible_cpus"))
> +		return;
> +
> +	values = calloc(nr_cpus, sizeof(u64));

[ ... ]

> +	err = bpf_map_lookup_elem(map_fd, &key, values);
> +	if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> +		goto out;
> +	if (!ASSERT_EQ(*values, 0, "bpf_map_lookup_elem value"))
> +		goto out;
> +
> +	for (key = 1; key < max_entries; key++) {
> +		err = bpf_map_lookup_elem(map_fd, &key, values);
> +		if (!ASSERT_OK(err, "bpf_map_lookup_elem"))
> +			goto out;
> +		if (!ASSERT_EQ(*values, value, "bpf_map_lookup_elem value"))
> +			goto out;
> +	}

When test_lru_hash_map_update_elem() is called with
BPF_MAP_TYPE_LRU_PERCPU_HASH via test_update_lru_percpu_hash_map(),
should all CPU values be checked rather than just *values?

For percpu maps, bpf_map_lookup_elem() returns an array with one value
per CPU. The test allocates space for nr_cpus values but only verifies
the first element. This means values for CPUs 1 through nr_cpus-1 are
not validated.

Looking at tools/testing/selftests/bpf/prog_tests/lookup_and_delete.c,
the pattern for percpu maps is to loop through all CPUs:

    for (i = 0; i < nr_cpus; i++) {
        val = value[i];
        /* check val */
    }

For BPF_MAP_TYPE_LRU_HASH this is correct, but for
BPF_MAP_TYPE_LRU_PERCPU_HASH the test may pass even if non-primary
CPUs have incorrect values.

> +
> +out:
> +	close(map_fd);
> +	free(values);
> +}
> +
> +static void test_update_lru_hash_map(void)
> +{
> +	test_lru_hash_map_update_elem(BPF_MAP_TYPE_LRU_HASH);
> +}
> +
> +static void test_update_lru_percpu_hash_map(void)
> +{
> +	test_lru_hash_map_update_elem(BPF_MAP_TYPE_LRU_PERCPU_HASH);
> +}

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19864460617