* [PATCH v1] tcp: Handle errors from tcp_send_flag()
@ 2026-04-10 7:55 Anshu Kumari
2026-04-13 6:22 ` David Gibson
0 siblings, 1 reply; 2+ messages in thread
From: Anshu Kumari @ 2026-04-10 7:55 UTC (permalink / raw)
To: passt-dev, anskuma, sbrivio; +Cc: david, lvivier
tcp_send_flag() can return error codes from tcp_prepare_flags()
failing TCP_INFO, or from failure to collect buffers on the
vhost-user path. These errors indicate the connection requires
resetting.
Most callers of tcp_send_flag() were ignoring the error code and
carrying on as if nothing was wrong. Check the return value at
each call site and handle the error appropriately:
- in tcp_data_from_tap(), return -1 so the caller resets
- in tcp_tap_handler(), goto reset
- in tcp_timer_handler()/tcp_sock_handler()/tcp_conn_from_sock_finish(),
call tcp_rst() and return
- in tcp_tap_conn_from_sock(), set CLOSING flag (flow not yet active)
- in tcp_keepalive(), call tcp_rst() and continue the loop
- in tcp_flow_migrate_target_ext(), goto fail
The call in tcp_rst_do() is left unchecked: we are already
resetting, and tcp_sock_rst() still needs to run regardless.
Bug: https://bugs.passt.top/show_bug.cgi?id=194
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
tcp.c | 59 ++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 44 insertions(+), 15 deletions(-)
diff --git a/tcp.c b/tcp.c
index 8ea9be8..9ce671a 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1917,7 +1917,9 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
"keep-alive sequence: %u, previous: %u",
seq, conn->seq_from_tap);
- tcp_send_flag(c, conn, ACK);
+ if (tcp_send_flag(c, conn, ACK))
+ return -1;
+
tcp_timer_ctl(c, conn);
if (setsockopt(conn->sock, SOL_SOCKET, SO_KEEPALIVE,
@@ -2043,14 +2045,16 @@ eintr:
* Then swiftly looked away and left.
*/
conn->seq_from_tap = seq_from_tap;
- tcp_send_flag(c, conn, ACK);
+ if (tcp_send_flag(c, conn, ACK))
+ return -1;
}
if (errno == EINTR)
goto eintr;
if (errno == EAGAIN || errno == EWOULDBLOCK) {
- tcp_send_flag(c, conn, ACK | DUP_ACK);
+ if (tcp_send_flag(c, conn, ACK | DUP_ACK))
+ return -1;
return p->count - idx;
}
@@ -2070,7 +2074,8 @@ out:
*/
if (conn->seq_dup_ack_approx != (conn->seq_from_tap & 0xff)) {
conn->seq_dup_ack_approx = conn->seq_from_tap & 0xff;
- tcp_send_flag(c, conn, ACK | DUP_ACK);
+ if (tcp_send_flag(c, conn, ACK | DUP_ACK))
+ return -1;
}
return p->count - idx;
}
@@ -2084,7 +2089,8 @@ out:
conn_event(c, conn, TAP_FIN_RCVD);
} else {
- tcp_send_flag(c, conn, ACK_IF_NEEDED);
+ if (tcp_send_flag(c, conn, ACK_IF_NEEDED))
+ return -1;
}
return p->count - idx;
@@ -2122,7 +2128,10 @@ static void tcp_conn_from_sock_finish(const struct ctx *c,
return;
}
- tcp_send_flag(c, conn, ACK);
+ if (tcp_send_flag(c, conn, ACK)) {
+ tcp_rst(c, conn);
+ return;
+ }
/* The client might have sent data already, which we didn't
* dequeue waiting for SYN,ACK from tap -- check now.
@@ -2308,7 +2317,9 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
goto reset;
}
- tcp_send_flag(c, conn, ACK);
+ if (tcp_send_flag(c, conn, ACK))
+ goto reset;
+
conn_event(c, conn, SOCK_FIN_SENT);
return 1;
@@ -2388,7 +2399,9 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
}
conn_event(c, conn, SOCK_FIN_SENT);
- tcp_send_flag(c, conn, ACK);
+ if (tcp_send_flag(c, conn, ACK))
+ goto reset;
+
ack_due = 0;
/* If we received a FIN, but the socket is in TCP_ESTABLISHED
@@ -2478,7 +2491,11 @@ static void tcp_tap_conn_from_sock(const struct ctx *c, union flow *flow,
conn->wnd_from_tap = WINDOW_DEFAULT;
- tcp_send_flag(c, conn, SYN);
+ if (tcp_send_flag(c, conn, SYN)) {
+ conn_flag(c, conn, CLOSING);
+ return;
+ }
+
conn_flag(c, conn, ACK_FROM_TAP_DUE);
tcp_get_sndbuf(conn);
@@ -2585,7 +2602,10 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
return;
if (conn->flags & ACK_TO_TAP_DUE) {
- tcp_send_flag(c, conn, ACK_IF_NEEDED);
+ if (tcp_send_flag(c, conn, ACK_IF_NEEDED)) {
+ tcp_rst(c, conn);
+ return;
+ }
tcp_timer_ctl(c, conn);
} else if (conn->flags & ACK_FROM_TAP_DUE) {
if (!(conn->events & ESTABLISHED)) {
@@ -2598,7 +2618,10 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
tcp_rst(c, conn);
} else {
flow_trace(conn, "SYN timeout, retry");
- tcp_send_flag(c, conn, SYN);
+ if (tcp_send_flag(c, conn, SYN)) {
+ tcp_rst(c, conn);
+ return;
+ }
conn->retries++;
conn_flag(c, conn, SYN_RETRIED);
tcp_timer_ctl(c, conn);
@@ -2662,8 +2685,11 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref,
tcp_data_from_sock(c, conn);
if (events & EPOLLOUT) {
- if (tcp_update_seqack_wnd(c, conn, false, NULL))
- tcp_send_flag(c, conn, ACK);
+ if (tcp_update_seqack_wnd(c, conn, false, NULL) &&
+ tcp_send_flag(c, conn, ACK)) {
+ tcp_rst(c, conn);
+ return;
+ }
}
return;
@@ -2903,7 +2929,8 @@ static void tcp_keepalive(struct ctx *c, const struct timespec *now)
if (conn->tap_inactive) {
flow_dbg(conn, "No tap activity for least %us, send keepalive",
KEEPALIVE_INTERVAL);
- tcp_send_flag(c, conn, KEEPALIVE);
+ if (tcp_send_flag(c, conn, KEEPALIVE))
+ tcp_rst(c, conn);
}
/* Ready to check fot next interval */
@@ -3926,7 +3953,9 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd
if (tcp_set_peek_offset(conn, peek_offset))
goto fail;
- tcp_send_flag(c, conn, ACK);
+ if (tcp_send_flag(c, conn, ACK))
+ goto fail;
+
tcp_data_from_sock(c, conn);
if ((rc = tcp_epoll_ctl(conn))) {
--
2.53.0
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v1] tcp: Handle errors from tcp_send_flag()
2026-04-10 7:55 [PATCH v1] tcp: Handle errors from tcp_send_flag() Anshu Kumari
@ 2026-04-13 6:22 ` David Gibson
0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2026-04-13 6:22 UTC (permalink / raw)
To: Anshu Kumari; +Cc: passt-dev, sbrivio, lvivier
[-- Attachment #1: Type: text/plain, Size: 6533 bytes --]
On Fri, Apr 10, 2026 at 01:25:39PM +0530, Anshu Kumari wrote:
> tcp_send_flag() can return error codes from tcp_prepare_flags()
> failing TCP_INFO, or from failure to collect buffers on the
> vhost-user path. These errors indicate the connection requires
> resetting.
>
> Most callers of tcp_send_flag() were ignoring the error code and
> carrying on as if nothing was wrong. Check the return value at
> each call site and handle the error appropriately:
> - in tcp_data_from_tap(), return -1 so the caller resets
> - in tcp_tap_handler(), goto reset
> - in tcp_timer_handler()/tcp_sock_handler()/tcp_conn_from_sock_finish(),
> call tcp_rst() and return
> - in tcp_tap_conn_from_sock(), set CLOSING flag (flow not yet active)
> - in tcp_keepalive(), call tcp_rst() and continue the loop
> - in tcp_flow_migrate_target_ext(), goto fail
Thanks for this detailed summary, it really helps a reviewer follow
the logic.
> The call in tcp_rst_do() is left unchecked: we are already
> resetting, and tcp_sock_rst() still needs to run regardless.
>
> Bug: https://bugs.passt.top/show_bug.cgi?id=194
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> tcp.c | 59 ++++++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 44 insertions(+), 15 deletions(-)
>
> diff --git a/tcp.c b/tcp.c
> index 8ea9be8..9ce671a 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1917,7 +1917,9 @@ static int tcp_data_from_tap(const struct ctx *c, struct tcp_tap_conn *conn,
> "keep-alive sequence: %u, previous: %u",
> seq, conn->seq_from_tap);
>
> - tcp_send_flag(c, conn, ACK);
> + if (tcp_send_flag(c, conn, ACK))
> + return -1;
> +
> tcp_timer_ctl(c, conn);
>
> if (setsockopt(conn->sock, SOL_SOCKET, SO_KEEPALIVE,
> @@ -2043,14 +2045,16 @@ eintr:
> * Then swiftly looked away and left.
> */
> conn->seq_from_tap = seq_from_tap;
> - tcp_send_flag(c, conn, ACK);
> + if (tcp_send_flag(c, conn, ACK))
> + return -1;
> }
>
> if (errno == EINTR)
> goto eintr;
>
> if (errno == EAGAIN || errno == EWOULDBLOCK) {
> - tcp_send_flag(c, conn, ACK | DUP_ACK);
> + if (tcp_send_flag(c, conn, ACK | DUP_ACK))
> + return -1;
> return p->count - idx;
>
> }
> @@ -2070,7 +2074,8 @@ out:
> */
> if (conn->seq_dup_ack_approx != (conn->seq_from_tap & 0xff)) {
> conn->seq_dup_ack_approx = conn->seq_from_tap & 0xff;
> - tcp_send_flag(c, conn, ACK | DUP_ACK);
> + if (tcp_send_flag(c, conn, ACK | DUP_ACK))
> + return -1;
> }
> return p->count - idx;
> }
> @@ -2084,7 +2089,8 @@ out:
>
> conn_event(c, conn, TAP_FIN_RCVD);
> } else {
> - tcp_send_flag(c, conn, ACK_IF_NEEDED);
> + if (tcp_send_flag(c, conn, ACK_IF_NEEDED))
> + return -1;
> }
>
> return p->count - idx;
> @@ -2122,7 +2128,10 @@ static void tcp_conn_from_sock_finish(const struct ctx *c,
> return;
> }
>
> - tcp_send_flag(c, conn, ACK);
> + if (tcp_send_flag(c, conn, ACK)) {
> + tcp_rst(c, conn);
> + return;
> + }
>
> /* The client might have sent data already, which we didn't
> * dequeue waiting for SYN,ACK from tap -- check now.
> @@ -2308,7 +2317,9 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
> goto reset;
> }
>
> - tcp_send_flag(c, conn, ACK);
> + if (tcp_send_flag(c, conn, ACK))
> + goto reset;
> +
> conn_event(c, conn, SOCK_FIN_SENT);
>
> return 1;
> @@ -2388,7 +2399,9 @@ int tcp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af,
> }
>
> conn_event(c, conn, SOCK_FIN_SENT);
> - tcp_send_flag(c, conn, ACK);
> + if (tcp_send_flag(c, conn, ACK))
> + goto reset;
> +
> ack_due = 0;
>
> /* If we received a FIN, but the socket is in TCP_ESTABLISHED
> @@ -2478,7 +2491,11 @@ static void tcp_tap_conn_from_sock(const struct ctx *c, union flow *flow,
>
> conn->wnd_from_tap = WINDOW_DEFAULT;
>
> - tcp_send_flag(c, conn, SYN);
> + if (tcp_send_flag(c, conn, SYN)) {
> + conn_flag(c, conn, CLOSING);
> + return;
> + }
> +
> conn_flag(c, conn, ACK_FROM_TAP_DUE);
>
> tcp_get_sndbuf(conn);
> @@ -2585,7 +2602,10 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
> return;
>
> if (conn->flags & ACK_TO_TAP_DUE) {
> - tcp_send_flag(c, conn, ACK_IF_NEEDED);
> + if (tcp_send_flag(c, conn, ACK_IF_NEEDED)) {
> + tcp_rst(c, conn);
> + return;
> + }
> tcp_timer_ctl(c, conn);
> } else if (conn->flags & ACK_FROM_TAP_DUE) {
> if (!(conn->events & ESTABLISHED)) {
> @@ -2598,7 +2618,10 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
> tcp_rst(c, conn);
> } else {
> flow_trace(conn, "SYN timeout, retry");
> - tcp_send_flag(c, conn, SYN);
> + if (tcp_send_flag(c, conn, SYN)) {
> + tcp_rst(c, conn);
> + return;
> + }
> conn->retries++;
> conn_flag(c, conn, SYN_RETRIED);
> tcp_timer_ctl(c, conn);
> @@ -2662,8 +2685,11 @@ void tcp_sock_handler(const struct ctx *c, union epoll_ref ref,
> tcp_data_from_sock(c, conn);
>
> if (events & EPOLLOUT) {
> - if (tcp_update_seqack_wnd(c, conn, false, NULL))
> - tcp_send_flag(c, conn, ACK);
> + if (tcp_update_seqack_wnd(c, conn, false, NULL) &&
> + tcp_send_flag(c, conn, ACK)) {
> + tcp_rst(c, conn);
> + return;
> + }
> }
>
> return;
> @@ -2903,7 +2929,8 @@ static void tcp_keepalive(struct ctx *c, const struct timespec *now)
> if (conn->tap_inactive) {
> flow_dbg(conn, "No tap activity for least %us, send keepalive",
> KEEPALIVE_INTERVAL);
> - tcp_send_flag(c, conn, KEEPALIVE);
> + if (tcp_send_flag(c, conn, KEEPALIVE))
> + tcp_rst(c, conn);
> }
>
> /* Ready to check fot next interval */
> @@ -3926,7 +3953,9 @@ int tcp_flow_migrate_target_ext(struct ctx *c, struct tcp_tap_conn *conn, int fd
> if (tcp_set_peek_offset(conn, peek_offset))
> goto fail;
>
> - tcp_send_flag(c, conn, ACK);
> + if (tcp_send_flag(c, conn, ACK))
> + goto fail;
> +
> tcp_data_from_sock(c, conn);
>
> if ((rc = tcp_epoll_ctl(conn))) {
> --
> 2.53.0
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-04-13 6:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-10 7:55 [PATCH v1] tcp: Handle errors from tcp_send_flag() Anshu Kumari
2026-04-13 6:22 ` David Gibson
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).