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=202608 header.b=aQjIY+pY; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 6D5C05A0269 for ; Mon, 10 Aug 2026 11:01:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202608; t=1786352496; bh=odI3ThJXWez4CfXaci9j2j/ryNsyfMsqkt8tZT3qDuc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=aQjIY+pYfwmnQibUQC1ZwOSay4xql1sXCFkHDdImva/D3U3XqjswlYFb9wqPEB1mB g5ArsP7v/IeZmOX42YSQt9jLDVYjTEBGkfa0VK77/zEiClALpivx5qSLQp9agm1Bnw tMZHjWzISEaWTnfxESNPaN9R1V6UXXz4kJPxMj5vgbtp57RdBM9kTG40jZarN2AiGZ +HOf1VkzXZA5EpLMy+wJKNYusYCGBjSL7GI2lXKTA2pywmIQ9SP27WXmmJoWa5uJ6T ed0lJ0W7FtoyFftzyZDyhGGNjwjh2hrCQMa8Iv72CBqrDP7SgStRrDEcH5elZVOhkx 5qOqJzF93k1mg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4hJTJc12hRz4w2b; Mon, 10 Aug 2026 19:01:36 +1000 (AEST) Date: Mon, 10 Aug 2026 19:01:30 +1000 From: David Gibson To: Ammar Yasser Subject: Re: [RFC v3 7/8] pasta: Implement pasta vhost TX (pasta->guest) prerequisites Message-ID: References: <20260802132155.870796-1-aerosound161@gmail.com> <20260802132155.870796-8-aerosound161@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="E1c3ZGNIUoY43g1T" Content-Disposition: inline In-Reply-To: <20260802132155.870796-8-aerosound161@gmail.com> Message-ID-Hash: 6XR4RXR37R2INPIEVRWWNIZVHOVXRG6Q X-Message-ID-Hash: 6XR4RXR37R2INPIEVRWWNIZVHOVXRG6Q 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, eperezma@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: --E1c3ZGNIUoY43g1T Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Aug 02, 2026 at 01:21:54PM +0000, Ammar Yasser wrote: > - tap_send_single was changed to directly call > tap_send_frames_passt/pasta instead of relying on tap_send_frames > because protocols that use tap_send_single won't use vhost > acceleration, only tcp and udp will. The function was also moved lower > in the file to by the time its called tap_send_frames_* functions are > defined and a forward declaration isn't needed Sorry, I'm not really following why the vhost change requires tap_send_single() to bypass tap_send_frames(). > - extend udp_meta_t to instead of always carrying a tap_hdr, to carry a > union of a tap_hdr and a virtio_net header because if vhost is used > the will the latter type of header and then build an iov from this > virtio net header in udp.c In fact, the intended role of tap_hdr itself was to be a union of whatever "below L2" headers we might need for all our backends. It was never very obvious because there was just the vnet_len (for the qemu -net socket protocol) and nothing at all (for tuntap via the chardev). With vhost-user, in theory it should have gained a virtio_net_mrg_rxbuf branch, but the vhost-user paths are different enough that we never quite needed it. Here for vhost-kernel, though, we should add the virtio_net header inside tap_hdr, rather than making a union including tap_hdr. tap_hdr_iov() and tap_hdr_update() should be updated to handle that case as well. > - create iov_from_virtio_net_hdr to build an iov from the vnet header > - in the case that vhost is requested, set the first frame fragment to > be a virtio_net header through calling iov_from_virtio_net_hdr > - convert the tcp_payload_tap_hdr array to be an array of virtio_net > header. in the case where we aren't using vhost, we will just be using > the first 4 bytes of it > - add a boolean argumen to tap_send_frames_pasta to determine if vhost > will be used >=20 > Signed-off-by: Ammar Yasser > --- > tap.c | 100 +++++++++++++++++++++++++++++++----------------------- > tcp_buf.c | 39 +++++++++++++++++---- > tcp_buf.h | 4 +-- > udp.c | 28 ++++++++++++++- > udp.h | 6 +++- > 5 files changed, 125 insertions(+), 52 deletions(-) >=20 > diff --git a/tap.c b/tap.c > index 9d601a2..e177eef 100644 > --- a/tap.c > +++ b/tap.c > @@ -124,45 +124,6 @@ unsigned long tap_l2_max_len(const struct ctx *c) > return 0; /* Unreachable, for cppcheck's sake */ > } > =20 > -/** > - * tap_send_single() - Send a single frame > - * @c: Execution context > - * @data: Packet buffer > - * @l2len: Total L2 packet length > - */ > -void tap_send_single(const struct ctx *c, const void *data, size_t l2len) > -{ > - uint8_t padded[ETH_ZLEN] =3D { 0 }; > - struct iovec iov[2]; > - size_t iovcnt =3D 0; > - uint32_t vnet_len; > - > - if (l2len < ETH_ZLEN) { > - memcpy(padded, data, l2len); > - data =3D padded; > - l2len =3D ETH_ZLEN; > - } > - > - vnet_len =3D htonl(l2len); > - > - switch (c->mode) { > - case MODE_PASST: > - iov[iovcnt] =3D IOV_OF_LVALUE(vnet_len); > - iovcnt++; > - /* fall through */ > - case MODE_PASTA: > - iov[iovcnt].iov_base =3D (void *)data; > - iov[iovcnt].iov_len =3D l2len; > - iovcnt++; > - > - tap_send_frames(c, iov, iovcnt, 1); > - break; > - case MODE_VU: > - vu_send_single(c, data, l2len); > - break; > - } > -} > - > /** > * tap_push_l2h() - Build an L2 header for an inbound packet > * @c: Execution context > @@ -415,7 +376,7 @@ void tap_icmp6_send(const struct ctx *c, > */ > static size_t tap_send_frames_pasta(const struct ctx *c, > const struct iovec *iov, > - size_t bufs_per_frame, size_t nframes) > + size_t bufs_per_frame, size_t nframes, bool vhost) > { > size_t nbufs =3D bufs_per_frame * nframes; > size_t i; > @@ -498,6 +459,59 @@ static size_t tap_send_frames_passt(const struct ctx= *c, > return i / bufs_per_frame; > } > =20 > +/** > + * tap_send_single() - Send a single frame > + * @c: Execution context > + * @data: Packet buffer > + * @l2len: Total L2 packet length > + */ > +void tap_send_single(const struct ctx *c, const void *data, size_t l2len) > +{ > + uint8_t padded[ETH_ZLEN] =3D { 0 }; > + struct iovec iov[2]; > + size_t iovcnt =3D 0; > + int m =3D 0; > + uint32_t vnet_len; > + > + if (l2len < ETH_ZLEN) { > + memcpy(padded, data, l2len); > + data =3D padded; > + l2len =3D ETH_ZLEN; > + } > + > + vnet_len =3D htonl(l2len); > + switch (c->mode) { > + case MODE_PASST: > + /* create an iov for the length */ > + iov[iovcnt] =3D IOV_OF_LVALUE(vnet_len); > + iovcnt++; > + /* create the data iov */ > + iov[iovcnt].iov_base =3D (void *)data; > + iov[iovcnt].iov_len =3D l2len; > + iovcnt++; > + m =3D tap_send_frames_passt(c, iov, iovcnt, 1); > + break; > + case MODE_PASTA: > + /* don't create a length iov in the case of pasta */ > + iov[iovcnt].iov_base =3D (void *)data; > + iov[iovcnt].iov_len =3D l2len; > + iovcnt++; > + > + m =3D tap_send_frames_pasta(c, iov, iovcnt, 1, false); You explicitly disable vhost here, even if available. As I've said before, tap_send_single() is a slow path that doesn't really need vhost acceleration. However, there's also not really a reason *not* to use vhost unless it simplifies things. So far this seems to be adding (slightly) complexity to avoid using vhost, and it's not clear to me if there's a simplification elsewhere that makes it worth it. > + break; > + case MODE_VU: > + m =3D vu_send_single(c, data, l2len); > + break; > + } > + > + if (m < 1) > + debug("tap: failed to send a single frame"); > + > + pcap_multiple(iov, iovcnt, m, > + c->mode =3D=3D MODE_PASST ? sizeof(uint32_t) : > + (c->fd_vhost !=3D -1) ? VNET_HLEN : 0); > +} > + > /** > * tap_send_frames() - Send out multiple prepared frames > * @c: Execution context > @@ -523,7 +537,8 @@ size_t tap_send_frames(const struct ctx *c, const str= uct iovec *iov, > =20 > switch (c->mode) { > case MODE_PASTA: > - m =3D tap_send_frames_pasta(c, iov, bufs_per_frame, nframes); > + /* use vhost in pasta sending only if the vhost setup was actually suc= cessful, indicated by a non-zero fd_vhost */ > + m =3D tap_send_frames_pasta(c, iov, bufs_per_frame, nframes, ((c->fd_v= host !=3D -1))); > break; > case MODE_PASST: > m =3D tap_send_frames_passt(c, iov, bufs_per_frame, nframes); > @@ -539,7 +554,8 @@ 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) : > + (c->fd_vhost !=3D -1) ? VNET_HLEN : 0); Rather than this nested ?: expression we should make a helper that gives the necessary length to exclude tap_hdr. tap_hdr_iov() should then use the same helper. > =20 > return m; > } > diff --git a/tcp_buf.c b/tcp_buf.c > index 4452337..080e4cb 100644 > --- a/tcp_buf.c > +++ b/tcp_buf.c > @@ -21,6 +21,7 @@ > #include > =20 > #include > +#include > =20 > #include "util.h" > #include "ip.h" > @@ -38,7 +39,7 @@ > /* Ethernet header for IPv4 and IPv6 frames */ > struct ethhdr tcp_eth_hdr[TCP_FRAMES_MEM]; > =20 > -struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM]; > +struct virtio_net_hdr_mrg_rxbuf tcp_payload_tap_hdr[TCP_FRAMES_MEM]; > =20 > /* IP headers for IPv4 and IPv6 */ > struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; > @@ -71,6 +72,14 @@ void tcp_update_l2_buf(const unsigned char *eth_d) > eth_update_mac(&tcp_eth_hdr[i], eth_d, NULL); > } > =20 > +static inline struct iovec iov_from_virtio_net_hdr(struct virtio_net_hdr= _mrg_rxbuf *hdr) > +{ > + return (struct iovec){ > + .iov_base =3D hdr, > + .iov_len =3D sizeof(*hdr), > + }; > +} As noted, this should become a new branch of tap_hdr_iov()... > /** > * tcp_sock_iov_init() - Initialise scatter-gather L2 buffers for IPv4 s= ockets > * @c: Execution context > @@ -89,7 +98,14 @@ 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]); > + /* If we are using pasta with vhost acceleration, the first entry in t= he tcp buffers > + * should point to a virtio_net header, otherwise a tap header=20 > + */ =09 > + if ((c->fd_vhost !=3D -1)) > + iov[TCP_IOV_TAP] =3D iov_from_virtio_net_hdr(&tcp_payload_tap_hdr[i]); > + else=20 > + iov[TCP_IOV_TAP] =3D tap_hdr_iov(c, (struct tap_hdr *)&tcp_payload_ta= p_hdr[i]); =2E.absorbing this if. > iov[TCP_IOV_ETH].iov_len =3D sizeof(struct ethhdr); > iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp_payload[i]; > iov[TCP_IOV_ETH_PAD].iov_base =3D eth_pad; > @@ -135,8 +151,7 @@ void tcp_payload_flush(const struct ctx *c, const str= uct timespec *now) > { > size_t m; > =20 > - m =3D tap_send_frames(c, &tcp_l2_iov[0][0], TCP_NUM_IOVS, > - tcp_payload_used); > + m =3D tap_send_frames(c, &tcp_l2_iov[0][0], TCP_NUM_IOVS, tcp_payload_u= sed); > if (m !=3D tcp_payload_used) { > tcp_revert_seq(c, &tcp_frame_conns[m], &tcp_l2_iov[m], > tcp_payload_used - m, now); > @@ -177,7 +192,7 @@ static void tcp_l2_buf_fill_headers(const struct ctx = *c, > { > struct iov_tail tail =3D IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0); > struct tcphdr th_storage, *th =3D IOV_REMOVE_HEADER(&tail, th_storage); > - struct tap_hdr *taph =3D iov[TCP_IOV_TAP].iov_base; > + > const struct flowside *tapside =3D TAPFLOW(conn); > const struct in_addr *a4 =3D inany_v4(&tapside->oaddr); > struct ethhdr *eh =3D iov[TCP_IOV_ETH].iov_base; > @@ -192,7 +207,19 @@ static void tcp_l2_buf_fill_headers(const struct ctx= *c, > =20 > l2len =3D tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &tail, > iov_tail_size(&tail), csum_flags, seq); > - tap_hdr_update(taph, l2len); > + > + /* when in pasta mode, the length of the tap header iov is zero, so this > + * l2len write doesn't do anything. But this function gets called from = both > + * pasta and passt. Make the l2len write in case we are in passt mode, = denoted by > + * fd_vhost being -1 since this field won't get initialized at all in p= asst. > + */ > + if (c->mode =3D=3D MODE_PASST) { > + iov[TCP_IOV_TAP].iov_len =3D sizeof(struct tap_hdr); > + struct tap_hdr *taph =3D iov[TCP_IOV_TAP].iov_base; > + tap_hdr_update(taph, l2len); > + } else if (c->fd_vhost =3D=3D -1) { /* pasta mode but without vhost */ > + iov[TCP_IOV_TAP].iov_len =3D 0; > + } Similarly this if should go inside tap_hdr_update(). > } > =20 > /** > diff --git a/tcp_buf.h b/tcp_buf.h > index c749038..da48cd0 100644 > --- a/tcp_buf.h > +++ b/tcp_buf.h > @@ -9,7 +9,7 @@ > #include "tcp_conn.h" > #include "tcp_internal.h" > =20 > -void tcp_sock_iov_init(); > +void tcp_sock_iov_init(const struct ctx *c); > void tcp_payload_flush(const struct ctx *c, const struct timespec *now); > int tcp_buf_data_from_sock(const struct ctx *c, struct tcp_tap_conn *con= n, > uint32_t already_sent, const struct timespec *now); > @@ -20,7 +20,7 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_t= ap_conn *conn, int flags, > #define TCP_FRAMES \ > (c->mode =3D=3D MODE_PASTA ? 1 : TCP_FRAMES_MEM) > =20 > -extern struct tap_hdr tcp_payload_tap_hdr[TCP_FRAMES_MEM]; > +extern struct virtio_net_hdr_mrg_rxbuf tcp_payload_tap_hdr[TCP_FRAMES_ME= M]; > extern struct ethhdr tcp_eth_hdr[TCP_FRAMES_MEM]; > extern struct tcp_payload_t tcp_payload[TCP_FRAMES_MEM]; > =20 > diff --git a/udp.c b/udp.c > index d05ee66..5f90dce 100644 > --- a/udp.c > +++ b/udp.c > @@ -103,6 +103,7 @@ > #include > #include > #include > +#include > =20 > #include "checksum.h" > #include "util.h" > @@ -119,6 +120,18 @@ > #include "udp_vu.h" > #include "epoll_ctl.h" > =20 > +/* UDP header and data for inbound messages */ > +struct udp_payload_t udp_payload[UDP_MAX_FRAMES]; > + > +/* Ethernet headers for IPv4 and IPv6 frames */ > +struct ethhdr udp_eth_hdr[UDP_MAX_FRAMES]; > + > +/* IOVs and msghdr arrays for receiving datagrams from sockets */ > +struct iovec udp_iov_recv [UDP_MAX_FRAMES]; > +struct mmsghdr udp_mh_recv [UDP_MAX_FRAMES]; > + > +/* Pre-cooked headers for UDP packets */ > +struct udp_meta_t udp_meta[UDP_MAX_FRAMES]; I'm slightly confused. I see these added, but I don't see the existing static declarations removed. It's also not clear to me why these need to become non-static. > #define UDP_TIMEOUT "/proc/sys/net/netfilter/nf_conntrack_udp_timeout" > #define UDP_TIMEOUT_STREAM \ > @@ -216,7 +229,20 @@ static void udp_iov_init_one(const struct ctx *c, si= ze_t i) > *siov =3D IOV_OF_LVALUE(payload->data); > =20 > tiov[UDP_IOV_ETH] =3D IOV_OF_LVALUE(udp_eth_hdr[i]); > - tiov[UDP_IOV_TAP] =3D tap_hdr_iov(c, &meta->taph); > + /* if the vhost tap fd is initialized, this is a sign for us that > + * we will be using virtio transport. make the iov that is supposed > + * to point to a tap header point instead to a virtio_net_mrg_rxbuf > + */ > + if (c->fd_vhost !=3D -1) { > + struct iovec vnet_iov =3D { > + .iov_base =3D (void *)(&meta->vnet_hdr), > + .iov_len =3D sizeof(meta->vnet_hdr) > + }; > + tiov[UDP_IOV_TAP] =3D vnet_iov; > + } else { > + tiov[UDP_IOV_TAP] =3D tap_hdr_iov(c, &meta->taph); > + } Again, this logic should go inside tap_hdr_iov(). > + > tiov[UDP_IOV_PAYLOAD].iov_base =3D payload; > tiov[UDP_IOV_ETH_PAD].iov_base =3D eth_pad; > =20 > diff --git a/udp.h b/udp.h > index b7a367c..3464b74 100644 > --- a/udp.h > +++ b/udp.h > @@ -9,6 +9,7 @@ > #include > #include > #include > +#include > =20 > #include "tap_hdr.h" > #include "fwd.h" > @@ -49,7 +50,10 @@ extern struct mmsghdr udp_mh_recv [UDP_MAX_FRAMES]; > struct udp_meta_t { > struct ipv6hdr ip6h; > struct iphdr ip4h; > - struct tap_hdr taph; > + union { > + struct virtio_net_hdr_mrg_rxbuf vnet_hdr; > + struct tap_hdr taph; > +}; > #ifdef __AVX2__ > } __attribute__((aligned(32))); > #else > --=20 > 2.34.1 >=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 --E1c3ZGNIUoY43g1T Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmp5k1sACgkQzQJF27ox 2GdAKg//SVIpq/s8L1lwycuTibZuAly8ZGnnIgdAwZyk8QRjCckwrJz4lyUdF6ST dhqGyQ4S6pXeCJmCg3bKkPPlSYaIV/vT6H4kaZlS0V4leDZljnxs7NbMvLiBzIPo fQNbJFKzb7C3qLlJ74eDzn6hMLK2wMGuE+5wbLwLGRQ+Xc84XiZWvu01U8F7hYy5 hDf5OtkMKsrf9ttQNFq6YbXjwoBQdd/aw27NiDwFjjI5kKkqG81Gq8MPcgoRaKoF cYb6qHYd4MyyE7Ac7qH9DpSat9CIHrOXsF5/qHS6/g5kCawfynQroqgwr1WGm8Wz 9g7MUdsfCngrGnNtXMQ4cByBuFtw994i+nbUiFqC5PHjvvly6AewY9BC5qKxQvUP 4bo1mAnc+2U64oSDnLicco2BQ9pW5Y+d+qVwUbzr46wU2D2R96WaBz8jQ2TbPoQz pXDYkcioaSFD+2v8VdAmEFlGMvcaAJUet0Tqiempjc8ep1NfRIX/yNB3pe/8hQWn oPZGhUj5MaMOxAHZWE1TP6XNqtq9xGLzCbwtoUiv8UcXyc9bSsrorr79XvI/3J3H 4fxhWtTX0sF2MKXJC2KqCFbJjxENCXmNed6N1ftN3ctzWhTuosDoRZNnTtACEBOv Rsa/jRx3x6/DJ0ALrjX1ZmOSbL6fYD6f1f0bYTH3NVfTMRB0QgE= =cIz2 -----END PGP SIGNATURE----- --E1c3ZGNIUoY43g1T--