Handle subflow join failures

squashto: mptcp: Add handling of outgoing MP_JOIN requests

When the join request fails we should send a reset and also
close the socket, we can't do anything useful with such a
connection.

Signed-off-by: Florian Westphal <fw@strlen.de>

---
 net/mptcp/subflow.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

On Tue, 2020-01-14 at 14:57 +0100, Florian Westphal wrote:
> squashto: mptcp: Add handling of outgoing MP_JOIN requests
> 
> When the join request fails we should send a reset and also
> close the socket, we can't do anything useful with such a
> connection.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> 
> ---
>  net/mptcp/subflow.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index f69a75f6bd6e..03f4604e2cb7 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -187,10 +187,13 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
>  
>  	subflow->icsk_af_ops->sk_rx_dst_set(sk, skb);
>  
> -	if (!subflow->conn)
> +	if (subflow->conn_finished || !tcp_sk(sk)->is_mptcp)
>  		return;
>  
> -	if (subflow->mp_capable && !subflow->conn_finished) {
> +	if (!subflow->conn)
> +		goto do_reset;
> +
> +	if (subflow->mp_capable) {
>  		pr_debug("subflow=%p, remote_key=%llu", mptcp_subflow_ctx(sk),
>  			 subflow->remote_key);
>  		mptcp_finish_connect(sk);
> @@ -200,14 +203,13 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
>  			pr_debug("synack seq=%u", TCP_SKB_CB(skb)->seq);
>  			subflow->ssn_offset = TCP_SKB_CB(skb)->seq;
>  		}
> -	} else if (subflow->mp_join && !subflow->conn_finished) {
> +	} else if (subflow->mp_join) {
>  		pr_debug("subflow=%p, thmac=%llu, remote_nonce=%u",
>  			 subflow, subflow->thmac,
>  			 subflow->remote_nonce);
>  		if (!subflow_thmac_valid(subflow)) {
>  			subflow->mp_join = 0;
> -			// @@ need to trigger RST
> -			return;
> +			goto do_reset;
>  		}
>  
>  		mptcp_crypto_hmac_sha(subflow->local_key, subflow->remote_key,
> 

> @@ -218,8 +220,14 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
>  		if (skb)
>  			subflow->ssn_offset = TCP_SKB_CB(skb)->seq;
>  
> -		if (mptcp_finish_join(sk))
> -			subflow->conn_finished = 1;
> +		if (!mptcp_finish_join(sk))
> +			goto do_reset;
> +
> +		subflow->conn_finished = 1;
> +	} else {
> +do_reset:
> +		tcp_send_active_reset(sk, GFP_ATOMIC);
> +		tcp_done(sk);
>  	}
>  }

LGTM! It's nice to remove that old todo ;)

Thank you!

Paolo

Hi Florian, Paolo,

On 15/01/2020 09:41, Paolo Abeni wrote:
> On Tue, 2020-01-14 at 14:57 +0100, Florian Westphal wrote:
>> squashto: mptcp: Add handling of outgoing MP_JOIN requests
>>
>> When the join request fails we should send a reset and also
>> close the socket, we can't do anything useful with such a
>> connection.

[...]

> LGTM! It's nice to remove that old todo ;)
Thank you for the patch and the review!

- cc94bbe4718f: "squashed" in "mptcp: Add handling of outgoing MP_JOIN 
requests"
- 704e4fa8a50c..4fe8bda1127e: result

Tests + export will be launched soon!

Cheers,
Matt

On Wed, 2020-01-15 at 09:41 +0100, Paolo Abeni wrote:
> On Tue, 2020-01-14 at 14:57 +0100, Florian Westphal wrote:
> > squashto: mptcp: Add handling of outgoing MP_JOIN requests
> > 
> > When the join request fails we should send a reset and also
> > close the socket, we can't do anything useful with such a
> > connection.
> > 
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > 
> > ---
> >  net/mptcp/subflow.c | 22 +++++++++++++++-------
> >  1 file changed, 15 insertions(+), 7 deletions(-)
> > 
> > diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> > index f69a75f6bd6e..03f4604e2cb7 100644
> > --- a/net/mptcp/subflow.c
> > +++ b/net/mptcp/subflow.c
> > @@ -187,10 +187,13 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
> >  
> >  	subflow->icsk_af_ops->sk_rx_dst_set(sk, skb);
> >  
> > -	if (!subflow->conn)
> > +	if (subflow->conn_finished || !tcp_sk(sk)->is_mptcp)
> >  		return;
> >  
> > -	if (subflow->mp_capable && !subflow->conn_finished) {
> > +	if (!subflow->conn)
> > +		goto do_reset;
> > +
> > +	if (subflow->mp_capable) {
> >  		pr_debug("subflow=%p, remote_key=%llu", mptcp_subflow_ctx(sk),
> >  			 subflow->remote_key);
> >  		mptcp_finish_connect(sk);
> > @@ -200,14 +203,13 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
> >  			pr_debug("synack seq=%u", TCP_SKB_CB(skb)->seq);
> >  			subflow->ssn_offset = TCP_SKB_CB(skb)->seq;
> >  		}
> > -	} else if (subflow->mp_join && !subflow->conn_finished) {
> > +	} else if (subflow->mp_join) {
> >  		pr_debug("subflow=%p, thmac=%llu, remote_nonce=%u",
> >  			 subflow, subflow->thmac,
> >  			 subflow->remote_nonce);
> >  		if (!subflow_thmac_valid(subflow)) {
> >  			subflow->mp_join = 0;
> > -			// @@ need to trigger RST
> > -			return;
> > +			goto do_reset;
> >  		}
> >  
> >  		mptcp_crypto_hmac_sha(subflow->local_key, subflow->remote_key,
> > 
> > @@ -218,8 +220,14 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
> >  		if (skb)
> >  			subflow->ssn_offset = TCP_SKB_CB(skb)->seq;
> >  
> > -		if (mptcp_finish_join(sk))
> > -			subflow->conn_finished = 1;
> > +		if (!mptcp_finish_join(sk))
> > +			goto do_reset;
> > +
> > +		subflow->conn_finished = 1;
> > +	} else {
> > +do_reset:
> > +		tcp_send_active_reset(sk, GFP_ATOMIC);
> > +		tcp_done(sk);
> >  	}
> >  }
> 
> LGTM! It's nice to remove that old todo ;)

Ditto this, I put that there a long time ago. Thank-you!

Peter.

> Thank you!
> 
> Paolo
> _______________________________________________
> mptcp mailing list -- mptcp@lists.01.org
> To unsubscribe send an email to mptcp-leave@lists.01.org