maintaining CRYPTO_TIMING

[Mark Johnston]

On Fri, Jun 12, 2020 at 12:46:27PM -0700, John Baldwin wrote:
> On 6/12/20 9:14 AM, Mark Johnston wrote:
> > Hi,
> > 
> > I noticed that the opencrypto framework maintains counters for various
> > operations.  These counters are all global and are updated
> > non-atomically, so they aren't SMP-friendly and won't be precise.  I
> > wrote a patch to convert them to counter(9), which fixes both issues,
> > and I note that kern.crypto_stats was renamed to kern.crypto.stats in
> > HEAD so presumably we can use this opportunity to break the sysctl ABI
> > as well (the counters have to be widened from 32 bits to 64 bits).
> > Nothing in the base system seems to actually fetch these counters
> > outside of some code under tools/, which wasn't updated when the sysctl
> > was renamed.
> 
> I think this is fine.  I'd probably not oppose just removing the stats
> outright though.  Not clear to me how useful they really are (I've never
> used them, always used dtrace or other stats that are more specific)

I think the current stats are not particularly useful, but I could
imagine adding stats that might be.  For instance, a counter for failed
CRYPTO_OP_VERIFY_DIGEST operations, or for operations handled by
software drivers vs. hardware drivers.  I noticed while testing GELI
with a WIP driver that I had forgotten to implement AES-XTS+HMAC, so the
tests were causing GELI to fall back to a software implementation.
Counters could perhaps make it easier to spot cases where a consumer is
unexpectedly using an unaccelerated implementation.  Or is there already
an easy way to spot that?

> > There is also CRYPTO_TIMING, which attempts to measure the time elapsed
> > during various stages of cryptop processing.  This similarly assumes
> > that processing is single-threaded and I guess is really only useful to
> > OCF driver developers.  It has been in the tree for a very long time,
> > but has anyone actually used it?  I would like to remove it since it
> > complicates the above-mentioned patch and is of limited usefulness in
> > SMP systems.  DTrace or some per-cryptop scheme could be used instead if
> > it is really worth having that functionality, but I don't want to write
> > a patch to implement that unless someone really wants it.
> 
> This can go.  I do use DTrace a fair bit when debugging driver issues, but
> I haven't used any of this stuff at all.  For real profiling pmcstat is a
> better tool.

Ok, thanks.