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

Boris Pismenny borisp at mellanox.com
Sun Sep 17 15:31:19 CEST 2017


Hi Hemant,

On 9/15/2017 8:33 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.
> 

Shouldn't the resize be double the original size to get the amortized O(1)?

> Also, do you want to make it a configurable variable. as some
> implementation may need really large number of SAs.
> 
> > +
> > +		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.
> 
> > +	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?
> 
> it will be similar to destroy, which is expected to free the 'sess'
> object to mempool?
> 
> > +	return sess;
> > +}
> > +
> 
> <snip>..
> 
> > diff --git a/lib/librte_security/rte_security.h
> > b/lib/librte_security/rte_security.h
> > new file mode 100644
> > index 0000000..2faac96
> > --- /dev/null
> > +++ b/lib/librte_security/rte_security.h
> > @@ -0,0 +1,494 @@
> > +/*-
> > + *   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.
> > + */
> > +
> > +#ifndef _RTE_SECURITY_H_
> > +#define _RTE_SECURITY_H_
> > +
> > +/**
> > + * @file rte_security.h
> > + *
> > + * RTE Security Common Definitions
> > + *
> > + */
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include <netinet/in.h>
> > +#include <netinet/ip.h>
> > +#include <netinet/ip6.h>
> > +
> > +#include <rte_mbuf.h>
> > +#include <rte_memory.h>
> > +#include <rte_mempool.h>
> > +#include <rte_common.h>
> > +#include <rte_crypto.h>
> > +
> > +/** IPSec protocol mode */
> > +enum rte_security_ipsec_sa_mode {
> > +	RTE_SECURITY_IPSEC_SA_MODE_TRANSPORT,
> > +	/**< IPSec Transport mode */
> > +	RTE_SECURITY_IPSEC_SA_MODE_TUNNEL,
> > +	/**< IPSec Tunnel mode */
> > +};
> > +
> > +/** IPSec Protocol */
> > +enum rte_security_ipsec_sa_protocol {
> > +	RTE_SECURITY_IPSEC_SA_PROTO_AH,
> > +	/**< AH protocol */
> > +	RTE_SECURITY_IPSEC_SA_PROTO_ESP,
> > +	/**< ESP protocol */
> > +};
> > +
> > +/** IPSEC tunnel type */
> > +enum rte_security_ipsec_tunnel_type {
> > +	RTE_SECURITY_IPSEC_TUNNEL_IPV4 = 0,
> > +	/**< Outer header is IPv4 */
> > +	RTE_SECURITY_IPSEC_TUNNEL_IPV6,
> > +	/**< Outer header is IPv6 */
> > +};
> > +
> > +/**
> > + * IPSEC tunnel parameters
> > + *
> > + * These parameters are used to build outbound tunnel headers.
> > + */
> > +struct rte_security_ipsec_tunnel_param {
> > +	enum rte_security_ipsec_tunnel_type type;
> > +	/**< Tunnel type: IPv4 or IPv6 */
> > +	union {
> > +		struct {
> > +			struct in_addr src_ip;
> > +			/**< IPv4 source address */
> > +			struct in_addr dst_ip;
> > +			/**< IPv4 destination address */
> > +			uint8_t dscp;
> > +			/**< IPv4 Differentiated Services Code Point */
> > +			uint8_t df;
> > +			/**< IPv4 Don't Fragment bit */
> > +			uint8_t ttl;
> > +			/**< IPv4 Time To Live */
> > +		} ipv4;
> > +		/**< IPv4 header parameters */
> > +		struct {
> > +			struct in6_addr src_addr;
> > +			/**< IPv6 source address */
> > +			struct in6_addr dst_addr;
> > +			/**< IPv6 destination address */
> > +			uint8_t dscp;
> > +			/**< IPv6 Differentiated Services Code Point */
> > +			uint32_t flabel;
> > +			/**< IPv6 flow label */
> > +			uint8_t hlimit;
> > +			/**< IPv6 hop limit */
> > +		} ipv6;
> > +		/**< IPv6 header parameters */
> > +	};
> > +};
> > +
> > +/**
> > + * IPsec Security Association option flags  */ struct
> > +rte_security_ipsec_sa_options {
> > +	/** Extended Sequence Numbers (ESN)
> > +	  *
> > +	  * * 1: Use extended (64 bit) sequence numbers
> > +	  * * 0: Use normal sequence numbers
> > +	  */
> > +	uint32_t esn : 1;
> > +
> > +	/** UDP encapsulation
> > +	  *
> > +	  * * 1: Do UDP encapsulation/decapsulation so that IPSEC packets
> can
> > +	  *      traverse through NAT boxes.
> > +	  * * 0: No UDP encapsulation
> > +	  */
> > +	uint32_t udp_encap : 1;
> > +
> > +	/** Copy DSCP bits
> > +	  *
> > +	  * * 1: Copy IPv4 or IPv6 DSCP bits from inner IP header to
> > +	  *      the outer IP header in encapsulation, and vice versa in
> > +	  *      decapsulation.
> > +	  * * 0: Use values from odp_ipsec_tunnel_param_t in encapsulation
> and
> > +	  *      do not change DSCP field in decapsulation.
> > +	  */
> > +	uint32_t copy_dscp : 1;
> > +
> > +	/** Copy IPv6 Flow Label
> > +	  *
> > +	  * * 1: Copy IPv6 flow label from inner IPv6 header to the
> > +	  *      outer IPv6 header.
> > +	  * * 0: Use value from odp_ipsec_tunnel_param_t
> > +	  */
> > +	uint32_t copy_flabel : 1;
> > +
> > +	/** Copy IPv4 Don't Fragment bit
> > +	  *
> > +	  * * 1: Copy the DF bit from the inner IPv4 header to the outer
> > +	  *      IPv4 header.
> > +	  * * 0: Use value from odp_ipsec_tunnel_param_t
> > +	  */
> > +	uint32_t copy_df : 1;
> > +
> > +	/** Decrement inner packet Time To Live (TTL) field
> > +	  *
> > +	  * * 1: In tunnel mode, decrement inner packet IPv4 TTL or
> > +	  *      IPv6 Hop Limit after tunnel decapsulation, or before tunnel
> > +	  *      encapsulation.
> > +	  * * 0: Inner packet is not modified.
> > +	  */
> > +	uint32_t dec_ttl : 1;
> > +
> > +	/** HW constructs/removes trailer of packets
> > +	  *
> > +	  * * 1: Transmitted packets will have the trailer added to them by
> > +	  *      hardawre. The next protocol field will be based on the
> > +	  *      mbuf->inner_esp_next_proto field.
> > +	  *      Received packets have no trailer, the next protocol field is
> > +	  *      supplied in the mbuf->inner_esp_next_proto field.
> > +	  * * 0: Inner packet is not modified.
> > +	  */
> > +	uint32_t no_trailer : 1;
> > +};
> > +
> > +/** IPSec security association direction */ enum
> > +rte_security_ipsec_sa_direction {
> > +	RTE_SECURITY_IPSEC_SA_DIR_EGRESS,
> > +	/**< Encrypt and generate digest */
> > +	RTE_SECURITY_IPSEC_SA_DIR_INGRESS,
> > +	/**< Verify digest and decrypt */
> > +};
> > +
> > +/**
> > + * IPsec security association configuration data.
> > + *
> > + * This structure contains data required to create an IPsec SA security
> session.
> > + */
> > +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?
> 
> > +	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?
> 
That's a good point. 

I've been think about it for some time. For inline we don't need any notifications,
but as we approach full offload it might be unavoidable.

I hope that we could cover some cases using the existing rte_flow facilities like
the MARK action which could indicate when the anti-replay window has reached a
critical point for both cases you've mentioned above.

> 
> > +/**
> > + * 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.
> 
> > + * @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);
> > +
> > +/**
> 
> 



More information about the dev mailing list