Re: [PATCH 3/5] tcp: Merge common sequence logic from tcp_{buf,vu}_data_from_sock()

[Laurent Vivier <lvivier@redhat.com>]

On 6/15/26 10:18, David Gibson wrote:
> tcp_buf_data_from_sock() and tcp_vu_data_from_sock() start with some
> identical logic to compute the amount of data from the socket that has
> already been sent to tap but not acked.  Replace it with a single copy in
> their common caller tcp_data_from_sock(), passing the already_sent amount
> in as a parameter.
> 
> This does mean that we now execute this logic in the VU case even if the
> queue is not active.  The logic is still valid in this case, and we don't
> need to worry too much about wasting cycles in this edge case.
> 
> Cc: Laurent Vivier <lvivier@redhat.com>
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

> ---
>   tcp.c     | 29 +++++++++++++++++++++++++++--
>   tcp_buf.c | 28 ++++------------------------
>   tcp_buf.h |  3 ++-
>   tcp_vu.c  | 27 ++++-----------------------
>   tcp_vu.h  |  3 ++-
>   5 files changed, 39 insertions(+), 51 deletions(-)
> 
> diff --git a/tcp.c b/tcp.c
> index c4000754..dcadc765 100644
> --- a/tcp.c
> +++ b/tcp.c
> @@ -1837,10 +1837,35 @@ static int tcp_sock_consume(const struct tcp_tap_conn *conn, uint32_t ack_seq)
>    */
>   static int tcp_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
>   {
> +	uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap;
> +	uint32_t already_sent;
> +
> +	/* How much have we read/sent since last received ack ? */
> +	already_sent = conn->seq_to_tap - conn->seq_ack_from_tap;
> +
> +	if (SEQ_LT(already_sent, 0)) {
> +		/* RFC 761, section 2.1. */
> +		flow_trace(conn, "ACK sequence gap: ACK for %u, sent: %u",
> +			   conn->seq_ack_from_tap, conn->seq_to_tap);
> +		conn->seq_to_tap = conn->seq_ack_from_tap;
> +		already_sent = 0;
> +		if (tcp_set_peek_offset(conn, 0)) {
> +			tcp_rst(c, conn);
> +			return -1;
> +		}
> +	}
> +
> +	if (!wnd_scaled || already_sent >= wnd_scaled) {
> +		conn_flag(c, conn, ACK_FROM_TAP_BLOCKS);
> +		conn_flag(c, conn, STALLED);
> +		conn_flag(c, conn, ACK_FROM_TAP_DUE);
> +		return 0;
> +	}
> +
>   	if (c->mode == MODE_VU)
> -		return tcp_vu_data_from_sock(c, conn);
> +		return tcp_vu_data_from_sock(c, conn, already_sent);
>   
> -	return tcp_buf_data_from_sock(c, conn);
> +	return tcp_buf_data_from_sock(c, conn, already_sent);
>   }
>   
>   /**
> diff --git a/tcp_buf.c b/tcp_buf.c
> index ca356089..1fc49959 100644
> --- a/tcp_buf.c
> +++ b/tcp_buf.c
> @@ -309,42 +309,22 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn,
>    * tcp_buf_data_from_sock() - Handle new data from socket, queue to tap, in window
>    * @c:		Execution context
>    * @conn:	Connection pointer
> + * @already_sent:	Number of bytes already sent to tap, but not acked
>    *
>    * Return: negative on connection reset, 0 otherwise
>    *
>    * #syscalls recvmsg
>    */
> -int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
> +int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn,
> +			   uint32_t already_sent)
>   {
>   	uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap;
>   	int fill_bufs, send_bufs = 0, last_len, iov_rem = 0;
>   	int len, dlen, i, s = conn->sock;
>   	struct msghdr mh_sock = { 0 };
>   	uint16_t mss = MSS_GET(conn);
> -	uint32_t already_sent, seq;
>   	struct iovec *iov;
> -
> -	/* How much have we read/sent since last received ack ? */
> -	already_sent = conn->seq_to_tap - conn->seq_ack_from_tap;
> -
> -	if (SEQ_LT(already_sent, 0)) {
> -		/* RFC 761, section 2.1. */
> -		flow_trace(conn, "ACK sequence gap: ACK for %u, sent: %u",
> -			   conn->seq_ack_from_tap, conn->seq_to_tap);
> -		conn->seq_to_tap = conn->seq_ack_from_tap;
> -		already_sent = 0;
> -		if (tcp_set_peek_offset(conn, 0)) {
> -			tcp_rst(c, conn);
> -			return -1;
> -		}
> -	}
> -
> -	if (!wnd_scaled || already_sent >= wnd_scaled) {
> -		conn_flag(c, conn, ACK_FROM_TAP_BLOCKS);
> -		conn_flag(c, conn, STALLED);
> -		conn_flag(c, conn, ACK_FROM_TAP_DUE);
> -		return 0;
> -	}
> +	uint32_t seq;
>   
>   	/* Set up buffer descriptors we'll fill completely and partially. */
>   	fill_bufs = DIV_ROUND_UP(wnd_scaled - already_sent, mss);
> diff --git a/tcp_buf.h b/tcp_buf.h
> index 54f5e53f..710ed377 100644
> --- a/tcp_buf.h
> +++ b/tcp_buf.h
> @@ -8,7 +8,8 @@
>   
>   void tcp_sock_iov_init(const struct ctx *c);
>   void tcp_payload_flush(const struct ctx *c);
> -int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn);
> +int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn,
> +			   uint32_t already_sent);
>   int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags);
>   
>   #endif  /*TCP_BUF_H */
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 7e2a7dbc..00616955 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -423,46 +423,27 @@ static void tcp_vu_prepare(const struct ctx *c, struct tcp_tap_conn *conn,
>    *			     in window
>    * @c:		Execution context
>    * @conn:	Connection pointer
> + * @already_sent:	Number of bytes already sent to tap, but not acked
>    *
>    * Return: negative on connection reset, 0 otherwise
>    */
> -int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
> +int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn,
> +			  uint32_t already_sent)
>   {
>   	uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap;
>   	struct vu_dev *vdev = c->vdev;
>   	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
> -	uint32_t already_sent, check;
>   	ssize_t len, previous_dlen;
>   	int i, elem_cnt, frame_cnt;
>   	size_t hdrlen, fillsize;
>   	int v6 = CONN_V6(conn);
> +	uint32_t check;
>   
>   	if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) {
>   		debug("Got packet, but RX virtqueue not usable yet");
>   		return 0;
>   	}
>   
> -	already_sent = conn->seq_to_tap - conn->seq_ack_from_tap;
> -
> -	if (SEQ_LT(already_sent, 0)) {
> -		/* RFC 761, section 2.1. */
> -		flow_trace(conn, "ACK sequence gap: ACK for %u, sent: %u",
> -			   conn->seq_ack_from_tap, conn->seq_to_tap);
> -		conn->seq_to_tap = conn->seq_ack_from_tap;
> -		already_sent = 0;
> -		if (tcp_set_peek_offset(conn, 0)) {
> -			tcp_rst(c, conn);
> -			return -1;
> -		}
> -	}
> -
> -	if (!wnd_scaled || already_sent >= wnd_scaled) {
> -		conn_flag(c, conn, ACK_FROM_TAP_BLOCKS);
> -		conn_flag(c, conn, STALLED);
> -		conn_flag(c, conn, ACK_FROM_TAP_DUE);
> -		return 0;
> -	}
> -
>   	/* Set up buffer descriptors we'll fill completely and partially. */
>   
>   	fillsize = wnd_scaled - already_sent;
> diff --git a/tcp_vu.h b/tcp_vu.h
> index 6ab6057f..30e46925 100644
> --- a/tcp_vu.h
> +++ b/tcp_vu.h
> @@ -7,6 +7,7 @@
>   #define TCP_VU_H
>   
>   int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags);
> -int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn);
> +int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn,
> +			  uint32_t already_sent);
>   
>   #endif  /*TCP_VU_H */