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=202408 header.b=OMH1HRwp; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 65C4D5A004E for ; Mon, 09 Sep 2024 03:17:49 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202408; t=1725844659; bh=h6yMmiThL58a85n/Bf8RvIzGNwJGQyw5Wgygu1C9NeI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=OMH1HRwpNKvLrPkvCE4Td84O1r1C0Z+Sv03VMB642nwYcA0WYg3t52IqOXrpkPHa8 3on53oquoLMX47IjtijMWbWTAjO95A2HadwA/sdrK5Qw1d0oAxSnXaMVaNsxhoe/xv ++xZxO5KWqHG3rhoqx/AbaEYMtWujfQsF4KBsskP1OVMdky5Wqkq5VpS1mZ6kUars5 8uWwclP1Gx64WdiQkz3rS/geMgAU+PHhi66D7ZMyOP5lLHOj6q0iHmqaHZ2gOTz0TA iqlpkGgsT3EYa4rBrWg56s9+n79iGxQ5Dy18VXSoKMlB214y9L7XNZw/BC/ZSIs4YN txDoewcC9bB6A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4X288M1xsgz4wcl; Mon, 9 Sep 2024 11:17:39 +1000 (AEST) Date: Mon, 9 Sep 2024 11:04:16 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH 1/4] tcp: create unified struct for IPv4 and IPv6 header Message-ID: References: <20240906213427.1915806-1-jmaloy@redhat.com> <20240906213427.1915806-2-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="E+Zsc88BHDC6b+R3" Content-Disposition: inline In-Reply-To: <20240906213427.1915806-2-jmaloy@redhat.com> Message-ID-Hash: QYFG2NVLINCCX2YJL2Q273DCYZ25NC57 X-Message-ID-Hash: QYFG2NVLINCCX2YJL2Q273DCYZ25NC57 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, sbrivio@redhat.com, lvivier@redhat.com, dgibson@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: --E+Zsc88BHDC6b+R3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 06, 2024 at 05:34:24PM -0400, Jon Maloy wrote: > As a preparation for unifying the l2 queues for TCPv4 and TCPv6 traffic > we prepare a unified struct that has space for both IP address types. >=20 > With this change, the arrays tcp4_l2_iov and tcp4_l2_iov, as well as > tcp4_l2_flags_iov and tcp4_l2_flags_iov, are identical in structure > and size. >=20 > Signed-off-by: Jon Maloy > --- > tcp_buf.c | 40 ++++++++++++++++++++++++++++------------ > 1 file changed, 28 insertions(+), 12 deletions(-) >=20 > diff --git a/tcp_buf.c b/tcp_buf.c > index c31e9f3..1af4786 100644 > --- a/tcp_buf.c > +++ b/tcp_buf.c > @@ -52,6 +52,18 @@ struct tcp_payload_t { > } __attribute__ ((packed, aligned(__alignof__(unsigned int)))); > #endif > =20 > +/** > + * struct iphdr_t - Area with space for both IPv4 and IPv6 header > + * @ip4: IPv4 header > + * @ip6: IPv6 header > + */ > +struct iphdr_t { > + union { > + struct iphdr ip4; > + struct ipv6hdr ip6; > + }; > +}; So, I haven't read the rest of the series yet, so it's possible something there would clarify this. Having the two headers in a union does require that essentially the whole header be re-initialized for every packet. For UDP, what we do instead is have both headers in paralllel, partially initialised. For each packet we update the appropriate header as needed, and point our IOV to the correct header (v4 or v6). I don't know if that's a better approach or not: it's plausible the dcache savings outweigh the extra re-initialisation, but it's plausible they don't too. I wonder if it might be better sticking with the same approach as UDP for consistency, at least until we have specific data suggesting one or the other approach is better. > + > /** > * struct tcp_flags_t - TCP header and data to send zero-length > * segments (flags) > @@ -72,7 +84,7 @@ static struct ethhdr tcp4_eth_src; > =20 > static struct tap_hdr tcp4_payload_tap_hdr[TCP_FRAMES_MEM]; > /* IPv4 headers */ > -static struct iphdr tcp4_payload_ip[TCP_FRAMES_MEM]; > +static struct iphdr_t tcp4_payload_ip[TCP_FRAMES_MEM]; > /* TCP segments with payload for IPv4 frames */ > static struct tcp_payload_t tcp4_payload[TCP_FRAMES_MEM]; > =20 > @@ -84,7 +96,7 @@ static unsigned int tcp4_payload_used; > =20 > static struct tap_hdr tcp4_flags_tap_hdr[TCP_FRAMES_MEM]; > /* IPv4 headers for TCP segment without payload */ > -static struct iphdr tcp4_flags_ip[TCP_FRAMES_MEM]; > +static struct iphdr_t tcp4_flags_ip[TCP_FRAMES_MEM]; > /* TCP segments without payload for IPv4 frames */ > static struct tcp_flags_t tcp4_flags[TCP_FRAMES_MEM]; > =20 > @@ -95,7 +107,7 @@ static struct ethhdr tcp6_eth_src; > =20 > static struct tap_hdr tcp6_payload_tap_hdr[TCP_FRAMES_MEM]; > /* IPv6 headers */ > -static struct ipv6hdr tcp6_payload_ip[TCP_FRAMES_MEM]; > +static struct iphdr_t tcp6_payload_ip[TCP_FRAMES_MEM]; > /* TCP headers and data for IPv6 frames */ > static struct tcp_payload_t tcp6_payload[TCP_FRAMES_MEM]; > =20 > @@ -107,7 +119,7 @@ static unsigned int tcp6_payload_used; > =20 > static struct tap_hdr tcp6_flags_tap_hdr[TCP_FRAMES_MEM]; > /* IPv6 headers for TCP segment without payload */ > -static struct ipv6hdr tcp6_flags_ip[TCP_FRAMES_MEM]; > +static struct iphdr_t tcp6_flags_ip[TCP_FRAMES_MEM]; > /* TCP segment without payload for IPv6 frames */ > static struct tcp_flags_t tcp6_flags[TCP_FRAMES_MEM]; > =20 > @@ -144,13 +156,13 @@ void tcp_sock4_iov_init(const struct ctx *c) > tcp4_eth_src.h_proto =3D htons_constant(ETH_P_IP); > =20 > for (i =3D 0; i < ARRAY_SIZE(tcp4_payload); i++) { > - tcp4_payload_ip[i] =3D iph; > + tcp4_payload_ip[i].ip4 =3D iph; > tcp4_payload[i].th.doff =3D sizeof(struct tcphdr) / 4; > tcp4_payload[i].th.ack =3D 1; > } > =20 > for (i =3D 0; i < ARRAY_SIZE(tcp4_flags); i++) { > - tcp4_flags_ip[i] =3D iph; > + tcp4_flags_ip[i].ip4 =3D iph; > tcp4_flags[i].th.doff =3D sizeof(struct tcphdr) / 4; > tcp4_flags[i].th.ack =3D 1; > } > @@ -160,7 +172,8 @@ void tcp_sock4_iov_init(const struct ctx *c) > =20 > iov[TCP_IOV_TAP] =3D tap_hdr_iov(c, &tcp4_payload_tap_hdr[i]); > iov[TCP_IOV_ETH] =3D IOV_OF_LVALUE(tcp4_eth_src); > - iov[TCP_IOV_IP] =3D IOV_OF_LVALUE(tcp4_payload_ip[i]); > + iov[TCP_IOV_IP].iov_base =3D &tcp4_payload_ip[i]; > + iov[TCP_IOV_IP].iov_len =3D sizeof(tcp4_payload_ip[i].ip4); > iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp4_payload[i]; > } > =20 > @@ -170,7 +183,8 @@ void tcp_sock4_iov_init(const struct ctx *c) > iov[TCP_IOV_TAP] =3D tap_hdr_iov(c, &tcp4_flags_tap_hdr[i]); > iov[TCP_IOV_ETH].iov_base =3D &tcp4_eth_src; > iov[TCP_IOV_ETH] =3D IOV_OF_LVALUE(tcp4_eth_src); > - iov[TCP_IOV_IP] =3D IOV_OF_LVALUE(tcp4_flags_ip[i]); > + iov[TCP_IOV_IP].iov_base =3D &tcp4_flags_ip[i]; > + iov[TCP_IOV_IP].iov_len =3D sizeof(tcp4_flags_ip[i].ip4); > iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp4_flags[i]; > } > } > @@ -188,13 +202,13 @@ void tcp_sock6_iov_init(const struct ctx *c) > tcp6_eth_src.h_proto =3D htons_constant(ETH_P_IPV6); > =20 > for (i =3D 0; i < ARRAY_SIZE(tcp6_payload); i++) { > - tcp6_payload_ip[i] =3D ip6; > + tcp6_payload_ip[i].ip6 =3D ip6; > tcp6_payload[i].th.doff =3D sizeof(struct tcphdr) / 4; > tcp6_payload[i].th.ack =3D 1; > } > =20 > for (i =3D 0; i < ARRAY_SIZE(tcp6_flags); i++) { > - tcp6_flags_ip[i] =3D ip6; > + tcp6_flags_ip[i].ip6 =3D ip6; > tcp6_flags[i].th.doff =3D sizeof(struct tcphdr) / 4; > tcp6_flags[i].th .ack =3D 1; > } > @@ -204,7 +218,8 @@ void tcp_sock6_iov_init(const struct ctx *c) > =20 > iov[TCP_IOV_TAP] =3D tap_hdr_iov(c, &tcp6_payload_tap_hdr[i]); > iov[TCP_IOV_ETH] =3D IOV_OF_LVALUE(tcp6_eth_src); > - iov[TCP_IOV_IP] =3D IOV_OF_LVALUE(tcp6_payload_ip[i]); > + iov[TCP_IOV_IP].iov_base =3D &tcp6_payload_ip[i]; > + iov[TCP_IOV_IP].iov_len =3D sizeof(tcp6_payload_ip[i].ip6); > iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp6_payload[i]; > } > =20 > @@ -213,7 +228,8 @@ void tcp_sock6_iov_init(const struct ctx *c) > =20 > iov[TCP_IOV_TAP] =3D tap_hdr_iov(c, &tcp6_flags_tap_hdr[i]); > iov[TCP_IOV_ETH] =3D IOV_OF_LVALUE(tcp6_eth_src); > - iov[TCP_IOV_IP] =3D IOV_OF_LVALUE(tcp6_flags_ip[i]); > + iov[TCP_IOV_IP].iov_base =3D &tcp6_flags_ip[i]; > + iov[TCP_IOV_IP].iov_len =3D sizeof(tcp6_flags_ip[i].ip6); > iov[TCP_IOV_PAYLOAD].iov_base =3D &tcp6_flags[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 --E+Zsc88BHDC6b+R3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmbeSX8ACgkQzQJF27ox 2Gdt7RAAgY4w3Vc7Z3g61y8RmKWn1WPtiUOA5ti5pHETwSn/Rw+IGxoSFeM5lYgd OZug5JOn8mTj4b5vS8oSZQaIbbZc4mUTZRhJXHNBXayqgalG+l/nyxynGokXyGG5 YvElUS+r89DCyNEhg4s384ezQB0AlnGuTTfPxS5nWLN4hJH5bAFkHJE0aNj15fH4 Mc4Vl6hzMRtoeFN1r4/ulmptGnVFDhotNoZQXsgM9PeioNgNu7jNLnXUdR25zoXR HPMO1G12TX11iBrWI6CIxmsIokh8xgq3ny4uwaP7CUJYtvJA3UwwT/GN1ceyvMDw u/kuLzl1c+isoceVUMzBUidht20mlMabUGS9/i1M+0Bl9gugG2Yr+KVHGXARxQet bkf1d7DJfyCoCxPx9tCBSYZexCQK93PFYWmOD7hbm55UtvBQB3BXP9WtlK5PdzTj u/kEKEp780o647MFoHGgB6MfR6C+IyRd9/BT1AU3qGq8WrpbKcLMYTijq5RQl4RF ELlb26cDRds9XJn4DVqCP/DFeXvTB0ePn+AygmlD9WHH0Z8xlKeGvi17ODR7H0rA JThTyr8Yn7iX8d+gj4bNDWI0NoqQUEBmYCnZ64dC+6h5Ufytiakym2jjeYke7BZM +re2PjeyCihZzU26Z4k6JeHPsryThyX4j8sn8FFvrgNM7XmgJUU= =E+pZ -----END PGP SIGNATURE----- --E+Zsc88BHDC6b+R3--