[PATCH mptcp-next v3] mptcp: full fully established support after ADD_ADDR

[PATCH mptcp-next v3] mptcp: full fully established support after ADD_ADDR

[Matthieu Baerts]

If directly after an MP_CAPABLE 3WHS, the client receives an ADD_ADDR
with HMAC from the server, it is enough to switch to a "fully
established" mode because it has received more MPTCP options.

It was then OK to enable the "fully_established" flag on the MPTCP
socket. Still, best to check if the ADD_ADDR looks valid by looking if
it contains an HMAC (no 'echo' bit). If an ADD_ADDR echo is received
while we are not in "fully established" mode, it is strange and then
we should not switch to this mode now.

But that is not enough. On one hand, the path-manager has be notified
the state has changed. On the other hand, the "fully_established" flag
on the subflow socket should be turned on as well not to re-send the
MP_CAPABLE 3rd ACK content with the next ACK.

Fixes: 84dfe3677a6f ("mptcp: send out dedicated ADD_ADDR packet")
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---

Notes:
    - v2: reword the commit message not to mention "valid" content (Mat)
    - v3: squash the two patches together and update the commit message (Mat)

 net/mptcp/options.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index d94ff50c29b3..bec3ed82e253 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -945,20 +945,16 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
 		return subflow->mp_capable;
 	}
 
-	if (mp_opt->dss && mp_opt->use_ack) {
+	if ((mp_opt->dss && mp_opt->use_ack) ||
+	    (mp_opt->add_addr && !mp_opt->echo)) {
 		/* subflows are fully established as soon as we get any
-		 * additional ack.
+		 * additional ack, including ADD_ADDR.
 		 */
 		subflow->fully_established = 1;
 		WRITE_ONCE(msk->fully_established, true);
 		goto fully_established;
 	}
 
-	if (mp_opt->add_addr) {
-		WRITE_ONCE(msk->fully_established, true);
-		return true;
-	}
-
 	/* If the first established packet does not contain MP_CAPABLE + data
 	 * then fallback to TCP. Fallback scenarios requires a reset for
 	 * MP_JOIN subflows.
-- 
2.32.0

Re: [PATCH mptcp-next v3] mptcp: full fully established support after ADD_ADDR

[Mat Martineau]

On Tue, 17 Aug 2021, Matthieu Baerts wrote:

> If directly after an MP_CAPABLE 3WHS, the client receives an ADD_ADDR
> with HMAC from the server, it is enough to switch to a "fully
> established" mode because it has received more MPTCP options.
>
> It was then OK to enable the "fully_established" flag on the MPTCP
> socket. Still, best to check if the ADD_ADDR looks valid by looking if
> it contains an HMAC (no 'echo' bit). If an ADD_ADDR echo is received
> while we are not in "fully established" mode, it is strange and then
> we should not switch to this mode now.
>
> But that is not enough. On one hand, the path-manager has be notified
> the state has changed. On the other hand, the "fully_established" flag
> on the subflow socket should be turned on as well not to re-send the
> MP_CAPABLE 3rd ACK content with the next ACK.
>
> Fixes: 84dfe3677a6f ("mptcp: send out dedicated ADD_ADDR packet")
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
> ---
>
> Notes:
>    - v2: reword the commit message not to mention "valid" content (Mat)
>    - v3: squash the two patches together and update the commit message (Mat)

Thanks for squashing! Still ok to tag:

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


>
> net/mptcp/options.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index d94ff50c29b3..bec3ed82e253 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -945,20 +945,16 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
> 		return subflow->mp_capable;
> 	}
>
> -	if (mp_opt->dss && mp_opt->use_ack) {
> +	if ((mp_opt->dss && mp_opt->use_ack) ||
> +	    (mp_opt->add_addr && !mp_opt->echo)) {
> 		/* subflows are fully established as soon as we get any
> -		 * additional ack.
> +		 * additional ack, including ADD_ADDR.
> 		 */
> 		subflow->fully_established = 1;
> 		WRITE_ONCE(msk->fully_established, true);
> 		goto fully_established;
> 	}
>
> -	if (mp_opt->add_addr) {
> -		WRITE_ONCE(msk->fully_established, true);
> -		return true;
> -	}
> -
> 	/* If the first established packet does not contain MP_CAPABLE + data
> 	 * then fallback to TCP. Fallback scenarios requires a reset for
> 	 * MP_JOIN subflows.
> -- 
> 2.32.0
>
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v3] mptcp: full fully established support after ADD_ADDR

[Matthieu Baerts]

Hi Mat,

On 17/08/2021 18:39, Mat Martineau wrote:
> On Tue, 17 Aug 2021, Matthieu Baerts wrote:
> 
>> If directly after an MP_CAPABLE 3WHS, the client receives an ADD_ADDR
>> with HMAC from the server, it is enough to switch to a "fully
>> established" mode because it has received more MPTCP options.
>>
>> It was then OK to enable the "fully_established" flag on the MPTCP
>> socket. Still, best to check if the ADD_ADDR looks valid by looking if
>> it contains an HMAC (no 'echo' bit). If an ADD_ADDR echo is received
>> while we are not in "fully established" mode, it is strange and then
>> we should not switch to this mode now.
>>
>> But that is not enough. On one hand, the path-manager has be notified
>> the state has changed. On the other hand, the "fully_established" flag
>> on the subflow socket should be turned on as well not to re-send the
>> MP_CAPABLE 3rd ACK content with the next ACK.
>>
>> Fixes: 84dfe3677a6f ("mptcp: send out dedicated ADD_ADDR packet")
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>
>> Notes:
>>    - v2: reword the commit message not to mention "valid" content (Mat)
>>    - v3: squash the two patches together and update the commit message
>> (Mat)
> 
> Thanks for squashing! Still ok to tag:
> 
> Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>

Thank you for the review!

Now in our tree (fix for net) with your RvB tag:

- 3c39f1cc7dcb: mptcp: full fully established support after ADD_ADDR
- Results: b2f0470e318c..0c5e0eca2d62

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20210817T164404
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export/20210817T164404

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