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

* [PATCH mptcp-next] mptcp: full fully established support after ADD_ADDR
@ 2021-08-12 16:27 Matthieu Baerts
  2021-08-12 16:33 ` Paolo Abeni
  2021-08-12 22:30 ` Mat Martineau
  0 siblings, 2 replies; 6+ messages in thread
From: Matthieu Baerts @ 2021-08-12 16:27 UTC (permalink / raw)
  To: mptcp; +Cc: Matthieu Baerts, Paolo Abeni

If directly after an MP_CAPABLE 3WHS, the client receives a valid
ADD_ADDR from the server, it is enough to prove exchanged MPTCP options
are valid. Indeed, the ADD_ADDR contains an HMAC generated using both
the client and server keys.

It was then OK to enable the fully_established flag on the MPTCP socket.

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")
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---

Notes:
    Once applied, we can close issue 221. But this patch alone is not enough
    to fix issue 221, hence no "Closes" tag.
    
    Because the PM was not notified, I think it is best to send it to -net.
    Before, I think we were overriding MPC ACK options when sending the echo
    ADD_ADDR so the issue was not visible.
    
    Here it is visible (and this is covered by out tests) since the
    introduction of commit 32c3e4d50806 ("Squash-to: "mptcp: move
    drop_other_suboptions check under pm lock"")
    
    With this patch, the packetdrill tests are all OK!
    (And the other selftests as well even if I still have other issues with
     mptcp_join.sh but it doesn't seem related.)

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

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index d94ff50c29b3..175563c95c0b 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -945,20 +945,15 @@ 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) {
 		/* 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread