On Tue, 16 Jun 2026 14:41:58 +0530 Shreesh Adiga <[email protected]> wrote:
> Add a 64-byte loop that maintains 4 fold registers and processes > 64 bytes at a time. The 4x fold registers is then reduced to 16 byte > single fold, similar to x86 SSE implementation. This technique is > described in the paper by Intel: > "Fast CRC Computation for Generic Polynomials Using PCLMULQDQ Instruction" > > This results in roughly 2x performance improvement due to better ILP > for large input sizes like 1024 observed on Cortex-X925. > > Signed-off-by: Shreesh Adiga <[email protected]> > --- Detailed AI review (not the CI one), spotted a correctness issue. On the 4x folding loop patch: The 4x-fold port matches the x86 SSE implementation and the constant tables are correct, but the patch introduces one correctness bug. Error: lib/net/net_crc_neon.c, "17 to 31 bytes" path uses the wrong fold constant. This patch repurposes rk1_rk2 as the fold-by-4 (512-bit) constant and moves the fold-by-1 (128-bit) constant into the new rk3_rk4, matching the SSE layout. The main paths were updated to select rk3_rk4 before falling into partial_bytes, and the partial_bytes comment was correctly updated to "k = rk3 & rk4". The 17-to-31 byte branch was missed. It still does: /* 17 to 31 bytes */ fold = vld1q_u64((const uint64_t *)data); fold = veorq_u64(fold, temp); n = 16; k = params->rk1_rk2; /* now the fold-by-4 constant */ goto partial_bytes; partial_bytes performs a single 128-bit fold and needs the fold-by-1 constant, but this path now feeds it rk1_rk2, which after the change holds the 512-bit constant. CRC results are therefore wrong for every input of length 17-31 bytes. This affects both crc32_eth and crc16_ccitt, since they share this routine. The fix is one line -- use rk3_rk4 here as the other paths do: k = params->rk3_rk4; Verification: I cross-compiled the routine (armv8-a+crypto) and ran it under qemu against a scalar reflected CRC-32 reference for lengths 1-256. As submitted it mismatches at exactly lengths 17-31 (15 lengths); with the one-line change above, all lengths pass. The >=64, >=32, ==16, and <16 paths are already correct. One suggestion for v2, not required: the SSE version does not carry a separate 17-31 branch at all -- it handles everything below 64 through the single_fold_loop plus partial_bytes with rk3_rk4. Collapsing the NEON path the same way would remove this class of bug rather than just this instance.
