From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <konstantin.ananyev@intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
 by dpdk.org (Postfix) with ESMTP id 6DABA1B199
 for <dev@dpdk.org>; Thu,  5 Oct 2017 18:31:11 +0200 (CEST)
Received: from fmsmga005.fm.intel.com ([10.253.24.32])
 by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
 05 Oct 2017 09:31:10 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.42,481,1500966000"; d="scan'208";a="159276444"
Received: from irsmsx107.ger.corp.intel.com ([163.33.3.99])
 by fmsmga005.fm.intel.com with ESMTP; 05 Oct 2017 09:31:08 -0700
Received: from irsmsx103.ger.corp.intel.com ([169.254.3.49]) by
 IRSMSX107.ger.corp.intel.com ([169.254.10.65]) with mapi id 14.03.0319.002;
 Thu, 5 Oct 2017 17:30:12 +0100
From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: Akhil Goyal <akhil.goyal@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>
CC: "Doherty, Declan" <declan.doherty@intel.com>, "De Lara Guarch, Pablo"
 <pablo.de.lara.guarch@intel.com>, "hemant.agrawal@nxp.com"
 <hemant.agrawal@nxp.com>, "Nicolau, Radu" <radu.nicolau@intel.com>,
 "borisp@mellanox.com" <borisp@mellanox.com>, "aviadye@mellanox.com"
 <aviadye@mellanox.com>, "thomas@monjalon.net" <thomas@monjalon.net>,
 "sandeep.malik@nxp.com" <sandeep.malik@nxp.com>,
 "jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>,
 "Mcnamara, John" <john.mcnamara@intel.com>, "olivier.matz@6wind.com"
 <olivier.matz@6wind.com>
Thread-Topic: [dpdk-dev] [PATCH v2 01/12] lib/rte_security: add security
 library
Thread-Index: AQHTPEn53qjhnXpVP0WZoPtrHXJms6LVb6AA
Date: Thu, 5 Oct 2017 16:30:11 +0000
Message-ID: <2601191342CEEE43887BDE71AB9772585FAA4E58@IRSMSX103.ger.corp.intel.com>
References: <20170914082651.26232-1-akhil.goyal@nxp.com>
 <20171003131413.23846-1-akhil.goyal@nxp.com>
 <20171003131413.23846-2-akhil.goyal@nxp.com>
In-Reply-To: <20171003131413.23846-2-akhil.goyal@nxp.com>
Accept-Language: en-IE, en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYTNjZTRlMjUtZjU3Ny00MjI5LWFhZGYtMmJkMDI5NDEwMmE2IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6Ild4N3I2V0I3Zzk4cUI2S3ZYSTdvVmhWbDZHY3MwVHN2VlZnend5dlZuUHc9In0=
x-ctpclassification: CTP_IC
dlp-product: dlpe-windows
dlp-version: 11.0.0.116
dlp-reaction: no-action
x-originating-ip: [163.33.239.181]
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Subject: Re: [dpdk-dev] [PATCH v2 01/12] lib/rte_security: add security
 library
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 05 Oct 2017 16:31:12 -0000

Hi lads,

>=20
> rte_security library provides APIs for security session
> create/free for protocol offload or offloaded crypto
> operation to ethernet device.
>=20
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>  lib/librte_security/Makefile                 |  53 +++
>  lib/librte_security/rte_security.c           | 275 +++++++++++++++
>  lib/librte_security/rte_security.h           | 495 +++++++++++++++++++++=
++++++
>  lib/librte_security/rte_security_driver.h    | 182 ++++++++++
>  lib/librte_security/rte_security_version.map |  13 +
>  5 files changed, 1018 insertions(+)
>  create mode 100644 lib/librte_security/Makefile
>  create mode 100644 lib/librte_security/rte_security.c
>  create mode 100644 lib/librte_security/rte_security.h
>  create mode 100644 lib/librte_security/rte_security_driver.h
>  create mode 100644 lib/librte_security/rte_security_version.map
>=20
> diff --git a/lib/librte_security/Makefile b/lib/librte_security/Makefile
> new file mode 100644
> index 0000000..af87bb2
> --- /dev/null
> +++ b/lib/librte_security/Makefile
> @@ -0,0 +1,53 @@
> +#   BSD LICENSE
> +#
> +#   Copyright(c) 2017 Intel Corporation. All rights reserved.
> +#
> +#   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 copyrigh=
t
> +#       notice, this list of conditions and the following disclaimer in
> +#       the documentation and/or other materials provided with the
> +#       distribution.
> +#     * Neither the name of Intel Corporation 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 FO=
R
> +#   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 AN=
Y
> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE US=
E
> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +# library name
> +LIB =3D librte_security.a
> +
> +# library version
> +LIBABIVER :=3D 1
> +
> +# build flags
> +CFLAGS +=3D -O3
> +CFLAGS +=3D $(WERROR_FLAGS)
> +
> +# library source files
> +SRCS-y +=3D rte_security.c
> +
> +# export include files
> +SYMLINK-y-include +=3D rte_security.h
> +SYMLINK-y-include +=3D rte_security_driver.h
> +
> +# versioning export map
> +EXPORT_MAP :=3D rte_security_version.map
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
> diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte=
_security.c
> new file mode 100644
> index 0000000..97d3857
> --- /dev/null
> +++ b/lib/librte_security/rte_security.c
> @@ -0,0 +1,275 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright 2017 NXP.
> + *   Copyright(c) 2017 Intel Corporation. All rights reserved.
> + *
> + *   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 copyrig=
ht
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of NXP 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 F=
OR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGH=
T
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTA=
L,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF US=
E,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON A=
NY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE U=
SE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE=
.
> + */
> +
> +#include <rte_malloc.h>
> +#include <rte_dev.h>
> +
> +#include "rte_security.h"
> +#include "rte_security_driver.h"
> +
> +#define RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ	(8)
> +
> +struct rte_security_ctx {
> +	uint16_t id;
> +	enum {
> +		RTE_SECURITY_INSTANCE_INVALID,
> +		RTE_SECURITY_INSTANCE_VALID
> +	} state;
> +	void *device;
> +	struct rte_security_ops *ops;
> +	uint16_t sess_cnt;
> +};
> +
> +static struct rte_security_ctx *security_instances;
> +static uint16_t max_nb_security_instances;
> +static uint16_t nb_security_instances;

