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
