Re: [PATCH v8 7/8] vhost-user: add vhost-user

On Thu, 14 Nov 2024 11:23:11 +0100
Laurent Vivier <lvivier(a)redhat.com> wrote:

...
  On 17/10/2024 02:10, Stefano Brivio wrote:
 
  +/**
 + * tcp_vu_data_from_sock() - Handle new data from socket, queue to vhost-user,
 + *			     in window
 + * @c:		Execution context
 + * @conn:	Connection pointer
 + *
 + * Return: Negative on connection reset, 0 otherwise
 + */
 +int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
 +{
 +	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];
 +	const struct flowside *tapside = TAPFLOW(conn);
 +	uint16_t mss = MSS_GET(conn);
 +	size_t l2_hdrlen, fillsize;
 +	int i, iov_cnt, iov_used;
 +	int v4 = CONN_V4(conn);
 +	uint32_t already_sent = 0;
 +	const uint16_t *check;
 +	struct iovec *first;
 +	int frame_size;
 +	int num_buffers;
 +	ssize_t len;
 +
 +	if (!vu_queue_enabled(vq) || !vu_queue_started(vq)) {
 +		flow_err(conn,
 +			 "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 (!wnd_scaled || already_sent >= wnd_scaled) {
 +		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;
 +
 +	if (peek_offset_cap)
 +		already_sent = 0;
 +
 +	iov_vu[0].iov_base = tcp_buf_discard;
 +	iov_vu[0].iov_len = already_sent;
 +	fillsize -= already_sent;
 +
 +	/* collect the buffers from vhost-user and fill them with the
 +	 * data from the socket
 +	 */
 +	iov_cnt = tcp_vu_sock_recv(c, conn, v4, fillsize, &len);
 +	if (iov_cnt <= 0)
 +		return iov_cnt;
 +
 +	len -= already_sent;
 +	if (len <= 0) {
 +		conn_flag(c, conn, STALLED);
 +		vu_queue_rewind(vq, iov_cnt);
 +		return 0;
 +	}
 +
 +	conn_flag(c, conn, ~STALLED);
 +
 +	/* Likely, some new data was acked too. */
 +	tcp_update_seqack_wnd(c, conn, 0, NULL);
 +
 +	/* initialize headers */
 +	l2_hdrlen = tcp_vu_l2_hdrlen(!v4);
 +	iov_used = 0;
 +	num_buffers = 0;
 +	check = NULL;
 +	frame_size = 0;
 +
 +	/* iov_vu is an array of buffers and the buffer size can be
 +	 * smaller than the frame size we want to use but with
 +	 * num_buffer we can merge several virtio iov buffers in one packet
 +	 * we need only to set the packet headers in the first iov and
 +	 * num_buffer to the number of iov entries   
 ...this part is clear to me, what I
don't understand is if we still
 have a way to guarantee that the sum of several buffers is big enough
 to fit frame_size bytes.   
 
 We don't have this garantee. But I think it's the same for the socket version?

Well, there we do:

	fill_bufs = DIV_ROUND_UP(wnd_scaled - already_sent, mss);
	if (fill_bufs > TCP_FRAMES) {
		fill_bufs = TCP_FRAMES;

and we don't fetch more data than that from the socket (in one pass).

Is this implicit in the i < iov_cnt loop condition here? That's the part
I don't understand: how do we limit the amount of data we can dequeue
from a socket in one single pass.

-- 
Stefano