Hi
On 13.10.23 23:05, brian m. carlson wrote:
On 2023-10-12 at 20:04:47, Feiyang Xue wrote:
Looking for comments on if this is a good idea to add SHA-NI option to git,
And if so, what'd be a good implementation.
The attached patch is more of a proof of concept, using a sha-ni example
found on internet and have it tapped into git's "hash-ll.h" when it sees make
flags "SHA1_SHANI=1" and/or "SHA256_SHANI=1" for sha1/sha256 respectively.
"git hash-object" is about 3-ish times faster for me
You almost certainly don't want to use SHA-1 using native instructions
because it doesn't detect collisions, unlike the default implementation
(SHA-1-DC). Since SHA-1 collisions are relatively cheap (less than USD
45,000) to make, not detecting collisions would be a security issue
worthy of a CVE.
It's fine for SHA-256, but see below.
There are few topics that i'm uncertain about.
1. Is it good idea to have machine-specific asm codes in git. I read there was
commit fd1ec82547 which removed assembly implementation for PPC_SHA1. Does that
imply maintainers doesn't want machine-specific assembly in source?
I'd prefer we didn't.
Nettle has SHA-NI extensions on systems that support it, and so does
OpenSSL. OpenSSL can't be used by distros for licensing reasons, but
Nettle can, and both of those libraries automatically detect the best
code to use based on the chip. One of those libraries is almost always
going to be linked into Git at some point because of libcurl anyway.
If we ship the code, then someone has to maintain it, and I don't know
if we have any assembly experts. We might also have a bug that produced
incorrect results, which would be pretty disastrous for the affected
users. It's much better for maintainability if we push that off onto
the cryptographic library maintainers who are much more competent in
that regard than I am (I will not speak for others) and let distro
maintainers simply link the appropriate library, which, as I said above,
they're already effectively doing.
In addition to all the valid points brian makes, the patch uses a
non-standard assembler (yasm, see http://yasm.tortall.net/) that's not
part of the default GNU toolchain.
Cheers, Beat
