From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id AFD5B5A0275 for ; Thu, 29 Feb 2024 09:49:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1709196554; bh=+4I1m46T/e6HqCuYRx3K949KyiYPFRVVNU5HhczC7KQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=pqHuXAXJg4ehllTtMeQPnn/e/sfccUTmQZZgJIlc1LiSSrxoYbjtiosSfcnJSf7kU OzmS8poHVmd1quULhnYikfYN2YemUVyaqAIzXtyQhgMiP66IhYyteK2SIgOe9o34WD abVhbDMD83kIGufziZw8k39zelJTuxebidqN9e3TK37n/9N8gRTz95/5xNPiUTwtAU y+t4jezSPPMznLdmxz39DD+7gLZia4jQA5eRDVvTVxbJnz8w9kJnx2QFeO5s8qX3Y6 wuGMmAAlGIGJWuEl0XNoxbMifhdndBA98AweBdH9sW8/DegZj5E9JMa4hqhYonU60O GinGqSnlExcMg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TllJV4XVYz4wyh; Thu, 29 Feb 2024 19:49:14 +1100 (AEDT) Date: Thu, 29 Feb 2024 19:49:09 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 7/9] checksum: introduce functions to compute the header part checksum for TCP/UDP Message-ID: References: <20240217150725.661467-1-lvivier@redhat.com> <20240217150725.661467-8-lvivier@redhat.com> <04c99072-02ea-46a9-aac6-23116cb05fa1@redhat.com> <20240229080509.4f534831@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="38kXi3If9Qihq7d7" Content-Disposition: inline In-Reply-To: <20240229080509.4f534831@elisabeth> Message-ID-Hash: KY54JWZ4BP5RMRGPY4ASIKNL7KIA4WOB X-Message-ID-Hash: KY54JWZ4BP5RMRGPY4ASIKNL7KIA4WOB 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: --38kXi3If9Qihq7d7 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 29, 2024 at 08:05:09AM +0100, Stefano Brivio wrote: > On Thu, 29 Feb 2024 11:38:53 +1100 > David Gibson wrote: >=20 > > On Wed, Feb 28, 2024 at 02:26:18PM +0100, Laurent Vivier wrote: > > > On 2/19/24 04:08, David Gibson wrote: =20 > > > > On Sat, Feb 17, 2024 at 04:07:23PM +0100, Laurent Vivier wrote: =20 > > > > > > > > [...] > > > > > > > > > +/** > > > > > + * proto_ipv6_header_psum() - Calculates the partial checksum of= an > > > > > + * IPv6 header for UDP or TCP > > > > > + * @payload_len: Payload length > > > > > + * @proto: Protocol number > > > > > + * @saddr: Source address > > > > > + * @daddr: Destination address > > > > > + * Returns: Partial checksum of the IPv6 header > > > > > + */ > > > > > +uint32_t proto_ipv6_header_psum(uint16_t payload_len, uint8_t pr= otocol, > > > > > + struct in6_addr saddr, struct in6_addr daddr) =20 > > > >=20 > > > > Hrm, this is passing 2 16-byte IPv6 addresses by value, which might > > > > not be what we want. =20 > > >=20 > > > The idea here is to avoid the pointer alignment problem (&ip6h->saddr= and > > > &ip6h->daddr can be misaligned). =20 > >=20 > > Ah, right. That's a neat idea, but I'm not sure it really helps: I > > think it will just move the misaligned access from inside the function > > to the call site, where we try to marshal the parameter from something > > unaligned. >=20 > I haven't tested this yet, but note that this is generally okay: the > problem is *dereferencing* an unaligned pointer. But if you load memory > from an aligned pointer, and extract a value from this memory, it's all > fine. Right, that's kind of what I'm getting at. Assuming this value starts in an unaligned buffer, then in order to pass this by value the caller will need to load from that unaligned pointer. AFAIK, the compiler will base the type of loads only on the pointed to type, which isn't changed whether we dereference in the caller or the callee. >=20 > Speaking MIPS, this is not safe on all CPU models: >=20 > la $1, 1002 # s1 now contains the value 1002 > lw $2, 0($1) # load word from memory at 1002 + 0 into s2 >=20 > but this is: >=20 > la $1, 1000 # s1 now contains the value 1000 > la $2, 1004 # s3 now contains the value 1004 > lw $3, 0($1) # load word from memory at 1000 + 0 into s3 > lw $4, 0($3) # load word from memory at 1004 + 0 into s4 > sll $5, $3, 16 # 16-bit shift left s3 into s5 > srl $6, $4, 16 # 16-bit shift right s4 into s6 > or $2, $5, $6 # OR s5 and s6 into s2 Right, but I don't think merely moving the dereference to the caller will necessarily induce the compiler to generate this rather than the former. > On x86, as far as I know, mov will digest the equivalent of the first > version without issues. >=20 > > > Is it a better solution to copy the content of ip6h->saddr and ip6h->= daddr > > > to some local variables and then provide the pointers of the local va= riables > > > to proto_ipv6_header_psum()? =20 > >=20 > > Honestly, I'm not sure. >=20 > I think it's pretty much the same. Let the compiler pass 16-byte > variables by value, and it will generally do this for us, but only if > needed. >=20 --=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 --38kXi3If9Qihq7d7 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXgROMACgkQzQJF27ox 2GeCRBAAkBnZFdS/631aSYT23QWFHHfQo2oP379VV2MqTTaSEmkb97q/gkYVclnV EHFvyj8po36P06NKE4JN2G/9vRfmTycyM7WF+oBSu0v4r0AxI68wfr3ZUQ8QTB8/ vsDYQfnYDQB6BldhG375aQEPj9je6OE1gkgFoCi9u1IWOrfFFH/evgaZGt2wgFH2 PrRg035kRH3EYrizSvuBFFZFm4kqFkZpZo9pOBd0hX1yE02uumrcGZW5f+NeWYgG w+sCxNEKP0HRv6tHtPnuzIgdJOjTnQW2gmm8T8S/s5Ij9uBqxIy9FgAyDqeYYAW+ iOQLy/plWKZQEvlp9cQm4bPdZ3Axa6nPexprZE+icPRGr0jg2u9fMUv962YV6es6 Bv9bMoy5s69rEWq4Hd8hIQd5gnKjO2d8MFQELJRmUPkGk+y5roQNey945H8VgF3p Le0sohdgaGtTHRt2Hjqu+h5jZhP2CiUCZ+vwFD/tgHtVjJiCEm65FnrAbteNlanF u2QemJhNafPv3Uhd5K/jpMKrU5gUjo5IPg4OPT//13BKiUsqi4cCq+cZT9VvmjGM 5tr6EpfkzbgyAZk48W0rX5VlavN49xksyZUMZJI4IcVjSel9jLiobdJHErOW8UQw BZAQLyv6ANRmURs0G7pZeu6BApcfcBBau1dSf7jgSzcbeuiO3Ps= =JRah -----END PGP SIGNATURE----- --38kXi3If9Qihq7d7--