[PATCH v2] tcp: Handle errors from tcp_send_flag()

[PATCH v2] tcp: Handle errors from tcp_send_flag()

[Anshu Kumari]

tcp_send_flag() can fail in two different ways:
   - tcp_prepare_flags() returns -ECONNRESET when getsockopt(TCP_INFO)
     fails: the socket is broken and the connection must be reset.
   - tcp_vu_send_flag() returns -EAGAIN when vu_collect() finds no
     available vhost-user buffers: this is a transient condition
     equivalent to a dropped packet on the wire.

Have tcp_vu_send_flag() return -EAGAIN instead of a bare -1 for the
buffer-unavailable case. Absorb -EAGAIN in the tcp_send_flag()
dispatcher so that callers only see fatal errors.

Check the return value at each call site and handle fatal errors:
   - 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, call
     FLOW_ACTIVATE() to avoid leaving the flow in TYPED state, and
     return
   - in tcp_connect_finish(), call tcp_rst() and return
   - 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.

Link: https://bugs.passt.top/show_bug.cgi?id=194
Signed-off-by: Anshu Kumari <anskuma@redhat.com>
---
v2:
  - updated commit message.
  - update tcp_vu_send_flag() to return -EAGAIN instead of -1 for
    buffer-unavailable case.
  - in tcp_tap_conn_from_sock(), added FLOW_ACTIVATE call.
  - inside tcp_send_flag(), updated the return statement. function will
    return 0 incase of -EAGAIN error.
---
 tcp.c    | 74 +++++++++++++++++++++++++++++++++++++++++---------------
 tcp_vu.c |  6 +++--
 2 files changed, 59 insertions(+), 21 deletions(-)

diff --git a/tcp.c b/tcp.c
index 8ea9be8..28139d8 100644
--- a/tcp.c
+++ b/tcp.c
@@ -1378,15 +1378,19 @@ int tcp_prepare_flags(const struct ctx *c, struct tcp_tap_conn *conn,
  * @conn:      Connection pointer
  * @flags:     TCP flags: if not set, send segment only if ACK is due
  *
- * Return: negative error code on connection reset, 0 otherwise
+ * Return: negative error code on fatal connection failure, 0 otherwise
  */
 static int tcp_send_flag(const struct ctx *c, struct tcp_tap_conn *conn,
 			 int flags)
 {
+	int ret;
+
 	if (c->mode == MODE_VU)
-		return tcp_vu_send_flag(c, conn, flags);
+		ret = tcp_vu_send_flag(c, conn, flags);
+	else
+		ret = tcp_buf_send_flag(c, conn, flags);
 
-	return tcp_buf_send_flag(c, conn, flags);
+	return ret == -EAGAIN ? 0 : ret;
 }
 
 /**
@@ -1917,7 +1921,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 +2049,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 +2078,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 +2093,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 +2132,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 +2321,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 +2403,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
@@ -2436,8 +2453,10 @@ static void tcp_connect_finish(const struct ctx *c, struct tcp_tap_conn *conn)
 		return;
 	}
 
-	if (tcp_send_flag(c, conn, SYN | ACK))
+	if (tcp_send_flag(c, conn, SYN | ACK)) {
+		tcp_rst(c, conn);
 		return;
+	}
 
 	conn_event(c, conn, TAP_SYN_ACK_SENT);
 	conn_flag(c, conn, ACK_FROM_TAP_DUE);
@@ -2478,7 +2497,12 @@ 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);
+		FLOW_ACTIVATE(conn);
+		return;
+	}
+
 	conn_flag(c, conn, ACK_FROM_TAP_DUE);
 
 	tcp_get_sndbuf(conn);
@@ -2585,7 +2609,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 +2625,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 +2692,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 +2936,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 +3960,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))) {
diff --git a/tcp_vu.c b/tcp_vu.c
index dc0e17c..9d63a30 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -65,7 +65,9 @@ static size_t tcp_vu_hdrlen(bool v6)
  * @conn:	Connection pointer
  * @flags:	TCP flags: if not set, send segment only if ACK is due
  *
- * Return: negative error code on connection reset, 0 otherwise
+ * Return: -ECONNRESET on fatal connection error,
+ *         -EAGAIN if vhost-user buffers are unavailable,
+ *         0 otherwise
  */
 int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 {
@@ -91,7 +93,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
 			      &flags_iov[0], 1, NULL,
 			      MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN), NULL);
 	if (elem_cnt != 1)
-		return -1;
+		return -EAGAIN;
 
 	assert(flags_elem[0].in_num == 1);
 	assert(flags_elem[0].in_sg[0].iov_len >=
-- 
2.53.0

Re: [PATCH v2] tcp: Handle errors from tcp_send_flag()

[Stefano Brivio]

On Thu, 23 Apr 2026 11:53:14 +0530
Anshu Kumari <anskuma@redhat.com> wrote:

> tcp_send_flag() can fail in two different ways:
>    - tcp_prepare_flags() returns -ECONNRESET when getsockopt(TCP_INFO)
>      fails: the socket is broken and the connection must be reset.
>    - tcp_vu_send_flag() returns -EAGAIN when vu_collect() finds no
>      available vhost-user buffers: this is a transient condition
>      equivalent to a dropped packet on the wire.
> 
> Have tcp_vu_send_flag() return -EAGAIN instead of a bare -1 for the
> buffer-unavailable case. Absorb -EAGAIN in the tcp_send_flag()
> dispatcher so that callers only see fatal errors.
> 
> Check the return value at each call site and handle fatal errors:
>    - 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, call
>      FLOW_ACTIVATE() to avoid leaving the flow in TYPED state, and
>      return
>    - in tcp_connect_finish(), call tcp_rst() and return
>    - 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.
> 
> Link: https://bugs.passt.top/show_bug.cgi?id=194
> Signed-off-by: Anshu Kumari <anskuma@redhat.com>
> ---
> v2:
>   - updated commit message.
>   - update tcp_vu_send_flag() to return -EAGAIN instead of -1 for
>     buffer-unavailable case.
>   - in tcp_tap_conn_from_sock(), added FLOW_ACTIVATE call.
>   - inside tcp_send_flag(), updated the return statement. function will
>     return 0 incase of -EAGAIN error.

Applied, thanks for the detailed change log!

Just one detail: this had two trivial conflicts with commit
831857e9b547 ("tcp: Replace send buffer boost with EPOLLOUT monitoring")
which I merged earlier this week (my Monday):

  $ git show -s --format=%cd 831857e9b547
  Mon Apr 20 23:20:41 2026 +0200

I solved those on merge, and I usually do that if the resolution is
obvious, but in general you should 'git pull --rebase' or similar
(depending on how / whether you use branches) just before submitting
patches, so that conflicts are less likely to happen.

They might still happen if there are other pending patches that are
then merged at the same time but that's unavoidable (and in general
I'll just solve conflicts in that case because that's usually my
fault for reviewing or merging things too late / slowly).

-- 
Stefano