icp_blake3_impl tunable broken

System information

Type	Version/Name
Distribution Name	any
Distribution Version	any
Kernel Version	any
Architecture	any
OpenZFS Version	master @ 1d3ba0b

Describe the problem you're observing

The icp_blake3_impl module parameter is updated incorrectly at run time, so only "cycle" and "fastest" will reflect being selected.

Having looked at the code (I am in the process of exposing this tunable for FreeBSD), it seems the parameter will only be displayed correctly if it is set at module load time rather than at run time. There is some overly-complicated looking IMPL_PARAM/IMPL_USER distinction that is to blame. It should probably work a lot more like in zfs_fletcher_4_impl in module/zcommon/zfs_fletcher.c.

Describe how to reproduce the problem

# cat /sys/module/zfs/parameters/icp_blake3_impl
cycle [fastest] generic sse2 sse41 %
# echo sse41 > /sys/module/zfs/parameters/icp_blake3_impl
# cat /sys/module/zfs/parameters/icp_blake3_impl
cycle fastest generic sse2 sse41 %
# echo fastest > /sys/module/zfs/parameters/icp_blake3_impl
# cat /sys/module/zfs/parameters/icp_blake3_impl
cycle [fastest] generic sse2 sse41 %

Include any warning/errors/backtraces from the system logs

[jumbi77]

ping @mcmilk (in case you are interested).

[rincebrain]

You know, I swear this worked when I tested it. (Of course, I believe that was before IMPL_PARAM/IMPL_USER was added...)

I don't recall seeing any discussion about it in the PR. Hopefully @mcmilk can clarify the problem this is solving, rather than someone just ripping it out and having to find out later...

[mcmilk]

I am on holiday currently. But I will fix this, when I am home again - in about one week.

Edit: I would also add some mini library for setting up more implementations... this can be re-used on sha, aes and other algs with multiple methods of implementations then.

In that case, it may be useful to incorporate or at least take into consideration the changes I've made here to support the tunable on FreeBSD: https://github.com/truenas/zfs/tree/blake3-tunable

In general FreeBSD doesn't use any icp code, except for edonr and now blake3, so the icp tunables not being implemented for FreeBSD hasn't been a problem until now.

By the way, this all came to my attention while investigating a report of BLAKE3 causing pool corruption:
https://lists.freebsd.org/archives/freebsd-current/2022-July/002281.html
In case you needed something to really make you appreciate that vacation :)

[mcmilk]

I am working on SHA2 SIMD code... I have to read more about the different ways of using the checksumming code in OpenZFS for Linux vs FreeBSD.
Maybe we can work together on this ... I will start a PR in the end of the next week I think.

[rincebrain]

I'm happy to take a crack at fixing the impl problem, I just wanted to understand what problem it was trying to solve before I just threw it out.

I also have a branch or two where I've shoved a dozen or so new checksums in as experiments with multiple implementations, so I would be willing to contribute to that as well.

[mcmilk]

The generic implementation interface is a bit more complex to do... so I will just fix this issue via extra PR... and then implement the SHA2 reworking.

Then, when the SHA2 stuff is in, we have these three algos within the ICP codebase which also get used bei FreeBSD:

• BLAKE3
• SHA2
• EdonR

Is the icp/illumos-crypto.c stuff of interest for FreeBSD OpenZFS?
In the codebase of illumos is ZFS very highly integrated, so I have no idea why we have some file called illumos-crypto.c with an ifdef __FreeBSD__ :-)

I will also reuse this here: https://github.com/truenas/zfs/tree/blake3-tunable - thanks for this beginning.

The ifdef __FreeBSD__ in icp/illumos-crypto.c is a mistake left by ad9e767 when ICP was welded into the zfs module for Linux. They apparently didn't realize FreeBSD does not use that file in the kernel.

[mcmilk]

@freqlabs - can this issue be closed?

[behlendorf]

Yes, I believe it can. Resolved by #13725.