DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
From: Akhil Goyal <akhil.goyal@nxp.com>
To: Nagadheeraj Rottela <rnagadheeraj@marvell.com>,
	"pablo.de.lara.guarch@intel.com" <pablo.de.lara.guarch@intel.com>,
	"mattias.ronnblom@ericsson.com" <mattias.ronnblom@ericsson.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Srikanth Jampala <jsrikanth@marvell.com>
Subject: Re: [dpdk-dev] [PATCH v4 04/11] crypto/nitrox: add basic symmetric cryptodev operations
Date: Fri, 20 Sep 2019 09:44:57 +0000
Message-ID: <VE1PR04MB6639F56C81FB1F43239C9AEBE6880@VE1PR04MB6639.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <20190826124836.21187-5-rnagadheeraj@marvell.com>

Hi Nagadheeraj,

> Add the following cryptodev operations,
> - dev_configure
> - dev_start
> - dev_stop
> - dev_close
> - dev_infos_get
> 
> Signed-off-by: Nagadheeraj Rottela <rnagadheeraj@marvell.com>
> ---
>  doc/guides/cryptodevs/features/nitrox.ini       | 38 ++++++++++++
>  doc/guides/cryptodevs/nitrox.rst                | 37 +++++++++++
>  drivers/crypto/nitrox/Makefile                  |  1 +
>  drivers/crypto/nitrox/meson.build               |  1 +
>  drivers/crypto/nitrox/nitrox_sym.c              | 81 ++++++++++++++++++++++++-
>  drivers/crypto/nitrox/nitrox_sym_capabilities.c | 57 +++++++++++++++++
>  drivers/crypto/nitrox/nitrox_sym_capabilities.h | 12 ++++
>  7 files changed, 226 insertions(+), 1 deletion(-)
>  create mode 100644 doc/guides/cryptodevs/features/nitrox.ini
>  create mode 100644 drivers/crypto/nitrox/nitrox_sym_capabilities.c
>  create mode 100644 drivers/crypto/nitrox/nitrox_sym_capabilities.h
> 
> diff --git a/doc/guides/cryptodevs/features/nitrox.ini
> b/doc/guides/cryptodevs/features/nitrox.ini
> new file mode 100644
> index 000000000..9f9e2619c
> --- /dev/null
> +++ b/doc/guides/cryptodevs/features/nitrox.ini
> @@ -0,0 +1,38 @@
> +;
> +; Supported features of the 'nitrox' crypto driver.
> +;
> +; Refer to default.ini for the full list of available PMD features.
> +;
> +[Features]
> +Symmetric crypto       = Y
> +Sym operation chaining = Y
> +HW Accelerated         = Y
> +In Place SGL           = Y
> +OOP SGL In SGL Out     = Y
> +OOP SGL In LB  Out     = Y
> +OOP LB  In SGL Out     = Y
> +OOP LB  In LB  Out     = Y

I cannot see any of these features getting supporting in any of the patches before 4/11.
You should add documentation update where the feature is supported.

> +
> +;
> +; Supported crypto algorithms of the 'nitrox' crypto driver.
> +;
> +[Cipher]
> +AES CBC (128)  = Y
> +AES CBC (192)  = Y
> +AES CBC (256)  = Y
> +
> +;
> +; Supported authentication algorithms of the 'nitrox' crypto driver.
> +;
> +[Auth]
> +SHA1 HMAC    = Y
> +
> +;
> +; Supported AEAD algorithms of the 'nitrox' crypto driver.
> +;
> +[AEAD]
> +
> +;
> +; Supported Asymmetric algorithms of the 'nitrox' crypto driver.
> +;
> +[Asymmetric]
> diff --git a/doc/guides/cryptodevs/nitrox.rst b/doc/guides/cryptodevs/nitrox.rst
> index b6b86dda5..c16a5e393 100644
> --- a/doc/guides/cryptodevs/nitrox.rst
> +++ b/doc/guides/cryptodevs/nitrox.rst
> @@ -9,3 +9,40 @@ cryptographic operations to the NITROX V security
> processor. Detailed
>  information about the NITROX V security processor can be obtained here:
> 
>  *
> https://www.marvell.com/security-solutions/nitrox-security-processors/nitrox-v/ 
> +

I believe you should add documentation in some later patch for features in each of the patch
Which add a certain feature.
Generic documentation should be part of your first patch where you introduced the driver.

