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=sUyCiF1B; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 2B9145A004F for ; Mon, 30 Sep 2024 06:24:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202408; t=1727670274; bh=dmdR+wXtth/LXKzruHh9NNS/cfe6WvcK8OwCctoEvnw=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=sUyCiF1BipjVxg+3i3Cjvl2KTfG80Nt9DnH2GjPTjANhQx8u5WxnEBoD91o+CkgCw wtcvZG0AV+Q4TlldG55ZM8vbzb7Exyh/mpNpM/k6TZKkZ/HQ/k1kkHMdAGLn4l4HKD 1xLhkOzYxqbce7637xBZ+SdKj9XiLT8BNdnJlP3hwfcNV4H/i8TZavZw2Ob2vyT52R Q3oHkPG72mqvl64Wk+cjvGxbC5rrX8dWzxa5ZED2PNergtWPfytsnb0k5w+K/UcaIR 5Fb3CdSgsrbRJzFYMle2xQpbFbYy8vysp37Zv7ziweWoL2SS9K5XR2mQaLID8WvVy7 8u6Z5YVHO3gvQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4XH7JL4xVSz4wb5; Mon, 30 Sep 2024 14:24:34 +1000 (AEST) Date: Mon, 30 Sep 2024 12:56:40 +1000 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH v5 4/5] tcp: Update TCP checksum using an iovec array Message-ID: References: <20240927135349.675850-1-lvivier@redhat.com> <20240927135349.675850-5-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="lIY96gENUXNLQPmy" Content-Disposition: inline In-Reply-To: <20240927135349.675850-5-lvivier@redhat.com> Message-ID-Hash: X2VD5XCFNP4BVUZUGEYJ3LYULUT52EZU X-Message-ID-Hash: X2VD5XCFNP4BVUZUGEYJ3LYULUT52EZU 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: --lIY96gENUXNLQPmy Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Sep 27, 2024 at 03:53:48PM +0200, Laurent Vivier wrote: > TCP header and payload are supposed to be in the same buffer, > and tcp_update_check_tcp4()/tcp_update_check_tcp6() compute > the checksum from the base address of the header using the > length of the IP payload. >=20 > In the future (for vhost-user) we need to dispatch the TCP header and > the TCP payload through several buffers. To be able to manage that, we > provide an iovec array that points to the data of the TCP frame. > We provide also an offset to be able to provide an array that contains > the TCP frame embedded in an lower level frame, and this offset points > to the TCP header inside the iovec array. >=20 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson > --- >=20 > Notes: > v5: > - s/IPv6/IPv4/ > - reintroduce ip6h and iph to avoid iov_size() > - check pointer alignment before casting to the type > =20 > v4: > - replace die() by err() in tcp_update_check_tcp6() too > =20 > v3: > - replace die() by err() and return > - add more information in the error message > =20 > v2: > - s/payload_offset/l4offset/ > - check memory address of the checksum (alignment, iovec boundaries) >=20 > checksum.c | 1 - > iov.c | 1 - > tcp.c | 122 ++++++++++++++++++++++++++++++++++++++++++++--------- > 3 files changed, 102 insertions(+), 22 deletions(-) >=20 > diff --git a/checksum.c b/checksum.c > index 05d002ab0c25..cf850196cca0 100644 > --- a/checksum.c > +++ b/checksum.c > @@ -503,7 +503,6 @@ uint16_t csum(const void *buf, size_t len, uint32_t i= nit) > * > * Return: 16-bit folded, complemented checksum > */ > -/* cppcheck-suppress unusedFunction */ > uint16_t csum_iov(const struct iovec *iov, size_t n, size_t offset, > uint32_t init) > { > diff --git a/iov.c b/iov.c > index 3f9e229a305f..9116dda94247 100644 > --- a/iov.c > +++ b/iov.c > @@ -25,7 +25,6 @@ > #include "util.h" > #include "iov.h" > =20 > - > /* iov_skip_bytes() - Skip leading bytes of an IO vector > * @iov: IO vector > * @n: Number of entries in @iov > diff --git a/tcp.c b/tcp.c > index c9472d905520..df3147a673d1 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -755,36 +755,106 @@ static void tcp_sock_set_bufsize(const struct ctx = *c, int s) > } > =20 > /** > - * tcp_update_check_tcp4() - Update TCP checksum from stored one > + * tcp_update_check_tcp4() - Calculate TCP checksum for IPv4 > * @iph: IPv4 header > - * @bp: TCP header followed by TCP payload > + * @iov: Pointer to the array of IO vectors > + * @iov_cnt: Length of the array > + * @l4offset: IPv4 payload offset in the iovec array > */ > -static void tcp_update_check_tcp4(const struct iphdr *iph, > - struct tcp_payload_t *bp) > +void tcp_update_check_tcp4(const struct iphdr *iph, > + const struct iovec *iov, int iov_cnt, > + size_t l4offset) > { > uint16_t l4len =3D ntohs(iph->tot_len) - sizeof(struct iphdr); > struct in_addr saddr =3D { .s_addr =3D iph->saddr }; > struct in_addr daddr =3D { .s_addr =3D iph->daddr }; > - uint32_t sum =3D proto_ipv4_header_psum(l4len, IPPROTO_TCP, saddr, dadd= r); > + size_t check_ofs; > + __sum16 *check; > + int check_idx; > + uint32_t sum; > + uintptr_t ptr; > + > + sum =3D proto_ipv4_header_psum(l4len, IPPROTO_TCP, saddr, daddr); > + > + check_idx =3D iov_skip_bytes(iov, iov_cnt, > + l4offset + offsetof(struct tcphdr, check), > + &check_ofs); > + > + if (check_idx >=3D iov_cnt) { > + err("TCP4 buffer is too small, iov size %zd, check offset %zd", > + iov_size(iov, iov_cnt), > + l4offset + offsetof(struct tcphdr, check)); > + return; > + } > =20 > - bp->th.check =3D 0; > - bp->th.check =3D csum(bp, l4len, sum); > + if (check_ofs + sizeof(*check) > iov[check_idx].iov_len) { > + err("TCP4 checksum field memory is not contiguous " > + "check_ofs %zd check_idx %d iov_len %zd", > + check_ofs, check_idx, iov[check_idx].iov_len); > + return; > + } > + > + ptr =3D (uintptr_t)((char *)iov[check_idx].iov_base + check_ofs); > + if (ptr & (__alignof__(*check) - 1)) { > + err("TCP4 checksum field is not correctly aligned in memory"); > + return; > + } > + > + check =3D (__sum16 *)ptr; > + > + *check =3D 0; > + *check =3D csum_iov(iov, iov_cnt, l4offset, sum); > } > =20 > /** > * tcp_update_check_tcp6() - Calculate TCP checksum for IPv6 > * @ip6h: IPv6 header > - * @bp: TCP header followed by TCP payload > + * @iov: Pointer to the array of IO vectors > + * @iov_cnt: Length of the array > + * @l4offset: IPv6 payload offset in the iovec array > */ > -static void tcp_update_check_tcp6(const struct ipv6hdr *ip6h, > - struct tcp_payload_t *bp) > +void tcp_update_check_tcp6(const struct ipv6hdr *ip6h, > + const struct iovec *iov, int iov_cnt, > + size_t l4offset) > { > uint16_t l4len =3D ntohs(ip6h->payload_len); > - uint32_t sum =3D proto_ipv6_header_psum(l4len, IPPROTO_TCP, > - &ip6h->saddr, &ip6h->daddr); > + size_t check_ofs; > + __sum16 *check; > + int check_idx; > + uint32_t sum; > + uintptr_t ptr; > + > + sum =3D proto_ipv6_header_psum(l4len, IPPROTO_TCP, &ip6h->saddr, > + &ip6h->daddr); > + > + check_idx =3D iov_skip_bytes(iov, iov_cnt, > + l4offset + offsetof(struct tcphdr, check), > + &check_ofs); > + > + if (check_idx >=3D iov_cnt) { > + err("TCP6 buffer is too small, iov size %zd, check offset %zd", > + iov_size(iov, iov_cnt), > + l4offset + offsetof(struct tcphdr, check)); > + return; > + } > + > + if (check_ofs + sizeof(*check) > iov[check_idx].iov_len) { > + err("TCP6 checksum field memory is not contiguous " > + "check_ofs %zd check_idx %d iov_len %zd", > + check_ofs, check_idx, iov[check_idx].iov_len); > + return; > + } > + > + ptr =3D (uintptr_t)((char *)iov[check_idx].iov_base + check_ofs); > + if (ptr & (__alignof__(*check) - 1)) { > + err("TCP6 checksum field is not correctly aligned in memory"); > + return; > + } > =20 > - bp->th.check =3D 0; > - bp->th.check =3D csum(bp, l4len, sum); > + check =3D (__sum16 *)ptr; > + > + *check =3D 0; > + *check =3D csum_iov(iov, iov_cnt, l4offset, sum); > } > =20 > /** > @@ -935,10 +1005,16 @@ static size_t tcp_fill_headers4(const struct tcp_t= ap_conn *conn, > =20 > tcp_fill_header(&bp->th, conn, seq); > =20 > - if (no_tcp_csum) > + if (no_tcp_csum) { > bp->th.check =3D 0; > - else > - tcp_update_check_tcp4(iph, bp); > + } else { > + const struct iovec iov =3D { > + .iov_base =3D bp, > + .iov_len =3D ntohs(iph->tot_len) - sizeof(struct iphdr), > + }; > + > + tcp_update_check_tcp4(iph, &iov, 1, 0); > + } > =20 > tap_hdr_update(taph, l3len + sizeof(struct ethhdr)); > =20 > @@ -980,10 +1056,16 @@ static size_t tcp_fill_headers6(const struct tcp_t= ap_conn *conn, > =20 > tcp_fill_header(&bp->th, conn, seq); > =20 > - if (no_tcp_csum) > + if (no_tcp_csum) { > bp->th.check =3D 0; > - else > - tcp_update_check_tcp6(ip6h, bp); > + } else { > + const struct iovec iov =3D { > + .iov_base =3D bp, > + .iov_len =3D ntohs(ip6h->payload_len) > + }; > + > + tcp_update_check_tcp6(ip6h, &iov, 1, 0); > + } > =20 > tap_hdr_update(taph, l4len + sizeof(*ip6h) + sizeof(struct ethhdr)); > =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 --lIY96gENUXNLQPmy Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmb6E2gACgkQzQJF27ox 2Ge0UBAAlfydi96pPDcUVk+b1LnKavYaR5M+JIUet3Es6wszULjDohqxy1G2Pa3O vb3Owr7zRQKty3mznqTXyGPjCBZCc2o4sMNh4SH9hqgjbke8nG2qT1bcOU9zY82l 7zlvaimX4EbT7Hx9BMO/NrbL9EHO4M7WsihfYhQZL4BxTKoM6hdRdURSMmJwY8zD o1crMB6WrJlf9CJ8rebJDZ/jfB0VjGBRccvPys5k5whD8X8d4T0b6WO6ZCpJnnNR o174Cjc/qOvWntnlxf5r2610c2L1z+37nP2n0uASdlMwHCEsbCmGFEVgGNUOqtLp BphJoSS6hRjzFkWSamWQaAYjyRiFMg6ULZCwUgU0QjJaPQ5ewZbcFeYlpymiPP25 rE0NmaT52Mf39iCyRK04Caj23YJDg7Syr4sBPlLxs5WDRN9EzfU1XvXSdK4AoQ3T 5WQh73LCxRHssQiJWdbgGjHQuW+KbOzXNsqAcGlLbBMCZw5bW1/EGDxm6etIQipR hsmpKhddnCAamMQnpoe9KMVPxNiIiexGQ7cRLoHwlqFWKMOBTHh65YYy7C4SKLi8 7hhZTHcq2zXF9k5ygcXsWJSIrWRDyqvPxF9BGJrB5/tqIwhC+bp/K3+BVjSROGCj Yf3pqcpKW8fU7r8MvVhffjLNuBPgBjOT05rxbzXGzZsaUydc2Nw= =mZuT -----END PGP SIGNATURE----- --lIY96gENUXNLQPmy--