[dpdk-dev] [PATCH v2 01/12] lib/rte_security: add security library

Akhil Goyal akhil.goyal at nxp.com
Fri Oct 6 20:11:00 CEST 2017


Hi Konstantin,

Thanks for your comments.
On 10/5/2017 10:00 PM, Ananyev, Konstantin wrote:
> Hi lads,
> 
>>
>> rte_security library provides APIs for security session
>> create/free for protocol offload or offloaded crypto
>> operation to ethernet device.
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal at nxp.com>
>> Signed-off-by: Boris Pismenny <borisp at mellanox.com>
>> Signed-off-by: Radu Nicolau <radu.nicolau at intel.com>
>> Signed-off-by: Declan Doherty <declan.doherty at 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
>>
>> 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 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 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 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 $(RTE_SDK)/mk/rte.vars.mk
>> +
>> +# library name
>> +LIB = librte_security.a
>> +
>> +# library version
>> +LIBABIVER := 1
>> +
>> +# build flags
>> +CFLAGS += -O3
>> +CFLAGS += $(WERROR_FLAGS)
>> +
>> +# library source files
>> +SRCS-y += rte_security.c
>> +
>> +# export include files
>> +SYMLINK-y-include += rte_security.h
>> +SYMLINK-y-include += rte_security_driver.h
>> +
>> +# versioning export map
>> +EXPORT_MAP := 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 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 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 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 <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_security_ops);
> 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.
> 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);
> ?
We would look into this separately.
> 
> Another question how currently proposed model with global static array and friends,
> supposed to work for DPDK MP model?
> Or MP support is not planned?
multi process case is planned for future enhancement. This is mentioned 
in the cover note.
> 
>> +
>> +static int
>> +rte_security_is_valid_id(uint16_t id)
>> +{
>> +	if (id >= nb_security_instances ||
>> +	    (security_instances[id].state != 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=%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=%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 == 0) {
>> +		security_instances = rte_malloc(
>> +				"rte_security_instances_ops",
>> +				sizeof(*security_instances) *
>> +				RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ, 0);
>> +
>> +		if (security_instances == NULL)
>> +			return -ENOMEM;
>> +		max_nb_security_instances =
>> +				RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ;
>> +	} else if (nb_security_instances >= max_nb_security_instances) {
> 
> You probably need try to reuse unregistered entries first?
> Konstantin
> 
These APIs are experimental at this moment as mentioned in the patchset. 
We will try accommodate your comments in future.
> 
>> +		uint16_t *instances = rte_realloc(security_instances,
>> +				sizeof(struct rte_security_ops *) *
>> +				(max_nb_security_instances +
>> +				RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ), 0);
>> +
>> +		if (instances == NULL)
>> +			return -ENOMEM;
>> +
>> +		max_nb_security_instances +=
>> +				RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ;
>> +	}
>> +
>> +	*id = nb_security_instances++;
>> +
>> +	security_instances[*id].id = *id;
>> +	security_instances[*id].state = RTE_SECURITY_INSTANCE_VALID;
>> +	security_instances[*id].device = device;
>> +	security_instances[*id].ops = ops;
>> +	security_instances[*id].sess_cnt = 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 = &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 = NULL;
>> +
>> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, NULL);
>> +	instance = &security_instances[id];
>> +
>> +	if (conf == 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 = &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 = &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 *sess)
>> +{
>> +	int ret;
>> +	struct rte_security_ctx *instance;
>> +	struct rte_mempool *mp = rte_mempool_from_obj(sess);
>> +
>> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, -ENODEV);
>> +	instance = &security_instances[id];
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->session_destroy, -ENOTSUP);
>> +
>> +	if (instance->sess_cnt)
>> +		instance->sess_cnt--;
>> +
>> +	ret = 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 = &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 = &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 = 0;
>> +
>> +	RTE_SEC_VALID_ID_OR_ERR_RET(id, NULL);
>> +	instance = &security_instances[id];
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->capabilities_get, NULL);
>> +	capabilities = instance->ops->capabilities_get(instance->device);
>> +
>> +	if (capabilities == NULL)
>> +		return NULL;
>> +
>> +	while ((capability = &capabilities[i++])->action
>> +			!= RTE_SECURITY_ACTION_TYPE_NONE) {
>> +		if (capability->action  == idx->action &&
>> +				capability->protocol == idx->protocol) {
>> +			if (idx->protocol == RTE_SECURITY_PROTOCOL_IPSEC) {
>> +				if (capability->ipsec.proto ==
>> +						idx->ipsec.proto &&
>> +					capability->ipsec.mode ==
>> +							idx->ipsec.mode &&
>> +					capability->ipsec.direction ==
>> +							idx->ipsec.direction)
>> +					return capability;
>> +			}
>> +		}
>> +	}
>> +
>> +	return NULL;
>> +}
> 

Regards,
Akhil


More information about the dev mailing list