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

[Halil Pasic <pasic@linux.ibm.com>]

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.

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.

I hope, this clarifies the idea behind this patch.

Regards,
Halil