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=202602 header.b=IRrPYrXN; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id A3C5E5A0262 for ; Tue, 24 Mar 2026 06:24:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202602; t=1774329885; bh=lfb/9D/i8kBGtLagE17YWd8HpwgMBA4V1qcjMiPZnfw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=IRrPYrXN8kG92hQd3HeAGepdsWxtsSFnUgM1otVGye+WyT8O9RWRUi14XaQAH1ONR C0PuDYVr6dSZNaf1Fuz7iub3RzhQwo88m/6tiU0PBn3n3++L8zRxiYtIvP4Mz4bnqX fug37P0MttdCNriQfYM+eXrg3RkyTX939JFisjf+JECOdz1a1rMkmZ2oYHD6X+vLpu DduUuIA2NekETWBn6+Rs+nd55P0uQj8na3n+LapFdylNeB3ghqmuSbzDGHP0I7cvGR zJgAI212puanCP01Vxt82z8retLo/3Na7m6VQzn8VHfPB4bvZ3mMDZxRO6kM/3a+63 J+RlkYK2ajVLg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4ffz4Y631Pz4wCB; Tue, 24 Mar 2026 16:24:45 +1100 (AEDT) Date: Tue, 24 Mar 2026 14:58:39 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH 2/7] tcp: use iov_tail to access headers in tcp_fill_headers() Message-ID: References: <20260323165259.1253482-1-lvivier@redhat.com> <20260323165259.1253482-3-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ylt+rxGp/K3p2ehW" Content-Disposition: inline In-Reply-To: <20260323165259.1253482-3-lvivier@redhat.com> Message-ID-Hash: JODLIHABXVUJIX36XXJWVYZH34Z77OE4 X-Message-ID-Hash: JODLIHABXVUJIX36XXJWVYZH34Z77OE4 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: --ylt+rxGp/K3p2ehW Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Mar 23, 2026 at 05:52:54PM +0100, Laurent Vivier wrote: > Instead of receiving individual pointers to each protocol header (eh, > ip4h, ip6h, th), have tcp_fill_headers() take an iov_tail starting at > the Ethernet header and walk through it using with_header() and > IOV_DROP_HEADER() to access each header in turn. >=20 > Replace the ip4h/ip6h NULL-pointer convention with a bool ipv4 > parameter, and move Ethernet header filling (MAC address and ethertype) > into tcp_fill_headers() as well, since the function now owns the full > header chain. >=20 > This simplifies callers, which no longer need to extract and pass > individual header pointers. >=20 > Signed-off-by: Laurent Vivier > --- > tcp.c | 106 +++++++++++++++++++++++++++---------------------- > tcp_buf.c | 16 ++------ > tcp_internal.h | 4 +- > tcp_vu.c | 12 +++--- > 4 files changed, 70 insertions(+), 68 deletions(-) >=20 > diff --git a/tcp.c b/tcp.c > index 158a5be0327e..058792d5b184 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -938,11 +938,8 @@ static void tcp_fill_header(struct tcphdr *th, > * tcp_fill_headers() - Fill 802.3, IP, TCP headers > * @c: Execution context > * @conn: Connection pointer > - * @eh: Pointer to Ethernet header > - * @ip4h: Pointer to IPv4 header, or NULL > - * @ip6h: Pointer to IPv6 header, or NULL > - * @th: Pointer to TCP header > - * @payload: TCP payload > + * @ipv4: True for IPv4, false for IPv6 > + * @payload: IOV tail starting at the Ethernet header As UDP I'm not sure I like removing the separater parameters. A little more so, since this doesn't (any more) have the ugly udp_payload_t equivalent. At minimum @payload is no longer a good name. > * @ip4_check: IPv4 checksum, if already known > * @seq: Sequence number for this segment > * @no_tcp_csum: Do not set TCP checksum > @@ -950,74 +947,89 @@ static void tcp_fill_header(struct tcphdr *th, > * Return: frame length (including L2 headers) > */ > size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn, > - struct ethhdr *eh, > - struct iphdr *ip4h, struct ipv6hdr *ip6h, > - struct tcphdr *th, struct iov_tail *payload, > + bool ipv4, const struct iov_tail *payload, > int ip4_check, uint32_t seq, bool no_tcp_csum) > { > const struct flowside *tapside =3D TAPFLOW(conn); > - size_t l4len =3D iov_tail_size(payload) + sizeof(*th); > + struct iov_tail current =3D *payload; > uint8_t *omac =3D conn->f.tap_omac; > - size_t l3len =3D l4len; > + size_t l3len, l4len; > uint32_t psum =3D 0; > =20 > - if (ip4h) { > + with_header(struct ethhdr, eh, ¤t) { > + if (ipv4) > + eh->h_proto =3D htons_constant(ETH_P_IP); > + else > + eh->h_proto =3D htons_constant(ETH_P_IPV6); > + > + /* Find if neighbour table has a recorded MAC address */ > + if (MAC_IS_UNDEF(omac)) > + fwd_neigh_mac_get(c, &tapside->oaddr, omac); > + eth_update_mac(eh, NULL, omac); > + } > + IOV_DROP_HEADER(¤t, struct ethhdr); > + > + l3len =3D iov_tail_size(¤t); > + > + if (ipv4) { > const struct in_addr *src4 =3D inany_v4(&tapside->oaddr); > const struct in_addr *dst4 =3D inany_v4(&tapside->eaddr); > =20 > assert(src4 && dst4); > =20 > - l3len +=3D + sizeof(*ip4h); > + l4len =3D l3len - sizeof(struct iphdr); > =20 > - ip4h->tot_len =3D htons(l3len); > - ip4h->saddr =3D src4->s_addr; > - ip4h->daddr =3D dst4->s_addr; > + with_header(struct iphdr, ip4h, ¤t) { > + ip4h->tot_len =3D htons(l3len); > + ip4h->saddr =3D src4->s_addr; > + ip4h->daddr =3D dst4->s_addr; > =20 > - if (ip4_check !=3D -1) > - ip4h->check =3D ip4_check; > - else > - ip4h->check =3D csum_ip4_header(l3len, IPPROTO_TCP, > - *src4, *dst4); > + if (ip4_check !=3D -1) > + ip4h->check =3D ip4_check; > + else > + ip4h->check =3D csum_ip4_header(l3len, > + IPPROTO_TCP, > + *src4, *dst4); > + } > + IOV_DROP_HEADER(¤t, struct iphdr); > =20 > if (!no_tcp_csum) { > psum =3D proto_ipv4_header_psum(l4len, IPPROTO_TCP, > *src4, *dst4); > } > - eh->h_proto =3D htons_constant(ETH_P_IP); > - } > - > - if (ip6h) { > - l3len +=3D sizeof(*ip6h); > + } else { > + l4len =3D l3len - sizeof(struct ipv6hdr); > =20 > - ip6h->payload_len =3D htons(l4len); > - ip6h->saddr =3D tapside->oaddr.a6; > - ip6h->daddr =3D tapside->eaddr.a6; > + with_header(struct ipv6hdr, ip6h, ¤t) { > + ip6h->payload_len =3D htons(l4len); > + ip6h->saddr =3D tapside->oaddr.a6; > + ip6h->daddr =3D tapside->eaddr.a6; > =20 > - ip6h->hop_limit =3D 255; > - ip6h->version =3D 6; > - ip6h->nexthdr =3D IPPROTO_TCP; > + ip6h->hop_limit =3D 255; > + ip6h->version =3D 6; > + ip6h->nexthdr =3D IPPROTO_TCP; > =20 > - ip6_set_flow_lbl(ip6h, conn->sock); > + ip6_set_flow_lbl(ip6h, conn->sock); > =20 > - if (!no_tcp_csum) { > - psum =3D proto_ipv6_header_psum(l4len, IPPROTO_TCP, > - &ip6h->saddr, > - &ip6h->daddr); > + if (!no_tcp_csum) { > + psum =3D proto_ipv6_header_psum(l4len, > + IPPROTO_TCP, > + &ip6h->saddr, > + &ip6h->daddr); > + } > } > - eh->h_proto =3D htons_constant(ETH_P_IPV6); > + IOV_DROP_HEADER(¤t, struct ipv6hdr); > } > =20 > - /* Find if neighbour table has a recorded MAC address */ > - if (MAC_IS_UNDEF(omac)) > - fwd_neigh_mac_get(c, &tapside->oaddr, omac); > - eth_update_mac(eh, NULL, omac); > - > - tcp_fill_header(th, conn, seq); > - > - if (no_tcp_csum) > + with_header(struct tcphdr, th, ¤t) { > + tcp_fill_header(th, conn, seq); > th->check =3D 0; > - else > - tcp_update_csum(psum, th, payload); > + } > + > + if (!no_tcp_csum) { > + with_header(struct tcphdr, th, ¤t) > + th->check =3D csum_iov_tail(¤t, psum); > + } > =20 > return MAX(l3len + sizeof(struct ethhdr), ETH_ZLEN); > } > diff --git a/tcp_buf.c b/tcp_buf.c > index bc0f58dd7a5e..891043c96dcb 100644 > --- a/tcp_buf.c > +++ b/tcp_buf.c > @@ -175,23 +175,15 @@ static void tcp_l2_buf_fill_headers(const struct ct= x *c, > struct iovec *iov, int check, > uint32_t seq, bool no_tcp_csum) > { > - struct iov_tail tail =3D IOV_TAIL(&iov[TCP_IOV_PAYLOAD], 1, 0); > - struct tcphdr th_storage, *th =3D IOV_REMOVE_HEADER(&tail, th_storage); > + struct iov_tail tail =3D IOV_TAIL(&iov[TCP_IOV_ETH], > + TCP_IOV_PAYLOAD + 1 - TCP_IOV_ETH, 0); > struct tap_hdr *taph =3D iov[TCP_IOV_TAP].iov_base; > const struct flowside *tapside =3D TAPFLOW(conn); > - const struct in_addr *a4 =3D inany_v4(&tapside->oaddr); > - struct ethhdr *eh =3D iov[TCP_IOV_ETH].iov_base; > - struct ipv6hdr *ip6h =3D NULL; > - struct iphdr *ip4h =3D NULL; > + bool ipv4 =3D inany_v4(&tapside->oaddr) !=3D NULL; > size_t l2len; > =20 > - if (a4) > - ip4h =3D iov[TCP_IOV_IP].iov_base; > - else > - ip6h =3D iov[TCP_IOV_IP].iov_base; > + l2len =3D tcp_fill_headers(c, conn, ipv4, &tail, check, seq, no_tcp_csu= m); > =20 > - l2len =3D tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &tail, check, s= eq, > - no_tcp_csum); > tap_hdr_update(taph, l2len); > } > =20 > diff --git a/tcp_internal.h b/tcp_internal.h > index bb7a6629839c..136e947f6e70 100644 > --- a/tcp_internal.h > +++ b/tcp_internal.h > @@ -184,9 +184,7 @@ void tcp_rst_do(const struct ctx *c, struct tcp_tap_c= onn *conn); > struct tcp_info_linux; > =20 > size_t tcp_fill_headers(const struct ctx *c, struct tcp_tap_conn *conn, > - struct ethhdr *eh, > - struct iphdr *ip4h, struct ipv6hdr *ip6h, > - struct tcphdr *th, struct iov_tail *payload, > + bool ipv4, const struct iov_tail *payload, > int ip4_check, uint32_t seq, bool no_tcp_csum); > =20 > int tcp_update_seqack_wnd(const struct ctx *c, struct tcp_tap_conn *conn, > diff --git a/tcp_vu.c b/tcp_vu.c > index a21ee3499aed..c6206b7a689c 100644 > --- a/tcp_vu.c > +++ b/tcp_vu.c > @@ -132,13 +132,12 @@ int tcp_vu_send_flag(const struct ctx *c, struct tc= p_tap_conn *conn, int flags) > } > =20 > vu_pad(&flags_iov[0], 1, 0, hdrlen + optlen); > - payload =3D IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen); > =20 > if (flags & KEEPALIVE) > seq--; > =20 > - tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload, > - -1, seq, !*c->pcap); > + payload =3D IOV_TAIL(flags_elem[0].in_sg, 1, VNET_HLEN); > + tcp_fill_headers(c, conn, CONN_V4(conn), &payload, -1, seq, !*c->pcap); > =20 > if (*c->pcap) > pcap_iov(&flags_elem[0].in_sg[0], 1, VNET_HLEN); > @@ -288,10 +287,10 @@ static void tcp_vu_prepare(const struct ctx *c, str= uct tcp_tap_conn *conn, > const struct flowside *toside =3D TAPFLOW(conn); > bool v6 =3D !(inany_v4(&toside->eaddr) && inany_v4(&toside->oaddr)); > size_t hdrlen =3D tcp_vu_hdrlen(v6); > - struct iov_tail payload =3D IOV_TAIL(iov, iov_cnt, hdrlen); > char *base =3D iov[0].iov_base; > struct ipv6hdr *ip6h =3D NULL; > struct iphdr *ip4h =3D NULL; > + struct iov_tail payload; > struct tcphdr *th; > struct ethhdr *eh; > =20 > @@ -326,8 +325,9 @@ static void tcp_vu_prepare(const struct ctx *c, struc= t tcp_tap_conn *conn, > th->ack =3D 1; > th->psh =3D push; > =20 > - tcp_fill_headers(c, conn, eh, ip4h, ip6h, th, &payload, > - *check, conn->seq_to_tap, no_tcp_csum); > + payload =3D IOV_TAIL(iov, iov_cnt, VNET_HLEN); > + tcp_fill_headers(c, conn, !v6, &payload, *check, conn->seq_to_tap, > + no_tcp_csum); > if (ip4h) > *check =3D ip4h->check; > } > --=20 > 2.53.0 >=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 --ylt+rxGp/K3p2ehW Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmnCC+4ACgkQzQJF27ox 2Gf1phAAm7zYqni9xcGXJ5SJi0aNqgLHM/yV+AHw/dW/P0bBHxtSUyyWKRpIS+UG LbAo0PwubBXzB7kiRxqXm1ppfAoUffvd6x2d1XrvNejMJUvutcz+VM8OiaHerpz/ Cv97kkMvj2sXGSJOQw4TmwuT5tb7WVG62fl70cRd1Wg7qZzzj8kU10KZdQ9Bbqma qf7mUVdYMDGrZbrrOHIJBeTaXX3rGTlc5mjt+hX1zEYn8PDSLbud5qvaxZ1BayN1 BMRTZ9hmVZP7ukjXBHRtqjjHulmjHFRtYbunXWaH+EJlQKG3Gnh+R7tzOXtOmUk3 g16y+RsN3wxR9VfWB5HP74GLSsOXCsnOEyqjuNRpKNHhbZ5WUaj1GUaFGD1nr/VZ cctTYQi/qQD2EQlTNId8HJcfPMLChv9g+YeXxh3dCuAx7nvztgtxycJ3USFWF/hY j5uRI241w7MhY97XqZEckol2bR+ycj9HBBTEATKRm0/M6HCmS1RvsERCQckBM0rm 2VnBh3KqBUN22eqF2wj7RU5R89aTk0pGx4JKn3zmzQgTe54HyqK+2xuIFG+lMacd cFFQNHAaP/sCBzScEJcOGmoJxDQC9oYLO7Qfn1a0VEuug5ZB237EDCtz76MAqrCL g5VtQKc6Qtmz5bCHFLHEOPEc3FkDIA410SZ8NnVIgz+kYErt4K4= =jZBM -----END PGP SIGNATURE----- --ylt+rxGp/K3p2ehW--