[dpdk-dev] [PATCH v3 3/8] crypto/armv8: add PMD optimized for ARMv8 processors

Jianbo Liu jianbo.liu at linaro.org
Fri Jan 13 08:41:38 CET 2017


On 12 January 2017 at 21:12, Zbigniew Bodek
<zbigniew.bodek at caviumnetworks.com> wrote:
> Hello  Jianbo Liu,
>
> Thanks for the review. Please check my answers in-line.
>
> Kind regards
> Zbigniew
>
>
> On 06.01.2017 03:45, Jianbo Liu wrote:
>>
>> On 5 January 2017 at 01:33,  <zbigniew.bodek at caviumnetworks.com> wrote:
>>>
>>> From: Zbigniew Bodek <zbigniew.bodek at caviumnetworks.com>
>>>
>>> This patch introduces crypto poll mode driver
>>> using ARMv8 cryptographic extensions.
>>> CPU compatibility with this driver is detected in
>>> run-time and virtual crypto device will not be
>>> created if CPU doesn't provide:
>>> AES, SHA1, SHA2 and NEON.
>>>
>>> This PMD is optimized to provide performance boost
>>> for chained crypto operations processing,
>>> such as encryption + HMAC generation,
>>> decryption + HMAC validation. In particular,
>>> cipher only or hash only operations are
>>> not provided.
>>>
>>> The driver currently supports AES-128-CBC
>>> in combination with: SHA256 HMAC and SHA1 HMAC
>>> and relies on the external armv8_crypto library:
>>> https://github.com/caviumnetworks/armv8_crypto
>>>
>>
>> It's standalone lib. I think you should change the following line in
>> its Makefile, so not depend on DPDK.
>> "include $(RTE_SDK)/mk/rte.lib.mk"
>>
>>> This patch adds driver's code only and does
>>> not include it in the build system.
>>>
>>> Signed-off-by: Zbigniew Bodek <zbigniew.bodek at caviumnetworks.com>
>>> ---
>>>  drivers/crypto/armv8/Makefile                  |  73 ++
>>>  drivers/crypto/armv8/rte_armv8_pmd.c           | 926
>>> +++++++++++++++++++++++++
>>>  drivers/crypto/armv8/rte_armv8_pmd_ops.c       | 369 ++++++++++
>>>  drivers/crypto/armv8/rte_armv8_pmd_private.h   | 211 ++++++
>>>  drivers/crypto/armv8/rte_armv8_pmd_version.map |   3 +
>>>  5 files changed, 1582 insertions(+)
>>>  create mode 100644 drivers/crypto/armv8/Makefile
>>>  create mode 100644 drivers/crypto/armv8/rte_armv8_pmd.c
>>>  create mode 100644 drivers/crypto/armv8/rte_armv8_pmd_ops.c
>>>  create mode 100644 drivers/crypto/armv8/rte_armv8_pmd_private.h
>>>  create mode 100644 drivers/crypto/armv8/rte_armv8_pmd_version.map
>>>
.....

