bpf: udp: Exactly-once socket iteration #8859
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![kernel-patches-daemon-bpf[bot]](https://avatars.githubusercontent.com/u/70728002?s=60&v=4){kind=small}
|
Upstream branch: 224ee86 |
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
b2666a3 to
9afa005
Compare
|
Upstream branch: 8e64c38 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
147beb6 to
3bbe5cf
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
9afa005 to
993c0c1
Compare
|
Upstream branch: 38d976c |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
3bbe5cf to
a8ae3a9
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
993c0c1 to
97831b9
Compare
|
Upstream branch: 358b1c0 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
a8ae3a9 to
d315c0b
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
97831b9 to
c60a0e6
Compare
|
Upstream branch: 358b1c0 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
d315c0b to
b47e42b
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
c60a0e6 to
bebc5be
Compare
|
Upstream branch: 7b05f43 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
b47e42b to
a1ec8c7
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
bebc5be to
bc80b4f
Compare
|
Upstream branch: 7b05f43 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
a1ec8c7 to
ca4d473
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
bc80b4f to
ad4665c
Compare
|
Upstream branch: 7b05f43 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
ca4d473 to
cbcb91d
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
ad4665c to
f363929
Compare
|
Upstream branch: 86870d0 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
cbcb91d to
23c5836
Compare
kernel-patches-daemon-bpf
bot
force-pushed
the
bpf-next_base
branch
from
f363929 to
632a377
Compare
|
Upstream branch: f263336 |
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
23c5836 to
f956863
Compare
Prepare for the next patch which needs to be able to choose either GFP_USER or GFP_NOWAIT for calls to bpf_iter_udp_realloc_batch. Signed-off-by: Jordan Rife <[email protected]> Reviewed-by: Kuniyuki Iwashima <[email protected]>
|
Upstream branch: f263336 |
Require that iter->batch always contains a full bucket snapshot. This invariant is important to avoid skipping or repeating sockets during iteration when combined with the next few patches. Before, there were two cases where a call to bpf_iter_udp_batch may only capture part of a bucket: 1. When bpf_iter_udp_realloc_batch() returns -ENOMEM [1]. 2. When more sockets are added to the bucket while calling bpf_iter_udp_realloc_batch(), making the updated batch size insufficient [2]. In cases where the batch size only covers part of a bucket, it is possible to forget which sockets were already visited, especially if we have to process a bucket in more than two batches. This forces us to choose between repeating or skipping sockets, so don't allow this: 1. Stop iteration and propagate -ENOMEM up to userspace if reallocation fails instead of continuing with a partial batch. 2. Try bpf_iter_udp_realloc_batch() with GFP_USER just as before, but if we still aren't able to capture the full bucket, call bpf_iter_udp_realloc_batch() again while holding the bucket lock to guarantee the bucket does not change. On the second attempt use GFP_NOWAIT since we hold onto the spin lock. Introduce the udp_portaddr_for_each_entry_from macro and use it instead of udp_portaddr_for_each_entry to make it possible to continue iteration from an arbitrary socket. This is required for this patch in the GFP_NOWAIT case to allow us to fill the rest of a batch starting from the middle of a bucket and the later patch which skips sockets that were already seen. Testing all scenarios directly is a bit difficult, but I did some manual testing to exercise the code paths where GFP_NOWAIT is used and where ERR_PTR(err) is returned. I used the realloc test case included later in this series to trigger a scenario where a realloc happens inside bpf_iter_udp_batch and made a small code tweak to force the first realloc attempt to allocate a too-small batch, thus requiring another attempt with GFP_NOWAIT. Some printks showed both reallocs with the tests passing: Apr 25 23:16:24 crow kernel: go again GFP_USER Apr 25 23:16:24 crow kernel: go again GFP_NOWAIT With this setup, I also forced each of the bpf_iter_udp_realloc_batch calls to return -ENOMEM to ensure that iteration ends and that the read() in userspace fails. [1]: https://lore.kernel.org/bpf/CABi4-ogUtMrH8-NVB6W8Xg_F_KDLq=yy-yu-tKr2udXE2Mu1Lg@mail.gmail.com/ [2]: https://lore.kernel.org/bpf/[email protected]/ Signed-off-by: Jordan Rife <[email protected]>
Get rid of the st_bucket_done field to simplify UDP iterator state and
logic. Before, st_bucket_done could be false if bpf_iter_udp_batch
returned a partial batch; however, with the last patch ("bpf: udp: Make
sure iter->batch always contains a full bucket snapshot"),
st_bucket_done == true is equivalent to iter->cur_sk == iter->end_sk.
Signed-off-by: Jordan Rife <[email protected]>
Prepare for the next patch that tracks cookies between iterations by converting struct sock **batch to union bpf_udp_iter_batch_item *batch inside struct bpf_udp_iter_state. Signed-off-by: Jordan Rife <[email protected]> Reviewed-by: Kuniyuki Iwashima <[email protected]>
Replace the offset-based approach for tracking progress through a bucket in the UDP table with one based on socket cookies. Remember the cookies of unprocessed sockets from the last batch and use this list to pick up where we left off or, in the case that the next socket disappears between reads, find the first socket after that point that still exists in the bucket and resume from there. This approach guarantees that all sockets that existed when iteration began and continue to exist throughout will be visited exactly once. Sockets that are added to the table during iteration may or may not be seen, but if they are they will be seen exactly once. Signed-off-by: Jordan Rife <[email protected]>
Extend the iter_udp_soreuse and iter_tcp_soreuse programs to write the cookie of the current socket, so that we can track the identity of the sockets that the iterator has seen so far. Update the existing do_test function to account for this change to the iterator program output. At the same time, teach both programs to work with AF_INET as well. Signed-off-by: Jordan Rife <[email protected]>
Introduce a set of tests that exercise various bucket resume scenarios: * remove_seen resumes iteration after removing a socket from the bucket that we've already processed. Before, with the offset-based approach, this test would have skipped an unseen socket after resuming iteration. With the cookie-based approach, we now see all sockets exactly once. * remove_unseen exercises the condition where the next socket that we would have seen is removed from the bucket before we resume iteration. This tests the scenario where we need to scan past the first cookie in our remembered cookies list to find the socket from which to resume iteration. * remove_all exercises the condition where all sockets we remembered were removed from the bucket to make sure iteration terminates and returns no more results. * add_some exercises the condition where a few, but not enough to trigger a realloc, sockets are added to the head of the current bucket between reads. Before, with the offset-based approach, this test would have repeated sockets we've already seen. With the cookie-based approach, we now see all sockets exactly once. * force_realloc exercises the condition that we need to realloc the batch on a subsequent read, since more sockets than can be held in the current batch array were added to the current bucket. This exercies the logic inside bpf_iter_udp_realloc_batch that copies cookies into the new batch to make sure nothing is skipped or repeated. Signed-off-by: Jordan Rife <[email protected]>
kernel-patches-daemon-bpf
bot
force-pushed
the
series/952598=>bpf-next
branch
from
f956863 to
e6d976b
Compare
|
At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=959148 irrelevant now. Closing PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: bpf: udp: Exactly-once socket iteration
version: 6
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=957801