Re: [MPTCP][PATCH mptcp-next] Squash to "mptcp: remove MPTCP_ADD_ADDR_IPV6 and MPTCP_ADD_ADDR_PORT"

[Geliang Tang <geliangtang@gmail.com>]

Hi Yonglong,

Yonglong Li <liyonglong@chinatelecom.cn> 于2021年7月13日周二 下午6:30写道：
>
>
>
> On 2021/7/13 14:44, Geliang Tang wrote:
> > Keep the debug info for "send ack".
> >
> > Don't drop mptcp_pm_should_add_signal_ipv6() and
> > mptcp_pm_should_add_signal_port().
> >
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> > ---
> >  net/mptcp/pm_netlink.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > index 3e35720317ae..2cd6caaedb08 100644
> > --- a/net/mptcp/pm_netlink.c
> > +++ b/net/mptcp/pm_netlink.c
> > @@ -543,8 +543,11 @@ void mptcp_pm_nl_addr_send_ack(struct mptcp_sock *msk)
> >               bool slow;
> >
> >               spin_unlock_bh(&msk->pm.lock);
> > -             pr_debug("send ack for %s",
> > -                      mptcp_pm_should_add_signal(msk) ? "add_addr" : "rm_addr");
> > +             pr_debug("send ack for %s%s%s",
> > +                      mptcp_pm_should_add_signal_addr(msk) ? "add_addr" :
> > +                      (mptcp_pm_should_add_signal_echo(msk) ? "add_echo" : "rm_addr"),
> > +                      mptcp_pm_should_add_signal_ipv6(msk) ? " [ipv6]" : "",
> > +                      mptcp_pm_should_add_signal_port(msk) ? " [port]" : "");
> >
> >               slow = lock_sock_fast(ssk);
> >               tcp_send_ack(ssk);
> >
> Hi Geliang,
>
> I think the debug log will be incorrect if add_addr and add_echo events trigger at the same time.
> WDYT?

Yes, how about moving this pr_debug line under the pm lock, just swap this
pr_debug line with the spin_unlock_bh line?

If so, no need to use READ_ONCE() in mptcp_pm_should_add_signal_ipv6/port
too.


I had tested this squash-to patches all night yesterday. And I got this
error in the debug log:

add_signal error, add_addr=2, echo=1

This means the race occurs, right?

Does it mean that this version cannot deal with the race either? I'm not
sure.

Could you please share your test scripts to me?

Thanks,
-Geliang



>
> --
> Li YongLong