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

[Daniel P . Berrangé]

On Fri, Jun 15, 2018 at 12:52:05AM +0000, Gonglei (Arei) wrote:
> 
> 
> > -----Original Message-----
> > From: Daniel P. Berrangé [mailto:address@hidden
> > Sent: Thursday, June 14, 2018 11:11 PM
> > To: Farhan Ali <address@hidden>
> > Cc: Halil Pasic <address@hidden>; address@hidden;
> > address@hidden; address@hidden; address@hidden; Gonglei
> > (Arei) <address@hidden>; longpeng <address@hidden>;
> > Viktor Mihajlovski <address@hidden>;
> > address@hidden
> > Subject: Re: [Qemu-devel] [RFC v1 1/1] virtio-crypto: Allow disabling of 
> > cipher
> > algorithms for virtio-crypto device
> > 
> > On Thu, Jun 14, 2018 at 10:50:40AM -0400, Farhan Ali wrote:
> > >
> > >
> > > On 06/14/2018 04:21 AM, Daniel P. Berrangé wrote:
> > > > 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.
> > > >
> > >
> > > How would the guest OS know which algo is better? As you mentioned it does
> > > depend on few factors and the best the kernel can do is use some sort of
> > > heuristics. Such a solution might not be very dynamic and might not work 
> > > for
> > > all the cases for a user.
> > 
> > Which is better will likely depend on the application using it. One might
> > be better for use by the kernel, while another is better for use by a
> > userspace application, or two userspace apps might have different
> > preferences.
> > 
> > > Shouldn't we use virtualization to give us the flexibility that we don't
> > > have with physical crypto accelerator? The crypto accelerator might not 
> > > know
> > > if it's implementation is any better, but the user can experiment and see
> > > what works better.
> > 
> > It is better to provide it all to the guest and let the guest decide which
> > is best to use.  If nothing else the virtio-crypto kernel module itself
> > can have module parameters to control the priority it gives to each
> > algorithm, or can avoid registering certain algorithms.  Doing it guest
> > side is more flexible, because realistically many virt host deployments
> > will never give the guest admin ability to control this from the host
> > side, so a guest kernel config ability will be the only thing available.
> > 
> 
> From the perspective of your communication and production deployment, 
> I tend to agree with Daniel’s point of view. AFAICT DPDK cryptodev scheduler
> PMD driver did this thing:
> 
> " Added a packet-size based distribution mode, which distributes the enqueued
> Crypto operations among two slaves, based on their data lengths."
> 
> Which it means that the guest's driver makes the decision.
> 
> Currently the Linux crypto framework uses the static priority of one algo to 
> decide 
> to choose what is simply. Maybe it should add a scheduler layer too?

The simplest option would probably be either a kernel module parameter,
or even better, sysfs tunables, to allow the priorities to be set
dynamically

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 :|