From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 1AC0A5A02ED for ; Wed, 1 May 2024 04:54:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1714532080; bh=JMTCI1AmrkwG64IrHEZS4qjg0yWCmJaXEz2wbq/+GnE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ph7fJw6GQIf1H5Yydua49hfsDgLtoC6jOe7st/0Hzs24YiZWzWz4GbgNTLPoGFyji I04HJhjl1MqQFkSjQRU8GVrhQEAutZ0Ig+Lae6fq/DUFgD7KOOv4+AuyPMUoxd01xa /rbaOaMJU1XGraHfgGV3c5fGc/2b9jqZ202/5UrpfhNojssGykf0p+4pDqa04KQBlF a46NM0V3vvzovCTIkd9OMUB49Wcp5AiGKIBsH8hUzlqMWMgQ3SqiJiknU9tZ/Y+ozb prrhNWSANkuLzdbb4/nCzvpYVY8crrV4dXFHbq3S5ZBCkSBQ+Be/JFLJvHFdaqjd6W ddl043C8ouO5A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4VThVm52hzz4wyk; Wed, 1 May 2024 12:54:40 +1000 (AEST) Date: Wed, 1 May 2024 12:54:33 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 7/7] udp: Single buffer for IPv4, IPv6 headers and metadata Message-ID: References: <20240430100541.381350-1-david@gibson.dropbear.id.au> <20240430100541.381350-8-david@gibson.dropbear.id.au> <20240430221634.5f7dbf39@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="2HAoXCjvM47MYplD" Content-Disposition: inline In-Reply-To: <20240430221634.5f7dbf39@elisabeth> Message-ID-Hash: C5CB5XGE6JEQ3FV6AJXUFDELIZL6F7PN X-Message-ID-Hash: C5CB5XGE6JEQ3FV6AJXUFDELIZL6F7PN 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: --2HAoXCjvM47MYplD Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 30, 2024 at 10:16:34PM +0200, Stefano Brivio wrote: > On Tue, 30 Apr 2024 20:05:41 +1000 > David Gibson wrote: >=20 > > Currently we have separate arrays for IPv4 and IPv6 which contain the > > headers for guest-bound packets, and also the originating socket addres= s. > > We can combine these into a single array of "metadata" structures with > > space for both pre-cooked IPv4 and IPv6 headers, as well as shared space > > for the tap specific header and socket address (using sockaddr_inany). >=20 > This is neat, definitely, I just wonder: will we need to essentially > revert this if we implement a flow-based mechanism where frames can be > sent to different guests/containers? No, I don't think so. There's still a separate IPv4 and IPv6 header, and socket address for each frame. It's just that those are stored in a parallel array, rather than alongside the paylaod buffers. If we are handling multiple guests, we can probably additionally merge the IPv4 and IPv6 header buffers, since I don't think there will be anything we can pre-fill any more. > On the other hand, chances are it would help instead because we would > have tables of metadata instead of those being buried into frames. >=20 > > Because we're using IOVs to separately address the pieces of each frame, > > these structures don't need to be packed to keep the headers contiguous > > so we can more naturally arrange for the alignment we want. > >=20 > > Signed-off-by: David Gibson > > --- > > udp.c | 131 ++++++++++++++++++++++++---------------------------------- > > 1 file changed, 55 insertions(+), 76 deletions(-) > >=20 > > diff --git a/udp.c b/udp.c > > index 64e7dcef..a9cc6ed2 100644 > > --- a/udp.c > > +++ b/udp.c > > @@ -184,44 +184,27 @@ udp_payload_buf[UDP_MAX_FRAMES]; > > /* Ethernet header for IPv4 frames */ > > static struct ethhdr udp4_eth_hdr; > > =20 > > -/** > > - * udp4_l2_buf_t - Pre-cooked IPv4 packet buffers for tap connections > > - * @s_in: Source socket address, filled in by recvmmsg() > > - * @taph: Tap backend specific header > > - * @iph: Pre-filled IP header (except for tot_len and saddr) > > - */ > > -static struct udp4_l2_buf_t { > > - struct sockaddr_in s_in; > > - > > - struct tap_hdr taph; > > - struct iphdr iph; > > -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))) > > -udp4_l2_buf[UDP_MAX_FRAMES]; > > - > > /* Ethernet header for IPv6 frames */ > > static struct ethhdr udp6_eth_hdr; > > =20 > > /** > > - * udp6_l2_buf_t - Pre-cooked IPv6 packet buffers for tap connections > > - * @s_in6: Source socket address, filled in by recvmmsg() > > + * udp_meta - Pre-cooked headers and metadata for UDP packets >=20 > Pre-existing, but... kernel-doc format wants *struct* x - Description. Adjusted. I also changed to udp_meta_t and udp_meta[] for consistency with the payload buffers. > > + * @ip6h: Pre-filled IPv6 header (except for payload_len and addresses) > > + * @ip4h: Pre-filled IPv4 header (except for tot_len and saddr) > > * @taph: Tap backend specific header > > - * @ip6h: Pre-filled IP header (except for payload_len and addresses) > > + * @s_in: Source socket address, filled in by recvmmsg() > > */ > > -struct udp6_l2_buf_t { > > - struct sockaddr_in6 s_in6; > > -#ifdef __AVX2__ > > - /* Align ip6h to 32-byte boundary. */ > > - uint8_t pad[64 - (sizeof(struct sockaddr_in6) + sizeof(struct tap_hdr= ))]; > > -#endif > > - > > - struct tap_hdr taph; > > +static struct udp_meta { > > struct ipv6hdr ip6h; > > + struct iphdr ip4h; > > + struct tap_hdr taph; > > + > > + union sockaddr_inany s_in; > > +} > > #ifdef __AVX2__ > > -} __attribute__ ((packed, aligned(32))) > > -#else > > -} __attribute__ ((packed, aligned(__alignof__(unsigned int)))) > > +__attribute__ ((aligned(32))) > > #endif > > -udp6_l2_buf[UDP_MAX_FRAMES]; > > +udp_meta_buf[UDP_MAX_FRAMES]; > > =20 > > /** > > * enum udp_iov_idx - Indices for the buffers making up a single UDP f= rame > > @@ -315,49 +298,46 @@ void udp_update_l2_buf(const unsigned char *eth_d= , const unsigned char *eth_s) > > static void udp_iov_init_one(const struct ctx *c, size_t i) > > { > > struct udp_payload *payload =3D &udp_payload_buf[i]; > > + struct udp_meta *meta =3D &udp_meta_buf[i]; > > struct iovec *siov =3D &udp_l2_iov_sock[i]; > > =20 > > + *meta =3D (struct udp_meta) { > > + .ip4h =3D L2_BUF_IP4_INIT(IPPROTO_UDP), > > + .ip6h =3D L2_BUF_IP6_INIT(IPPROTO_UDP), > > + }; > > + > > + >=20 > One extra newline should be enough. Fixed. --=20 David Gibson | 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 --2HAoXCjvM47MYplD Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmYxrugACgkQzQJF27ox 2GcAFA//XAe0uhEZWgUNkslvtyJA0XY3kQee/y6qmxsWzdNHdpxCxNsygEewT5Bu R7yytUQHxCHX9duvQVPVzTvo3X0aZ4onGS4qq3XsK98RKuBxmtmlaVBnYio6HPpz bfsvhIdEB3Oi/E7gRF3vkbwV/NA2XRjegZfBluxVYtNYH0tdWv136x15XE5uvKzz sLSj4gvKURLdHpauwbp9sUUfipmz/waDDuysPwsdRK2aCV19nx5FGC+U7Ac3Oi6J fbMPFBRUIly85n5Vw+q61a7VgUmK29HkCcB2aYnKAjl85VEelAAL8wsEAciJn2uv gzVh6ywTOSZH5LxaVCFgMT8XPkyCCOK/z8UQim2F9psh3HQ+AsAWUJC/RBHOzkuC A1fl3+nCHYE59FQVlub0FHtzU0bP1ER+/XJ/A76bENpzZkarVQgCbGEs4Bz1tT7A gRmxU5QgAw0+/L/f44fmBoxr+15gJWBi8QPqg97hhcHFH6Du/84iqMnhoeBrF8EP sfTySr8klaRSWXRazrgEhcvnzfBzllDMhOSPavQgwiKcT+rYhh9NaqHYu8IFIqU0 irTKvC0VXqUpkHvGjl5bOcUB5GerOEHbg8VFsdxCCkjImfGsAtJw8BTgRpyISmnM x9SBsPQnH/qY6vNHrPseo/XQQtr0gY8g9mm+sa+bEOc/eN+zd3U= =614E -----END PGP SIGNATURE----- --2HAoXCjvM47MYplD--