Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpf: Support atomic update for htab of maps #8634

Open
wants to merge 6 commits into
base: bpf-next_base
Choose a base branch
from
Open

bpf: Support atomic update for htab of maps #8634

wants to merge 6 commits into from
+289 −76

Conversation

kernel-patches-daemon-bpf[bot]
Copy link

Pull request for series with
subject: bpf: Support atomic update for htab of maps
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f282146
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 79d93c8
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 74f36a9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 157a502
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 233732b
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: a68894a
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: be741c7
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 46d38f4
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 956e816
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f3f8649
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: ae0a457
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f4edc66
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6ca2162
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: a259804
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 79db658
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: e16e64f
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

Hou Tao added 3 commits March 20, 2025 17:06
bpf: Factor out htab_elem_value helper()
04cd6c1 
All hash maps store map key and map value together. The relative offset
of the map value compared to the map key is round_up(key_size, 8).
Therefore, factor out a common helper htab_elem_value() to calculate the
address of the map value instead of duplicating the logic.

Signed-off-by: Hou Tao <[email protected]>
bpf: Rename __htab_percpu_map_update_elem to htab_map_update_elem_in_…
6639287 
…place

Rename __htab_percpu_map_update_elem to htab_map_update_elem_in_place,
and add a new percpu argument for the helper to support in-place update
for both per-cpu htab and htab of maps.

Signed-off-by: Hou Tao <[email protected]>
bpf: Support atomic update for htab of maps
8acf06e 
As reported by Cody Haas [1], when there is concurrent map lookup and
map update operation in an existing element for htab of maps, the map
lookup procedure may return -ENOENT unexpectedly.

The root cause is twofold:

1) the update of existing element involves two separated list operation
In htab_map_update_elem(), it first inserts the new element at the head
of list, then it deletes the old element. Therefore, it is possible a
lookup operation has already iterated to the middle of the list when a
concurrent update operation begins, and the lookup operation will fail
to find the target element.

2) the immediate reuse of htab element.
It is more subtle. Even through the lookup operation finds the old
element, it is possible that the target element has been removed by a
concurrent update operation, and the element has been reused immediately
by other update operation which runs on the same CPU as the previous
update operation, and the element is inserted into the same bucket list.
After these steps above, when the lookup operation tries to compare the
key in the old element with the expected key, the match will fail
because the key in the old element have been overwritten by other update
operation.

The two-step update process is relatively straightforward to address.
The more challenging aspect is the immediate reuse. As Alexei pointed
out:

 So since 2022 both prealloc and no_prealloc reuse elements.
 We can consider a new flag for the hash map like F_REUSE_AFTER_RCU_GP
 that will use _rcu() flavor of freeing into bpf_ma,
 but it has to have a strong reason.

Given that htab of maps doesn't support special field in value and
directly stores the inner map pointer in htab_element, just do in-place
update for htab of maps instead of attempting to address the immediate
reuse issue.

[1]: https://lore.kernel.org/xdp-newbies/CAH7f-ULFTwKdoH_t2SFc5rWCVYLEg-14d1fBYWH2eekudsnTRg@mail.gmail.com/

Signed-off-by: Hou Tao <[email protected]>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 51d6504
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=941775
version: 2

Hou Tao added 3 commits March 20, 2025 17:06
bpf: Add is_fd_htab() helper
1a1455d 
Add is_fd_htab() helper to check whether the map is htab of maps.

Signed-off-by: Hou Tao <[email protected]>
bpf: Don't allocate per-cpu extra_elems for fd htab
48e5eb4 
The update of element in fd htab is in-place now, therefore, there is no
need to allocate per-cpu extra_elems, just remove it.

Signed-off-by: Hou Tao <[email protected]>
selftests/bpf: Add test case for atomic update of fd htab
26030ed 
Add a test case to verify the atomic update of existing elements in the
htab of maps. The test proceeds in three steps:

1) fill the outer map with keys in the range [0, 8]
For each inner array map, the value of its first element is set as the
key used to lookup the inner map.

2) create 16 threads to lookup these keys concurrently
Each lookup thread first lookups the inner map, then it checks whether
the first value of the inner array map is the same as the key used to
lookup the inner map.

3) create 8 threads to overwrite these keys concurrently
Each update thread first creates an inner array, it sets the first value
of the array to the key used to update the outer map, then it uses the
key and the inner map to update the outer map.

Without atomic update support, the lookup operation may return -ENOENT
during the lookup of outer map, or return -EINVAL during the comparison
of the first value in the inner map and the key used for inner map, and
the test will fail. After the atomic update change, both the lookup and
the comparison will succeed.

Given that the update of outer map is slow, the test case sets the loop
number for each thread as 5 to reduce the total running time. However,
the loop number could also be adjusted through FD_HTAB_LOOP_NR
environment variable.

Signed-off-by: Hou Tao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bpf-next new V2-ci-pass V2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
0 participants