* [PATCH mptcp-next] mptcp: Retransmit DATA_FIN
@ 2021-04-19 21:38 Mat Martineau
2021-04-21 8:17 ` Paolo Abeni
0 siblings, 1 reply; 3+ messages in thread
From: Mat Martineau @ 2021-04-19 21:38 UTC (permalink / raw)
To: mptcp; +Cc: Mat Martineau, pabeni
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 with a couple of attempts 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
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
net/mptcp/protocol.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index a8180a917649..1fddaeeb8051 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -11,6 +11,7 @@
#include <linux/netdevice.h>
#include <linux/sched/signal.h>
#include <linux/atomic.h>
+#include <linux/log2.h>
#include <net/sock.h>
#include <net/inet_common.h>
#include <net/inet_hashtables.h>
@@ -402,6 +403,15 @@ static void mptcp_set_timeout(const struct sock *sk, const struct sock *ssk)
mptcp_sk(sk)->timer_ival = tout > 0 ? tout : TCP_RTO_MIN;
}
+static void mptcp_set_datafin_timeout(const struct sock *sk)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+ int retransmits = clamp_val(icsk->icsk_retransmits, 1,
+ ilog2(TCP_RTO_MAX / TCP_RTO_MIN));
+
+ mptcp_sk(sk)->timer_ival = TCP_RTO_MIN << retransmits;
+}
+
static bool tcp_can_send_ack(const struct sock *ssk)
{
return !((1 << inet_sk_state_load(ssk)) &
@@ -1061,7 +1071,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 +2298,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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH mptcp-next] mptcp: Retransmit DATA_FIN
2021-04-19 21:38 [PATCH mptcp-next] mptcp: Retransmit DATA_FIN Mat Martineau
@ 2021-04-21 8:17 ` Paolo Abeni
2021-04-21 15:01 ` Mat Martineau
0 siblings, 1 reply; 3+ messages in thread
From: Paolo Abeni @ 2021-04-21 8:17 UTC (permalink / raw)
To: Mat Martineau, mptcp
Hello,
On Mon, 2021-04-19 at 14:38 -0700, 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 with a couple of attempts 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
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> ---
> net/mptcp/protocol.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index a8180a917649..1fddaeeb8051 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -11,6 +11,7 @@
> #include <linux/netdevice.h>
> #include <linux/sched/signal.h>
> #include <linux/atomic.h>
> +#include <linux/log2.h>
> #include <net/sock.h>
> #include <net/inet_common.h>
> #include <net/inet_hashtables.h>
> @@ -402,6 +403,15 @@ static void mptcp_set_timeout(const struct sock *sk, const struct sock *ssk)
> mptcp_sk(sk)->timer_ival = tout > 0 ? tout : TCP_RTO_MIN;
> }
>
> +static void mptcp_set_datafin_timeout(const struct sock *sk)
> +{
> + struct inet_connection_sock *icsk = inet_csk(sk);
> + int retransmits = clamp_val(icsk->icsk_retransmits, 1,
> + ilog2(TCP_RTO_MAX / TCP_RTO_MIN));
> +
> + mptcp_sk(sk)->timer_ival = TCP_RTO_MIN << retransmits;
The patch LGTM! I have only a couple of minor comment WRT to the above.
To me something alike:
mptcp_sk(sk)->timer_ival = min(TCP_RTO_MAX,
TCP_RTO_MIN << icsk->icsk_retransmits);
would be more readable.
Also I'm wondering if we could use max subflows 'icsk->icsk_rto'
instead of TCP_RTO_MIN - so that e.g. we are not too "aggressive" on
slow links
The latter could be really a follow-up, if needed at all.
Thanks!
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH mptcp-next] mptcp: Retransmit DATA_FIN
2021-04-21 8:17 ` Paolo Abeni
@ 2021-04-21 15:01 ` Mat Martineau
0 siblings, 0 replies; 3+ messages in thread
From: Mat Martineau @ 2021-04-21 15:01 UTC (permalink / raw)
To: Paolo Abeni; +Cc: mptcp
On Wed, 21 Apr 2021, Paolo Abeni wrote:
> Hello,
>
> On Mon, 2021-04-19 at 14:38 -0700, 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 with a couple of attempts 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
>> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
>> ---
>> net/mptcp/protocol.c | 26 ++++++++++++++++++++++++--
>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index a8180a917649..1fddaeeb8051 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -11,6 +11,7 @@
>> #include <linux/netdevice.h>
>> #include <linux/sched/signal.h>
>> #include <linux/atomic.h>
>> +#include <linux/log2.h>
>> #include <net/sock.h>
>> #include <net/inet_common.h>
>> #include <net/inet_hashtables.h>
>> @@ -402,6 +403,15 @@ static void mptcp_set_timeout(const struct sock *sk, const struct sock *ssk)
>> mptcp_sk(sk)->timer_ival = tout > 0 ? tout : TCP_RTO_MIN;
>> }
>>
>> +static void mptcp_set_datafin_timeout(const struct sock *sk)
>> +{
>> + struct inet_connection_sock *icsk = inet_csk(sk);
>> + int retransmits = clamp_val(icsk->icsk_retransmits, 1,
>> + ilog2(TCP_RTO_MAX / TCP_RTO_MIN));
>> +
>> + mptcp_sk(sk)->timer_ival = TCP_RTO_MIN << retransmits;
>
> The patch LGTM! I have only a couple of minor comment WRT to the above.
> To me something alike:
>
> mptcp_sk(sk)->timer_ival = min(TCP_RTO_MAX,
> TCP_RTO_MIN << icsk->icsk_retransmits);
>
> would be more readable.
Yeah, good point. I didn't want to start overflowing the result of
TCP_RTO_MIN << x, but that would take an hour or two without any subflow
catching up to the DATA_FIN sequence number. Probably too paranoid there.
>
> Also I'm wondering if we could use max subflows 'icsk->icsk_rto'
> instead of TCP_RTO_MIN - so that e.g. we are not too "aggressive" on
> slow links
>
> The latter could be really a follow-up, if needed at all.
By "max subflows icsk_rto", you mean look through all the subflows and use
the max icsk_rto?
Follow-up sounds right to me. Thanks!
--
Mat Martineau
Intel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-21 15:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-19 21:38 [PATCH mptcp-next] mptcp: Retransmit DATA_FIN Mat Martineau
2021-04-21 8:17 ` Paolo Abeni
2021-04-21 15:01 ` Mat Martineau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.