| Message ID | 20220929070407.965581-1-martin.lau@linux.dev (mailing list archive) |
|---|---|
| Headers | show |
| Series | bpf: Remove recursion check for struct_ops prog | expand |
Hello: This series was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <ast@kernel.org>: On Thu, 29 Sep 2022 00:04:02 -0700 you wrote: > From: Martin KaFai Lau <martin.lau@kernel.org> > > The struct_ops is sharing the tracing-trampoline's enter/exit > function which tracks prog->active to avoid recursion. It turns > out the struct_ops bpf prog will hit this prog->active and > unnecessarily skipped running the struct_ops prog. eg. The > '.ssthresh' may run in_task() and then interrupted by softirq > that runs the same '.ssthresh'. > > [...] Here is the summary with links: - [v3,bpf-next,1/5] bpf: Add __bpf_prog_{enter,exit}_struct_ops for struct_ops trampoline https://git.kernel.org/bpf/bpf-next/c/64696c40d03c - [v3,bpf-next,2/5] bpf: Move the "cdg" tcp-cc check to the common sol_tcp_sockopt() https://git.kernel.org/bpf/bpf-next/c/37cfbe0bf2e8 - [v3,bpf-next,3/5] bpf: Refactor bpf_setsockopt(TCP_CONGESTION) handling into another function https://git.kernel.org/bpf/bpf-next/c/1e7d217faa11 - [v3,bpf-next,4/5] bpf: tcp: Stop bpf_setsockopt(TCP_CONGESTION) in init ops to recur itself https://git.kernel.org/bpf/bpf-next/c/061ff040710e - [v3,bpf-next,5/5] selftests/bpf: Check -EBUSY for the recurred bpf_setsockopt(TCP_CONGESTION) https://git.kernel.org/bpf/bpf-next/c/3411c5b6f8d6 You are awesome, thank you!
From: Martin KaFai Lau <martin.lau@kernel.org> The struct_ops is sharing the tracing-trampoline's enter/exit function which tracks prog->active to avoid recursion. It turns out the struct_ops bpf prog will hit this prog->active and unnecessarily skipped running the struct_ops prog. eg. The '.ssthresh' may run in_task() and then interrupted by softirq that runs the same '.ssthresh'. The kernel does not call the tcp-cc's ops in a recursive way, so this set is to remove the recursion check for struct_ops prog. v3: - Clear the bpf_chg_cc_inprogress from the newly cloned tcp_sock in tcp_create_openreq_child() because the listen sk can be cloned without lock being held. (Eric Dumazet) v2: - v1 [0] turned into a long discussion on a few cases and also whether it needs to follow the bpf_run_ctx chain if there is tracing bpf_run_ctx (kprobe/trace/trampoline) running in between. It is a good signal that it is not obvious enough to reason about it and needs a tradeoff for a more straight forward approach. This revision uses one bit out of an existing 1 byte hole in the tcp_sock. It is in Patch 4. [0]: https://lore.kernel.org/bpf/20220922225616.3054840-1-kafai@fb.com/T/#md98d40ac5ec295fdadef476c227a3401b2b6b911 Martin KaFai Lau (5): bpf: Add __bpf_prog_{enter,exit}_struct_ops for struct_ops trampoline bpf: Move the "cdg" tcp-cc check to the common sol_tcp_sockopt() bpf: Refactor bpf_setsockopt(TCP_CONGESTION) handling into another function bpf: tcp: Stop bpf_setsockopt(TCP_CONGESTION) in init ops to recur itself selftests/bpf: Check -EBUSY for the recurred bpf_setsockopt(TCP_CONGESTION) arch/x86/net/bpf_jit_comp.c | 3 + include/linux/bpf.h | 4 ++ include/linux/tcp.h | 6 ++ kernel/bpf/trampoline.c | 23 ++++++ net/core/filter.c | 70 ++++++++++++++----- net/ipv4/tcp_minisocks.c | 1 + .../selftests/bpf/prog_tests/bpf_tcp_ca.c | 4 ++ tools/testing/selftests/bpf/progs/bpf_dctcp.c | 25 ++++--- 8 files changed, 112 insertions(+), 24 deletions(-)