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.go...@nxp.com> Signed-off-by: Boris Pismenny <bor...@mellanox.com> Signed-off-by: Radu Nicolau <radu.nico...@intel.com> Signed-off-by: Declan Doherty <declan.dohe...@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(ð_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.
multi process case is planned for future enhancement. This is mentioned in the cover note.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?
These APIs are experimental at this moment as mentioned in the patchset. We will try accommodate your comments in future.+ +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
+ 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
