| Message ID | 1617263626-66920-1-git-send-email-liyonglong@chinatelecom.cn |
|---|---|
| State | Rejected, archived |
| Delegated to: | Paolo Abeni |
| Headers | show |
| Series | [v3] mptcp: ensure there is unacked data at all subflow | expand |
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
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 > > >
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)
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(-)