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=pol7jiJO; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 634165A026E for ; Thu, 13 Aug 2026 04:09:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202608; t=1786586982; bh=smbfdIF4ObbJnI7WxA6fYLLXaiMmTCiR90rqycYQHQ8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=pol7jiJOS+Nrh/vqyG+SCQfXf7sAwwuzwjelXrCMRWX1hqNDHK90w83O5zXK1E++D rTx1jU5U8noefnlBXZVjTKp/PWAhsfJ6jwoRu6Qzvqgc9xfvoY270WAzBR2CgoYz2F Y72t9Vt1iKLAvnXCZEihnH/gyQO+J6Zpf2GZb8YaZPwmcabnBwdLTxaWAEj3Pfo6S1 AxKUOw1Cxm7uVzan1O8VjgVpJ1KrzREbq8VgcH++oHzIWxCjlStv35Pq5erGsJuCGa +BwfOLE7+e9YYFDZ1sDUW9HxgiUjhZC3dTGseKieXOdaH3QrvCZ2hdjd0XbO4iksc/ ojFV8Fd5D83nQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4hL81y1tmzz4wHr; Thu, 13 Aug 2026 12:09:42 +1000 (AEST) Date: Thu, 13 Aug 2026 12:09:35 +1000 From: David Gibson To: Ammar Yasser Subject: Re: [RFC v3 8/8] tap: Implement pasta vhost TX Message-ID: References: <20260802132155.870796-1-aerosound161@gmail.com> <20260802132155.870796-9-aerosound161@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="IfL9qtzy3NwSSYQK" Content-Disposition: inline In-Reply-To: Message-ID-Hash: IZKCH2RYXQARHCRRMHXUBXAGFLUJFLVL X-Message-ID-Hash: IZKCH2RYXQARHCRRMHXUBXAGFLUJFLVL 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: --IfL9qtzy3NwSSYQK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 12, 2026 at 09:45:12PM +0300, Ammar Yasser wrote: > On Mon Aug 10, 2026 at 12:18 PM EEST, David Gibson wrote: > > On Sun, Aug 02, 2026 at 01:21:55PM +0000, Ammar Yasser wrote: > >> Callers will specify whether vhost should be used or no by passing the > >> vhost argument to tap_send_frames_pasta. If specified, sending will go > >> through a function called tap_send_frames_vhost, which pops a descript= or > >> from the queue shared with the kernel and sets the address of that > >> descriptor to be the base address of a single iov from the group of > >> buffers_per_frame * nframes iovs we pass to the function > >>=20 > >> Signed-off-by: Ammar Yasser > > > > Sorry, didn't spot this earlier in the series: if these are based on > > Eugenio's earlier patches, they should have his Signed-off-by in > > addition to your own. >=20 > They have significantly diverged in both order and structure so there's > no clear mapping between mine and his anymore. That's ok. S-o-b isn't really about tracing the origin of any single patch, but about the developer's certificate of origin (see CONTRIBUTING.md). By adding your S-o-b you're asserting the things it says, but if the code is baed on Eugenio's work, even significantly rearranged, then to assert that you're relying on _his_ assertion of the DCO. That's what having both S-o-b lines is documenting. > if there doesn't need to > be a mapping i'm happy to add his Signed-off-by. Otherwise, i did > mention his original series in the cover letter >=20 > > > >> --- > >> tap.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > >> 1 file changed, 107 insertions(+), 1 deletion(-) > >>=20 > >> diff --git a/tap.c b/tap.c > >> index e177eef..66bb71d 100644 > >> --- a/tap.c > >> +++ b/tap.c > >> @@ -361,7 +361,32 @@ void tap_icmp6_send(const struct ctx *c, > >> } > >> =20 > >> /** > >> - * tap_send_frames_pasta() - Send multiple frames to the pasta tap > >> + * tx_reap() - Reclaim the descriptors the kernel has already process= ed > >> + */ > >> +static void tx_reap(void) { > >> + struct vring_used *used =3D &vring_used_all[1].used; > >> + uint16_t used_idx =3D le16toh(used->idx); > >> + > >> + smp_rmb(); > >> + > >> + /* increment last_used_idx until it reaches the kernel's used index = */ > >> + while (vqs[1].last_used_idx !=3D used_idx) { > >> + uint16_t desc_id =3D le32toh(used->ring[vqs[1].last_used_idx % VHOS= T_NDESCS].id); > >> + =09 > >> + for (;;) { > >> + /* keep going until we find a descriptor without the next flag */ > >> + vqs[1].num_free++; > >> + if (!(le16toh(vring_desc[1][desc_id].flags) & VRING_DESC_F_NEXT)) > >> + break; > >> + /* this descriptor wasn't the last, set desc_id to the next one an= d keep going */ > >> + desc_id =3D le16toh(vring_desc[1][desc_id].next); > >> + } > >> + vqs[1].last_used_idx++; > >> + } > >> +} > >> + > >> +/** > >> + * tap_send_frames_vhost() - Send multiple frames to the pasta tap > >> * @c: Execution context > >> * @iov: Array of buffers > >> * @bufs_per_frame: Number of buffers (iovec entries) per frame > >> @@ -371,6 +396,84 @@ void tap_icmp6_send(const struct ctx *c, > >> * @bufs_per_frame contiguous buffers representing a single frame. > >> * > >> * Return: number of frames successfully sent > >> + */ > >> +static size_t tap_send_frames_vhost(const struct ctx *c, > >> + const struct iovec *iov, > >> + size_t bufs_per_frame, size_t nframes) > >> +{ > >> + size_t i; > >> + size_t processed_frames =3D 0; > > > > Reverse christmas tree. >=20 > Noted >=20 > > > >> + > >> + /* update our local counters first */ > >> + tx_reap(); > >> + > >> + #define AVAIL_Q(i)(vring_avail_all[i].avail) > > > > # for preprocessor directives should always go in column 0. >=20 > Ok >=20 > > > >> + > >> + for (i =3D 0; i < nframes; i++) { > >> + size_t j; > >> + > >> + if (vqs[1].num_free < bufs_per_frame) > >> + break; > > > > IIRC, we were discussing during the last call that at least for the > > first version it's fine if we synchronously wait for buffers to be > > available. Since then it occurred to me that another acceptable > > option would be to simply drop frames if there are no available > > buffers (this is kind of how IP is designed to work - congestion is > > reported implicitly as packet loss). Do whichever is easier for now > > and it can be refined later. >=20 > Will do >=20 > > > >> + > >> + /* set the index of the avail ring in the tx queue to be our last_u= sed_idx */ > >> + uint16_t head =3D vqs[1].next_free % VHOST_NDESCS; > > > > No inline decls. >=20 > Ok >=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 --IfL9qtzy3NwSSYQK Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmp9J1QACgkQzQJF27ox 2GeOgQ/+MvR3gAzREWleUufn2vRr/XKiSJexu/4b7dr0yaeuuRPNLPRXPYIhDiyn JiACPuZ+vwhakVl0BA3GTdSJDgYfMdPLex0TnIitA6l2NL0cvm6KNLbUQonbXBsX oYSKy24zXTCs6Bar6CA6YI0Rz1Imu9PWXlim48OlatLCx1nkUyrpdBNSjxtYKfqZ fYiPKNRtz1KqvzLgpr0SQuPKBSlhbOvl1o4KeiI3dBz/ukYFkQ8Cgxx2nCOVFUR4 OOrsRqMuvRqtMn0rsEKigWDWuESQTPvA21cvcqqzBrNxxladFqNy1LJro6UPiVn7 HVaU9Z601PmHse4ThojWWOZteV7VNzPWzMWrRTtiHxZaAIzbfO5c7v+tdo/hsRqT u4A98bT/PrFc+PlHneRS4uLeV4Tud/yz0FUwU/Ey8bFGf5JmmZ2iqa+O50+RCend XmKSZjgYzpbPyK8KiottXGU0JH2xhZ93d/PxTIhUbCYz3XIaJC0XWGBQ00mbDnpg MDOcNr220oE7GTRLkmFBGg4o8b++MUpDpvcsJUNtg93H4Hl8nqbmveTbBf0RhI+B f1QpGUneTZCJoF8zkuX3x/WK6t8t8yZG2QToHuVmz5Y5mV4yWgpPxDqqOiCvaDXh Garz7bveOBZzLV8FjlzO8d9ycYO2O2o/rFIOj0LWM2gP+SsxHHw= =njuk -----END PGP SIGNATURE----- --IfL9qtzy3NwSSYQK--