Re: [PATCH v2 1/2] vhost_user: Offer VIRTIO_NET_F_GUEST_CSUM

[Laurent Vivier <lvivier@redhat.com>]

Hi Stefano,

could you apply this one too?

Thanks,
Laurent

On 4/16/26 18:21, Laurent Vivier wrote:
> According to the virtio-net specification, when the VIRTIO_NET_F_GUEST_CSUM
> is negotiated, the device can set VIRTIO_NET_HDR_F_DATA_VALID in the
> virtio-net header to indicate that packet checksums have been validated,
> allowing the guest to skip verification. Without this feature, the device
> must provide fully checksummed packets.
> 
> The vhost-user TCP and UDP paths were unconditionally skipping checksum
> computation, regardless of whether GUEST_CSUM was negotiated. This
> went undetected with Linux guests because Linux's virtio-net driver
> honours VIRTIO_NET_HDR_F_DATA_VALID regardless of whether
> VIRTIO_NET_F_GUEST_CSUM was negotiated, marking such packets as
> CHECKSUM_UNNECESSARY and skipping verification.
> 
> iPXE, however, does not negotiate GUEST_CSUM, ignores the DATA_VALID
> flag entirely, and always verifies checksums. This caused TCP
> connections to fail: the SYN-ACK had a zero TCP checksum, iPXE rejected
> it, and the connection timed out in SYN_RCVD.
> 
> Adding --pcap happened to mask the bug, because the pcap code path
> forces checksum computation to ensure correct captures.
> 
> Offer VIRTIO_NET_F_GUEST_CSUM in the device features, and only skip
> checksum computation when the guest has actually negotiated it. When
> GUEST_CSUM is not negotiated, always compute valid checksums as required
> by the specification.
> 
> We keep setting VIRTIO_NET_HDR_F_DATA_VALID unconditionally in
> VU_HEADER: when GUEST_CSUM is negotiated, the flag lets the guest skip
> checksum verification; when it is not, the spec says the guest should
> ignore the flags field, so setting it is harmless.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   tcp_vu.c     | 8 ++++++--
>   udp_vu.c     | 7 ++++---
>   vhost_user.c | 1 +
>   3 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 7f7e43860b10..3da14a4942d6 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -124,6 +124,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>   	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];
> +	uint32_t csum_flags = IP4_CSUM;
>   	struct iovec flags_iov[64];
>   	int elem_cnt, dup_elem_cnt = 0;
>   	struct tcp_syn_opts opts;
> @@ -135,6 +136,9 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>   	uint32_t seq;
>   	int ret;
>   
> +	if (*c->pcap || !vu_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
> +		csum_flags |= TCP_CSUM;
> +
>   	hdrlen = tcp_vu_hdrlen(CONN_V6(conn));
>   
>   	elem_cnt = vu_collect(vdev, vq, &flags_elem[0], 1,
> @@ -173,7 +177,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>   	iov_from_buf(payload.iov, payload.cnt, payload.off, &opts, optlen);
>   	tcp_fill_headers(c, conn, &eh, CONN_V4(conn) ? &ip4h : NULL,
>   			 CONN_V6(conn) ? &ip6h : NULL, &th, &payload,
> -			 optlen, IP4_CSUM | (*c->pcap ? TCP_CSUM : 0), seq);
> +			 optlen, csum_flags, seq);
>   
>   	vu_pad(flags_elem[0].in_sg, iov_cnt, hdrlen + optlen);
>   
> @@ -519,7 +523,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
>   
>   	hdrlen = tcp_vu_hdrlen(v6);
>   	check = IP4_CSUM;
> -	if (*c->pcap)
> +	if (*c->pcap || !vu_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
>   		check |= TCP_CSUM;
>   	for (i = 0, previous_dlen = -1; i < frame_cnt; i++) {
>   		struct iovec *iov = &iov_vu[frame[i].idx_iovec];
> diff --git a/udp_vu.c b/udp_vu.c
> index 8cf50ca1c38f..54d824b5530c 100644
> --- a/udp_vu.c
> +++ b/udp_vu.c
> @@ -233,12 +233,13 @@ void udp_vu_sock_to_tap(const struct ctx *c, int s, int n, flow_sidx_t tosidx)
>   		if (iov_cnt > 0) {
>   			struct iov_tail data = IOV_TAIL(iov_vu, iov_cnt, VNET_HLEN);
>   			udp_vu_prepare(c, &data, toside, dlen);
> -			if (*c->pcap) {
> +			if (!vu_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM) ||
> +			    *c->pcap)
>   				udp_vu_csum(toside, &data, dlen);
> +			vu_pad(iov_vu, iov_cnt, hdrlen + dlen);
> +			if (*c->pcap)
>   				pcap_iov(iov_vu, iov_cnt, VNET_HLEN,
>   					 hdrlen + dlen - VNET_HLEN);
> -			}
> -			vu_pad(iov_vu, iov_cnt, hdrlen + dlen);
>   			vu_flush(vdev, vq, elem, elem_used, hdrlen + dlen);
>   			vu_queue_notify(vdev, vq);
>   		}
> diff --git a/vhost_user.c b/vhost_user.c
> index f062badd3311..a1259c2624c0 100644
> --- a/vhost_user.c
> +++ b/vhost_user.c
> @@ -322,6 +322,7 @@ static bool vu_get_features_exec(struct vu_dev *vdev,
>   {
>   	uint64_t features =
>   		1ULL << VIRTIO_F_VERSION_1 |
> +		1ULL << VIRTIO_NET_F_GUEST_CSUM |
>   		1ULL << VIRTIO_NET_F_MRG_RXBUF |
>   		1ULL << VHOST_F_LOG_ALL |
>   		1ULL << VHOST_USER_F_PROTOCOL_FEATURES;