Enabling PROVE_RCU_LIST (and RCU_EXPERT) shows list_for_each_entry_rcu() from mptcp_sched_find() not being used with RCU read lock held. The first patch is a fix for -net. The other one is for a commit that is only in our tree. Link: https://lore.kernel.org/20241016011144.3058445-1-kuba@kernel.org Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> --- Matthieu Baerts (NGI0) (2): mptcp: init: protect sched with rcu_read_lock Squash to "bpf: Add bpf_mptcp_sched_ops" net/mptcp/bpf.c | 10 +++++++--- net/mptcp/protocol.c | 2 ++ 2 files changed, 9 insertions(+), 3 deletions(-) --- base-commit: baccd7675477b1db387aa71b48c3312b5fb67a5d change-id: 20241016-mptcp-sched-find-rcu-649ce3399334 Best regards, -- Matthieu Baerts (NGI0) <matttbe@kernel.org>
Hi, On 10/16/24 21:05, Matthieu Baerts (NGI0) wrote: > Enabling PROVE_RCU_LIST (and RCU_EXPERT) shows list_for_each_entry_rcu() > from mptcp_sched_find() not being used with RCU read lock held. > > The first patch is a fix for -net. The other one is for a commit that is > only in our tree. > > Link: https://lore.kernel.org/20241016011144.3058445-1-kuba@kernel.org > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > Matthieu Baerts (NGI0) (2): > mptcp: init: protect sched with rcu_read_lock > Squash to "bpf: Add bpf_mptcp_sched_ops" > > net/mptcp/bpf.c | 10 +++++++--- > net/mptcp/protocol.c | 2 ++ > 2 files changed, 9 insertions(+), 3 deletions(-) > --- > base-commit: baccd7675477b1db387aa71b48c3312b5fb67a5d > change-id: 20241016-mptcp-sched-find-rcu-649ce3399334 > > Best regards, Only slightly related, but mptcp_get_available_schedulers() currently acquires both the rcu and the sched_list lock. I think the latter is not needed and should be dropped. Cheers, Paolo
Hi Paolo, On 17/10/2024 11:38, Paolo Abeni wrote: > On 10/16/24 21:05, Matthieu Baerts (NGI0) wrote: >> Enabling PROVE_RCU_LIST (and RCU_EXPERT) shows list_for_each_entry_rcu() >> from mptcp_sched_find() not being used with RCU read lock held. >> >> The first patch is a fix for -net. The other one is for a commit that is >> only in our tree. >> >> Link: https://lore.kernel.org/20241016011144.3058445-1-kuba@kernel.org >> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> >> --- >> Matthieu Baerts (NGI0) (2): >> mptcp: init: protect sched with rcu_read_lock >> Squash to "bpf: Add bpf_mptcp_sched_ops" >> >> net/mptcp/bpf.c | 10 +++++++--- >> net/mptcp/protocol.c | 2 ++ >> 2 files changed, 9 insertions(+), 3 deletions(-) >> --- >> base-commit: baccd7675477b1db387aa71b48c3312b5fb67a5d >> change-id: 20241016-mptcp-sched-find-rcu-649ce3399334 >> >> Best regards, > > Only slightly related, but mptcp_get_available_schedulers() currently > acquires both the rcu and the sched_list lock. I think the latter is not > needed and should be dropped. Good point, the list is not modified. I can send a patch for that. (sorry, I should not have applied these patches that quickly, I was busy adding 'debug' support for the BPF tests in our CI, and I didn't think it was too quick) Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Geliang, On 16/10/2024 21:05, Matthieu Baerts (NGI0) wrote: > Enabling PROVE_RCU_LIST (and RCU_EXPERT) shows list_for_each_entry_rcu() > from mptcp_sched_find() not being used with RCU read lock held. > > The first patch is a fix for -net. The other one is for a commit that is > only in our tree. > > Link: https://lore.kernel.org/20241016011144.3058445-1-kuba@kernel.org > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org> > --- > Matthieu Baerts (NGI0) (2): > mptcp: init: protect sched with rcu_read_lock > Squash to "bpf: Add bpf_mptcp_sched_ops" Thank you for the review! I just applied these two patches in our tree: New patches for t/upstream-net and t/upstream: - e80dfc53fa3d: mptcp: init: protect sched with rcu_read_lock - Results: 32fda8b8e0f1..4367da1fbf63 (export-net) - 8e293a58ad78: "squashed" patch 2/2 in "bpf: Add bpf_mptcp_sched_ops" - Results: baccd7675477..0004ac084daf (export) Tests are now in progress: - export-net: https://github.com/multipath-tcp/mptcp_net-next/commit/6b56229c81f9ee84435f808186d11e9bcb7ad6c3/checks - export: https://github.com/multipath-tcp/mptcp_net-next/commit/e8bb62da034e057e6d778b8d7c8c0c9c5cd5d5ab/checks Cheers, Matt -- Sponsored by the NGI0 Core fund.
Hi Matthieu, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal: Success! ✅ - KVM Validation: debug: Success! ✅ - KVM Validation: btf (only bpftest_all): Success! ✅ - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/11372377607 Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/443b0dde1d65 Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=899900 If there are some issues, you can reproduce them using the same environment as the one used by the CI thanks to a docker image, e.g.: $ cd [kernel source code] $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \ --pull always mptcp/mptcp-upstream-virtme-docker:latest \ auto-normal For more details: https://github.com/multipath-tcp/mptcp-upstream-virtme-docker Please note that despite all the efforts that have been already done to have a stable tests suite when executed on a public CI like here, it is possible some reported issues are not due to your modifications. Still, do not hesitate to help us improve that ;-) Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (NGI0 Core)
© 2016 - 2024 Red Hat, Inc.