>>> +       /* Select auth algo */
>>> +       switch (auth_xform->auth.algo) {
>>> +       /* Cover supported hash algorithms */
>>> +       case RTE_CRYPTO_AUTH_SHA256:
>>> +               aalg = auth_xform->auth.algo;
>>> +               sess->auth.mode = ARMV8_CRYPTO_AUTH_AS_AUTH;
>>> +               break;
>>> +       case RTE_CRYPTO_AUTH_SHA1_HMAC:
>>> +       case RTE_CRYPTO_AUTH_SHA256_HMAC: /* Fall through */
>>> +               aalg = auth_xform->auth.algo;
>>> +               sess->auth.mode = ARMV8_CRYPTO_AUTH_AS_HMAC;
>>> +               break;
>>> +       default:
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       /* Verify supported key lengths and extract proper algorithm */
>>> +       switch (cipher_xform->cipher.key.length << 3) {
>>> +       case 128:
>>> +               sess->crypto_func =
>>> +                               CRYPTO_GET_ALGO(order, cop, calg, aalg,
>>> 128);
>>> +               sess->cipher.key_sched =
>>> +                               CRYPTO_GET_KEY_SCHED(cop, calg, 128);
>>> +               break;
>>> +       case 192:
>>> +               sess->crypto_func =
>>> +                               CRYPTO_GET_ALGO(order, cop, calg, aalg,
>>> 192);
>>> +               sess->cipher.key_sched =
>>> +                               CRYPTO_GET_KEY_SCHED(cop, calg, 192);
>>> +               break;
>>> +       case 256:
>>> +               sess->crypto_func =
>>> +                               CRYPTO_GET_ALGO(order, cop, calg, aalg,
>>> 256);
>>> +               sess->cipher.key_sched =
>>> +                               CRYPTO_GET_KEY_SCHED(cop, calg, 256);
>>> +               break;
>>> +       default:
>>> +               sess->crypto_func = NULL;
>>> +               sess->cipher.key_sched = NULL;
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       if (unlikely(sess->crypto_func == NULL)) {
>>> +               /*
>>> +                * If we got here that means that there must be a bug
>>
>>
>> Since AES-128-CBC is only supported in your patch. It means that
>> crypto_func could be NULL according to the switch above if
>> cipher.key.length > 128?
>
>
> Yes. Instead of checking for key lengths in a similar way that we check for
> algorithms, etc. we just fail when we don't find appropriate function. Do
> you suggest that this should be changed?
>

I mean to return error directly if length is not 128 in the above
switch, so this "if" is no necessary.

>
>>
>>> +                * in the algorithms selection above. Nevertheless keep
>>> +                * it here to catch bug immediately and avoid NULL
>>> pointer
>>> +                * dereference in OPs processing.
>>> +                */
>>> +               ARMV8_CRYPTO_LOG_ERR(
>>> +                       "No appropriate crypto function for given
>>> parameters");
>>> +               return -EINVAL;
>>> +       }
>>> +
>>> +       /* Set up cipher session prerequisites */
>>> +       if (cipher_set_prerequisites(sess, cipher_xform) != 0)
>>> +               return -EINVAL;
>>> +
>>> +       /* Set up authentication session prerequisites */
>>> +       if (auth_set_prerequisites(sess, auth_xform) != 0)
>>> +               return -EINVAL;
>>> +
>>> +       return 0;
>>> +}
>>> +
>>
>>
>> ....
>>
>>> diff --git a/drivers/crypto/armv8/rte_armv8_pmd_ops.c
>>> b/drivers/crypto/armv8/rte_armv8_pmd_ops.c
>>> new file mode 100644
>>> index 0000000..2bf6475
>>> --- /dev/null
>>> +++ b/drivers/crypto/armv8/rte_armv8_pmd_ops.c
>>> @@ -0,0 +1,369 @@
>>> +/*
>>> + *   BSD LICENSE
>>> + *
>>> + *   Copyright (C) Cavium networks Ltd. 2017.
>>> + *
>>> + *   Redistribution and use in source and binary forms, with or without
>>> + *   modification, are permitted provided that the following conditions
>>> + *   are met:
>>> + *
>>> + *     * Redistributions of source code must retain the above copyright
>>> + *       notice, this list of conditions and the following disclaimer.
>>> + *     * Redistributions in binary form must reproduce the above
>>> copyright
>>> + *       notice, this list of conditions and the following disclaimer in
>>> + *       the documentation and/or other materials provided with the
>>> + *       distribution.
>>> + *     * Neither the name of Cavium networks nor the names of its
>>> + *       contributors may be used to endorse or promote products derived
>>> + *       from this software without specific prior written permission.
>>> + *
>>> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>>> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>>> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
>>> FOR
>>> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
>>> COPYRIGHT
>>> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
>>> INCIDENTAL,
>>> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>>> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
>>> USE,
>>> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
>>> ANY
>>> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>>> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
>>> USE
>>> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
>>> DAMAGE.
>>> + */
>>> +
>>> +#include <string.h>
>>> +
>>> +#include <rte_common.h>
>>> +#include <rte_malloc.h>
>>> +#include <rte_cryptodev_pmd.h>
>>> +
>>> +#include "armv8_crypto_defs.h"
>>> +
>>> +#include "rte_armv8_pmd_private.h"
>>> +
>>> +static const struct rte_cryptodev_capabilities
>>> +       armv8_crypto_pmd_capabilities[] = {
>>> +       {       /* SHA1 HMAC */
>>> +               .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
>>> +                       {.sym = {
>>> +                               .xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
>>> +                               {.auth = {
>>> +                                       .algo =
>>> RTE_CRYPTO_AUTH_SHA1_HMAC,
>>> +                                       .block_size = 64,
>>> +                                       .key_size = {
>>> +                                               .min = 16,
>>> +                                               .max = 128,
>>> +                                               .increment = 0
>>> +                                       },
>>> +                                       .digest_size = {
>>> +                                               .min = 20,
>>> +                                               .max = 20,
>>> +                                               .increment = 0
>>> +                                       },
>>> +                                       .aad_size = { 0 }
>>> +                               }, }
>>> +                       }, }
>>> +       },
>>> +       {       /* SHA256 HMAC */
>>> +               .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
>>> +                       {.sym = {
>>> +                               .xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
>>> +                               {.auth = {
>>> +                                       .algo =
>>> RTE_CRYPTO_AUTH_SHA256_HMAC,
>>> +                                       .block_size = 64,
>>> +                                       .key_size = {
>>> +                                               .min = 16,
>>> +                                               .max = 128,
>>> +                                               .increment = 0
>>> +                                       },
>>> +                                       .digest_size = {
>>> +                                               .min = 32,
>>> +                                               .max = 32,
>>> +                                               .increment = 0
>>> +                                       },
>>> +                                       .aad_size = { 0 }
>>> +                               }, }
>>> +                       }, }
>>> +       },
>>> +       {       /* AES CBC */
>>> +               .op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
>>> +                       {.sym = {
>>> +                               .xform_type =
>>> RTE_CRYPTO_SYM_XFORM_CIPHER,
>>> +                               {.cipher = {
>>> +                                       .algo =
>>> RTE_CRYPTO_CIPHER_AES_CBC,
>>> +                                       .block_size = 16,
>>> +                                       .key_size = {
>>> +                                               .min = 16,
>>> +                                               .max = 16,
>>> +                                               .increment = 0
>>> +                                       },
>>> +                                       .iv_size = {
>>> +                                               .min = 16,
>>> +                                               .max = 16,
>>> +                                               .increment = 0
>>> +                                       }
>>> +                               }, }
>>> +                       }, }
>>> +       },
>>> +
>>
>>
>> It's strange that you defined aes and hmac here, but not implemented
>> them, though their combinations are implemented.
>> Will you add later?
>
>
> We may add standalone algorithms in the future but those ops here are not
> for that purpose. I thought that since there is no chained operations
> capability we should export what we can do even though that it will work
> (mean not return error) only if the operations are chained.
> Do you have some other suggestion?
>

Nothing special. Either implement them later, or add new chained ops
(is that possible?)
BTW, can you explain what optimization you have done, so I can better
understand your asm code, thanks!

>
>>
>>> +       RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
>>> +};
>>> +
>>> +


More information about the dev mailing list