Re: [PATCH v6 4/4] tcp_vu: Support multibuffer frames in tcp_vu_send_flag()

[Laurent Vivier <lvivier@redhat.com>]

On 5/11/26 12:52, David Gibson wrote:
> On Thu, Apr 16, 2026 at 06:16:18PM +0200, Laurent Vivier wrote:
>> Build the Ethernet, IP, and TCP headers on the stack instead of
>> directly in the buffer via pointer casts, then write them into the
>> iovec with IOV_PUSH_HEADER().  This mirrors the approach already used
>> in tcp_vu_prepare() and udp_vu_prepare().
>>
>> Remove the vu_eth(), vu_ip(), vu_payloadv4() and vu_payloadv6() helpers
>> from vu_common.h, as they are no longer used anywhere.
>>
>> Introduce tcp_vu_send_dup() to handle DUP_ACK duplication using
>> vu_collect() and iov_memcopy() instead of a plain memcpy(), so that
> 
> s/iov_memcopy/iov_memcpy/
> 
>> the duplicated frame is also properly scattered across multiple iovecs.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>   iov.c       |   1 -
>>   tcp_vu.c    | 153 ++++++++++++++++++++++++++++++----------------------
>>   vu_common.h |  20 -------
>>   3 files changed, 90 insertions(+), 84 deletions(-)
>>
>> diff --git a/iov.c b/iov.c
>> index c0d9c6d21322..31fc3e479572 100644
>> --- a/iov.c
>> +++ b/iov.c
>> @@ -208,7 +208,6 @@ void iov_memset(const struct iovec *iov, size_t iov_cnt, size_t offset, int c,
>>    *
>>    * Return: total number of bytes copied
>>    */
>> -/* cppcheck-suppress unusedFunction */
>>   size_t iov_memcpy(struct iovec *dst_iov, size_t dst_iov_cnt, size_t dst_offset,
>>   		  const struct iovec *src_iov, size_t src_iov_cnt,
>>   		  size_t src_offset, size_t length)
>> diff --git a/tcp_vu.c b/tcp_vu.c
>> index 96b16007701d..7f7e43860b10 100644
>> --- a/tcp_vu.c
>> +++ b/tcp_vu.c
>> @@ -74,6 +74,42 @@ static size_t tcp_vu_hdrlen(bool v6)
>>   	return hdrlen;
>>   }
>>   
>> +/**
>> + * tcp_vu_send_dup() - Duplicate a frame into a new virtqueue element
>> + * @c:		Execution context
>> + * @vq:		Receive virtqueue
>> + * @dest_elem:	Destination virtqueue element to collect
>> + * @dest_iov:	Destination iovec array for collected buffers
>> + * @max_dest_iov: Maximum number of entries in @dest_iov
>> + * @src_iov:	Source iovec array containing the frame to duplicate
>> + * @src_cnt:	Number of entries in @src_iov
> 
> As Jon noted, @vnlen is missing.
> 
>> + *
>> + * Return: number of virtqueue elements collected (0 if none available)
>> + */
>> +static int tcp_vu_send_dup(const struct ctx *c, struct vu_virtq *vq,
>> +			   struct vu_virtq_element *dest_elem,
>> +			   struct iovec *dest_iov, size_t max_dest_iov,
>> +			   const struct iovec *src_iov, size_t src_cnt,
>> +			   size_t vnlen)
>> +{
>> +	const struct vu_dev *vdev = c->vdev;
>> +	size_t dest_cnt;
>> +	int elem_cnt;
>> +
>> +	elem_cnt = vu_collect(vdev, vq, dest_elem, 1, dest_iov, max_dest_iov,
>> +			      &dest_cnt, vnlen, NULL);
>> +	if (elem_cnt == 0)
>> +		return 0;
>> +
>> +	iov_memcpy(dest_iov, dest_cnt, 0, src_iov, src_cnt, 0,
>> +		   MAX(VNET_HLEN + ETH_ZLEN, vnlen));
>> +
>> +	if (*c->pcap)
>> +		pcap_iov(dest_iov, dest_cnt, VNET_HLEN, vnlen - VNET_HLEN);
>> +
>> +	return elem_cnt;
>> +}
>> +
>>   /**
>>    * tcp_vu_send_flag() - Send segment with flags to vhost-user (no payload)
>>    * @c:		Execution context
>> @@ -86,97 +122,88 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>>   {
>>   	struct vu_dev *vdev = c->vdev;
>>   	struct vu_virtq *vq = &vdev->vq[VHOST_USER_RX_QUEUE];
>> +	size_t optlen, hdrlen, iov_cnt, iov_used;
>>   	struct vu_virtq_element flags_elem[2];
> 
> Does the rationale for this having only 2 elements still apply?

Yes, this is element, not iovec. One for the flags, an other one for the DUP.

> 
>> -	size_t optlen, hdrlen, l2len;
>> -	struct ipv6hdr *ip6h = NULL;
>> -	struct iphdr *ip4h = NULL;
>> -	struct iovec flags_iov[2];
>> -	struct tcp_syn_opts *opts;
>> +	struct iovec flags_iov[64];
>> +	int elem_cnt, dup_elem_cnt = 0;
>> +	struct tcp_syn_opts opts;
>>   	struct iov_tail payload;
>> -	struct tcphdr *th;
>> -	struct ethhdr *eh;
>> +	struct ipv6hdr ip6h;
>> +	struct iphdr ip4h;
>> +	struct tcphdr th;
>> +	struct ethhdr eh;
>>   	uint32_t seq;
>> -	int elem_cnt;
>>   	int ret;
>>   
>>   	hdrlen = tcp_vu_hdrlen(CONN_V6(conn));
>>   
>>   	elem_cnt = vu_collect(vdev, vq, &flags_elem[0], 1,
>> -			      &flags_iov[0], 1, NULL,
>> -			      hdrlen + sizeof(*opts), NULL);
>> -	if (elem_cnt != 1)
>> +			      flags_iov, ARRAY_SIZE(flags_iov), &iov_cnt,
>> +			      hdrlen + sizeof(opts), NULL);
>> +	if (elem_cnt == 0)
>>   		return -1;
>>   
>> -	assert(flags_elem[0].in_num == 1);
>> -	assert(flags_elem[0].in_sg[0].iov_len >=
>> -	       MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN));
>> -
>> -	eh = vu_eth(flags_elem[0].in_sg[0].iov_base);
>> -
>> -	memcpy(eh->h_dest, c->guest_mac, sizeof(eh->h_dest));
>> -	memcpy(eh->h_source, c->our_tap_mac, sizeof(eh->h_source));
>> -
>> -	if (CONN_V4(conn)) {
>> -		eh->h_proto = htons(ETH_P_IP);
>> -
>> -		ip4h = vu_ip(flags_elem[0].in_sg[0].iov_base);
>> -		*ip4h = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
>> -
>> -		th = vu_payloadv4(flags_elem[0].in_sg[0].iov_base);
>> -	} else {
>> -		eh->h_proto = htons(ETH_P_IPV6);
>> +	memcpy(eh.h_dest, c->guest_mac, sizeof(eh.h_dest));
>>   
>> -		ip6h = vu_ip(flags_elem[0].in_sg[0].iov_base);
>> -		*ip6h = (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_TCP);
>> -		th = vu_payloadv6(flags_elem[0].in_sg[0].iov_base);
>> -	}
>> +	if (CONN_V4(conn))
>> +		ip4h = (struct iphdr)L2_BUF_IP4_INIT(IPPROTO_TCP);
>> +	else
>> +		ip6h = (struct ipv6hdr)L2_BUF_IP6_INIT(IPPROTO_TCP);
>>   
>> -	memset(th, 0, sizeof(*th));
>> -	th->doff = sizeof(*th) / 4;
>> -	th->ack = 1;
> 
> Where are doff and ack now initialised?  Or were these already unneeded?

They are set in tcp_prepare_flags().

Thanks,
Laurent