From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202602 header.b=KDh2vUAQ; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 2ED8E5A026D for ; Wed, 04 Mar 2026 02:44:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1772588661; bh=wDLEcPR3BKnDl99rbxFa76cGoUVVLpHHYtRNBkhIsGs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KDh2vUAQIonr/ZVnVaQLVEcvCWXyszOstn9RcuKXhNa8t0lGqwUg3ySUvzEagAO0S /fTIr/Tf9VQBPgWmnR/AbMM/r4Ugx52tgiv7hi61why1UasioLBOhNG6d3mATbP4p9 LhHRXIKYg/QadvdGuuu5IOgCCqcXP62wKgtzbhkHYyNazg9NRPVFr4hkNNalKaKlwx LJS8nsNsyqsCsn8Juc/V9lm3moy1oH9jSRpwkBeps7OvFkLDT6hfDeV2aYMbuLx0mW K9gOSm7jNIS0Bzd71rQIJyvm7SikgLTlVh6aXjWTHUw2e0C0EDCvJbR/KyHrBYdrrI cux9EixVPYB4w== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4fQb7T0CKPz4wCm; Wed, 04 Mar 2026 12:44:21 +1100 (AEDT) Date: Wed, 4 Mar 2026 12:44:16 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH] vu_common: Always set num_buffers in virtio-net header Message-ID: References: <20260303151734.1582315-1-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="MXzc7qbvnu4ZuH3/" Content-Disposition: inline In-Reply-To: <20260303151734.1582315-1-lvivier@redhat.com> Message-ID-Hash: GQO6CI3MJF64NDKD4RTHQGALUOZFTQ72 X-Message-ID-Hash: GQO6CI3MJF64NDKD4RTHQGALUOZFTQ72 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --MXzc7qbvnu4ZuH3/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 03, 2026 at 04:17:34PM +0100, Laurent Vivier wrote: > Legacy virtio used two different header formats: struct virtio_net_hdr > (10 bytes) when VIRTIO_NET_F_MRG_RXBUF was not negotiated, and > struct virtio_net_hdr_mrg_rxbuf (12 bytes) when it was. The > num_buffers field only existed in the larger header. >=20 > Modern virtio (VIRTIO_F_VERSION_1, i.e. virtio 1.0+) always uses the > 12-byte struct virtio_net_hdr_mrg_rxbuf header regardless of whether > VIRTIO_NET_F_MRG_RXBUF is negotiated, so num_buffers is always present > in the header. passt only supports modern virtio and dies if > VIRTIO_F_VERSION_1 is not negotiated (vhost_user.c), and VNET_HLEN is > unconditionally defined as sizeof(struct virtio_net_hdr_mrg_rxbuf). >=20 > The virtio specification (v1.1, section 5.1.6) requires that: >=20 > "The device MUST set num_buffers to 1 if VIRTIO_NET_F_MRG_RXBUF has > not been negotiated." >=20 > vu_set_vnethdr() only set num_buffers when VIRTIO_NET_F_MRG_RXBUF was > negotiated. When it was not, num_buffers was left uninitialised, > violating the spec. >=20 > Since vu_collect() already limits buffer collection to a single element > when VIRTIO_NET_F_MRG_RXBUF is not negotiated, num_buffers passed by > callers is guaranteed to be 1 in that case. We can therefore > unconditionally set num_buffers, which makes the vdev parameter > unnecessary. >=20 > Drop the vdev parameter from vu_set_vnethdr() and update all callers. >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson > --- > tcp_vu.c | 4 ++-- > udp_vu.c | 2 +- > vu_common.c | 13 ++++++------- > vu_common.h | 4 +--- > 4 files changed, 10 insertions(+), 13 deletions(-) >=20 > diff --git a/tcp_vu.c b/tcp_vu.c > index bb05fbf45826..88be232dca66 100644 > --- a/tcp_vu.c > +++ b/tcp_vu.c > @@ -97,7 +97,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_ta= p_conn *conn, int flags) > ASSERT(flags_elem[0].in_sg[0].iov_len >=3D > MAX(hdrlen + sizeof(*opts), ETH_ZLEN + VNET_HLEN)); > =20 > - vu_set_vnethdr(vdev, flags_elem[0].in_sg[0].iov_base, 1); > + vu_set_vnethdr(flags_elem[0].in_sg[0].iov_base, 1); > =20 > eh =3D vu_eth(flags_elem[0].in_sg[0].iov_base); > =20 > @@ -452,7 +452,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct= tcp_tap_conn *conn) > bool push =3D i =3D=3D head_cnt - 1; > size_t l2len; > =20 > - vu_set_vnethdr(vdev, iov->iov_base, buf_cnt); > + vu_set_vnethdr(iov->iov_base, buf_cnt); > =20 > /* The IPv4 header checksum varies only with dlen */ > if (previous_dlen !=3D dlen) > diff --git a/udp_vu.c b/udp_vu.c > index 51f3718f5925..3520f89e5671 100644 > --- a/udp_vu.c > +++ b/udp_vu.c > @@ -127,7 +127,7 @@ static int udp_vu_sock_recv(const struct ctx *c, stru= ct vu_virtq *vq, int s, > l2len =3D *dlen + hdrlen - VNET_HLEN; > vu_pad(&iov_vu[0], l2len); > =20 > - vu_set_vnethdr(vdev, iov_vu[0].iov_base, iov_used); > + vu_set_vnethdr(iov_vu[0].iov_base, iov_used); > =20 > /* release unused buffers */ > vu_queue_rewind(vq, iov_cnt - iov_used); > diff --git a/vu_common.c b/vu_common.c > index aa14598ea028..5f2ce18e5b71 100644 > --- a/vu_common.c > +++ b/vu_common.c > @@ -121,17 +121,16 @@ int vu_collect(const struct vu_dev *vdev, struct vu= _virtq *vq, > =20 > /** > * vu_set_vnethdr() - set virtio-net headers > - * @vdev: vhost-user device > * @vnethdr: Address of the header to set > * @num_buffers: Number of guest buffers of the frame > */ > -void vu_set_vnethdr(const struct vu_dev *vdev, > - struct virtio_net_hdr_mrg_rxbuf *vnethdr, > - int num_buffers) > +void vu_set_vnethdr(struct virtio_net_hdr_mrg_rxbuf *vnethdr, int num_bu= ffers) > { > vnethdr->hdr =3D VU_HEADER; > - if (vu_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF)) > - vnethdr->num_buffers =3D htole16(num_buffers); > + /* Note: if VIRTIO_NET_F_MRG_RXBUF is not negotiated, > + * num_buffers must be 1 > + */ > + vnethdr->num_buffers =3D htole16(num_buffers); > } > =20 > /** > @@ -269,7 +268,7 @@ int vu_send_single(const struct ctx *c, const void *b= uf, size_t size) > goto err; > } > =20 > - vu_set_vnethdr(vdev, in_sg[0].iov_base, elem_cnt); > + vu_set_vnethdr(in_sg[0].iov_base, elem_cnt); > =20 > total -=3D VNET_HLEN; > =20 > diff --git a/vu_common.h b/vu_common.h > index 052aff710502..20868a7f62ce 100644 > --- a/vu_common.h > +++ b/vu_common.h > @@ -49,9 +49,7 @@ void vu_init_elem(struct vu_virtq_element *elem, struct= iovec *iov, > int vu_collect(const struct vu_dev *vdev, struct vu_virtq *vq, > struct vu_virtq_element *elem, int max_elem, size_t size, > size_t *collected); > -void vu_set_vnethdr(const struct vu_dev *vdev, > - struct virtio_net_hdr_mrg_rxbuf *vnethdr, > - int num_buffers); > +void vu_set_vnethdr(struct virtio_net_hdr_mrg_rxbuf *vnethdr, int num_bu= ffers); > void vu_flush(const struct vu_dev *vdev, struct vu_virtq *vq, > struct vu_virtq_element *elem, int elem_cnt); > void vu_kick_cb(struct vu_dev *vdev, union epoll_ref ref, > --=20 > 2.53.0 >=20 --=20 David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson --MXzc7qbvnu4ZuH3/ Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmmnjm8ACgkQzQJF27ox 2GeYQRAAkIVzTxxYv6oSoqDFGcDxJpEIJyZHuJgJeA5wePrVhFugqsQDikSWpdSy 14GhfIXbu1cQ/nDBCcE10xoaLRIFWUMwEbHZTU5R+EG3M08JJh/aVB9qoA++CJ4g h19J/MLETlo1Su4//q4zGFDRb5f4425bJhHzW2J94sV1hoBPvbxc4WFudMTqDza+ 4SaQDfgJfeQVyYyBbx679YFBNod0WUUBWUh+7mH8cuRTMhvi+Sa2C66USipJaUtG +K5zEbXuw1/cgX9CsmjqN7QTc1BPTGciiH+XSUFFdIayGTu+camlWYBRstFC5TT3 w1t8s/CYMV+ANZ6dbCzDdWZCX9jmm7iqIOJYIL88R5izLptJKPaW8HILJ9k2DEPG kgR1i5x+jnT5xbfl8AGxdh6EVLWfh0Es+L7VS6OQ4k6U6izuaiteVkW7PpAzQ7m1 0vyaRwg1vongWP4fy8hT9mBIHyHTg8RM43B9j2vmcY+78w7n3+N2+8f4dxejvHZZ Uyv28PwpYmtKjlNXAfsfdJPsOXpn/DDPvSGbfWynnKA53wrGTSuZlYRik9YH28MC G+kS3yL3Y7XJal7yn0N8En+O1XbgT1/4gwVcZtFIMSUExFtC6NhjHnP913sOagX9 xoFZBxwalr32AKJ0hA/CXvyWmdU4TqFXlaa71zxIOK6WAnjs/Ng= =de+Y -----END PGP SIGNATURE----- --MXzc7qbvnu4ZuH3/--