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=202506 header.b=1uhCoYPH; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 966C55A0271 for ; Thu, 24 Jul 2025 02:24:52 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202506; t=1753316518; bh=5aR9wNqr8j7Hi34dkYg11Wea7zilJRLT/1JNpgydw6w=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=1uhCoYPHJPAhYQV35nJmEjipoIp4RWNu+Oln50Z9USb9qBFNaW0ZaE84dt4H3pOTT h21+x/+htRAwAaMGt9OHfmK8NU7obSPhh1yFhxWWfmUVGsKgeOX01xP+lm8jRMN4uc 4F1keufUrTMqGZuAOsvL2eHN0k8ouJZ80eCfgmoumNTGehaWq5x8aXOmmoO4eh48hq zWWRECD11AZsIXsIX39i0WN/NBpvcHXhNw9a6txlhy4TljVCU/minjJFn9ScH8a0+S InoFZrfnixwiDyS+Gg+a0W9MYyFj6E84VT98XMcUl+ovqVeJNJ2vLpe3R2fDap8O/6 SA29H3N6TtL2A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4bnWsL31Zzz4x3d; Thu, 24 Jul 2025 10:21:58 +1000 (AEST) Date: Thu, 24 Jul 2025 10:17:00 +1000 From: David Gibson To: Eugenio =?iso-8859-1?Q?P=E9rez?= Subject: Re: [RFC v2 03/11] tap: replace tx tap hdr with virtio_nethdr_mrg_rxbuf Message-ID: References: <20250709174748.3514693-1-eperezma@redhat.com> <20250709174748.3514693-4-eperezma@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="yjsFifN/GNpifQXn" Content-Disposition: inline In-Reply-To: <20250709174748.3514693-4-eperezma@redhat.com> Message-ID-Hash: 6Q7VYWYW55ILKABTIVJ5X7ZLRXP6OF2L X-Message-ID-Hash: 6Q7VYWYW55ILKABTIVJ5X7ZLRXP6OF2L 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, jasowang@redhat.com 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: --yjsFifN/GNpifQXn Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 09, 2025 at 07:47:40PM +0200, Eugenio P=E9rez wrote: > vhost kernel expects this as the first data of the frame. >=20 > Signed-off-by: Eugenio P=E9rez > --- > tap.c | 2 +- > tcp_buf.c | 15 +++++++++++++-- > 2 files changed, 14 insertions(+), 3 deletions(-) >=20 > diff --git a/tap.c b/tap.c > index 0c49e6d..0656294 100644 > --- a/tap.c > +++ b/tap.c > @@ -593,7 +593,7 @@ size_t tap_send_frames(const struct ctx *c, const str= uct iovec *iov, > nframes - m, nframes); >=20 > pcap_multiple(iov, bufs_per_frame, m, > - c->mode =3D=3D MODE_PASST ? sizeof(uint32_t) : 0); > + c->mode =3D=3D MODE_PASST ? sizeof(uint32_t) : sizeof(struct vir= tio_net_hdr_mrg_rxbuf)); If I understand correctly, you're always considering virtio_net_hdr_mrg_rxbuf part of the extended frame in PASTA mode, it's just unused when you're not using the vhost path. Is that correct? We probably want a helper that returns the correct tap header length, a companion to tap_hdr_iov(). >=20 > return m; > } > diff --git a/tcp_buf.c b/tcp_buf.c > index d1fca67..2fbd056 100644 > --- a/tcp_buf.c > +++ b/tcp_buf.c > @@ -22,6 +22,8 @@ >=20 > #include >=20 > +#include > + > #include "util.h" > #include "ip.h" > #include "iov.h" > @@ -43,7 +45,7 @@ > static struct ethhdr tcp4_eth_src; > static struct ethhdr tcp6_eth_src; >=20 > -static struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM]; > +static struct virtio_net_hdr_mrg_rxbuf tcp_payload_tap_hdr[TCP_FRAMES_ME= M]; The intention is that struct tap_hdr can store whatever sort of "below L2" header we need - it's just that so far we only had the trivial qemu socket header or nothing. Now we're adding another option, so it should be a union branch of tap_hdr, rather than a separate structure. > /* IP headers for IPv4 and IPv6 */ > struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; > @@ -75,6 +77,14 @@ void tcp_update_l2_buf(const unsigned char *eth_d, con= st unsigned char *eth_s) > eth_update_mac(&tcp6_eth_src, eth_d, eth_s); > } >=20 > +static inline struct iovec virtio_net_hdr_iov(struct virtio_net_hdr_mrg_= rxbuf *hdr) > +{ > + return (struct iovec){ > + .iov_base =3D hdr, > + .iov_len =3D sizeof(*hdr), > + }; > +} With that unification of tap_hdr_iov(), this should be a branch in tap_hdr_iov(), rather than a separate function. tap_hdr_update() will also need to be updated to only update vnet_len in PASST (qemu socket) mode. It is now occurring to me that adding vhost is highlighting an existing ambiguity in our terminology: "tap" can mean either "the guest facing interface, of whatever mechanics", or specifically the kernel tuntap device. Maybe we need to look at a great renaming. > + > /** > * tcp_sock_iov_init() - Initialise scatter-gather L2 buffers for IPv4 s= ockets > * @c: Execution context > @@ -85,6 +95,7 @@ void tcp_sock_iov_init(const struct ctx *c) > struct iphdr iph =3D L2_BUF_IP4_INIT(IPPROTO_TCP); > int i; >=20 > + (void)c; > tcp6_eth_src.h_proto =3D htons_constant(ETH_P_IPV6); > tcp4_eth_src.h_proto =3D htons_constant(ETH_P_IP); >=20 > @@ -96,7 +107,7 @@ void tcp_sock_iov_init(const struct ctx *c) > for (i =3D 0; i < TCP_FRAMES_MEM; i++) { > struct iovec *iov =3D tcp_l2_iov[i]; >=20 > - iov[TCP_IOV_TAP] =3D tap_hdr_iov(c, &tcp_payload_tap_hdr[i]); > + iov[TCP_IOV_TAP] =3D virtio_net_hdr_iov(&tcp_payload_tap_hdr[i]); > iov[TCP_IOV_ETH].iov_len =3D sizeof(struct ethhdr); > iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp_payload[i]; > } --=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 --yjsFifN/GNpifQXn Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmiBe2sACgkQzQJF27ox 2Gdzcg/+LiexS9u/x5QjTeep8WdgnjCSqpZ+/sOGXpHmt+VUT2GsF2jKBC9/mwaW XQxZRt27XpiDYRekhoyRICexLNPZgpsofBdR60lzGrGMBKNP1yPJA/qRqegwAHQO L91zbHO4oLTZdUV7jufSOyeMog3mlwUGywk/eLzsJECwAY6CyydvXBmyv9PeqewK 4OslveqLWClaDr8MMnbqpz/mkh8MQvr+C7j6wQ/6aWrsi9ul+ROHoM8fCGQ4mRqn ME20sMUtYRnUPsTP39d/k8NhZVmLqkmwdB+imxfYALC64xEa99CdbBhxaW0LAu7f 92uM/fp9bnS6p0b0GmCQamdLqmQa9Uc80sD2rMp7VIOn4CQUg2Z2FfXRqHwSDqTi zi4kH5YF5DNj1/2MkEH38y2iiHkrezTpWb4bihU5iAuNqGn7FC8P4Wz3vE9z4sYD d6Hfp0rP2AmApw2+2rlu0bsqqWQ4L/rDWfbnRmAQh4oxaRfPS6gNjUoUioWf6kOX q9bL7UKr0gi8DHcIW1Ph/e+pLiL8kKHfTdfqoyGTCDZlegXCb4cOwf/AE2/xFZb+ Gir3oGKcFYOpet5+EQxiYkp8lSrpQbjO83+t4Zk3HcVinHByeFifLK8LeAs/HjhG d+vLU8T8aJ7yfrO6/LQJR3st19AQnRuKgDaOyUtvkUw5oMM3Vcg= =RY96 -----END PGP SIGNATURE----- --yjsFifN/GNpifQXn--