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 A4D625A0278 for ; Fri, 1 Mar 2024 00:54:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1709250845; bh=n3dyjHX9tUBxDAmEZJ6J2yC47e+4NCaOLK29gbzk9kc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=la5zxgBY+2SuQ2ogL1Imfm/26+zOpPPTm6SLl5+QGRyR0HUfSJkxyVmrAd4JNRyqx O4gqd5WtPPDj1q14Gk2h+PQktndCRYeEIg0M4094VIOIumehUAxo6/Z36T39+dCV1O djqaiUSoIhXXpZm0PbuY/oOxIuBOqSRrNk6/5ADurXwXfvBp1dbKLrywVG/Z/yLZ/b sW80fubynx1CBaNS/9445IYNbS/n5RvdZgOleiijFryxWarparzNvO1aVCEA1rVFIs tlfNeXNdsXmY9iSM81kmjV1Ftb4Md/wHqtVEhYslEJxa3fjdpM9Cohnqcy2jeTcqNZ t+B/PCrVPEnvw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Tm7NY4hctz4wcb; Fri, 1 Mar 2024 10:54:05 +1100 (AEDT) Date: Fri, 1 Mar 2024 10:54:00 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v4 8/8] tcp: Introduce ipv4_fill_headers()/ipv6_fill_headers() Message-ID: References: <20240229165955.829476-1-lvivier@redhat.com> <20240229165955.829476-9-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NST2RGGKPnNCNoqO" Content-Disposition: inline In-Reply-To: <20240229165955.829476-9-lvivier@redhat.com> Message-ID-Hash: JMWSU5L3OOJLQXSG7VOBCTKBERJJU2G3 X-Message-ID-Hash: JMWSU5L3OOJLQXSG7VOBCTKBERJJU2G3 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: --NST2RGGKPnNCNoqO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 29, 2024 at 05:59:55PM +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: > v4: > - group all the ip6g initialisations together and > remove flow_lbl preset to 0 > - add ASSERT(a4) > =20 > 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 5b2fdf662a6c..ced22534a103 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -1326,6 +1326,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 I don't love the name, since it does also fill the TCP header. Maybe 'tcp_fill_headers4()'? > + * @c: Execution context > + * @conn: Connection pointer > + * @iph: Pointer to IPv4 header, immediately followed by a TCP header Again, really don't love accessing beyond a given pointer's type. > + * @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 AFAICT the return value does *not* include the L2 headers.. > + */ > +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); > + > + ASSERT(a4); > + > + 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); It's a bit ugly that tcp_fill_header() fills the TCP header, but *not* the checksum. Could we handle this by passing the pseudo-header psum into tcp_fill_header()? Then the logic for that in tcp_update_check_tcp4() would become part of this function. > + 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; > + > + 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; > + > + tcp_fill_header(th, conn, seq); > + > + tcp_update_check_tcp6(ip6h); > + > + return ip_len; > +} > + > /** > * tcp_l2_buf_fill_headers() - Fill 802.3, IP, TCP headers in pre-cooked= buffers > * @c: Execution context > @@ -1345,67 +1447,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 --NST2RGGKPnNCNoqO Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXhGRcACgkQzQJF27ox 2Gf7gQ//WPAHQG8En+z5/UV5fd+MEuu7fkomazdwi1VblhReMZyFES0Od9odaTdt 7YAAbo2G3o4zO7QJpnfDPEK/bg+ZWDimCzz1wJQIqRA5JiJhvFpAs63pYEtokE4b aMx80ekSMB0+RWQvQceY+KfRTnQWDPvUA7APl7VaCP2WF73Zhr542XtA7qAUkAv6 CsTZh5hD6zLbys05jckp0PCY9ExWze6DRJXk/hQZ8Y/Xgn1Lp1LUKy1eXlVGra6w JcCsuBA4EkqhKtdCrHtFpf3VXA/aMetTBxY25domd45tvR/X68vuR4sQfa/xemOm 1YgyoQbApuHoxKmYRgqmSRaNmqjRJMq6RtqtpsmELR61mcj4SFjMaK2O1cRdN+w8 +ULn+aIBaXqLmoO6hFrhZ7YW0AIid35FMJgj4AouXYo8JQTXitA9iYZDUb2g6BWJ P5/KY0EvOQAbaR7aJNOSGVQ0nCL3dQe9Sxw0dL6BfcSD+vgXCP7wSxZMKPObB9rD XpuFdIgbz2DM6pf2yRPHnjOeB1p0MVh5crwByeAr2akc6GUf/oNzjnk6xqht8tSn 1S6UcsfChFtNzxU0q2V3R70lAUTS0dt/OBCNxbSYZy0b6HG1JSrSYV+QqeX6H3xr y2JzbE5x0S8Fha2YaMFUDPQ3nhNqccn23hBjRgU9zxmEcaM41Fc= =XDB7 -----END PGP SIGNATURE----- --NST2RGGKPnNCNoqO--