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=202412 header.b=BoPnw9xx; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 2CFC15A026F for ; Fri, 10 Jan 2025 03:40:47 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202412; t=1736476829; bh=x9RiigXg164z+Ld8SPVhB9RLfdUGZlXVeqXSvdvUr3s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=BoPnw9xxnu8SDwSJNUWf+Qb8o28VAUI8itO6PKPCFsx/hQ4IZrLA9yz1JdG/Qyvbz dubHbAAaZMIKVuPBjz67990+IzIWXxPMGtfCkjqid1i3liUPYWCxu6CmFGzjl9b9NU jVXtWPghzUL0NLtmZI+qg23uAZP7zhv2YaYurB/F4uiMYzALSVsZ842o+YVxfsUWOD wQ0SpKAKTLrkSv5/pZ+U9RPAR9QxV0NVB7O4GHA6KZjrLK+QjmL2wRvnCcvgJkUfYD mr2OHQ0gQTw3dJNyQA3j5MKjKvl7rNMK6G/Wi560TOr66l3WhM2H250M1AQM98nOi0 NwEwI8ZA3CyBg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4YTm992pKtz4wcZ; Fri, 10 Jan 2025 13:40:29 +1100 (AEDT) Date: Fri, 10 Jan 2025 13:40:28 +1100 From: David Gibson To: Laurent Vivier Subject: Re: [PATCH] checksum: fix checksum with odd base address Message-ID: References: <20250109130648.326933-1-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="npfcLT72hyOcHDuM" Content-Disposition: inline In-Reply-To: <20250109130648.326933-1-lvivier@redhat.com> Message-ID-Hash: ICW3ON67C25ZSS5NN5566OBE5B4ZM25O X-Message-ID-Hash: ICW3ON67C25ZSS5NN5566OBE5B4ZM25O 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: --npfcLT72hyOcHDuM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jan 09, 2025 at 02:06:48PM +0100, Laurent Vivier wrote: > csum_unfolded() must call csum_avx2() with a 32byte aligned base address. >=20 > To be able to do that if the buffer is not correctly aligned, > it splits the buffers in 2 parts, the second part is 32byte aligned and > can be used with csum_avx2(), the first part is the remaining part, that > is not 32byte aligned and we use sum_16b() to compute the checksum. >=20 > A problem appears if the length of the first part is odd because > the checksum is using 16bit words to do the checksum. >=20 > If the length is odd, when the second part is computed, all words are > shifted by 1 byte, meaning weight of upper and lower byte is swapped. >=20 > For instance a 13 bytes buffer: >=20 > bytes: >=20 > aa AA bb BB cc CC dd DD ee EE ff FF gg >=20 > 16bit words: >=20 > AAaa BBbb CCcc DDdd EEee FFff 00gg >=20 > If we don't split the sequence, the checksum is: >=20 > AAaa + BBbb + CCcc + DDdd + EEee + FFff + 00gg >=20 > If we split the sequence with an even length for the first part: >=20 > (AAaa + BBbb) + (CCcc + DDdd + EEee + FFff + 00gg) >=20 > But if the first part has an odd length: >=20 > (AAaa + BBbb + 00cc) + (ddCC + eeDD + ffEE + ggFF) >=20 > To avoid the problem, do not call csum_avx2() if the first part cannot > have an even length, and compute the checksum of all the buffer using > sum_16b(). >=20 > This is slower but it can only happen if the buffer base address is odd, > and this can only happen if the binary is built using '-Os', and that > means we have chosen to prioritize size over speed. >=20 > Link: https://bugs.passt.top/show_bug.cgi?id=3D108 > Signed-off-by: Laurent Vivier Reviewed-by: David Gibson In that it's a real bug and we need to fix it quickly. That said, I think we can do a bit better long term: I believe it should be possible to correct the value from the of-by-one csum_avx2(), I think with just an unconditional byteswap. The TCP/UDP checksum has the curious property that it doesn't matter if you compute it big-endian or little-endian, as long as you're consistent. We already rely on this. Having one odd byte piece essentially means we're using inconsistent endianness between the two pieces. --=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 --npfcLT72hyOcHDuM Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmeAiIsACgkQzQJF27ox 2GcebhAAiccCIylpNTvVT1LD9eopiXU0i3c2UsX4arQ6+HS4WToEmereGQqPCdX9 1LD6IDE0c/HauQxEpgkfZK0MX3PTFQFcm9ggSSb0eVCoKp18ly3LYWuzc31OOD9g fZwyNAXCdwyARaQFPF3Lz+j5WxUvty6TfrxyUB3XBY/ODdl90FWs7JG6dZKYKmTi 3tBF37lmqGPfeAQMMDOiL4T4GTzBy8RjPgAo/EOk/UzThs3057AqwhSx70RcNIkh qgrtLQe4PLQ3XWdrp/8xtkczGv5eLBqwgc6TKvMbeIP/EOkb6kwqNdlNS15G7h+v 2PUQLaZ2egLwFg1HDx/soZsLX8e8NbG+jBJaf88jcAgm9FGnA1FZW5LBO1AIccdQ w1DOiDZKWWQ5g0L01EEy5yMCHHnjZRdPZhY2RCxv+DlIxD0B4bGz5nitS05DWIgt Q0hPk3Ed2MjdfIxuJ00Fs8WxU9Hr3Ih5KVlTJ9Y9dsTNDXHKXPcOo8T+C5dZVGnU DgPdfUb8oMwpYAEGiyNSH9if4yK5Q5CPJ9+jElvc5LDSa4X0Qb/FU+kA5iBb5zLK tRp5HPC84Wo7UKsSPI0gDst/9oq1J0VXjQTyHK4Iqsk2siYk3fXCF3G0Js1FbEV2 Z80QK52R8lPIRX+UGNCznZAfft2MgqOG/eGpJIUcV/ice7/Snno= =VlP+ -----END PGP SIGNATURE----- --npfcLT72hyOcHDuM--