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 051845A0279 for ; Mon, 19 Feb 2024 04:14:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1708312470; bh=yt+OrrJZOk1oHYmpJRq/hfpHRQQ84T2Xl2CQ8YBjbI8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Ikg//dh2708wQ0OYMy9d7aV4JbOq80FwD6VFCG3w1fLZpHY++DFx2xsKoNIznsFgi CB9+AWdzbxEYwQZ9uAbXS3i8J9cbwWDI4zmvBCxzmAeSEme2DAm+W5gCjxYa3/J+14 yStbB29OJV9ZiJ/N+B4gQ6RwThZ7YKgDO63CvEIXL27w2qdZgMIYLQ6L9bUeaP0z3+ LBqGIJDjtEylXEuwgj/qeQmqdoUTUgLhoH0xFXbpFaX9khZCE9WVLwf52tBR6hVPJw Std+J8yf8Z+ZLE23dRiyDte6W5kDu/91z1ypNjKnUdGd2geqBNAJuu/MB4jNWTGkFJ mCh2htHSpWtUw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TdSLt0Wc4z4wc7; Mon, 19 Feb 2024 14:14:30 +1100 (AEDT) Date: Mon, 19 Feb 2024 14:14:26 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v3 9/9] tcp: Introduce ipv4_fill_headers()/ipv6_fill_headers() Message-ID: References: <20240217150725.661467-1-lvivier@redhat.com> <20240217150725.661467-10-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="xQXyEPgNio+JGWof" Content-Disposition: inline In-Reply-To: <20240217150725.661467-10-lvivier@redhat.com> Message-ID-Hash: T5G2YAFNYVLMTW7WDFB6XTBSLMDI2ZPD X-Message-ID-Hash: T5G2YAFNYVLMTW7WDFB6XTBSLMDI2ZPD 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: --xQXyEPgNio+JGWof Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Feb 17, 2024 at 04:07:25PM +0100, Laurent Vivier wrote: > Replace the macro SET_TCP_HEADER_COMMON_V4_V6() by a new function > tcp_fill_header(). >=20 > Move IPv4 and IPv6 code from tcp_l2_buf_fill_headers() to > tcp_fill_ipv4_header() and tcp_fill_ipv6_header() >=20 > Signed-off-by: Laurent Vivier > --- >=20 > Notes: > v3: > - add to sub-series part 1 > =20 > v2: > - extract header filling functions from > "tcp: extract buffer management from tcp_send_flag()" > - rename them tcp_fill_flag_header()/tcp_fill_ipv4_header(), > tcp_fill_ipv6_header() > - use upside-down Christmas tree arguments order > - replace (void *) by (struct tcphdr *) >=20 > tcp.c | 154 +++++++++++++++++++++++++++++++++++++++------------------- > 1 file changed, 104 insertions(+), 50 deletions(-) >=20 > diff --git a/tcp.c b/tcp.c > index aa03c20712f6..bc57a4f6e611 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -1324,6 +1324,108 @@ void tcp_defer_handler(struct ctx *c) > tcp_l2_data_buf_flush(c); > } > =20 > +/** > + * tcp_fill_header() - Fill the TCP header fields for a given TCP segmen= t. > + * > + * @th: Pointer to the TCP header structure > + * @conn: Pointer to the TCP connection structure > + * @seq: Sequence number > + */ > +static void tcp_fill_header(struct tcphdr *th, > + const struct tcp_tap_conn *conn, uint32_t seq) > +{ > + th->source =3D htons(conn->fport); > + th->dest =3D htons(conn->eport); > + th->seq =3D htonl(seq); > + th->ack_seq =3D htonl(conn->seq_ack_to_tap); > + if (conn->events & ESTABLISHED) { > + th->window =3D htons(conn->wnd_to_tap); > + } else { > + unsigned wnd =3D conn->wnd_to_tap << conn->ws_to_tap; > + > + th->window =3D htons(MIN(wnd, USHRT_MAX)); > + } > +} > + > +/** > + * tcp_fill_ipv4_header() - Fill 802.3, IPv4, TCP headers in pre-cooked = buffers > + * @c: Execution context > + * @conn: Connection pointer > + * @iph: Pointer to IPv4 header, immediately followed by a TCP header > + * @plen: Payload length (including TCP header options) > + * @check: Checksum, if already known > + * @seq: Sequence number for this segment > + * > + * Return: IP frame length including L2 headers, host order > + */ > +static size_t tcp_fill_ipv4_header(const struct ctx *c, > + const struct tcp_tap_conn *conn, > + struct iphdr *iph, size_t plen, > + const uint16_t *check, uint32_t seq) > +{ > + size_t ip_len =3D plen + sizeof(struct iphdr) + sizeof(struct tcphdr); > + const struct in_addr *a4 =3D inany_v4(&conn->faddr); > + struct tcphdr *th =3D (struct tcphdr *)(iph + 1); > + > + iph->tot_len =3D htons(ip_len); > + iph->saddr =3D a4->s_addr; > + iph->daddr =3D c->ip4.addr_seen.s_addr; > + > + iph->check =3D check ? *check : > + csum_ip4_header(iph->tot_len, IPPROTO_TCP, > + iph->saddr, iph->daddr); > + > + > + tcp_fill_header(th, conn, seq); > + > + tcp_update_check_tcp4(iph); > + > + return ip_len; > +} > + > +/** > + * tcp_fill_ipv6_header() - Fill 802.3, IPv6, TCP headers in pre-cooked = buffers > + * @c: Execution context > + * @conn: Connection pointer > + * @ip6h: Pointer to IPv6 header, immediately followed by a TCP header > + * @plen: Payload length (including TCP header options) > + * @check: Checksum, if already known > + * @seq: Sequence number for this segment > + * > + * Return: The total length of the IPv6 packet, host order > + */ > +static size_t tcp_fill_ipv6_header(const struct ctx *c, > + const struct tcp_tap_conn *conn, > + struct ipv6hdr *ip6h, size_t plen, > + uint32_t seq) > +{ > + size_t ip_len =3D plen + sizeof(struct ipv6hdr) + sizeof(struct tcphdr); > + struct tcphdr *th =3D (struct tcphdr *)(ip6h + 1); > + > + ip6h->payload_len =3D htons(plen + sizeof(struct tcphdr)); > + ip6h->saddr =3D conn->faddr.a6; > + if (IN6_IS_ADDR_LINKLOCAL(&ip6h->saddr)) > + ip6h->daddr =3D c->ip6.addr_ll_seen; > + else > + ip6h->daddr =3D c->ip6.addr_seen; > + > + memset(ip6h->flow_lbl, 0, 3); > + > + tcp_fill_header(th, conn, seq); > + > + tcp_update_check_tcp6(ip6h); > + > + ip6h->hop_limit =3D 255; > + ip6h->version =3D 6; > + ip6h->nexthdr =3D IPPROTO_TCP; > + > + ip6h->flow_lbl[0] =3D (conn->sock >> 16) & 0xf; > + ip6h->flow_lbl[1] =3D (conn->sock >> 8) & 0xff; > + ip6h->flow_lbl[2] =3D (conn->sock >> 0) & 0xff; IIUC, the reason part of the ip6h update is done after the TCP header update, but part before was a consequence of how we did the checksumming: we computed the pseudo-header checksum by doing a full checksum operation over the partially filled header, meaning filling the fields not in the pseudo-header had to be done afterwards. Now that you've reworked the checksumming, that's no longer necessary, so you could group all the ip6g initialisations together. Oh.. and also avoid the pre-filling of the flow_lbl with 0s before filling the real values. > + return ip_len; > +} > + > /** > * tcp_l2_buf_fill_headers() - Fill 802.3, IP, TCP headers in pre-cooked= buffers > * @c: Execution context > @@ -1343,67 +1445,19 @@ static size_t tcp_l2_buf_fill_headers(const struc= t ctx *c, > const struct in_addr *a4 =3D inany_v4(&conn->faddr); > size_t ip_len, tlen; > =20 > -#define SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq) \ > -do { \ > - b->th.source =3D htons(conn->fport); \ > - b->th.dest =3D htons(conn->eport); \ > - b->th.seq =3D htonl(seq); \ > - b->th.ack_seq =3D htonl(conn->seq_ack_to_tap); \ > - if (conn->events & ESTABLISHED) { \ > - b->th.window =3D htons(conn->wnd_to_tap); \ > - } else { \ > - unsigned wnd =3D conn->wnd_to_tap << conn->ws_to_tap; \ > - \ > - b->th.window =3D htons(MIN(wnd, USHRT_MAX)); \ > - } \ > -} while (0) > - > if (a4) { > struct tcp4_l2_buf_t *b =3D (struct tcp4_l2_buf_t *)p; > =20 > - ip_len =3D plen + sizeof(struct iphdr) + sizeof(struct tcphdr); > - b->iph.tot_len =3D htons(ip_len); > - b->iph.saddr =3D a4->s_addr; > - b->iph.daddr =3D c->ip4.addr_seen.s_addr; > - > - b->iph.check =3D check ? *check : > - csum_ip4_header(b->iph.tot_len, IPPROTO_TCP, > - b->iph.saddr, b->iph.daddr); > - > - SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq); > - > - tcp_update_check_tcp4(&b->iph); > + ip_len =3D tcp_fill_ipv4_header(c, conn, &b->iph, plen, check, seq); > =20 > tlen =3D tap_iov_len(c, &b->taph, ip_len); > } else { > struct tcp6_l2_buf_t *b =3D (struct tcp6_l2_buf_t *)p; > =20 > - ip_len =3D plen + sizeof(struct ipv6hdr) + sizeof(struct tcphdr); > - > - b->ip6h.payload_len =3D htons(plen + sizeof(struct tcphdr)); > - b->ip6h.saddr =3D conn->faddr.a6; > - if (IN6_IS_ADDR_LINKLOCAL(&b->ip6h.saddr)) > - b->ip6h.daddr =3D c->ip6.addr_ll_seen; > - else > - b->ip6h.daddr =3D c->ip6.addr_seen; > - > - memset(b->ip6h.flow_lbl, 0, 3); > - > - SET_TCP_HEADER_COMMON_V4_V6(b, conn, seq); > - > - tcp_update_check_tcp6(&b->ip6h); > - > - b->ip6h.hop_limit =3D 255; > - b->ip6h.version =3D 6; > - b->ip6h.nexthdr =3D IPPROTO_TCP; > - > - b->ip6h.flow_lbl[0] =3D (conn->sock >> 16) & 0xf; > - b->ip6h.flow_lbl[1] =3D (conn->sock >> 8) & 0xff; > - b->ip6h.flow_lbl[2] =3D (conn->sock >> 0) & 0xff; > + ip_len =3D tcp_fill_ipv6_header(c, conn, &b->ip6h, plen, seq); > =20 > tlen =3D tap_iov_len(c, &b->taph, ip_len); > } > -#undef SET_TCP_HEADER_COMMON_V4_V6 > =20 > return tlen; > } --=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 --xQXyEPgNio+JGWof Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXSx5EACgkQzQJF27ox 2GfZqBAAmKuDmQKCBBb7lTc1dwD/p05YA9fBWvulsbX7ZOmzGIWQJH3BJA9wqvEu jAIw8K3GlVipu0RLjEoEE2yTv5OVq5BFs351KjfVhr4XrEZbPB0TgPl0UEULyMR6 8MqglrrzTudg+PviPaOgpF1l9cMaEQa3vV9AUgINHYuZz+ARwPqAsDRatjyhOwEH 9G8iiSFJL+DLVU5oOoKvel2Lg5F2Zd8ZhDY3rmhzj5lhH/6IgJcMivVK4pBoL+kl SD0CZ1E5m2TRba5g5K0N1sg93/+RXlw4bFg+DDqMfE1plNJL14FiE6j3TXcdyQaE oyZMsQapqqz5QNaFDU6Fv4xyMe8tke1OocOHDo8gbzKiGqe/4UjpV0LBrVj+uLxI HVGLoET1RTZpFw+yKTfdt6yUJrDSJUeeQao3+PA3SziX7or3uE9YXLN1dxWikjBC +10J5Qr38rIOYInAHO34KQm+W7QpZf69a3QieRSSxn3xf1e8uTZ/rTFU+4qOuCUy 0AuvZgXlFzpFeLZrHOvkHxvg7MNDRK4Pj/xDoZ42HwyN6owUZ6xu8kg7UdTNrb5m PELI2O5mgs52NTZhcoSyOVJaCCvhpooAsYWrv1XQI4pXJfkMjDGtvEKiomCA6+Bn 7JI607p8/4538CzN4q4rPP+rrXOx9zdahjufuOY4DfSaa5eJPrk= =vHyZ -----END PGP SIGNATURE----- --xQXyEPgNio+JGWof--