You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Jordan Rife says:
====================
bpf: udp: Exactly-once socket iteration
Both UDP and TCP socket iterators use iter->offset to track progress
through a bucket, which is a measure of the number of matching sockets
from the current bucket that have been seen or processed by the
iterator. On subsequent iterations, if the current bucket has
unprocessed items, we skip at least iter->offset matching items in the
bucket before adding any remaining items to the next batch. However,
iter->offset isn't always an accurate measure of "things already seen"
when the underlying bucket changes between reads which can lead to
repeated or skipped sockets. Instead, this series remembers the cookies
of the sockets we haven't seen yet in the current bucket and resumes
from the first cookie in that list that we can find on the next
iteration. This series focuses on UDP socket iterators, but a later
series will apply a similar approach to TCP socket iterators.
To be more specific, this series replaces struct sock **batch inside
struct bpf_udp_iter_state with union bpf_udp_iter_batch_item *batch,
where union bpf_udp_iter_batch_item can contain either a pointer to a
socket or a socket cookie. During reads, batch contains pointers to all
sockets in the current batch while between reads batch contains all the
cookies of the sockets in the current bucket that have yet to be
processed. On subsequent reads, when iteration resumes,
bpf_iter_udp_batch finds the first saved cookie that matches a socket in
the bucket's socket list and picks up from there to construct the next
batch. On average, assuming it's rare that the next socket disappears
before the next read occurs, we should only need to scan as much as we
did with the offset-based approach to find the starting point. In the
case that the next socket is no longer there, we keep scanning through
the saved cookies list until we find a match. The worst case is when
none of the sockets from last time exist anymore, but again, this should
be rare.
CHANGES
=======
v6 -> v7:
* Move initialization of iter->state.bucket to -1 from patch five ("bpf:
udp: Avoid socket skips and repeats during iteration") to patch three
("bpf: udp: Get rid of st_bucket_done") to avoid skipping the first
bucket in the patch three and four (Martin).
* Rename sock to sk in bpf_iter_batch_item (Martin).
* Use ASSERT_OK_PTR in do_resume_test to check if counts is NULL
(Martin).
* goto done in do_resume_test when calloc or sock_iter_batch__open fails
to make sure things are cleaned up properly, and initialize pointers
to NULL explicitly to silence warnings from llvm 20 in CI.
v5 -> v6:
* Rework the logic in patch two ("bpf: udp: Make sure iter->batch
always contains a full bucket snapshot") again to simplify it:
* Only try realloc with GFP_USER one time instead of two (Alexei).
* v5 introduced a second call to bpf_iter_udp_realloc_batch inside the
loop to handle the GFP_ATOMIC case. In v6, move the GFP_USER case
inside the loop as well, so it's all in once place. This, I feel,
makes it a bit easier to understand the control flow. Consequently,
it also simplifies the logic outside the loop.
* Use GFP_NOWAIT instead of GFP_ATOMIC to avoid depleting memory
reserves, since iterators are not critical operation (Alexei). Alexei
suggested using __GFP_NOWARN as well with GFP_NOWAIT, but this is
already set inside bpf_iter_udp_realloc_batch, so no change was needed
there.
* Introduce patch three ("bpf: udp: Get rid of st_bucket_done") to
simplify things further, since with patch two, st_bucket_done == true
is equivalent to iter->cur_sk == iter->end_sk.
* In patch five ("bpf: udp: Avoid socket skips and repeats during
iteration"), initialize iter->state.bucket to -1 so that on the first
call to bpf_iter_udp_batch, the resume_bucket condition is not hit.
This avoids adding a special case to the condition around
bpf_iter_udp_resume for bucket zero.
v4 -> v5:
* Rework the logic from patch two ("bpf: udp: Make sure iter->batch
always contains a full bucket snapshot") to move the handling of the
GFP_ATOMIC case inside the main loop and get rid of the extra lock
variable. This makes the logic clearer and makes it clearer that the
bucket lock is always released (Martin).
* Introduce udp_portaddr_for_each_entry_from in patch two instead of
patch four ("bpf: udp: Avoid socket skips and repeats during
iteration"), since patch two now needs to be able to resume list
iteration from an arbitrary point in the GFP_ATOMIC case.
* Similarly, introduce the memcpy inside bpf_iter_udp_realloc_batch in
patch two instead of patch four, since in the GFP_ATOMIC case the new
batch needs to remember the sockets from the old batch.
* Use sock_gen_cookie instead of __sock_gen_cookie inside
bpf_iter_udp_put_batch, since it can be called from a preemptible
context (Martin).
v3 -> v4:
* Explicitly assign sk = NULL on !iter->end_sk exit condition
(Kuniyuki).
* Reword the commit message of patch two ("bpf: udp: Make sure
iter->batch always contains a full bucket snapshot") to make the
reasoning for GFP_ATOMIC more clear.
v2 -> v3:
* Guarantee that iter->batch is always a full snapshot of a bucket to
prevent socket repeat scenarios [3]. This supercedes the patch from v2
that simply propagated ENOMEM up from bpf_iter_udp_batch and covers
the scenario where the batch size is still too small after a realloc.
* Fix up self tests (Martin)
* ASSERT_EQ(nread, sizeof(out), "nread") instead of
ASSERT_GE(nread, 1, "nread) in read_n.
* Use ASSERT_OK and ASSERT_OK_FD in several places.
* Add missing free(counts) to do_resume_test.
* Move int local_port declaration to the top of do_resume_test.
* Remove unnecessary guards before close and free.
v1 -> v2:
* Drop WARN_ON_ONCE from bpf_iter_udp_realloc_batch (Kuniyuki).
* Fixed memcpy size parameter in bpf_iter_udp_realloc_batch; before it
was missing sizeof(elem) * (Kuniyuki).
* Move "bpf: udp: Propagate ENOMEM up from bpf_iter_udp_batch" to patch
two in the series (Kuniyuki).
rfc [1] -> v1:
* Use hlist_entry_safe directly to retrieve the first socket in the
current bucket's linked list instead of immediately breaking from
udp_portaddr_for_each_entry (Martin).
* Cancel iteration if bpf_iter_udp_realloc_batch() can't grab enough
memory to contain a full snapshot of the current bucket to prevent
unwanted skips or repeats [2].
[1]: https://lore.kernel.org/bpf/[email protected]/
[2]: https://lore.kernel.org/bpf/CABi4-ogUtMrH8-NVB6W8Xg_F_KDLq=yy-yu-tKr2udXE2Mu1Lg@mail.gmail.com/
[3]: https://lore.kernel.org/bpf/[email protected]/
====================
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Martin KaFai Lau <[email protected]>
0 commit comments