| Message ID | cover.1743562290.git.tanggeliang@kylinos.cn (mailing list archive) |
|---|---|
| Headers | show |
| Series | only remove entry from local_addr_list when sending a REMOVE_ADDR | expand |
Hi Geliang, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal: Unstable: 1 failed test(s): selftest_mptcp_connect
On Wed, 2 Apr 2025, Geliang Tang wrote: > From: Geliang Tang <tanggeliang@kylinos.cn> > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/403 Geliang and Matthieu - Since this is a bug fix, should it go to mptcp-net rather than -next? Or does the associated selftest change make that complicated? I think there's a nearby bug in mptcp_pm_nl_remove_doit() that I noticed while reviewing this series. If we upstream this series to the net tree it might make sense to send the fixes together. In this code: list_del_rcu(&match->list); msk->pm.local_addr_used--; spin_unlock_bh(&msk->pm.lock); mptcp_pm_remove_addr_entry(msk, match); release_sock(sk); sock_kfree_s(sk, match, sizeof(*match)); The struct is removed from a linked list with list_del_rcu(), but doesn't wait for a grace period before the sock_kfree_s() in the final line. The easy solution is to make the kfree rcu-aware and open-code the handling of sk_omem_alloc, like this: atomic_sub(sizeof(*match), &sk->sk_omem_alloc); kfree_rcu(match); instead of calling sock_kfree_s(). tcp_md5_do_del() takes this approach. This does allow possible allocation beyond the optmem limit, but only for the duration of the rcu grace period. I can send a patch tomorrow, unless someone wants to implement it before then. - Mat > > Geliang Tang (3): > mptcp: pm: userspace: local_addr_used-- after sending REMOVE_ADDR > mptcp: pm: userspace: drop delete_local_addr helper > selftests: mptcp: send REMOVE_ADDR after subflow is deleted > > net/mptcp/pm_userspace.c | 37 +++---------------- > .../testing/selftests/net/mptcp/mptcp_join.sh | 4 +- > .../selftests/net/mptcp/userspace_pm.sh | 6 +++ > 3 files changed, 14 insertions(+), 33 deletions(-) > > -- > 2.43.0 > > >
Hi Mat, On Tue, 2025-04-08 at 17:23 -0700, Mat Martineau wrote: > On Wed, 2 Apr 2025, Geliang Tang wrote: > > > From: Geliang Tang <tanggeliang@kylinos.cn> > > > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/403 > > Geliang and Matthieu - > > Since this is a bug fix, should it go to mptcp-net rather than -next? > Or > does the associated selftest change make that complicated? > > I think there's a nearby bug in mptcp_pm_nl_remove_doit() that I > noticed > while reviewing this series. If we upstream this series to the net > tree it > might make sense to send the fixes together. > > In this code: > > list_del_rcu(&match->list); > msk->pm.local_addr_used--; > spin_unlock_bh(&msk->pm.lock); > > mptcp_pm_remove_addr_entry(msk, match); > > release_sock(sk); > > sock_kfree_s(sk, match, sizeof(*match)); > > The struct is removed from a linked list with list_del_rcu(), but > doesn't > wait for a grace period before the sock_kfree_s() in the final line. > The > easy solution is to make the kfree rcu-aware and open-code the > handling of > sk_omem_alloc, like this: > > atomic_sub(sizeof(*match), &sk->sk_omem_alloc); > kfree_rcu(match); How about adding a new helper sock_kfree_rcu_s() here? > > instead of calling sock_kfree_s(). tcp_md5_do_del() takes this > approach. > This does allow possible allocation beyond the optmem limit, but only > for > the duration of the rcu grace period. > > I can send a patch tomorrow, unless someone wants to implement it > before Great! Looking forward to your patch. Thanks, -Geliang > then. > > > - Mat > > > > > > Geliang Tang (3): > > mptcp: pm: userspace: local_addr_used-- after sending REMOVE_ADDR > > mptcp: pm: userspace: drop delete_local_addr helper > > selftests: mptcp: send REMOVE_ADDR after subflow is deleted > > > > net/mptcp/pm_userspace.c | 37 +++------------- > > --- > > .../testing/selftests/net/mptcp/mptcp_join.sh | 4 +- > > .../selftests/net/mptcp/userspace_pm.sh | 6 +++ > > 3 files changed, 14 insertions(+), 33 deletions(-) > > > > -- > > 2.43.0 > > > > > >
From: Geliang Tang <tanggeliang@kylinos.cn> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/403 Geliang Tang (3): mptcp: pm: userspace: local_addr_used-- after sending REMOVE_ADDR mptcp: pm: userspace: drop delete_local_addr helper selftests: mptcp: send REMOVE_ADDR after subflow is deleted net/mptcp/pm_userspace.c | 37 +++---------------- .../testing/selftests/net/mptcp/mptcp_join.sh | 4 +- .../selftests/net/mptcp/userspace_pm.sh | 6 +++ 3 files changed, 14 insertions(+), 33 deletions(-)