Re: [Qemu-devel] [RFC v1 1/1] virtio-crypto: Allow disabling of cipher algorithms for virtio-crypto device

[Daniel P . Berrangé]

On Wed, Jun 13, 2018 at 07:28:08PM +0200, Halil Pasic wrote:
> 
> 
> On 06/13/2018 05:05 PM, Daniel P. Berrangé wrote:
> > On Wed, Jun 13, 2018 at 11:01:05AM -0400, Farhan Ali wrote:
> > > Hi Daniel
> > > 
> > > On 06/13/2018 05:37 AM, Daniel P. Berrangé wrote:
> > > > On Tue, Jun 12, 2018 at 03:48:34PM -0400, Farhan Ali wrote:
> > > > > The virtio-crypto driver currently propagates to the guest
> > > > > all the cipher algorithms that the backend cryptodev can
> > > > > support. But in certain cases where the guest has more
> > > > > performant mechanism to handle some algorithms, it would be
> > > > > useful to propagate only a subset of the algorithms.
> > > > 
> > > > I'm not really convinced by this.
> > > > 
> > > > The performance of crypto algorithms has many influencing
> > > > factors, making it pretty hard to decide which is best
> > > > without actively testing specific impls and comparing
> > > > them in a manner which matches the application usage
> > > > pattern. eg in theory the kernel crypto impl of an alg
> > > > is faster than a userspace impl, if the kernel uses
> > > > hardware accel and userspace does not. This, however,
> > > > ignores the overhead of the kernel/userspace switch.
> > > > The real world performance winner, thus depends on the
> > > > amount of data being processed in each operation. Some
> > > > times userspace can win & sometimes kernel space can
> > > > win. This is even more relevant to virtio-crypto as
> > > > it has more expensive context switches.
> > > 
> > > True. But what if the guest can perform some crypto algorithms without a
> > > incurring a VM exit? For example in s390 we have the cpacf instructions to
> > > perform crypto and this instruction is implemented for us by our hardware
> > > virtualization technology. In such a case it would be better not to use
> > > virtio-crypto's implementation of such a crypto algorithm.
> > > 
> > > At the same time we would like to take advantage of virtio-crypto's
> > > acceleration capabilities for certain crypto algorithms for which there is
> > > no hardware assistance.
> > 
> > IIUC, the kernel's crypto layer can support multiple implementations of
> > any algorithm. Providers can report a priority against implementations
> > which influences which impl is used in practice. So if there's a native
> > instruction for a partiuclar algorithm I would expect the impl registered
> > for that to be designated higher priority than other impls, so that it is
> > used in preference to other impls.
> > 
> 
> AFAIR the problem here is that in (the guest) kernel the virtio-crypto
> driver has to register it's crypto algo implementations with a priority
> (single number), which dictates if it's going to be the preferred (used)
> implementation of the algorithm or not. The virtio-crypto driver does this
> without having information about the (comparative or absolute) performance
> of it's implementation (which depends on the backend among others). I don't 
> think
> any dynamic re-prioritization of the algorithms takes place (e.g. based on 
> how these
> perform in for the given configuration).
> 
> I think the strategy of the virtio-crypto is to rather overstate, than
> understate the performance of it's implementation. If we were to 'be
> conservative' and say, 'hey we don't know nothing about the performance,
> let's make it lowest priority implementation' the implementations provided
> by virtio-crypto would end up being used only if there is no other
> implementation. And that does not sound like a good idea either.


This problem you describe, however, is something that applies to *any*
kerenl code that is registering a crypto algo impl for accelerator
hardware. A non-virtualized crypto cards in bare metal likewise cannot
assume that its AES impl is better then the host CPU's  aes-ni instruction.

> So the idea is to give the user the power to effectively not provide
> an algorithm via virtio-crypto. That is, if the user observes a performance
> degradation because of virtio-crypto, he can turn off the bad algorithms
> at the device. That way overstatement becomes a much smaller problem.
> The user can turn off the bad algorithms for reasons other than performance
> too.
> 
> Of course there are other ways to deal with the problem of virtio-crypto
> driver not knowing how good it's implementation of a given algo is. We
> could make the in kernel crypto priorities dynamically adjustable in general
> or we could provide the user with means to specify the priorities (e.g.
> as module parameter) with which the virtio-crypto driver registers each algo.
> Both of these would be knobs in the guest. It's hard to tell if these first
> one would be useful in scenarios not involving virtualization. Same goes
> for some kind of dynamic priority management for crypto algorithm 
> implementations
> in the Linux kernel. I assume the people involved with the respective
> subsystem do not see the necessity for something like that.

It still feels like this is a problem for the guest OS to solve. If you
put a physical crypto accelerator in a bare metal machine, that has the
same problem you describe here, so the kernel surely already needs to find
a viable solution for this problem. 


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|