Probably a dumb question - but why do you need a global security_instances =
[]
and why security_instance has to be refrenced by index?
As I understand, with proposed model all drivers have to do something like:
rte_security_register(&eth_dev->data->sec_id,  (void *)eth_dev, &ixgbe_secu=
rity_ops); =20
and then all apps would have to:
rte_eth_dev_get_sec_id(portid)
to retrieve that security_instance index.
Why not just treat struct rte_security_ctx* as opaque pointer and make
all related API get/accept it as a paratemer.=20
To retrieve sec_ctx from device just:
struct rte_security_ctx* rte_ethdev_get_sec_ctx(portid);
struct rte_security_ctx* rte_cryptodev_get_sec_ctx(portid);
?

Another question how currently proposed model with global static array and =
friends,
supposed to work for DPDK MP model?=20
Or MP support is not planned?

> +
> +static int
> +rte_security_is_valid_id(uint16_t id)
> +{
> +	if (id >=3D nb_security_instances ||
> +	    (security_instances[id].state !=3D RTE_SECURITY_INSTANCE_VALID))
> +		return 0;
> +	else
> +		return 1;
> +}
> +
> +/* Macros to check for valid id */
> +#define RTE_SEC_VALID_ID_OR_ERR_RET(id, retval) do { \
> +	if (!rte_security_is_valid_id(id)) { \
> +		RTE_PMD_DEBUG_TRACE("Invalid sec_id=3D%d\n", id); \
> +		return retval; \
> +	} \
> +} while (0)
> +
> +#define RTE_SEC_VALID_ID_OR_RET(id) do { \
> +	if (!rte_security_is_valid_id(id)) { \
> +		RTE_PMD_DEBUG_TRACE("Invalid sec_id=3D%d\n", id); \
> +		return; \
> +	} \
> +} while (0)
> +
> +int
> +rte_security_register(uint16_t *id, void *device,
> +		      struct rte_security_ops *ops)
> +{
> +	if (max_nb_security_instances =3D=3D 0) {
> +		security_instances =3D rte_malloc(
> +				"rte_security_instances_ops",
> +				sizeof(*security_instances) *
> +				RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ, 0);
> +
> +		if (security_instances =3D=3D NULL)
> +			return -ENOMEM;
> +		max_nb_security_instances =3D
> +				RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ;
> +	} else if (nb_security_instances >=3D max_nb_security_instances) {

You probably need try to reuse unregistered entries first?=20
Konstantin


> +		uint16_t *instances =3D rte_realloc(security_instances,
> +				sizeof(struct rte_security_ops *) *
> +				(max_nb_security_instances +
> +				RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ), 0);
> +
> +		if (instances =3D=3D NULL)
> +			return -ENOMEM;
> +
> +		max_nb_security_instances +=3D
> +				RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ;
> +	}
> +
> +	*id =3D nb_security_instances++;
> +
> +	security_instances[*id].id =3D *id;
> +	security_instances[*id].state =3D RTE_SECURITY_INSTANCE_VALID;
> +	security_instances[*id].device =3D device;
> +	security_instances[*id].ops =3D ops;
> +	security_instances[*id].sess_cnt =3D 0;
> +
> +	return 0;
> +}
> +
> +int
> +rte_security_unregister(uint16_t id)
> +{
> +	struct rte_security_ctx *instance;
> +
> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, -ENODEV);
> +	instance =3D &security_instances[id];
> +
> +	if (instance->sess_cnt)
> +		return -EBUSY;
> +
> +	memset(instance, 0, sizeof(*instance));
> +	return 0;
> +}
> +
> +struct rte_security_session *
> +rte_security_session_create(uint16_t id,
> +			    struct rte_security_session_conf *conf,
> +			    struct rte_mempool *mp)
> +{
> +	struct rte_security_ctx *instance;
> +	struct rte_security_session *sess =3D NULL;
> +
> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, NULL);
> +	instance =3D &security_instances[id];
> +
> +	if (conf =3D=3D NULL)
> +		return NULL;
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->session_create, NULL);
> +
> +	if (rte_mempool_get(mp, (void *)&sess))
> +		return NULL;
> +
> +	if (instance->ops->session_create(instance->device, conf, sess, mp)) {
> +		rte_mempool_put(mp, (void *)sess);
> +		return NULL;
> +	}
> +	instance->sess_cnt++;
> +
> +	return sess;
> +}
> +
> +int
> +rte_security_session_update(uint16_t id,
> +			    struct rte_security_session *sess,
> +			    struct rte_security_session_conf *conf)
> +{
> +	struct rte_security_ctx *instance;
> +
> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, -ENODEV);
> +	instance =3D &security_instances[id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->session_update, -ENOTSUP);
> +	return instance->ops->session_update(instance->device, sess, conf);
> +}
> +
> +int
> +rte_security_session_stats_get(uint16_t id,
> +			       struct rte_security_session *sess,
> +			       struct rte_security_stats *stats)
> +{
> +	struct rte_security_ctx *instance;
> +
> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, -ENODEV);
> +	instance =3D &security_instances[id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->session_stats_get, -ENOTSUP);
> +	return instance->ops->session_stats_get(instance->device, sess, stats);
> +}
> +
> +int
> +rte_security_session_destroy(uint16_t id, struct rte_security_session *s=
ess)
> +{
> +	int ret;
> +	struct rte_security_ctx *instance;
> +	struct rte_mempool *mp =3D rte_mempool_from_obj(sess);
> +
> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, -ENODEV);
> +	instance =3D &security_instances[id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->session_destroy, -ENOTSUP);
> +
> +	if (instance->sess_cnt)
> +		instance->sess_cnt--;
> +
> +	ret =3D instance->ops->session_destroy(instance->device, sess);
> +	if (!ret)
> +		rte_mempool_put(mp, (void *)sess);
> +
> +	return ret;
> +}
> +
> +int
> +rte_security_set_pkt_metadata(uint16_t id,
> +			      struct rte_security_session *sess,
> +			      struct rte_mbuf *m, void *params)
> +{
> +	struct rte_security_ctx *instance;
> +
> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, -ENODEV);
> +	instance =3D &security_instances[id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->set_pkt_metadata, -ENOTSUP);
> +	return instance->ops->set_pkt_metadata(instance->device,
> +					       sess, m, params);
> +}
> +
> +const struct rte_security_capability *
> +rte_security_capabilities_get(uint16_t id)
> +{
> +	struct rte_security_ctx *instance;
> +
> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, NULL);
> +	instance =3D &security_instances[id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->capabilities_get, NULL);
> +	return instance->ops->capabilities_get(instance->device);
> +}
> +
> +const struct rte_security_capability *
> +rte_security_capability_get(uint16_t id,
> +			    struct rte_security_capability_idx *idx)
> +{
> +	struct rte_security_ctx *instance;
> +	const struct rte_security_capability *capabilities;
> +	const struct rte_security_capability *capability;
> +	uint16_t i =3D 0;
> +
> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, NULL);
> +	instance =3D &security_instances[id];
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->capabilities_get, NULL);
> +	capabilities =3D instance->ops->capabilities_get(instance->device);
> +
> +	if (capabilities =3D=3D NULL)
> +		return NULL;
> +
> +	while ((capability =3D &capabilities[i++])->action
> +			!=3D RTE_SECURITY_ACTION_TYPE_NONE) {
> +		if (capability->action  =3D=3D idx->action &&
> +				capability->protocol =3D=3D idx->protocol) {
> +			if (idx->protocol =3D=3D RTE_SECURITY_PROTOCOL_IPSEC) {
> +				if (capability->ipsec.proto =3D=3D
> +						idx->ipsec.proto &&
> +					capability->ipsec.mode =3D=3D
> +							idx->ipsec.mode &&
> +					capability->ipsec.direction =3D=3D
> +							idx->ipsec.direction)
> +					return capability;
> +			}
> +		}
> +	}
> +
> +	return NULL;
> +}