From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v20 5/5] fixup: TCP_REPAIR_WINDOW before send unsent
Date: Thu, 13 Feb 2025 15:58:13 +1100 [thread overview]
Message-ID: <20250213045813.3767488-6-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20250213045813.3767488-1-david@gibson.dropbear.id.au>
We, like libsoccr handle restoring the send queue in two pieces. The
"already sent" piece is written in repair mode, to repopulate the kernel
sndbuf without actually sending new packets to the peer. The "not sent"
piece is written out of repair mode, so that we both put it into sndbuf
*and* actually send it out.
To do that we temporarily drop out of repair mode. However we do so before
we've called TCP_REPAIR_WINDOW, meaning we're doing real send()s on a real,
non-repair socket that has bad window information. That seems bad.
Despite it differing from libsoccr, move the sending of the non sent
queued data until after the *final* repair off. I strongly suspect
that both we and libsoccr were only (kind of) getting away with this
because notsent is usually 0.
This seems to fix an intermittent hang I was seeing on
migrate/iperf3_bidir6. I was seeing that perhaps 1 time in 3, or 1
time in 5 with DEBUG=1. I did observe a nonzero notsent the one time
I reproduced after I knew what I was looking for.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
tcp.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/tcp.c b/tcp.c
index 39035e14..3f41cf0b 100644
--- a/tcp.c
+++ b/tcp.c
@@ -3532,25 +3532,22 @@ int tcp_flow_migrate_target_ext(struct ctx *c, union flow *flow, int fd)
shutdown(s, SHUT_WR);
}
+ if ((rc = tcp_flow_repair_wnd(s, &t)))
+ return rc;
+
+ tcp_flow_repair_off(c, conn);
+ repair_flush(c);
+
if (t.notsent) {
- tcp_flow_repair_off(c, conn);
- repair_flush(c);
+ err("socket %i, t.sndq=%u t.notsent=%u",
+ s, t.sndq, t.notsent);
if ((rc = tcp_flow_repair_queue(s, t.notsent,
tcp_migrate_snd_queue +
(t.sndq - t.notsent))))
return rc;
-
- tcp_flow_repair_on(c, conn);
- repair_flush(c);
}
- if ((rc = tcp_flow_repair_wnd(s, &t)))
- return rc;
-
- tcp_flow_repair_off(c, conn);
- repair_flush(c);
-
/* If we sent a FIN but it wasn't acknowledged yet (TCP_FIN_WAIT1), send
* it out, because we don't know if we already sent it.
*
--
@@ -3532,25 +3532,22 @@ int tcp_flow_migrate_target_ext(struct ctx *c, union flow *flow, int fd)
shutdown(s, SHUT_WR);
}
+ if ((rc = tcp_flow_repair_wnd(s, &t)))
+ return rc;
+
+ tcp_flow_repair_off(c, conn);
+ repair_flush(c);
+
if (t.notsent) {
- tcp_flow_repair_off(c, conn);
- repair_flush(c);
+ err("socket %i, t.sndq=%u t.notsent=%u",
+ s, t.sndq, t.notsent);
if ((rc = tcp_flow_repair_queue(s, t.notsent,
tcp_migrate_snd_queue +
(t.sndq - t.notsent))))
return rc;
-
- tcp_flow_repair_on(c, conn);
- repair_flush(c);
}
- if ((rc = tcp_flow_repair_wnd(s, &t)))
- return rc;
-
- tcp_flow_repair_off(c, conn);
- repair_flush(c);
-
/* If we sent a FIN but it wasn't acknowledged yet (TCP_FIN_WAIT1), send
* it out, because we don't know if we already sent it.
*
--
2.48.1
prev parent reply other threads:[~2025-02-13 4:58 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-13 4:58 [PATCH v20 0/5] State migration David Gibson
2025-02-13 4:58 ` [PATCH v20 1/5] migrate: Migrate TCP flows David Gibson
2025-02-13 4:58 ` [PATCH v20 2/5] test: Add migration tests David Gibson
2025-02-13 4:58 ` [PATCH v20 3/5] fixup?: loop on EGAIN in repair queue David Gibson
2025-02-13 4:58 ` [PATCH v20 4/5] depause David Gibson
2025-02-13 4:58 ` David Gibson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250213045813.3767488-6-david@gibson.dropbear.id.au \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).