Re: [PATCH v5 3/4] mptcp: fix syncookie process if mptcp can not_accept new subflow

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Wed, 16 Jun 2021, wujianguo106@163.com wrote:

> From: Jianguo Wu <wujianguo@chinatelecom.cn>
>
> Lots of "TCP: tcp_fin: Impossible, sk->sk_state=7" in client side
> when doing stress testing.

Hi Jianguo -

Like patch 1, do the existing syncookie selftests trigger this sometimes?

Or are there selftests that could be added to this series, or packetdrill 
tests, to do such syncookie stress tests?

Thanks,

Mat


>
> There are at least two cases may trigger this warning:
> 1.mptcp is in syncookie, and server recv MP_JOIN SYN request,
>  in subflow_check_req(), the mptcp_can_accept_new_subflow()
>  return false, so subflow_init_req_cookie_join_save() isn't
>  called, i.e. not store the data present in the MP_JOIN syn
>  request and the random nonce in hash table - join_entries[],
>  but still send synack. When recv 3rd-ack,
>  mptcp_token_join_cookie_init_state() will return false, and
>  3rd-ack is dropped, then if mptcp conn is closed by client,
>  client will send a DATA_FIN and a MPTCP FIN, the DATA_FIN
>  doesn't have MP_CAPABLE or MP_JOIN,
>  so mptcp_subflow_init_cookie_req() will return 0, and pass
>  the cookie check, MP_JOIN request is fallback to normal TCP.
>  Server will send a TCP FIN if closed, in client side,
>  when process TCP FIN, it will do reset, the code path is:
>    tcp_data_queue()->mptcp_incoming_options()
>      ->check_fully_established()->mptcp_subflow_reset().
>  mptcp_subflow_reset() will set sock state to TCP_CLOSE,
>  so tcp_fin will hit TCP_CLOSE, and print the warning.
> 2.mptcp is in syncookie, and server recv 3rd-ack, in
>  mptcp_subflow_init_cookie_req(), mptcp_can_accept_new_subflow()
>  return false, and subflow_req->mp_join is not set to 1,
>  so in subflow_syn_recv_sock() will not reset the MP_JOIN
>  subflow, but fallback to normal TCP, and then the same thing
>  happens when server will send a TCP FIN if closed.
>
> For case1, subflow_check_req() return -EPERM,
> then tcp_conn_request() will drop MP_JOIN SYN.
>
> For case2, let subflow_syn_recv_sock() call mptcp_can_accept_new_subflow(),
> and do fatal fallback, send reset.
>
> Fixes: 9466a1ccebbe("mptcp: enable JOIN requests even if cookies are in use")
> Signed-off-by: Jianguo Wu <wujianguo@chinatelecom.cn>
> ---
> net/mptcp/subflow.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 75ed530706c0..6d98e19a20aa 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -224,6 +224,8 @@ static int subflow_check_req(struct request_sock *req,
> 		if (unlikely(req->syncookie)) {
> 			if (mptcp_can_accept_new_subflow(subflow_req->msk))
> 				subflow_init_req_cookie_join_save(subflow_req, skb);
> +			else
> +				return -EPERM;
> 		}
>
> 		pr_debug("token=%u, remote_nonce=%u msk=%p", subflow_req->token,
> @@ -263,9 +265,7 @@ int mptcp_subflow_init_cookie_req(struct request_sock *req,
> 		if (!mptcp_token_join_cookie_init_state(subflow_req, skb))
> 			return -EINVAL;
>
> -		if (mptcp_can_accept_new_subflow(subflow_req->msk))
> -			subflow_req->mp_join = 1;
> -
> +		subflow_req->mp_join = 1;
> 		subflow_req->ssn_offset = TCP_SKB_CB(skb)->seq - 1;
> 	}
>
> -- 
> 1.8.3.1
>
>
>

--
Mat Martineau
Intel