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