Re: [PATCH mptcp-next v8 1/8] mptcp: add NO_INITIAL_SUBFLOW flag

[Geliang Tang <geliang.tang@suse.com>]

Hi Matt,

On Thu, Sep 14, 2023 at 12:19:01PM +0200, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 14/09/2023 12:03, Geliang Tang wrote:
> > If the initial subflow has been removed, we cannot know without checking
> > other counters, e.g. ss -ti <filter> | grep -c tcp-ulp-mptcp or
> > getsockopt(SOL_MPTCP, MPTCP_FULL_INFO, ...) (or others except MPTCP_INFO
> > of course) and then check mptcp_subflow_data->num_subflows to get the
> > total amount of subflows.
> > 
> > This patch adds a new flag NO_INITIAL_SUBFLOW in mptcpi_flags to know if
> > the initial subflow has been removed. With this bit, we can then compute
> > the total amount of subflows from mptcp_info by doing something like:
> > 
> >   if (mptcpi_flags & MPTCP_INFO_FLAG_HAS_INITIAL_SUBFLOW)
> > 	  mptcpi_subflows += 1;
> 
> Thank you for looking at that!
> 
> (...)
> 
> > diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
> > index 8260202c0066..aebf692543a5 100644
> > --- a/net/mptcp/sockopt.c
> > +++ b/net/mptcp/sockopt.c
> > @@ -919,6 +919,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
> >  		flags |= MPTCP_INFO_FLAG_FALLBACK;
> >  	if (READ_ONCE(msk->can_ack))
> >  		flags |= MPTCP_INFO_FLAG_REMOTE_KEY_RECEIVED;
> > +	if (__mptcp_check_no_initial_subflow(msk))
> > +		flags |= MPTCP_INFO_FLAG_NO_INITIAL_SUBFLOW;
> 
> I had a very brief look but can we not just check if msk->first == NULL
> instead?

msk->first is not NULL in this case.

When closing the initial subflow in __mptcp_close_ssk(), dispose_it is
false, it goes to:

        if (!dispose_it) {
                /* The MPTCP code never wait on the subflow sockets, TCP-level
                 * disconnect should never fail
                 */
                WARN_ON_ONCE(tcp_disconnect(ssk, 0));
                mptcp_subflow_ctx_reset(subflow);
                release_sock(ssk);

                goto out;
        }

... ...

        if (ssk == msk->first)
                WRITE_ONCE(msk->first, NULL);

out:
        if (need_push)
                __mptcp_push_pending(sk, 0);

"WRITE_ONCE(msk->first, NULL)" is skipped in this case.

Thanks,
-Geliang

> 
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net