[MPTCP] Re: [PATCH 1/9] token: handle join-before-accept-completion case

[MPTCP] Re: [PATCH 1/9] token: handle join-before-accept-completion case

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 1373 bytes --]

On Sat, 2019-11-30 at 23:48 +0100, Florian Westphal wrote:
> squashto: mptcp: Add handling of incoming MP_JOIN requests
> 
> If we get a join request while the socket hasn't been accepted yet,
> the tree still contains the dummy reserved value.  Return NULL in
> this case, the token is still invalid.
> 
> Only alternative would be to serialize such request into a work queue
> to replay it at a future data when the address might have become
> available.
> 
> Signed-off-by: Florian Westphal <fw(a)strlen.de>
> ---
>  net/mptcp/token.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/token.c b/net/mptcp/token.c
> index 56f1bd2641f6..15cd71a33205 100644
> --- a/net/mptcp/token.c
> +++ b/net/mptcp/token.c
> @@ -182,8 +182,13 @@ struct mptcp_sock *mptcp_token_get_sock(u32 token)
>  
>  	spin_lock_bh(&token_tree_lock);
>  	conn = radix_tree_lookup(&token_tree, token);
> -	if (conn)
> -		sock_hold(conn);
> +	if (conn) {
> +		/* token still reserved? */
> +		if (conn == (struct sock *)&token_used)
> +			conn = NULL;
> +		else
> +			sock_hold(conn);
> +	}
>  	spin_unlock_bh(&token_tree_lock);
>  
>  	return mptcp_sk(conn);

LGTM.

Note, with MPTCP v1, we will probably have to add additional checks
here (e.g. con->fourth_ack must be true). Perhaps we can add that
already?

/P

[MPTCP] Re: [PATCH 1/9] token: handle join-before-accept-completion case

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

Paolo Abeni <pabeni(a)redhat.com> wrote:
> Note, with MPTCP v1, we will probably have to add additional checks
> here (e.g. con->fourth_ack must be true). Perhaps we can add that
> already?

I'm not sure this is the right location.  Should that check not
be done where we add the ssk to the conn_list?

[MPTCP] Re: [PATCH 1/9] token: handle join-before-accept-completion case

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 591 bytes --]

On Mon, 2019-12-02 at 14:31 +0100, Florian Westphal wrote:
> Paolo Abeni <pabeni(a)redhat.com> wrote:
> > Note, with MPTCP v1, we will probably have to add additional checks
> > here (e.g. con->fourth_ack must be true). Perhaps we can add that
> > already?
> 
> I'm not sure this is the right location.  Should that check not
> be done where we add the ssk to the conn_list?

My [mis?]understanding is that MP_JOIN should not be attemped before
the msk is fully established (fourth_ack), so I thought it would be
nice to catch that erroneous condition ASAP. 

Cheers,

Paolo

[MPTCP] Re: [PATCH 1/9] token: handle join-before-accept-completion case

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 696 bytes --]

Paolo Abeni <pabeni(a)redhat.com> wrote:
> On Mon, 2019-12-02 at 14:31 +0100, Florian Westphal wrote:
> > Paolo Abeni <pabeni(a)redhat.com> wrote:
> > > Note, with MPTCP v1, we will probably have to add additional checks
> > > here (e.g. con->fourth_ack must be true). Perhaps we can add that
> > > already?
> > 
> > I'm not sure this is the right location.  Should that check not
> > be done where we add the ssk to the conn_list?
> 
> My [mis?]understanding is that MP_JOIN should not be attemped before
> the msk is fully established (fourth_ack), so I thought it would be
> nice to catch that erroneous condition ASAP.

I see.  What would that buy us though wrt. error handling?