[PATCH mptcp-next v2] mptcp: Retransmit DATA_FIN

[PATCH mptcp-next v2] mptcp: Retransmit DATA_FIN

[Mat Martineau]

With this change, the MPTCP-level retransmission timer is used to resend
DATA_FIN. The retranmit timer is not stopped while waiting for a
MPTCP-level ACK of DATA_FIN, and retransmitted DATA_FINs are sent on all
subflows. The retry interval starts at TCP_RTO_MIN and then doubles on
each attempt, up to TCP_RTO_MAX.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/146
Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---

v2: Simplify timeout limits, move function to avoid -net conflict

v1: Add backoff

Paolo and Matthieu mentioned sending to -net instead of net-next on IRC,
I will test out on that branch too.

 net/mptcp/protocol.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index a8180a917649..661873b373e3 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -392,6 +392,14 @@ static bool mptcp_pending_data_fin(struct sock *sk, u64 *seq)
 	return false;
 }
 
+static void mptcp_set_datafin_timeout(const struct sock *sk)
+{
+	struct inet_connection_sock *icsk = inet_csk(sk);
+
+	mptcp_sk(sk)->timer_ival = min(TCP_RTO_MAX,
+				       TCP_RTO_MIN << icsk->icsk_retransmits);
+}
+
 static void mptcp_set_timeout(const struct sock *sk, const struct sock *ssk)
 {
 	long tout = ssk && inet_csk(ssk)->icsk_pending ?
@@ -1061,7 +1069,8 @@ static void __mptcp_clean_una(struct sock *sk)
 		}
 	}
 
-	if (snd_una == READ_ONCE(msk->snd_nxt)) {
+	if (snd_una == READ_ONCE(msk->snd_nxt) &&
+	    !mptcp_data_fin_enabled(msk)) {
 		if (msk->timer_ival)
 			mptcp_stop_timer(sk);
 	} else {
@@ -2287,8 +2296,19 @@ static void __mptcp_retrans(struct sock *sk)
 
 	__mptcp_clean_una_wakeup(sk);
 	dfrag = mptcp_rtx_head(sk);
-	if (!dfrag)
+	if (!dfrag) {
+		if (mptcp_data_fin_enabled(msk)) {
+			struct inet_connection_sock *icsk = inet_csk(sk);
+
+			icsk->icsk_retransmits++;
+			mptcp_set_datafin_timeout(sk);
+			mptcp_send_ack(msk);
+
+			goto reset_timer;
+		}
+
 		return;
+	}
 
 	ssk = mptcp_subflow_get_retrans(msk);
 	if (!ssk)
-- 
2.31.1

Re: [PATCH mptcp-next v2] mptcp: Retransmit DATA_FIN

[Matthieu Baerts]

Hi Mat,

On 21/04/2021 17:38, Mat Martineau wrote:
> With this change, the MPTCP-level retransmission timer is used to resend
> DATA_FIN. The retranmit timer is not stopped while waiting for a
> MPTCP-level ACK of DATA_FIN, and retransmitted DATA_FINs are sent on all
> subflows. The retry interval starts at TCP_RTO_MIN and then doubles on
> each attempt, up to TCP_RTO_MAX.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/146
> Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
> 
> v2: Simplify timeout limits, move function to avoid -net conflict
> 
> v1: Add backoff
> 
> Paolo and Matthieu mentioned sending to -net instead of net-next on IRC,
> I will test out on that branch too.

Thank you for the patch!

As discussed on IRC, I just applied this patch in our tree without
Paolo's ACK. It seems you correctly addressed all the comments so no
need to wait. We can apply some squash-to patches later if needed.

- 7137c5d18e29: mptcp: Retransmit DATA_FIN
- Results: 5eaa9f1d555d..c5409d6a14e5

Builds and tests are now in progress:

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

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