Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1555219 > unrolled thread

Re: [PATCH v8 1/1] crypto: add virtio-crypto driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v8 1/1] crypto: add virtio-crypto driver Christian Borntraeger <borntraeger@de.ibm.com> - 2017-01-10 13:30 +0100
    RE: [PATCH v8 1/1] crypto: add virtio-crypto driver "Gonglei (Arei)" <arei.gonglei@huawei.com> - 2017-01-10 13:40 +0100
      Re: [PATCH v8 1/1] crypto: add virtio-crypto driver Christian Borntraeger <borntraeger@de.ibm.com> - 2017-01-10 14:00 +0100
        Re: [PATCH v8 1/1] crypto: add virtio-crypto driver Christian Borntraeger <borntraeger@de.ibm.com> - 2017-01-12 15:20 +0100
          Re: [PATCH v8 1/1] crypto: add virtio-crypto driver "Michael S. Tsirkin" <mst@redhat.com> - 2017-01-12 15:30 +0100
            RE: [PATCH v8 1/1] crypto: add virtio-crypto driver "Gonglei (Arei)" <arei.gonglei@huawei.com> - 2017-01-13 03:00 +0100

#1555219 — Re: [PATCH v8 1/1] crypto: add virtio-crypto driver

On 12/15/2016 03:03 AM, Gonglei wrote:
[...]
> +
> +static struct crypto_alg virtio_crypto_algs[] = { {
> +	.cra_name = "cbc(aes)",
> +	.cra_driver_name = "virtio_crypto_aes_cbc",
> +	.cra_priority = 501,


This is still higher than the hardware-accelerators (like intel aesni or the
s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported by the
hardware virtualization and available to the guests. I do not see a way how virtio
crypto can be faster than that (in the end it might be cpacf/aesni + overhead)
instead it will very likely be slower.
So we should use a number that is higher than software implementations but
lower than the hw ones.

Just grepping around, the software ones seem be be around 100 and the hardware 
ones around 200-400. So why was 150 not enough?

Christian

[toc] | [next] | [standalone]

#1555224

Hi,

> 
> On 12/15/2016 03:03 AM, Gonglei wrote:
> [...]
> > +
> > +static struct crypto_alg virtio_crypto_algs[] = { {
> > +	.cra_name = "cbc(aes)",
> > +	.cra_driver_name = "virtio_crypto_aes_cbc",
> > +	.cra_priority = 501,
> 
> 
> This is still higher than the hardware-accelerators (like intel aesni or the
> s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported by the
> hardware virtualization and available to the guests. I do not see a way how
> virtio
> crypto can be faster than that (in the end it might be cpacf/aesni + overhead)
> instead it will very likely be slower.
> So we should use a number that is higher than software implementations but
> lower than the hw ones.
> 
> Just grepping around, the software ones seem be be around 100 and the
> hardware
> ones around 200-400. So why was 150 not enough?
> 
I didn't find a documentation about how we use the priority, and I assumed
people use virtio-crypto will configure hardware accelerators in the
host. So I choosed the number which bigger than aesni's priority.

Regards,
-Gonglei

[toc] | [prev] | [next] | [standalone]

#1555232

On 01/10/2017 01:36 PM, Gonglei (Arei) wrote:
> Hi,
> 
>>
>> On 12/15/2016 03:03 AM, Gonglei wrote:
>> [...]
>>> +
>>> +static struct crypto_alg virtio_crypto_algs[] = { {
>>> +	.cra_name = "cbc(aes)",
>>> +	.cra_driver_name = "virtio_crypto_aes_cbc",
>>> +	.cra_priority = 501,
>>
>>
>> This is still higher than the hardware-accelerators (like intel aesni or the
>> s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported by the
>> hardware virtualization and available to the guests. I do not see a way how
>> virtio
>> crypto can be faster than that (in the end it might be cpacf/aesni + overhead)
>> instead it will very likely be slower.
>> So we should use a number that is higher than software implementations but
>> lower than the hw ones.
>>
>> Just grepping around, the software ones seem be be around 100 and the
>> hardware
>> ones around 200-400. So why was 150 not enough?
>>
> I didn't find a documentation about how we use the priority, and I assumed
> people use virtio-crypto will configure hardware accelerators in the
> host. So I choosed the number which bigger than aesni's priority.

Yes, but the aesni driver will only bind if there is HW support in the guest.
And if aesni is available in the guest (or the s390 aes function from cpacf)
it will always be faster than the same in the host via virtio.So your priority
should be smaller.

[toc] | [prev] | [next] | [standalone]

#1557475

On 01/10/2017 01:56 PM, Christian Borntraeger wrote:
> On 01/10/2017 01:36 PM, Gonglei (Arei) wrote:
>> Hi,
>>
>>>
>>> On 12/15/2016 03:03 AM, Gonglei wrote:
>>> [...]
>>>> +
>>>> +static struct crypto_alg virtio_crypto_algs[] = { {
>>>> +	.cra_name = "cbc(aes)",
>>>> +	.cra_driver_name = "virtio_crypto_aes_cbc",
>>>> +	.cra_priority = 501,
>>>
>>>
>>> This is still higher than the hardware-accelerators (like intel aesni or the
>>> s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported by the
>>> hardware virtualization and available to the guests. I do not see a way how
>>> virtio
>>> crypto can be faster than that (in the end it might be cpacf/aesni + overhead)
>>> instead it will very likely be slower.
>>> So we should use a number that is higher than software implementations but
>>> lower than the hw ones.
>>>
>>> Just grepping around, the software ones seem be be around 100 and the
>>> hardware
>>> ones around 200-400. So why was 150 not enough?
>>>
>> I didn't find a documentation about how we use the priority, and I assumed
>> people use virtio-crypto will configure hardware accelerators in the
>> host. So I choosed the number which bigger than aesni's priority.
> 
> Yes, but the aesni driver will only bind if there is HW support in the guest.
> And if aesni is available in the guest (or the s390 aes function from cpacf)
> it will always be faster than the same in the host via virtio.So your priority
> should be smaller.


any opinion on this? 

[toc] | [prev] | [next] | [standalone]

#1557480

On Thu, Jan 12, 2017 at 03:10:25PM +0100, Christian Borntraeger wrote:
> On 01/10/2017 01:56 PM, Christian Borntraeger wrote:
> > On 01/10/2017 01:36 PM, Gonglei (Arei) wrote:
> >> Hi,
> >>
> >>>
> >>> On 12/15/2016 03:03 AM, Gonglei wrote:
> >>> [...]
> >>>> +
> >>>> +static struct crypto_alg virtio_crypto_algs[] = { {
> >>>> +	.cra_name = "cbc(aes)",
> >>>> +	.cra_driver_name = "virtio_crypto_aes_cbc",
> >>>> +	.cra_priority = 501,
> >>>
> >>>
> >>> This is still higher than the hardware-accelerators (like intel aesni or the
> >>> s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported by the
> >>> hardware virtualization and available to the guests. I do not see a way how
> >>> virtio
> >>> crypto can be faster than that (in the end it might be cpacf/aesni + overhead)
> >>> instead it will very likely be slower.
> >>> So we should use a number that is higher than software implementations but
> >>> lower than the hw ones.
> >>>
> >>> Just grepping around, the software ones seem be be around 100 and the
> >>> hardware
> >>> ones around 200-400. So why was 150 not enough?
> >>>
> >> I didn't find a documentation about how we use the priority, and I assumed
> >> people use virtio-crypto will configure hardware accelerators in the
> >> host. So I choosed the number which bigger than aesni's priority.
> > 
> > Yes, but the aesni driver will only bind if there is HW support in the guest.
> > And if aesni is available in the guest (or the s390 aes function from cpacf)
> > it will always be faster than the same in the host via virtio.So your priority
> > should be smaller.
> 
> 
> any opinion on this? 

Going forward, we might add an emulated aesni device and that might
become slower than virtio. OTOH if or when this happens, we can solve it
by adding a priority or a feature flag to virtio to raise its priority.

So I think I agree with Christian here, let's lower the priority.
Gonglei, could you send a patch like this?

-- 
MST

[toc] | [prev] | [next] | [standalone]

#1557959

> 
> On Thu, Jan 12, 2017 at 03:10:25PM +0100, Christian Borntraeger wrote:
> > On 01/10/2017 01:56 PM, Christian Borntraeger wrote:
> > > On 01/10/2017 01:36 PM, Gonglei (Arei) wrote:
> > >> Hi,
> > >>
> > >>>
> > >>> On 12/15/2016 03:03 AM, Gonglei wrote:
> > >>> [...]
> > >>>> +
> > >>>> +static struct crypto_alg virtio_crypto_algs[] = { {
> > >>>> +	.cra_name = "cbc(aes)",
> > >>>> +	.cra_driver_name = "virtio_crypto_aes_cbc",
> > >>>> +	.cra_priority = 501,
> > >>>
> > >>>
> > >>> This is still higher than the hardware-accelerators (like intel aesni or the
> > >>> s390 cpacf functions or the arm hw). aesni and s390/cpacf are supported
> by the
> > >>> hardware virtualization and available to the guests. I do not see a way
> how
> > >>> virtio
> > >>> crypto can be faster than that (in the end it might be cpacf/aesni +
> overhead)
> > >>> instead it will very likely be slower.
> > >>> So we should use a number that is higher than software implementations
> but
> > >>> lower than the hw ones.
> > >>>
> > >>> Just grepping around, the software ones seem be be around 100 and the
> > >>> hardware
> > >>> ones around 200-400. So why was 150 not enough?
> > >>>
> > >> I didn't find a documentation about how we use the priority, and I assumed
> > >> people use virtio-crypto will configure hardware accelerators in the
> > >> host. So I choosed the number which bigger than aesni's priority.
> > >
> > > Yes, but the aesni driver will only bind if there is HW support in the guest.
> > > And if aesni is available in the guest (or the s390 aes function from cpacf)
> > > it will always be faster than the same in the host via virtio.So your priority
> > > should be smaller.
> >
> >
> > any opinion on this?
> 
> Going forward, we might add an emulated aesni device and that might
> become slower than virtio. OTOH if or when this happens, we can solve it
> by adding a priority or a feature flag to virtio to raise its priority.
> 
> So I think I agree with Christian here, let's lower the priority.
> Gonglei, could you send a patch like this?
> 
OK, will do.

Thanks,
-Gonglei

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web