[PATCH v3] mptcp: ensure there is unacked data at all subflow

[PATCH v3] mptcp: ensure there is unacked data at all subflow

[Yonglong Li]

MPTCP-level retransmit may take place if first subflow without data in
its write queue and second subflow is likely to have unacked data left
in its write queue. In this condition datamaybe double retransmitted.

Signed-off-by: Yonglong Li <liyonglong@chinatelecom.cn>
---
 net/mptcp/protocol.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 9d7e7e1..66707ab 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2072,6 +2072,7 @@ static struct sock *mptcp_subflow_get_retrans(const struct mptcp_sock *msk)
 {
 	struct mptcp_subflow_context *subflow;
 	struct sock *backup = NULL;
+	struct sock *idle = NULL;
 
 	sock_owned_by_me((const struct sock *)msk);
 
@@ -2097,10 +2098,11 @@ static struct sock *mptcp_subflow_get_retrans(const struct mptcp_sock *msk)
 			continue;
 		}
 
-		return ssk;
+		if (!idle)
+			idle = ssk;
 	}
 
-	return backup;
+	return idle ? : backup;
 }
 
 static void mptcp_dispose_initial_subflow(struct mptcp_sock *msk)
-- 
1.8.3.1

Re: [PATCH v3] mptcp: ensure there is unacked data at all subflow

[Paolo Abeni]

Hello,

I'm sorry for the late feeback.

On Thu, 2021-04-01 at 15:53 +0800, Yonglong Li wrote:
> MPTCP-level retransmit may take place if first subflow without data in
> its write queue and second subflow is likely to have unacked data left
> in its write queue. In this condition datamaybe double retransmitted.

Note that the above does not cause problems at the byte stream level,
and can improve the aggregate tput in some cases, as it could allow
faster forward progress when e.g. the substream which originally
handled the relevant data has experienced some congestion.

Do you have any specific scenario which is badly affected by the
current code? How does the self-tests behave on top of this patch?
(specifically: simult_flows.sh)

Side note: we could improve re-injection keeping explicit track at the
 mptcp_data_frag level of the already used subflows, and avoding them
at mptcp_subflow_get_retrans() time.

Thanks!

Paolo

Re: [PATCH v3] mptcp: ensure there is unacked data at all subflow

[Yonglong Li]

Hi Paolo,

Thanks for your feeback.

On 2021/4/1 18:08, Paolo Abeni wrote:
> Hello,
> 
> I'm sorry for the late feeback.
> 
> On Thu, 2021-04-01 at 15:53 +0800, Yonglong Li wrote:
>> MPTCP-level retransmit may take place if first subflow without data in
>> its write queue and second subflow is likely to have unacked data left
>> in its write queue. In this condition datamaybe double retransmitted.
> 
> Note that the above does not cause problems at the byte stream level,
> and can improve the aggregate tput in some cases, as it could allow
> faster forward progress when e.g. the substream which originally
> handled the relevant data has experienced some congestion.
> 
> Do you have any specific scenario which is badly affected by the
> current code? How does the self-tests behave on top of this patch?
> (specifically: simult_flows.sh)

In my test, current code doesn't cause any badly affected. And in some
cases it can improve the tput in theory (re-injection likely). But the
behave of current code is not stable. In some cases it 're-injection'
data, and some cases it does not. And as you said at "Side note" the
're-injection' should not do at mptcp_subflow_get_retrans()

> 
> Side note: we could improve re-injection keeping explicit track at the
>  mptcp_data_frag level of the already used subflows, and avoding them
> at mptcp_subflow_get_retrans() time.
> 
> Thanks!
> 
> Paolo
> 
> 
>