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 2A4A15A0275 for ; Fri, 1 Mar 2024 00:11:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1709248281; bh=6CN5G2jMVbzKaMVs9N2kldVM4wGAypahozxww0uiJNE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=SnHGMuqreNoYXmlFy/fwTyQDMQ8J+DX3dEKWl2snRuD2ixoV6vcYvXTlm6q4S7iKY C5DtXsnheyCv5+3dRzjtlCvY9mJQwowhsMmPyXNqHIKbWh81i2y28Nx1DlrbG388Fz F69BYBNSUo3gz3gKQ68mTBu1a7mKAlhSBA4eY+5rGAMurOsBbMEhgEE8/zvqQ+MnH/ Plj0Iq5WrofeISrT1fGHnFXF+9EeHGUu/vKoLUd35axwGYVcM9ven6edyeXWNHxCOE DA8Rp0oRnTTBQImrkyc2UoDCIyKq/w++UIMwNzmsQKJP2rThsTuy9QLpCFQIMnVXW7 WQh+JLx2IXjCw== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Tm6RF4WMnz4wck; Fri, 1 Mar 2024 10:11:21 +1100 (AEDT) Date: Fri, 1 Mar 2024 10:10:52 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 6/9] checksum: use csum_ip4_header() in udp.c and tcp.c Message-ID: References: <20240217150725.661467-1-lvivier@redhat.com> <20240217150725.661467-7-lvivier@redhat.com> <20240229172406.6a8331f1@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qT2NuCGqPqnhrR12" Content-Disposition: inline In-Reply-To: <20240229172406.6a8331f1@elisabeth> Message-ID-Hash: H22Z53HHXQHKN4WBDKEI3VXXLCCTHZ5Z X-Message-ID-Hash: H22Z53HHXQHKN4WBDKEI3VXXLCCTHZ5Z 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: Laurent Vivier , 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: --qT2NuCGqPqnhrR12 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 29, 2024 at 05:24:06PM +0100, Stefano Brivio wrote: > On Sat, 17 Feb 2024 16:07:22 +0100 > Laurent Vivier wrote: >=20 > > We can find the same function to compute the IPv4 header > > checksum in tcp.c, udp.c and tap.c > >=20 > > Use the function defined for tap.c, csum_ip4_header(), but > > with the code used in tcp.c and udp.c as it doesn't need a fully > > initialiazed IPv4 header, only protocol, tot_len, saddr and daddr. > >=20 > > Signed-off-by: Laurent Vivier > > --- > >=20 > > Notes: > > v3: > > - function parameters provide tot_len, saddr, daddr and protocol > > rather than an iphdr > > =20 > > v2: > > - use csum_ip4_header() from checksum.c > > - use code from tcp.c and udp.c in csum_ip4_header() > > - use "const struct iphfr *", check is not updated by the > > function but by the caller. > >=20 > > checksum.c | 17 +++++++++++++---- > > checksum.h | 3 ++- > > tap.c | 3 ++- > > tcp.c | 24 +++--------------------- > > udp.c | 20 ++------------------ > > 5 files changed, 22 insertions(+), 45 deletions(-) > >=20 > > diff --git a/checksum.c b/checksum.c > > index 74e3742bc6f6..511b296a9a80 100644 > > --- a/checksum.c > > +++ b/checksum.c > > @@ -57,6 +57,7 @@ > > #include > > =20 > > #include "util.h" > > +#include "ip.h" > > #include "checksum.h" > > =20 > > /* Checksums are optional for UDP over IPv4, so we usually just set > > @@ -116,13 +117,21 @@ uint16_t csum_fold(uint32_t sum) > > uint16_t csum(const void *buf, size_t len, uint32_t init); > > =20 > > /** > > - * csum_ip4_header() - Calculate and set IPv4 header checksum > > + * csum_ip4_header() - Calculate IPv4 header checksum > > * @ip4h: IPv4 header > > */ > > -void csum_ip4_header(struct iphdr *ip4h) > > +uint16_t csum_ip4_header(uint16_t tot_len, uint8_t protocol, > > + uint32_t saddr, uint32_t daddr) > > { > > - ip4h->check =3D 0; > > - ip4h->check =3D csum(ip4h, (size_t)ip4h->ihl * 4, 0); > > + uint32_t sum =3D L2_BUF_IP4_PSUM(protocol); >=20 > Now that we use this macro, Coverity Scan realises that it's broken: >=20 > #define L2_BUF_IP4_PSUM(proto) ((uint32_t)htons_constant(0x4500) + \ > (uint32_t)htons_constant(0xff00 | (proto))) >=20 > ...but proto is eight (lower) bits, so this actually ignores 'proto'. Uh... how so? --=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 --qT2NuCGqPqnhrR12 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXhDvsACgkQzQJF27ox 2GcyNA/+Jog8wEJxas1wNIr8gfzQal6KynAonq/gKAYgcqgarnUpm4LGSboi3zxH EyVgt12AzLHfaCgCK4ZQ4fhCNKFJMxsueNvonx4GsTJk/PNv2SgSrUk7W9LtoEit 4JnlU/9nYUWjZTmtzoGSkXf6VN/GWPqXK5e/xFcFbjZ21MxX0mK7tMqpVeAi5a7r jcP+VTOgjfh9wXQy5LtjPZFIOzaEk1wu6hXr1DCWlBB9j1JIpHZXD/AhDJQbknDN C4ZKdyXJkUI0LaWzOvxrnKpNX4SiknuelLyj/PNf41DOIu7p76kBEDF2wIxWdm5N /ZOiosyMj90KMkcffeg/UAOi//tibG60pv0Qt08eJr1oVGYMrUZpflZgP7IlYH/+ qSbB20ucFHSjbIAn20kB13/KVHeCgUfqdDH9qlu046dGuKZb6suf35stQbKE7ex+ foz6Rdc5lVUJtYx9YpUi1RGtQBRpfIBQqDOhjioYFQ8sJEtEK+gTzXYmU0A3GQU7 Wbc4FHyX61/NFia4nsd2MXV85mmzatnE1wfsmrUppHF165WuDUgDsgqcg/vgQUjo ChjOl8qu0qkJEzDA524QDP7qW0bx+PBlBcvXHsS2QfStoVuSCrGHu/fy7wLMeyHM viPvROTld0X5qZ8JEteRn0rMtbt3OM88Rlx/GCZPOgiB3nDVg90= =v5IX -----END PGP SIGNATURE----- --qT2NuCGqPqnhrR12--