From: Stefano Brivio <sbrivio@redhat.com>
To: passt-dev@passt.top
Cc: Jon Maloy <jmaloy@redhat.com>,
Paul Holzinger <pholzing@redhat.com>,
David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH v4 7/8] tcp: Fast re-transmit if half-closed, make TAP_FIN_RCVD path consistent
Date: Tue, 9 Sep 2025 20:16:54 +0200 [thread overview]
Message-ID: <20250909181655.2990223-8-sbrivio@redhat.com> (raw)
In-Reply-To: <20250909181655.2990223-1-sbrivio@redhat.com>
We currently have a number of discrepancies in the tcp_tap_handler()
path between the half-closed connection path and the regular one, and
they are mostly a result of code duplication, which comes in turn from
the fact that tcp_data_from_tap() deals with data transfers as well as
general connection bookkeeping, so we can't use it for half-closed
connections.
This suggests that we should probably rework it into two or more
functions, in the long term, but for the moment being I'm just fixing
one obvious issue, which is the lack of fast retransmissions in the
TAP_FIN_RCVD path, and a potential one, which is the fact we don't
handle socket flush failures.
Add fast re-transmit for half-closed connections, and handle the case
of socket flush (tcp_sock_consume()) flush failure in the same way as
tcp_data_from_tap() handles it.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
tcp.c | 42 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/tcp.c b/tcp.c
index 9c70a25..5163dbf 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1652,6 +1652,23 @@ static int tcp_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
return tcp_buf_data_from_sock(c, conn);
}
+/**
+ * tcp_packet_data_len() - Get data (TCP payload) length for a TCP packet
+ * @th: Pointer to TCP header
+ * @l4len: TCP packet length, including TCP header
+ *
+ * Return: data length of TCP packet, -1 on invalid value of Data Offset field
+ */
+static ssize_t tcp_packet_data_len(const struct tcphdr *th, size_t l4len)
+{
+ size_t off = th->doff * 4UL;
+
+ if (off < sizeof(*th) || off > l4len)
+ return -1;
+
+ return l4len - off;
+}
+
/**
* tcp_data_from_tap() - tap/guest data for established connection
* @c: Execution context
@@ -2113,9 +2130,28 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
/* Established connections not accepting data from tap */
if (conn->events & TAP_FIN_RCVD) {
- tcp_sock_consume(conn, ntohl(th->ack_seq));
- tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq));
- if (tcp_tap_window_update(c, conn, ntohs(th->window)))
+ bool retr;
+
+ retr = th->ack && !tcp_packet_data_len(th, l4len) && !th->fin &&
+ ntohl(th->ack_seq) == conn->seq_ack_from_tap &&
+ ntohs(th->window) == conn->wnd_from_tap;
+
+ /* On socket flush failure, pretend there was no ACK, try again
+ * later
+ */
+ if (th->ack && !tcp_sock_consume(conn, ntohl(th->ack_seq)))
+ tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq));
+
+ if (retr) {
+ flow_trace(conn,
+ "fast re-transmit, ACK: %u, previous sequence: %u",
+ ntohl(th->ack_seq), conn->seq_to_tap);
+
+ if (tcp_rewind_seq(c, conn))
+ return -1;
+ }
+
+ if (tcp_tap_window_update(c, conn, ntohs(th->window)) || retr)
tcp_data_from_sock(c, conn);
if (conn->seq_ack_from_tap == conn->seq_to_tap) {
--
@@ -1652,6 +1652,23 @@ static int tcp_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
return tcp_buf_data_from_sock(c, conn);
}
+/**
+ * tcp_packet_data_len() - Get data (TCP payload) length for a TCP packet
+ * @th: Pointer to TCP header
+ * @l4len: TCP packet length, including TCP header
+ *
+ * Return: data length of TCP packet, -1 on invalid value of Data Offset field
+ */
+static ssize_t tcp_packet_data_len(const struct tcphdr *th, size_t l4len)
+{
+ size_t off = th->doff * 4UL;
+
+ if (off < sizeof(*th) || off > l4len)
+ return -1;
+
+ return l4len - off;
+}
+
/**
* tcp_data_from_tap() - tap/guest data for established connection
* @c: Execution context
@@ -2113,9 +2130,28 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
/* Established connections not accepting data from tap */
if (conn->events & TAP_FIN_RCVD) {
- tcp_sock_consume(conn, ntohl(th->ack_seq));
- tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq));
- if (tcp_tap_window_update(c, conn, ntohs(th->window)))
+ bool retr;
+
+ retr = th->ack && !tcp_packet_data_len(th, l4len) && !th->fin &&
+ ntohl(th->ack_seq) == conn->seq_ack_from_tap &&
+ ntohs(th->window) == conn->wnd_from_tap;
+
+ /* On socket flush failure, pretend there was no ACK, try again
+ * later
+ */
+ if (th->ack && !tcp_sock_consume(conn, ntohl(th->ack_seq)))
+ tcp_update_seqack_from_tap(c, conn, ntohl(th->ack_seq));
+
+ if (retr) {
+ flow_trace(conn,
+ "fast re-transmit, ACK: %u, previous sequence: %u",
+ ntohl(th->ack_seq), conn->seq_to_tap);
+
+ if (tcp_rewind_seq(c, conn))
+ return -1;
+ }
+
+ if (tcp_tap_window_update(c, conn, ntohs(th->window)) || retr)
tcp_data_from_sock(c, conn);
if (conn->seq_ack_from_tap == conn->seq_to_tap) {
--
2.43.0
next prev parent reply other threads:[~2025-09-09 18:16 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-09 18:16 [PATCH v4 0/8] tcp: Fixes for issues uncovered by tests with 6.17-rc1 kernels Stefano Brivio
2025-09-09 18:16 ` [PATCH v4 1/8] tcp: FIN flags have to be retransmitted as well Stefano Brivio
2025-09-09 18:16 ` [PATCH v4 2/8] tcp: Factor sequence rewind for retransmissions into a new function Stefano Brivio
2025-09-09 18:16 ` [PATCH v4 3/8] tcp: Rewind sequence when guest shrinks window to zero Stefano Brivio
2025-09-10 2:20 ` David Gibson
2025-09-10 6:37 ` Stefano Brivio
2025-09-10 7:18 ` David Gibson
2025-09-10 23:48 ` Jon Maloy
2025-09-09 18:16 ` [PATCH v4 4/8] tcp: Fix closing logic for half-closed connections Stefano Brivio
2025-09-10 23:56 ` Jon Maloy
2025-09-09 18:16 ` [PATCH v4 5/8] tcp: Don't try to transmit right after the peer shrank the window to zero Stefano Brivio
2025-09-09 18:16 ` [PATCH v4 6/8] tcp: Cast operands of sequence comparison macros to uint32_t before using them Stefano Brivio
2025-09-10 2:21 ` David Gibson
2025-09-09 18:16 ` Stefano Brivio [this message]
2025-09-10 2:27 ` [PATCH v4 7/8] tcp: Fast re-transmit if half-closed, make TAP_FIN_RCVD path consistent David Gibson
2025-09-10 9:57 ` Stefano Brivio
2025-09-09 18:16 ` [PATCH v4 8/8] tcp: Don't send FIN segment to guest yet if we have pending unacknowledged data Stefano Brivio
2025-09-10 2:29 ` David Gibson
2025-09-10 6:37 ` Stefano Brivio
2025-09-10 9:10 ` [PATCH v4 0/8] tcp: Fixes for issues uncovered by tests with 6.17-rc1 kernels Paul Holzinger
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=20250909181655.2990223-8-sbrivio@redhat.com \
--to=sbrivio@redhat.com \
--cc=david@gibson.dropbear.id.au \
--cc=jmaloy@redhat.com \
--cc=passt-dev@passt.top \
--cc=pholzing@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).