On Mon, Jul 3, 2023 at 8:48 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > Hi folks, really sorry for the big delay, this thread really slipped my mind :) > IMO the best API for CRC's is like zlib's where you pass in 0 to start the CRC > and it does both the pre and post inversions for you. Note, "updates" still > work as expected, since two inversions cancel each other out. I agree, I did that when adding CRC32c to EFI. u32 calculate_crc32c(const void *buf, size_t len, u32 initial) with inversions on initial and the result is pretty simple and effective. > Unfortunately, many but not all of the CRC APIs in Linux decided to go with the > other convention, which is to leave the inversions entirely to the caller. > > I think the kernel should also make the architecture-specific CRC > implementations accessible directly via a library API, similar to what's done > for Blake2s and ChaCha20. There should be no need to go through shash at all... > > > > > This misuse could be fixed, but you'd have to burn an incompat flag to > > do it. I'm less smart about crc32* than I was back in 2008, so I also > > don't have the skills to figure out if the correction is worth the cost. > > > > --D > > No, it's not worth changing the ext4 on-disk format for this. I don't think we'd need to change the on-disk format for this? Or for any other hash algorithm change (as long as the resulting digest is 32-bit), right? Given we have s_checksum_type. Or do existing tools dangerously assume CRC32c at the moment? In any case, thank you both for the background on this, I'll try to submit a patch to the docs to clarify this point. -- Pedro