> +Features
> +--------
> +
> +Nitrox crypto PMD has support for:
> +
> +Cipher algorithms:
> +
> +* ``RTE_CRYPTO_CIPHER_AES_CBC``
> +
> +Hash algorithms:
> +
> +* ``RTE_CRYPTO_AUTH_SHA1_HMAC``
> +
> +Limitations
> +-----------
> +
> +* AES_CBC Cipher Only combination is not supported.
> +
> +Installation
> +------------
> +
> +For compiling the Nitrox crypto PMD, please check if the
> +CONFIG_RTE_LIBRTE_PMD_NITROX setting is set to `y` in
> config/common_base file.
> +
> +* ``CONFIG_RTE_LIBRTE_PMD_NITROX=y``
> +
> +Initialization
> +--------------
> +
> +Nitrox crypto PMD depend on Nitrox kernel PF driver being installed on the
> +platform. Nitrox PF driver is required to create VF devices which will
> +be used by the PMD. Each VF device can enable one cryptodev PMD.
> +
> +Nitrox kernel PF driver is available as part of CNN55XX-Driver SDK. The SDK
> +and it's installation instructions can be obtained from:
> +`Marvell Technical Documentation Portal
> < https://support.cavium.com/websilo/portal>`_.
> diff --git a/drivers/crypto/nitrox/Makefile b/drivers/crypto/nitrox/Makefile
> index 06c96ccd7..dedb74a34 100644
> --- a/drivers/crypto/nitrox/Makefile
> +++ b/drivers/crypto/nitrox/Makefile
> @@ -27,5 +27,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_NITROX) +=
> nitrox_device.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_NITROX) += nitrox_hal.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_NITROX) += nitrox_logs.c
>  SRCS-$(CONFIG_RTE_LIBRTE_PMD_NITROX) += nitrox_sym.c
> +SRCS-$(CONFIG_RTE_LIBRTE_PMD_NITROX) += nitrox_sym_capabilities.c
> 

Capabilities should be added in the patch where they are supported.

>  include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/drivers/crypto/nitrox/meson.build
> b/drivers/crypto/nitrox/meson.build
> index 1277cf58e..7c565c5a4 100644
> --- a/drivers/crypto/nitrox/meson.build
> +++ b/drivers/crypto/nitrox/meson.build
> @@ -13,4 +13,5 @@ sources = files(
>  		'nitrox_hal.c',
>  		'nitrox_logs.c',
>  		'nitrox_sym.c',
> +		'nitrox_sym_capabilities.c',
>  		)
> diff --git a/drivers/crypto/nitrox/nitrox_sym.c
> b/drivers/crypto/nitrox/nitrox_sym.c
> index c72016dd0..c05042e54 100644
> --- a/drivers/crypto/nitrox/nitrox_sym.c
> +++ b/drivers/crypto/nitrox/nitrox_sym.c
> @@ -9,6 +9,7 @@
> 
>  #include "nitrox_sym.h"
>  #include "nitrox_device.h"
> +#include "nitrox_sym_capabilities.h"
>  #include "nitrox_logs.h"
> 
>  #define CRYPTODEV_NAME_NITROX_PMD crypto_nitrox
> @@ -25,6 +26,84 @@ static const struct rte_driver nitrox_rte_sym_drv = {
>  	.alias = nitrox_sym_drv_name
>  };
> 
> +static int nitrox_sym_dev_qp_release(struct rte_cryptodev *cdev,
> +				     uint16_t qp_id);
> +
> +static int
> +nitrox_sym_dev_config(__rte_unused struct rte_cryptodev *cdev,
> +		      __rte_unused struct rte_cryptodev_config *config)
> +{
> +	return 0;
> +}
> +
> +static int
> +nitrox_sym_dev_start(__rte_unused struct rte_cryptodev *cdev)
> +{
> +	return 0;
> +}
> +
> +static void
> +nitrox_sym_dev_stop(__rte_unused struct rte_cryptodev *cdev)
> +{
> +}

Why are these functions empty? Atleast a comment can be added here.
I see that you have used __rte_unused, you can also use RTE_SET_USED.


