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=SfQXm6Vi; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id D6D0E5A0265 for ; Mon, 10 Aug 2026 09:44:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202608; t=1786347883; bh=00SeLPx1srrHyUJwp1YIigJ4492WeK0+KD8AVsNr3qk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=SfQXm6Viar30PHLJlSFDbXnqdz+2S/WQ0KtLHA8jzhKO+6K2O/ANMkhHZfTYF7SpE HfT1gJzBOESX0xM7UGUgj80mh/Wp0FHmP/+hgRAruWduZfjKrhiAU47H5hgjX16Lsm pmz7aP4Df+wZH+g2ymQkHRqn/WcEFPNXri9emYOTYmNO9XEqlxSRFBMHnVSFMzOYYg 3ayP563g/7UK1HEZu2ZcEtSlO3mJ3UaD3zGYHSyfeXHA7XkfMXFJJXIBZdkP5SU9Ue E2kZS48afRzdUr2FICNe/6Aeo0anXUJ9/TYOiMYv/k7TXzSZMk+dvOD8MCGiw7t7pM vFj5QZFiilnug== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4hJRbv46cpz4w91; Mon, 10 Aug 2026 17:44:43 +1000 (AEST) Date: Mon, 10 Aug 2026 17:34:23 +1000 From: David Gibson To: Ammar Yasser Subject: Re: [RFC v3 6/8] virtio: Implement pasta vhost acceleration guest->pasta path Message-ID: References: <20260802132155.870796-1-aerosound161@gmail.com> <20260802132155.870796-7-aerosound161@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="xy31zBBmSNlP1pNV" Content-Disposition: inline In-Reply-To: <20260802132155.870796-7-aerosound161@gmail.com> Message-ID-Hash: LPWJZBB7NTRY3EKC7OTIPUI3S5PP7T6S X-Message-ID-Hash: LPWJZBB7NTRY3EKC7OTIPUI3S5PP7T6S 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: --xy31zBBmSNlP1pNV Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Aug 02, 2026 at 01:21:53PM +0000, Ammar Yasser wrote: > Add a new function called tap_vhost_input that will be used to indicate > that the guest is trying to send data to us through the rx queue and the > kernel is informing us to handle this data. >=20 > We will however still recieve epoll events on the fd normally. We should "the fd" is not very clear here, there are a bunch of fds. I'm guessing you mean the character device fd? > explicitly use either the vhost path of the tap_handler_pasta path and > so if vhost was requested the tap_handler_pasta path gets skipped. >=20 > Add vhost bootstrapping to tap_sock_tun_init in case vhost was > requested. >=20 > consume_one_rx_descriptor: fetch one descriptor from what the kernel has > advertised to us was written to, advance the tracking data structure to > reflect the last index we used, and the count of descriptors we are > ready to hand back to the kernel. return a void pointer to the start of > this descriptor's data >=20 > tap_vhost_input: fetch descriptors using consume_one_rx_descriptor, > and skip over the vnet header, then build an iov tail from data and > queue it for processing, process the frames, and lastly, advertise to > the kernel that the descriptors are free to reuse >=20 > Signed-off-by: Ammar Yasser > --- > passt.c | 6 ++++ > tap.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++++++--- > tap.h | 1 + > 3 files changed, 112 insertions(+), 4 deletions(-) >=20 > diff --git a/passt.c b/passt.c > index 865b331..fdf9070 100644 > --- a/passt.c > +++ b/passt.c > @@ -305,6 +305,12 @@ static void passt_worker(void *opaque, int nfds, str= uct epoll_event *events) > case EPOLL_TYPE_CONF: > conf_handler(c, eventmask); > break; > + case EPOLL_TYPE_VHOST_CALL: > + tap_vhost_input(c, ref, &now); > + break; > + case EPOLL_TYPE_VHOST_ERROR: > + die("Error on vhost-kernel socket"); Eventually we probably want to try to reset and recover here, or even fall back to character device. die() is probably ok in the interim, though. > + break; > default: > /* Can't happen */ > assert(0); > diff --git a/tap.c b/tap.c > index dfa66c7..9d601a2 100644 > --- a/tap.c > +++ b/tap.c > @@ -13,6 +13,7 @@ > * > */ > =20 > +#include "common.h" > #include > #include > #include > @@ -38,6 +39,7 @@ > #include > #include > #include > +#include > =20 > #include > #include > @@ -61,6 +63,7 @@ > #include "vhost_user.h" > #include "vu_common.h" > #include "epoll_ctl.h" > +#include "virtio.h" > =20 > /* Maximum allowed frame lengths (including L2 header) */ > =20 > @@ -1359,7 +1362,8 @@ void tap_handler_pasta(struct ctx *c, uint32_t even= ts, > if (events & (EPOLLRDHUP | EPOLLHUP | EPOLLERR)) > die("Disconnect event on /dev/net/tun device, exiting"); > =20 > - if (events & EPOLLIN) > + /* don't proceed with the normal tap processing in case vhost accelerat= ion was required */ > + if (events & EPOLLIN && !c->vhost) Seems like it would be more elegant to remove the character device fd =66rom the epoll set (or at least remove EPOLLIN) when vhost is activated, rather than take it's events then ignore the. > tap_pasta_input(c, now); > } > =20 > @@ -1514,6 +1518,90 @@ void tap_listen_handler(struct ctx *c, uint32_t ev= ents) > tap_start_connection(c); > } > =20 > +/** > + * consume_one_rx_descriptor() - Consume one used RX descriptor from the= kernel I'd suggest avoiding "Rx" or "Tx" terminology throughout, using "toguest" or "fromguest" instead. Note that for extra confusion Rx here is meaning "fromguest", but for vhost-user it means "toguest". > + * @len: Set to the length of data written by the kernel > + * > + * Pops a single entry from the used ring. Advances vqs[0].last_used_idx > + * (the number of entries we have consumed) and vqs[0].num_free (the cou= nt > + * of descriptors awaiting refill announcement). > + *=20 > + * NOTE: This function assumes the kernel is going to post single descri= ptors > + * always, No chains. If that changes, we would need to increment num_fr= ee > + * as we advertise back to the kernel the free descriptors by the length= of the chain. > + * > + * Return: pointer to the packet buffer, or NULL if no data is available > + */ > +static void *consume_one_rx_descriptor(unsigned *len) For a length of data, we'd typically use a size_t rather than a bare unsigned. > +{ > + struct vring_used *used =3D &vring_used_all[0].used; > + uint32_t i; > + uint16_t used_idx, last_used; > + > + used_idx =3D le16toh(used->idx); > + > + smp_rmb(); > + > + /* if the kernel's last_used index matches our last_used_idx, then > + * we have finished consuming data. > + */ > + if (used_idx =3D=3D vqs[0].last_used_idx) { > + *len =3D 0; > + return NULL; > + } > + > + /* read the last index we consumed */ > + last_used =3D vqs[0].last_used_idx % VHOST_NDESCS; > + /* read the index of what the */ > + i =3D le32toh(used->ring[last_used].id); > + *len =3D le32toh(used->ring[last_used].len); > + > + if (i !=3D last_used) { > + die("vhost: id %u at used position %u !=3D %u", i, last_used, i); > + } Braces not necessary for single line if blocks. > + > + /* the kernel has queued for us something we cannot receive */ > + if (*len > PKT_BUF_BYTES/VHOST_NDESCS) { This seems oddly circular: deriving the maximum frame size from the buffer and number of descriptors, when the number of descriptors was derived from the buffer size and maximum frame size. Better to use (or add) an L2_MAX_LEN_XX constant. > + die("vhost: id %d len %u > %zu", i, *len, PKT_BUF_BYTES/VHOST_NDESCS); > + } > + > + vqs[0].last_used_idx++; > + vqs[0].num_free++; > + return pkt_buf + i * (PKT_BUF_BYTES/VHOST_NDESCS); > +} > + > + > +/** > + * tap_vhost_input() - Handler for new data on the tun socket to hypervi= sor vq > + * @c: Execution context > + * @ref: epoll reference > + * @now: Current timestamp > + */ > +void tap_vhost_input(struct ctx *c, union epoll_ref ref, const struct ti= mespec *now) > +{ > + eventfd_read(ref.fd, (eventfd_t[]){ 0 }); > + > + tap_flush_pools(); > + > + struct virtio_net_hdr_mrg_rxbuf *hdr; > + struct iov_tail data; > + unsigned len; My convention, we don't use inline declarations, even though we could in C11 - move these up to the top. > + > + while ((hdr =3D consume_one_rx_descriptor(&len))) { > + if (len < sizeof(*hdr)) { I think you want to check that at least an ethernet (L2) header is there as well as the virtio_net_hdr_mrg_rxbuf: both tap_pasta_input() and tap_passt_input() appear to check for this. > + warn("vhost: invalid len %u", len); > + continue; > + } > + =09 > + /* skip over the vnet header, we wanna add the packet without it*/ > + data =3D IOV_TAIL_FROM_BUF((void *)(hdr+1), len - sizeof(*hdr), 0); > + tap_add_packet(c, &data, now); > + } > + > + tap_handler(c, now); > + rx_descriptor_handoff(c); > +} > + > /** > * tap_ns_tun() - Get tuntap fd in namespace > * @c: Execution context > @@ -1524,16 +1612,15 @@ void tap_listen_handler(struct ctx *c, uint32_t e= vents) > */ > static int tap_ns_tun(void *arg) > { > - struct ifreq ifr =3D { .ifr_flags =3D IFF_TAP | IFF_NO_PI }; > - int flags =3D O_RDWR | O_NONBLOCK | O_CLOEXEC; > struct ctx *c =3D (struct ctx *)arg; > + struct ifreq ifr =3D { .ifr_flags =3D IFF_TAP | IFF_NO_PI }; > int fd, rc; > =20 > c->fd_tap =3D -1; > memcpy(ifr.ifr_name, c->pasta_ifn, IFNAMSIZ); > ns_enter(c); > =20 > - fd =3D open("/dev/net/tun", flags); > + fd =3D open("/dev/net/tun", O_RDWR | O_NONBLOCK | O_CLOEXEC); This change seems unrelated to the actual aim of this patch. > if (fd < 0) > die_perror("Failed to open() /dev/net/tun"); > =20 > @@ -1561,6 +1648,20 @@ static void tap_sock_tun_init(struct ctx *c) > die("Failed to set up tap device in namespace"); > } > =20 > + /* initialize the vhost-net dev file descriptor */ > + if (c->vhost) { > + setup_vhost_net(c); > + > + for (int i =3D 0; i < ARRAY_SIZE(c->vq); i++) > + setup_eventfds(c, i); > + > + if (setup_memory_table(c) < 0) > + die_perror("VHOST_SET_MEM_TABLE ioctl on /dev/vhost-net failed"); > + > + for (int i =3D 0; i < ARRAY_SIZE(c->vq); i++) > + set_vring_for_queue(c, i, c->fd_tap); > + } It's not clear to me if this code block needs to execute within the guest namespace. If not, it should probably be in tap_sock_tun_init() rather than tap_ns_tun(). If it _does_ have to be in tap_ns_tun(), then tap_ns_tun() probably needs a rename, since it's now doing rather more in the vhost case that just getting a device fd. > + > pasta_ns_conf(c); > =20 > if (!c->splice_only) > diff --git a/tap.h b/tap.h > index 1625975..eb02da8 100644 > --- a/tap.h > +++ b/tap.h > @@ -66,6 +66,7 @@ static inline void tap_hdr_update(struct tap_hdr *thdr,= size_t l2len) > thdr->vnet_len =3D htonl(l2len); > } > =20 > +void tap_vhost_input(struct ctx *c, union epoll_ref ref, const struct ti= mespec *now); > unsigned long tap_l2_max_len(const struct ctx *c); > void *tap_push_l2h(const struct ctx *c, void *buf, > const void *src_mac, uint16_t proto); > --=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 --xy31zBBmSNlP1pNV Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmp5fvAACgkQzQJF27ox 2GeRvRAApCeW1uRzgw1SUmBWST0uGt5LCmjkfZxzrm8tk4SoqXRXwLRI2a8+xLu8 T/GJu3wwaW4/H+AnkuEza4G04Pk+Mf1qJgUwNr3hYT2x+SVhyauUv00We3YD8OyQ Ov2ziCVItyUQ0D7twEpgcc/9bywex086eDlJHZ4Aw+POMVjN3wFSJ2GDbq0t5dly o6hA1SM3/kx4txB/YHtXnT4Oa6BUhKLR85rVcbiBhAPprvBgJL2AAHVtAmUxqzoT mMEocXly/nDEIjpeOS22yLF7hUWO/iHCWjdF+Ltnr90TrtPRABQB1yf+sVN5YzOZ UUIgmtueGzrOQGQUFYQyQ/xhHRbn3BSJj5cOWSZGjs+U/n6qeFpX8KVTe4HJ8nWp RR+V5fw5VvUfLpr0LD+R/S4ks3rddu+kPqbm1MZOcp6dSjXRt7uulKrbaTxuTMVU h/SFcfRYkHWm5FZcub4k89GnPS+npLdaoD/Rwk7OHtUAiUoUQR+J0AcTnVPL05EZ DP4FI+g+5X7yFIzhgIvuVip2cU/7eSU47GjM7wY9u9PpgeAvfdsHgnXUTUgYt0DD uH4z04sSu9+LWkzZKVJ+IPVQdlhaNsoBzRKYHOGrjQAFmYHUnCYfN4OW8QQLLzn7 xr4Upr8wY7we4UbBf8rNm8L0B4tkXrp9ei7HovWchl2jcdU9TVw= =/qkE -----END PGP SIGNATURE----- --xy31zBBmSNlP1pNV--