[dpdk-dev] [PATCH 01/11] lib/rte_security: add security library
Akhil Goyal
akhil.goyal at nxp.com
Wed Sep 20 13:35:52 CEST 2017
Hi Hemant,
On 9/15/2017 11:02 AM, Hemant Agrawal wrote:
> Hi,
>
> On 9/14/2017 1:56 PM, Akhil Goyal wrote:
> <snip>..
>
>> diff --git a/lib/librte_security/rte_security.c
>> b/lib/librte_security/rte_security.c
>> new file mode 100644
>> index 0000000..5776246
>> --- /dev/null
>> +++ b/lib/librte_security/rte_security.c
>> @@ -0,0 +1,252 @@
>> +/*-
>> + * 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 = 0,
>> + RTE_SECURITY_INSTANCE_VALID
>> + } state;
>> + void *device;
>> + struct rte_security_ops *ops;
>> +};
>> +
>> +static struct rte_security_ctx *security_instances;
>> +static uint16_t max_nb_security_instances;
>> +static uint16_t nb_security_instances;
>> +
>> +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) {
>> + uint16_t *instances = rte_realloc(security_instances,
>> + sizeof(struct rte_security_ops *) *
>> + (max_nb_security_instances +
>> + RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ), 0);
>
> I think "RTE_SECURITY_INSTANCES_BLOCK_ALLOC_SZ" value as 8 is relatively
> small. you may want to keep it "64" or more.
>
> you may change it into two parts
> - Initial block size and incremental block size for realloc.
>
> Also, do you want to make it a configurable variable. as some
> implementation may need really large number of SAs.
Security Instances are not per SA, these are per eth/crypto device which
support security offload.
>
>> +
>> + 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;
>> +
>> + return 0;
>> +}
>> +
>> +int
>> +rte_security_unregister(__rte_unused uint16_t *id)
>> +{
>> + /* To be implemented */
>> + 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;
>> +
>> + if (rte_mempool_get(mp, (void *)&sess))
>> + return NULL;
>> +
>> + RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->session_create, NULL);
>
> it will leak the sess memory, if returned on error.
ok I will fix this.
>
>> + if (instance->ops->session_create(instance->device, conf, sess,
>> mp)) {
>> + rte_mempool_put(mp, (void *)sess);
>> + return NULL;
>> + }
>
> can the mempool operations be part of session_create api?
No, this is used for struct rte_security_session. session_create() would
take another object for its private data which it would free in the
session_destroy() in the driver.
>
> it will be similar to destroy, which is expected to free the 'sess'
> object to mempool?
rte_security_session_destroy should free the mempool object used for
struct rte_security_session in the rte_security_session_create
I will fix this in the next version.
>
>> + return sess;
>> +}
>> +
>
> <snip>..
>
>> +struct rte_security_ipsec_xform {
>> + uint32_t spi;
>> + /**< SA security parameter index */
>> + uint32_t salt;
>> + /**< SA salt */
>> + struct rte_security_ipsec_sa_options options;
>> + /**< various SA options */
>> + enum rte_security_ipsec_sa_direction direction;
>> + /**< IPSec SA Direction - Egress/Ingress */
>> + enum rte_security_ipsec_sa_protocol proto;
>> + /**< IPsec SA Protocol - AH/ESP */
>> + enum rte_security_ipsec_sa_mode mode;
>> + /**< IPsec SA Mode - transport/tunnel */
>> + struct rte_security_ipsec_tunnel_param tunnel;
>> + /**< Tunnel parameters, NULL for transport mode */
>> +};
>> +
>> +/**
>> + * MACsec security session configuration
>> + */
>> +struct rte_security_macsec_xform {
>> + /** To be Filled */
>> +};
>> +
>> +/**
>> + * Security session action type.
>> + */
>> +enum rte_security_session_action_type {
>> + RTE_SECURITY_ACTION_TYPE_NONE,
>> + /**< No security actions */
>
> This is not being used, it seems that you are only using it as marker to
> indicate end of capability set?
Yes.
>
>> + RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO,
>> + /**< Crypto processing for security protocol is processed inline
>> + * during transmission */
>> + RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL,
>> + /**< All security protocol processing is performed inline during
>> + * transmission */
>> + RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL
>> + /**< All security protocol processing including crypto is performed
>> + * on a lookaside accelerator */
>> +};
>> +
>> +/** Security session protocol definition */
>> +enum rte_security_session_protocol {
>> + RTE_SECURITY_PROTOCOL_IPSEC,
>> + /**< IPsec Protocol */
>> + RTE_SECURITY_PROTOCOL_MACSEC,
>> + /**< MACSec Protocol */
>> +};
>> +
>> +/**
>> + * Security session configuration
>> + */
>> +struct rte_security_session_conf {
>> + enum rte_security_session_action_type action_type;
>> + /**< Type of action to be performed on the session */
>> + enum rte_security_session_protocol protocol;
>> + /**< Security protocol to be configured */
>> + union {
>> + struct rte_security_ipsec_xform ipsec;
>> + struct rte_security_macsec_xform macsec;
>> + };
>> + /**< Configuration parameters for security session */
>> + struct rte_crypto_sym_xform *crypto_xform;
>> + /**< Security Session Crypto Transformations */
>> +};
>> +
>> +struct rte_security_session {
>> + __extension__ void *sess_private_data;
>> + /**< Private session material */
>> +};
>> +
>
>
> Do you need specific error handling for security sessions as well?
> In case of full protocol offloads, you will need indications for
> 1. SEQ number overflow (egress side, if the SA is not refreshed on time)
> 2. Anti replay window config and err handlings?
>
This is in our TODO list for future.
>
>> +/**
>> + * Create security session as specified by the session configuration
>> + *
>> + * @param id security instance identifier id
>> + * @param conf session configuration parameters
>
> fix the indentation here and other places in this file.
ok.
>
>> + * @param mp mempool to allocate session objects from
>> + * @return
>> + * - On success, pointer to session
>> + * - On failure, NULL
>> + */
>> +struct rte_security_session *
>> +rte_security_session_create(uint16_t id,
>> + struct rte_security_session_conf *conf,
>> + struct rte_mempool *mp);
>> +
>> +/**
>
Regards,
Akhil
More information about the dev
mailing list