> +
> +static int
> +nitrox_sym_dev_close(struct rte_cryptodev *cdev)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < cdev->data->nb_queue_pairs; i++) {
> +		ret = nitrox_sym_dev_qp_release(cdev, i);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void
> +nitrox_sym_dev_info_get(struct rte_cryptodev *cdev,
> +			struct rte_cryptodev_info *info)
> +{
> +	struct nitrox_sym_device *sym_dev = cdev->data->dev_private;
> +	struct nitrox_device *ndev = sym_dev->ndev;
> +
> +	if (!info)
> +		return;
> +
> +	info->max_nb_queue_pairs = ndev->nr_queues;
> +	info->feature_flags = cdev->feature_flags;
> +	info->capabilities = nitrox_get_sym_capabilities();
> +	info->driver_id = nitrox_sym_drv_id;
> +	info->sym.max_nb_sessions = 0;
> +}
> +
> +static int
> +nitrox_sym_dev_qp_release(struct rte_cryptodev *cdev, uint16_t qp_id)
> +{
> +	RTE_SET_USED(cdev);
> +	RTE_SET_USED(qp_id);
> +	return 0;
> +}
> +
> +static struct rte_cryptodev_ops nitrox_cryptodev_ops = {
> +	.dev_configure		= nitrox_sym_dev_config,
> +	.dev_start		= nitrox_sym_dev_start,
> +	.dev_stop		= nitrox_sym_dev_stop,
> +	.dev_close		= nitrox_sym_dev_close,
> +	.dev_infos_get		= nitrox_sym_dev_info_get,

I see that in this patch, none of the ops are really implemented except the dev_infos_get.
This patch may be squashed in your previous patch after removing the capabilities and documentation
Which again need to be squashed in some other patch.


> +
> +	.stats_get		= NULL,
> +	.stats_reset		= NULL,
> +
> +	.queue_pair_setup	= NULL,
> +	.queue_pair_release     = NULL,
> +
> +	.sym_session_get_size   = NULL,
> +	.sym_session_configure  = NULL,
> +	.sym_session_clear      = NULL
> +};
> +
>  int
>  nitrox_sym_pmd_create(struct nitrox_device *ndev)
>  {
> @@ -50,7 +129,7 @@ nitrox_sym_pmd_create(struct nitrox_device *ndev)
> 
>  	ndev->rte_sym_dev.name = cdev->data->name;
>  	cdev->driver_id = nitrox_sym_drv_id;
> -	cdev->dev_ops = NULL;
> +	cdev->dev_ops = &nitrox_cryptodev_ops;
>  	cdev->enqueue_burst = NULL;
>  	cdev->dequeue_burst = NULL;
>  	cdev->feature_flags = RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO |
> diff --git a/drivers/crypto/nitrox/nitrox_sym_capabilities.c
> b/drivers/crypto/nitrox/nitrox_sym_capabilities.c
> new file mode 100644
> index 000000000..aa1ff2638
> --- /dev/null
> +++ b/drivers/crypto/nitrox/nitrox_sym_capabilities.c
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2019 Marvell International Ltd.
> + */
> +
> +#include "nitrox_sym_capabilities.h"
> +
> +static const struct rte_cryptodev_capabilities nitrox_capabilities[] = {


Should not be part of this patch.

> +	{	/* 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 = 1,
> +					.max = 64,
> +					.increment = 1
> +				},
> +				.digest_size = {
> +					.min = 1,
> +					.max = 20,
> +					.increment = 1
> +				},
> +				.iv_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 = 32,
> +					.increment = 8
> +				},
> +				.iv_size = {
> +					.min = 16,
> +					.max = 16,
> +					.increment = 0
> +				}
> +			}, }
> +		}, }
> +	},
> +
> +	RTE_CRYPTODEV_END_OF_CAPABILITIES_LIST()
> +};
> +
> +const struct rte_cryptodev_capabilities *
> +nitrox_get_sym_capabilities(void)
> +{
> +	return nitrox_capabilities;
> +}
> diff --git a/drivers/crypto/nitrox/nitrox_sym_capabilities.h
> b/drivers/crypto/nitrox/nitrox_sym_capabilities.h
> new file mode 100644
> index 000000000..cb2d97572
> --- /dev/null
> +++ b/drivers/crypto/nitrox/nitrox_sym_capabilities.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2019 Marvell International Ltd.
> + */
> +
> +#ifndef _NITROX_SYM_CAPABILITIES_H_
> +#define _NITROX_SYM_CAPABILITIES_H_
> +
> +#include <rte_cryptodev.h>
> +
> +const struct rte_cryptodev_capabilities *nitrox_get_sym_capabilities(void);
> +
> +#endif /* _NITROX_SYM_CAPABILITIES_H_ */
> --
> 2.13.6


  reply index

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-17  5:29 [dpdk-dev] [PATCH 00/10] add Nitrox crypto device support Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 01/10] crypto/nitrox: add Nitrox build and doc skeleton Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 02/10] crypto/nitrox: add PCI probe and remove routines Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 03/10] crypto/nitrox: create Nitrox symmetric cryptodev Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 04/10] crypto/nitrox: add basic symmetric cryptodev operations Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 05/10] crypto/nitrox: add software queue management functionality Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 06/10] crypto/nitrox: add hardware " Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 07/10] crypto/nitrox: add session management operations Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 08/10] crypto/nitrox: add burst enqueue and dequeue operations Nagadheeraj Rottela
2019-07-17 14:16   ` Aaron Conole
2019-07-17  5:29 ` [dpdk-dev] [PATCH 09/10] crypto/nitrox: add cipher auth crypto chain processing Nagadheeraj Rottela
2019-07-17  5:29 ` [dpdk-dev] [PATCH 10/10] test/crypto: add tests for Nitrox PMD Nagadheeraj Rottela
2019-07-19 12:33 ` [dpdk-dev] [PATCH v2 00/10] add Nitrox crypto device support Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 01/10] crypto/nitrox: add Nitrox build and doc skeleton Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 02/10] crypto/nitrox: add PCI probe and remove routines Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 03/10] crypto/nitrox: create Nitrox symmetric cryptodev Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 04/10] crypto/nitrox: add basic symmetric cryptodev operations Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 05/10] crypto/nitrox: add software queue management functionality Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 06/10] crypto/nitrox: add hardware " Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 07/10] crypto/nitrox: add session management operations Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 08/10] crypto/nitrox: add burst enqueue and dequeue operations Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 09/10] crypto/nitrox: add cipher auth crypto chain processing Nagadheeraj Rottela
2019-07-19 12:33   ` [dpdk-dev] [PATCH v2 10/10] test/crypto: add tests for Nitrox PMD Nagadheeraj Rottela
2019-08-23 10:42 ` [dpdk-dev] [PATCH v3 00/11] add Nitrox crypto device support Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 01/11] crypto/nitrox: add Nitrox build and doc skeleton Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 02/11] crypto/nitrox: add PCI probe and remove routines Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 03/11] crypto/nitrox: create Nitrox symmetric cryptodev Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 04/11] crypto/nitrox: add basic symmetric cryptodev operations Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 05/11] crypto/nitrox: add software queue management functionality Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 06/11] crypto/nitrox: add hardware " Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 07/11] crypto/nitrox: add session management operations Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 08/11] crypto/nitrox: add burst enqueue and dequeue operations Nagadheeraj Rottela
2019-08-25 20:55     ` Mattias Rönnblom
2019-08-26 12:49       ` [dpdk-dev] [PATCH v4 00/11] add Nitrox crypto device support Nagadheeraj Rottela
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 01/11] crypto/nitrox: add Nitrox build and doc skeleton Nagadheeraj Rottela
2019-09-20  8:56           ` Akhil Goyal
2019-09-20  9:02             ` Akhil Goyal
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 02/11] crypto/nitrox: add PCI probe and remove routines Nagadheeraj Rottela
2019-09-20  9:15           ` Akhil Goyal
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 03/11] crypto/nitrox: create Nitrox symmetric cryptodev Nagadheeraj Rottela
2019-09-20  9:29           ` Akhil Goyal
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 04/11] crypto/nitrox: add basic symmetric cryptodev operations Nagadheeraj Rottela
2019-09-20  9:44           ` Akhil Goyal [this message]
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 05/11] crypto/nitrox: add software queue management functionality Nagadheeraj Rottela
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 06/11] crypto/nitrox: add hardware " Nagadheeraj Rottela
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 07/11] crypto/nitrox: add session management operations Nagadheeraj Rottela
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 08/11] crypto/nitrox: add burst enqueue and dequeue operations Nagadheeraj Rottela
2019-09-20 10:15           ` Akhil Goyal
2019-09-20 11:11             ` Nagadheeraj Rottela
2019-09-20 11:13               ` Akhil Goyal
2019-09-20 11:23                 ` Nagadheeraj Rottela
2019-09-20 11:25                   ` Akhil Goyal
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 09/11] crypto/nitrox: add cipher auth crypto chain processing Nagadheeraj Rottela
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 10/11] test/crypto: add tests for Nitrox PMD Nagadheeraj Rottela
2019-08-26 12:49         ` [dpdk-dev] [PATCH v4 11/11] crypto/nitrox: add SHA224 and SHA256 HMAC algorithms Nagadheeraj Rottela
2019-09-20  8:49           ` Akhil Goyal
2019-09-20 10:16             ` Akhil Goyal
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 09/11] crypto/nitrox: add cipher auth crypto chain processing Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 10/11] test/crypto: add tests for Nitrox PMD Nagadheeraj Rottela
2019-08-23 10:42   ` [dpdk-dev] [PATCH v3 11/11] crypto/nitrox: add SHA224 and SHA256 HMAC algorithms Nagadheeraj Rottela

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VE1PR04MB6639F56C81FB1F43239C9AEBE6880@VE1PR04MB6639.eurprd04.prod.outlook.com \
    --to=akhil.goyal@nxp.com \
    --cc=dev@dpdk.org \
    --cc=jsrikanth@marvell.com \
    --cc=mattias.ronnblom@ericsson.com \
    --cc=pablo.de.lara.guarch@intel.com \
    --cc=rnagadheeraj@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git