Repository navigation
crypto: offload OpenSSL SHA-1 compression to SG2002 - #509
dormancygrace wants to merge 11 commits into
Conversation
dormancygrace
left a comment
dormancygrace
left a comment
There was a problem hiding this comment.
I found nothing specific to this PR. I checked:
- the wrapper placement in
sha_local.h, for both the normal and the small-footprint definitions; - the BE state conversion;
- that state is committed only after completed chunks;
- the 5-word state.
The CI gap from #508 applies here as well: the OpenSSL hunks are never applied or compiled in CI. The hardware run is still pending.
Build outputsCompared with the build of main at 2571baa; codex/openssl-sha256 is at 359ab55, so outputs changed since then are listed too. 4 changed, 159 added, 145 removed of 584 outputs.
|
dormancygrace
left a comment
dormancygrace
left a comment
There was a problem hiding this comment.
The code is correct, and the 8 KiB threshold fits the three measured points. My question is whether this patch should ship at all. On the Go side, SRTP HMAC-SHA1 runs in internal/sha1mb and does not use OpenSSL.
| + size_t completed = 0; | ||
| + int fd; | ||
| + | ||
| + if (blocks < 2 * DMA_BLOCKS) |
There was a problem hiding this comment.
Which OpenSSL consumer on the device hashes more than 8 KiB per update with SHA-1? The gain exists only above 8 KiB per call: 1.2× at 8 KiB and 1.65× at 16 KiB. The cost is that the patch wraps upstream's block function in sha_local.h, which has to be requalified at every OpenSSL bump.
The 8 KiB crossover comes mostly from the transport's fixed cost per call. By the rows in the README, that is about 66 µs, against about 26 µs per 4 KiB request. Unless there is a named consumer, I would hold this patch until the #507 per-call cost is reduced, then re-measure. The crossover should drop to about 4 KiB (my estimate).
| Native correctness/counters, software candidate comparison, size threshold, | ||
| RAM and concurrent-video measurements remain pending. | ||
|
|
||
| Native C906: 864 digest EVP cases passed; hardware counters confirmed SHA-1 submissions. Three rotated trials found a 4 KiB call slower with DMA (42.54 versus 54.01 MiB/CPU-s), while 8 KiB reached 68.14 versus 55.44 and 16 KiB 92.55 versus 56.14. The minimum is therefore 8 KiB; each submitted request remains 4 KiB. The separate GNU adapter of the Go scalar core passed correctness but achieved only 41.78 MiB/CPU-s at 16 KiB, so upstream OpenSSL software is retained. GCC already emits C906 rotates and byte swaps for it. |
There was a problem hiding this comment.
Please commit the raw trials, and add 64 B / 1 KiB / 4 KiB rows with DMA disabled against the #508 head. That shows the new wrapper in the common SHA-1 path costs small hashes nothing (#504 notes code placement alone moves results by up to 5%). Line 11, the comment at patch line 11 and the PR description still say 4 KiB, while the code says 8 KiB. The threshold is also written twice (blocks < 2 * DMA_BLOCKS and num >= 128).
# Conflicts: # firmware/crypto/openssl-c906/README.md
Add the separate SHA-1 OpenSSL patch using the SG2002 direct ioctl transport. OpenSSL keeps initialization, streaming, padding, digest output and its original software implementation. Successful compression chunks update the five chaining words; any remaining input resumes in software after hardware absence or failure. Both normal and small-footprint code paths are covered, with no digest-context growth and no FIPS-module change.
Validation: full OpenSSL 3.5.9 RISC-V cross-build without warnings; 960 ASan/UBSan fault-continuation cases covering every input offset, first/later failures and unchanged buffer/length fields. CI runs these tests. The shared native digest verifier includes SHA-1 and checks expected outputs independently derived from Python hashlib and host OpenSSL.
Draft: physical C906 correctness/counters, comparison against our Go scalar SHA-1, crossover threshold, RAM and concurrent-video measurements are pending. The 4 KiB minimum is an initial candidate. Stacked after #508; SHA-1 remains its own patch and review. No ENGINE or cryptodev dependency.