From: Martin KaFai Lau Date: Fri, 2 May 2025 17:32:02 +0000 (-0700) Subject: Merge branch 'bpf-udp-exactly-once-socket-iteration' X-Git-Tag: v6.16-rc1~132^2~173^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1b1f563a2526625a9125c1a63f47239f40f5e259;p=thirdparty%2Flinux.git Merge branch 'bpf-udp-exactly-once-socket-iteration' 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/20250404220221.1665428-1-jordan@jrife.io/ [2]: https://lore.kernel.org/bpf/CABi4-ogUtMrH8-NVB6W8Xg_F_KDLq=yy-yu-tKr2udXE2Mu1Lg@mail.gmail.com/ [3]: https://lore.kernel.org/bpf/d323d417-3e8b-48af-ae94-bc28469ac0c1@linux.dev/ ==================== Link: https://patch.msgid.link/20250502161528.264630-1-jordan@jrife.io Signed-off-by: Martin KaFai Lau --- 1b1f563a2526625a9125c1a63f47239f40f5e259