patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH 1/6] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
       [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
@ 2019-03-06 16:00 ` Bernard Iremonger
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 2/6] examples/ipsec-secgw: fix 1st packet dropped patch two Bernard Iremonger
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Bernard Iremonger @ 2019-03-06 16:00 UTC (permalink / raw)
  To: dev, konstantin.ananyev, akhil.goyal; +Cc: Bernard Iremonger, stable

Refactor create_session() into create_inline_session() and
create_lookaside_session() in ipsec.c
Use socket_ctx in create_inline_session()

Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload")
Cc: stable@dpdk.org
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 examples/ipsec-secgw/ipsec.c         | 123 +++++++++++++++++++++++------------
 examples/ipsec-secgw/ipsec.h         |   5 +-
 examples/ipsec-secgw/ipsec_process.c |   9 +--
 3 files changed, 90 insertions(+), 47 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index 4352cb8..7090dbe 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -40,7 +40,7 @@ set_ipsec_conf(struct ipsec_sa *sa, struct rte_security_ipsec_xform *ipsec)
 }
 
 int
-create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
+create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 {
 	struct rte_cryptodev_info cdev_info;
 	unsigned long cdev_id_qp = 0;
@@ -108,23 +108,82 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 				"SEC Session init failed: err: %d\n", ret);
 				return -1;
 			}
-		} else if (sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
+		} else if ((sa->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) ||
+				(sa->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)) {
+					RTE_LOG(ERR, IPSEC,
+						"Inline not supported\n");
+					return -1;
+		}
+	} else {
+		sa->crypto_session = rte_cryptodev_sym_session_create(
+				ipsec_ctx->session_pool);
+		rte_cryptodev_sym_session_init(ipsec_ctx->tbl[cdev_id_qp].id,
+				sa->crypto_session, sa->xforms,
+				ipsec_ctx->session_priv_pool);
+
+		rte_cryptodev_info_get(ipsec_ctx->tbl[cdev_id_qp].id,
+				&cdev_info);
+	}
+
+	sa->cdev_id_qp = cdev_id_qp;
+
+	return 0;
+}
+
+int
+create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa)
+{
+	unsigned long cdev_id_qp = 0;
+	int32_t ret = 0;
+	struct rte_security_ctx *sec_ctx;
+
+	RTE_LOG_DP(DEBUG, IPSEC, "Create session for SA spi %u on port %u\n",
+		sa->spi, sa->portid);
+
+	if (sa->type != RTE_SECURITY_ACTION_TYPE_NONE) {
+		struct rte_security_session_conf sess_conf = {
+			.action_type = sa->type,
+			.protocol = RTE_SECURITY_PROTOCOL_IPSEC,
+			{.ipsec = {
+				.spi = sa->spi,
+				.salt = sa->salt,
+				.options = { 0 },
+				.direction = sa->direction,
+				.proto = RTE_SECURITY_IPSEC_SA_PROTO_ESP,
+				.mode = (sa->flags == IP4_TUNNEL ||
+						sa->flags == IP6_TUNNEL) ?
+					RTE_SECURITY_IPSEC_SA_MODE_TUNNEL :
+					RTE_SECURITY_IPSEC_SA_MODE_TRANSPORT,
+			} },
+			.crypto_xform = sa->xforms,
+			.userdata = NULL,
+		};
+
+		if (sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
 			struct rte_flow_error err;
-			struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-							rte_eth_dev_get_sec_ctx(
-							sa->portid);
 			const struct rte_security_capability *sec_cap;
 			int ret = 0;
 
-			sa->sec_session = rte_security_session_create(ctx,
-					&sess_conf, ipsec_ctx->session_pool);
+			sec_ctx = (struct rte_security_ctx *)
+							rte_eth_dev_get_sec_ctx(
+							sa->portid);
+			if (sec_ctx == NULL) {
+				RTE_LOG(ERR, IPSEC,
+				" rte_eth_dev_get_sec_ctx failed\n");
+				return -1;
+			}
+
+			sa->sec_session = rte_security_session_create(sec_ctx,
+					&sess_conf, skt_ctx->session_pool);
 			if (sa->sec_session == NULL) {
 				RTE_LOG(ERR, IPSEC,
 				"SEC Session init failed: err: %d\n", ret);
 				return -1;
 			}
 
-			sec_cap = rte_security_capabilities_get(ctx);
+			sec_cap = rte_security_capabilities_get(sec_ctx);
 
 			/* iterate until ESP tunnel*/
 			while (sec_cap->action !=
@@ -147,7 +206,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 			}
 
 			sa->ol_flags = sec_cap->ol_flags;
-			sa->security_ctx = ctx;
+			sa->security_ctx = sec_ctx;
 			sa->pattern[0].type = RTE_FLOW_ITEM_TYPE_ETH;
 
 			sa->pattern[1].type = RTE_FLOW_ITEM_TYPE_IPV4;
@@ -196,7 +255,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 				/* Try RSS. */
 				sa->action[1].type = RTE_FLOW_ACTION_TYPE_RSS;
 				sa->action[1].conf = &action_rss;
-				eth_dev = ctx->device;
+				eth_dev = sec_ctx->device;
 				rte_eth_dev_rss_hash_conf_get(sa->portid,
 							      &rss_conf);
 				for (i = 0, j = 0;
@@ -252,12 +311,12 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 			}
 		} else if (sa->type ==
 				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
-			struct rte_security_ctx *ctx =
-					(struct rte_security_ctx *)
-					rte_eth_dev_get_sec_ctx(sa->portid);
 			const struct rte_security_capability *sec_cap;
 
-			if (ctx == NULL) {
+			sec_ctx = (struct rte_security_ctx *)
+					rte_eth_dev_get_sec_ctx(sa->portid);
+
+			if (sec_ctx == NULL) {
 				RTE_LOG(ERR, IPSEC,
 				"Ethernet device doesn't have security features registered\n");
 				return -1;
@@ -279,15 +338,15 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 
 			sess_conf.userdata = (void *) sa;
 
-			sa->sec_session = rte_security_session_create(ctx,
-					&sess_conf, ipsec_ctx->session_pool);
+			sa->sec_session = rte_security_session_create(sec_ctx,
+					&sess_conf, skt_ctx->session_pool);
 			if (sa->sec_session == NULL) {
 				RTE_LOG(ERR, IPSEC,
 				"SEC Session init failed: err: %d\n", ret);
 				return -1;
 			}
 
-			sec_cap = rte_security_capabilities_get(ctx);
+			sec_cap = rte_security_capabilities_get(sec_ctx);
 
 			if (sec_cap == NULL) {
 				RTE_LOG(ERR, IPSEC,
@@ -316,17 +375,8 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 			}
 
 			sa->ol_flags = sec_cap->ol_flags;
-			sa->security_ctx = ctx;
+			sa->security_ctx = sec_ctx;
 		}
-	} else {
-		sa->crypto_session = rte_cryptodev_sym_session_create(
-				ipsec_ctx->session_pool);
-		rte_cryptodev_sym_session_init(ipsec_ctx->tbl[cdev_id_qp].id,
-				sa->crypto_session, sa->xforms,
-				ipsec_ctx->session_priv_pool);
-
-		rte_cryptodev_info_get(ipsec_ctx->tbl[cdev_id_qp].id,
-				&cdev_info);
 	}
 	sa->cdev_id_qp = cdev_id_qp;
 
@@ -395,7 +445,7 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 			rte_prefetch0(&priv->sym_cop);
 
 			if ((unlikely(sa->sec_session == NULL)) &&
-					create_session(ipsec_ctx, sa)) {
+				create_lookaside_session(ipsec_ctx, sa)) {
 				rte_pktmbuf_free(pkts[i]);
 				continue;
 			}
@@ -414,7 +464,7 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 			rte_prefetch0(&priv->sym_cop);
 
 			if ((unlikely(sa->crypto_session == NULL)) &&
-					create_session(ipsec_ctx, sa)) {
+				create_lookaside_session(ipsec_ctx, sa)) {
 				rte_pktmbuf_free(pkts[i]);
 				continue;
 			}
@@ -429,12 +479,7 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 			}
 			break;
 		case RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL:
-			if ((unlikely(sa->sec_session == NULL)) &&
-					create_session(ipsec_ctx, sa)) {
-				rte_pktmbuf_free(pkts[i]);
-				continue;
-			}
-
+			RTE_ASSERT(sa->sec_session != NULL);
 			ipsec_ctx->ol_pkts[ipsec_ctx->ol_pkts_cnt++] = pkts[i];
 			if (sa->ol_flags & RTE_SECURITY_TX_OLOAD_NEED_MDATA)
 				rte_security_set_pkt_metadata(
@@ -442,17 +487,11 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 						sa->sec_session, pkts[i], NULL);
 			continue;
 		case RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO:
+			RTE_ASSERT(sa->sec_session != NULL);
 			priv->cop.type = RTE_CRYPTO_OP_TYPE_SYMMETRIC;
 			priv->cop.status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
 
 			rte_prefetch0(&priv->sym_cop);
-
-			if ((unlikely(sa->sec_session == NULL)) &&
-					create_session(ipsec_ctx, sa)) {
-				rte_pktmbuf_free(pkts[i]);
-				continue;
-			}
-
 			rte_security_attach_session(&priv->cop,
 					sa->sec_session);
 
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index 99f49d6..db5a5c1 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -306,6 +306,9 @@ void
 enqueue_cop_burst(struct cdev_qp *cqp);
 
 int
-create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa);
+create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa);
+
+int
+create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa);
 
 #endif /* __IPSEC_H__ */
diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
index e403c46..152a684 100644
--- a/examples/ipsec-secgw/ipsec_process.c
+++ b/examples/ipsec-secgw/ipsec_process.c
@@ -95,22 +95,23 @@ fill_ipsec_session(struct rte_ipsec_session *ss, struct ipsec_ctx *ctx,
 	/* setup crypto section */
 	if (ss->type == RTE_SECURITY_ACTION_TYPE_NONE) {
 		if (sa->crypto_session == NULL) {
-			rc = create_session(ctx, sa);
+			rc = create_lookaside_session(ctx, sa);
 			if (rc != 0)
 				return rc;
 		}
 		ss->crypto.ses = sa->crypto_session;
 	/* setup session action type */
-	} else {
+	} else if (sa->type == RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) {
 		if (sa->sec_session == NULL) {
-			rc = create_session(ctx, sa);
+			rc = create_lookaside_session(ctx, sa);
 			if (rc != 0)
 				return rc;
 		}
 		ss->security.ses = sa->sec_session;
 		ss->security.ctx = sa->security_ctx;
 		ss->security.ol_flags = sa->ol_flags;
-	}
+	} else
+		RTE_ASSERT(0);
 
 	rc = rte_ipsec_session_prepare(ss);
 	if (rc != 0)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-stable] [PATCH 2/6] examples/ipsec-secgw: fix 1st packet dropped patch two
       [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 1/6] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto Bernard Iremonger
@ 2019-03-06 16:00 ` Bernard Iremonger
  2019-03-06 19:39   ` Ananyev, Konstantin
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 3/6] examples/ipsec-secgw: fix 1st packet dropped patch three Bernard Iremonger
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 17+ messages in thread
From: Bernard Iremonger @ 2019-03-06 16:00 UTC (permalink / raw)
  To: dev, konstantin.ananyev, akhil.goyal; +Cc: Bernard Iremonger, stable

Call create_inline_session() at initialisition in sa.c
Call rte_ipsec_session_prepare() in fill_ipsec_session() for inline.

Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload")
Cc: stable@dpdk.org
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 examples/ipsec-secgw/sa.c | 46 ++++++++++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 414fcd2..7fb1929 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -762,11 +762,13 @@ check_eth_dev_caps(uint16_t portid, uint32_t inbound)
 
 static int
 sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
-		uint32_t nb_entries, uint32_t inbound)
+		uint32_t nb_entries, uint32_t inbound,
+		struct socket_ctx *skt_ctx)
 {
 	struct ipsec_sa *sa;
 	uint32_t i, idx;
 	uint16_t iv_length, aad_length;
+	int32_t rc;
 
 	/* for ESN upper 32 bits of SQN also need to be part of AAD */
 	aad_length = (app_sa_prm.enable_esn != 0) ? sizeof(uint32_t) : 0;
@@ -819,6 +821,17 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 
 			sa->xforms = &sa_ctx->xf[idx].a;
 
+			if (sa->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL ||
+				sa->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
+				rc = create_inline_session(skt_ctx, sa);
+				if (rc != 0) {
+					RTE_LOG(ERR, IPSEC_ESP,
+						"create_inline_session() failed\n");
+					return -EINVAL;
+				}
+			}
 			print_one_sa_rule(sa, inbound);
 		} else {
 			switch (sa->cipher_algo) {
@@ -894,16 +907,16 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 
 static inline int
 sa_out_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
-		uint32_t nb_entries)
+		uint32_t nb_entries, struct socket_ctx *skt_ctx)
 {
-	return sa_add_rules(sa_ctx, entries, nb_entries, 0);
+	return sa_add_rules(sa_ctx, entries, nb_entries, 0, skt_ctx);
 }
 
 static inline int
 sa_in_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
-		uint32_t nb_entries)
+		uint32_t nb_entries, struct socket_ctx *skt_ctx)
 {
-	return sa_add_rules(sa_ctx, entries, nb_entries, 1);
+	return sa_add_rules(sa_ctx, entries, nb_entries, 1, skt_ctx);
 }
 
 /*
@@ -997,10 +1010,12 @@ fill_ipsec_sa_prm(struct rte_ipsec_sa_prm *prm, const struct ipsec_sa *ss,
 	return 0;
 }
 
-static void
+static int
 fill_ipsec_session(struct rte_ipsec_session *ss, struct rte_ipsec_sa *sa,
 	const struct ipsec_sa *lsa)
 {
+	int32_t rc = 0;
+
 	ss->sa = sa;
 	ss->type = lsa->type;
 
@@ -1013,6 +1028,17 @@ fill_ipsec_session(struct rte_ipsec_session *ss, struct rte_ipsec_sa *sa,
 		ss->security.ctx = lsa->security_ctx;
 		ss->security.ol_flags = lsa->ol_flags;
 	}
+
+	if (ss->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO ||
+		ss->type == RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
+		if (ss->security.ses != NULL) {
+			rc = rte_ipsec_session_prepare(ss);
+			if (rc != 0)
+				memset(ss, 0, sizeof(*ss));
+		}
+	}
+
+	return rc;
 }
 
 /*
@@ -1047,8 +1073,8 @@ ipsec_sa_init(struct ipsec_sa *lsa, struct rte_ipsec_sa *sa, uint32_t sa_size)
 	if (rc < 0)
 		return rc;
 
-	fill_ipsec_session(&lsa->ips, sa, lsa);
-	return 0;
+	rc = fill_ipsec_session(&lsa->ips, sa, lsa);
+	return rc;
 }
 
 /*
@@ -1126,7 +1152,7 @@ sa_init(struct socket_ctx *ctx, int32_t socket_id)
 				"context %s in socket %d\n", rte_errno,
 				name, socket_id);
 
-		sa_in_add_rules(ctx->sa_in, sa_in, nb_sa_in);
+		sa_in_add_rules(ctx->sa_in, sa_in, nb_sa_in, ctx);
 
 		if (app_sa_prm.enable != 0) {
 			rc = ipsec_satbl_init(ctx->sa_in, sa_in, nb_sa_in,
@@ -1146,7 +1172,7 @@ sa_init(struct socket_ctx *ctx, int32_t socket_id)
 				"context %s in socket %d\n", rte_errno,
 				name, socket_id);
 
-		sa_out_add_rules(ctx->sa_out, sa_out, nb_sa_out);
+		sa_out_add_rules(ctx->sa_out, sa_out, nb_sa_out, ctx);
 
 		if (app_sa_prm.enable != 0) {
 			rc = ipsec_satbl_init(ctx->sa_out, sa_out, nb_sa_out,
-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-stable] [PATCH 3/6] examples/ipsec-secgw: fix 1st packet dropped patch three
       [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 1/6] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto Bernard Iremonger
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 2/6] examples/ipsec-secgw: fix 1st packet dropped patch two Bernard Iremonger
@ 2019-03-06 16:00 ` Bernard Iremonger
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 4/6] examples/ipsec-secgw: fix debug in esp.c Bernard Iremonger
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Bernard Iremonger @ 2019-03-06 16:00 UTC (permalink / raw)
  To: dev, konstantin.ananyev, akhil.goyal; +Cc: Bernard Iremonger, stable

Refactor cryprodev_init() and main() in ipsec-secgw.c
Add max_session_size().
Start ports before adding flows in main().

Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
Cc: stable@dpdk.org
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 examples/ipsec-secgw/ipsec-secgw.c | 246 ++++++++++++++++++-------------------
 1 file changed, 123 insertions(+), 123 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 8e7cd1b..805e6b4 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -1029,6 +1029,9 @@ main_loop(__attribute__((unused)) void *dummy)
 				drain_outbound_crypto_queues(qconf,
 					&qconf->outbound);
 		}
+
+		drain_inbound_crypto_queues(qconf, &qconf->inbound);
+		drain_outbound_crypto_queues(qconf, &qconf->outbound);
 	}
 }
 
@@ -1627,7 +1630,7 @@ cryptodevs_init(void)
 	struct rte_cryptodev_config dev_conf;
 	struct rte_cryptodev_qp_conf qp_conf;
 	uint16_t idx, max_nb_qps, qp, i;
-	int16_t cdev_id, port_id;
+	int16_t cdev_id;
 	struct rte_hash_parameters params = { 0 };
 
 	params.entries = CDEV_MAP_ENTRIES;
@@ -1650,45 +1653,6 @@ cryptodevs_init(void)
 
 	printf("lcore/cryptodev/qp mappings:\n");
 
-	uint32_t max_sess_sz = 0, sess_sz;
-	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
-		void *sec_ctx;
-
-		/* Get crypto priv session size */
-		sess_sz = rte_cryptodev_sym_get_private_session_size(cdev_id);
-		if (sess_sz > max_sess_sz)
-			max_sess_sz = sess_sz;
-
-		/*
-		 * If crypto device is security capable, need to check the
-		 * size of security session as well.
-		 */
-
-		/* Get security context of the crypto device */
-		sec_ctx = rte_cryptodev_get_sec_ctx(cdev_id);
-		if (sec_ctx == NULL)
-			continue;
-
-		/* Get size of security session */
-		sess_sz = rte_security_session_get_size(sec_ctx);
-		if (sess_sz > max_sess_sz)
-			max_sess_sz = sess_sz;
-	}
-	RTE_ETH_FOREACH_DEV(port_id) {
-		void *sec_ctx;
-
-		if ((enabled_port_mask & (1 << port_id)) == 0)
-			continue;
-
-		sec_ctx = rte_eth_dev_get_sec_ctx(port_id);
-		if (sec_ctx == NULL)
-			continue;
-
-		sess_sz = rte_security_session_get_size(sec_ctx);
-		if (sess_sz > max_sess_sz)
-			max_sess_sz = sess_sz;
-	}
-
 	idx = 0;
 	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
 		struct rte_cryptodev_info cdev_info;
@@ -1726,45 +1690,6 @@ cryptodevs_init(void)
 				"Device does not support at least %u "
 				"sessions", CDEV_MP_NB_OBJS);
 
-		if (!socket_ctx[dev_conf.socket_id].session_pool) {
-			char mp_name[RTE_MEMPOOL_NAMESIZE];
-			struct rte_mempool *sess_mp;
-
-			snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
-					"sess_mp_%u", dev_conf.socket_id);
-			sess_mp = rte_cryptodev_sym_session_pool_create(
-					mp_name, CDEV_MP_NB_OBJS,
-					0, CDEV_MP_CACHE_SZ, 0,
-					dev_conf.socket_id);
-			socket_ctx[dev_conf.socket_id].session_pool = sess_mp;
-		}
-
-		if (!socket_ctx[dev_conf.socket_id].session_priv_pool) {
-			char mp_name[RTE_MEMPOOL_NAMESIZE];
-			struct rte_mempool *sess_mp;
-
-			snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
-					"sess_mp_priv_%u", dev_conf.socket_id);
-			sess_mp = rte_mempool_create(mp_name,
-					CDEV_MP_NB_OBJS,
-					max_sess_sz,
-					CDEV_MP_CACHE_SZ,
-					0, NULL, NULL, NULL,
-					NULL, dev_conf.socket_id,
-					0);
-			socket_ctx[dev_conf.socket_id].session_priv_pool =
-					sess_mp;
-		}
-
-		if (!socket_ctx[dev_conf.socket_id].session_priv_pool ||
-				!socket_ctx[dev_conf.socket_id].session_pool)
-			rte_exit(EXIT_FAILURE,
-				"Cannot create session pool on socket %d\n",
-				dev_conf.socket_id);
-		else
-			printf("Allocated session pool on socket %d\n",
-					dev_conf.socket_id);
-
 		if (rte_cryptodev_configure(cdev_id, &dev_conf))
 			rte_panic("Failed to initialize cryptodev %u\n",
 					cdev_id);
@@ -1785,38 +1710,6 @@ cryptodevs_init(void)
 					cdev_id);
 	}
 
-	/* create session pools for eth devices that implement security */
-	RTE_ETH_FOREACH_DEV(port_id) {
-		if ((enabled_port_mask & (1 << port_id)) &&
-				rte_eth_dev_get_sec_ctx(port_id)) {
-			int socket_id = rte_eth_dev_socket_id(port_id);
-
-			if (!socket_ctx[socket_id].session_pool) {
-				char mp_name[RTE_MEMPOOL_NAMESIZE];
-				struct rte_mempool *sess_mp;
-
-				snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
-						"sess_mp_%u", socket_id);
-				sess_mp = rte_mempool_create(mp_name,
-						(CDEV_MP_NB_OBJS * 2),
-						max_sess_sz,
-						CDEV_MP_CACHE_SZ,
-						0, NULL, NULL, NULL,
-						NULL, socket_id,
-						0);
-				if (sess_mp == NULL)
-					rte_exit(EXIT_FAILURE,
-						"Cannot create session pool "
-						"on socket %d\n", socket_id);
-				else
-					printf("Allocated session pool "
-						"on socket %d\n", socket_id);
-				socket_ctx[socket_id].session_pool = sess_mp;
-			}
-		}
-	}
-
-
 	printf("\n");
 
 	return 0;
@@ -1982,6 +1875,99 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 	printf("\n");
 }
 
+static size_t
+max_session_size(void)
+{
+	size_t max_sz, sz;
+	void *sec_ctx;
+	int16_t cdev_id, port_id, n;
+
+	max_sz = 0;
+	n =  rte_cryptodev_count();
+	for (cdev_id = 0; cdev_id != n; cdev_id++) {
+		sz = rte_cryptodev_sym_get_private_session_size(cdev_id);
+		if (sz > max_sz)
+			max_sz = sz;
+		/*
+		 * If crypto device is security capable, need to check the
+		 * size of security session as well.
+		 */
+
+		/* Get security context of the crypto device */
+		sec_ctx = rte_cryptodev_get_sec_ctx(cdev_id);
+		if (sec_ctx == NULL)
+			continue;
+
+		/* Get size of security session */
+		sz = rte_security_session_get_size(sec_ctx);
+		if (sz > max_sz)
+			max_sz = sz;
+	}
+
+	RTE_ETH_FOREACH_DEV(port_id) {
+		if ((enabled_port_mask & (1 << port_id)) == 0)
+			continue;
+
+		sec_ctx = rte_eth_dev_get_sec_ctx(port_id);
+		if (sec_ctx == NULL)
+			continue;
+
+		sz = rte_security_session_get_size(sec_ctx);
+		if (sz > max_sz)
+			max_sz = sz;
+	}
+
+	return max_sz;
+}
+
+static void
+session_pool_init(struct socket_ctx *ctx, int32_t socket_id, size_t sess_sz)
+{
+	char mp_name[RTE_MEMPOOL_NAMESIZE];
+	struct rte_mempool *sess_mp;
+
+	snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+			"sess_mp_%u", socket_id);
+	sess_mp = rte_cryptodev_sym_session_pool_create(
+			mp_name, CDEV_MP_NB_OBJS,
+			sess_sz, CDEV_MP_CACHE_SZ, 0,
+			socket_id);
+	ctx->session_pool = sess_mp;
+
+	if (ctx->session_pool == NULL)
+		rte_exit(EXIT_FAILURE,
+			"Cannot init session pool on socket %d\n", socket_id);
+	else
+		printf("Allocated session pool on socket %d\n",	socket_id);
+}
+
+static void
+session_priv_pool_init(struct socket_ctx *ctx, int32_t socket_id,
+	size_t sess_sz)
+{
+	char mp_name[RTE_MEMPOOL_NAMESIZE];
+	struct rte_mempool *sess_mp;
+
+	snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+			"sess_mp_priv_%u", socket_id);
+	sess_mp = rte_mempool_create(mp_name,
+			CDEV_MP_NB_OBJS,
+			sess_sz,
+			CDEV_MP_CACHE_SZ,
+			0, NULL, NULL, NULL,
+			NULL, socket_id,
+			0);
+	ctx->session_priv_pool = sess_mp;
+
+	if (ctx->session_priv_pool == NULL)
+		rte_exit(EXIT_FAILURE,
+			"Cannot init session priv pool on socket %d\n",
+			socket_id);
+	else
+		printf("Allocated session priv pool on socket %d\n",
+			socket_id);
+}
+
 static void
 pool_init(struct socket_ctx *ctx, int32_t socket_id, uint32_t nb_mbuf)
 {
@@ -2062,9 +2048,11 @@ main(int32_t argc, char **argv)
 {
 	int32_t ret;
 	uint32_t lcore_id;
+	uint32_t i;
 	uint8_t socket_id;
 	uint16_t portid;
 	uint64_t req_rx_offloads, req_tx_offloads;
+	size_t sess_sz;
 
 	/* init EAL */
 	ret = rte_eal_init(argc, argv);
@@ -2092,7 +2080,8 @@ main(int32_t argc, char **argv)
 
 	nb_lcores = rte_lcore_count();
 
-	/* Replicate each context per socket */
+	sess_sz = max_session_size();
+
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		if (rte_lcore_is_enabled(lcore_id) == 0)
 			continue;
@@ -2102,20 +2091,14 @@ main(int32_t argc, char **argv)
 		else
 			socket_id = 0;
 
+		/* mbuf_pool is initialised by the pool_init() function*/
 		if (socket_ctx[socket_id].mbuf_pool)
 			continue;
 
-		/* initilaze SPD */
-		sp4_init(&socket_ctx[socket_id], socket_id);
-
-		sp6_init(&socket_ctx[socket_id], socket_id);
-
-		/* initilaze SAD */
-		sa_init(&socket_ctx[socket_id], socket_id);
-
-		rt_init(&socket_ctx[socket_id], socket_id);
-
 		pool_init(&socket_ctx[socket_id], socket_id, NB_MBUF);
+		session_pool_init(&socket_ctx[socket_id], socket_id, sess_sz);
+		session_priv_pool_init(&socket_ctx[socket_id], socket_id,
+			sess_sz);
 	}
 
 	RTE_ETH_FOREACH_DEV(portid) {
@@ -2133,7 +2116,11 @@ main(int32_t argc, char **argv)
 		if ((enabled_port_mask & (1 << portid)) == 0)
 			continue;
 
-		/* Start device */
+		/*
+		 * Start device
+		 * note: device must be started before a flow rule
+		 * can be installed.
+		 */
 		ret = rte_eth_dev_start(portid);
 		if (ret < 0)
 			rte_exit(EXIT_FAILURE, "rte_eth_dev_start: "
@@ -2151,6 +2138,19 @@ main(int32_t argc, char **argv)
 			RTE_ETH_EVENT_IPSEC, inline_ipsec_event_callback, NULL);
 	}
 
+	/* Replicate each context per socket */
+	for (i = 0; i < NB_SOCKETS && i < rte_socket_count(); i++) {
+		socket_id = rte_socket_id_by_idx(i);
+		if ((socket_ctx[socket_id].mbuf_pool != NULL) &&
+			(socket_ctx[socket_id].sa_in == NULL) &&
+			(socket_ctx[socket_id].sa_out == NULL)) {
+			sa_init(&socket_ctx[socket_id], socket_id);
+			sp4_init(&socket_ctx[socket_id], socket_id);
+			sp6_init(&socket_ctx[socket_id], socket_id);
+			rt_init(&socket_ctx[socket_id], socket_id);
+		}
+	}
+
 	check_all_ports_link_status(enabled_port_mask);
 
 	/* launch per-lcore init on every lcore */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-stable] [PATCH 4/6] examples/ipsec-secgw: fix debug in esp.c
       [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
                   ` (2 preceding siblings ...)
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 3/6] examples/ipsec-secgw: fix 1st packet dropped patch three Bernard Iremonger
@ 2019-03-06 16:00 ` Bernard Iremonger
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 5/6] examples/ipsec-secgw: fix debug in sa.c Bernard Iremonger
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Bernard Iremonger @ 2019-03-06 16:00 UTC (permalink / raw)
  To: dev, konstantin.ananyev, akhil.goyal; +Cc: Bernard Iremonger, stable

Improve debug code in esp.c

Fixes: f159e70b0922 ("examples/ipsec-secgw: support transport mode")
Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload")
Cc: stable@dpdk.org
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 examples/ipsec-secgw/esp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/esp.c b/examples/ipsec-secgw/esp.c
index e33232c..faa84dd 100644
--- a/examples/ipsec-secgw/esp.c
+++ b/examples/ipsec-secgw/esp.c
@@ -162,7 +162,7 @@ esp_inbound_post(struct rte_mbuf *m, struct ipsec_sa *sa,
 	}
 
 	if (cop->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
-		RTE_LOG(ERR, IPSEC_ESP, "failed crypto op\n");
+		RTE_LOG(ERR, IPSEC_ESP, "%s() failed crypto op\n", __func__);
 		return -1;
 	}
 
@@ -455,7 +455,8 @@ esp_outbound_post(struct rte_mbuf *m,
 	} else {
 		RTE_ASSERT(cop != NULL);
 		if (cop->status != RTE_CRYPTO_OP_STATUS_SUCCESS) {
-			RTE_LOG(ERR, IPSEC_ESP, "Failed crypto op\n");
+			RTE_LOG(ERR, IPSEC_ESP, "%s() failed crypto op\n",
+				__func__);
 			return -1;
 		}
 	}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-stable] [PATCH 5/6] examples/ipsec-secgw: fix debug in sa.c
       [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
                   ` (3 preceding siblings ...)
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 4/6] examples/ipsec-secgw: fix debug in esp.c Bernard Iremonger
@ 2019-03-06 16:00 ` Bernard Iremonger
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 6/6] examples/ipsec-secgw: fix debug in ipsec-secgw.c Bernard Iremonger
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: Bernard Iremonger @ 2019-03-06 16:00 UTC (permalink / raw)
  To: dev, konstantin.ananyev, akhil.goyal; +Cc: Bernard Iremonger, stable

Improve debug code in sa.c

Fixes: 0d547ed03717 ("examples/ipsec-secgw: support configuration file")
Cc: stable@dpdk.org
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 examples/ipsec-secgw/sa.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 7fb1929..4e583cd 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -688,7 +688,22 @@ print_one_sa_rule(const struct ipsec_sa *sa, int inbound)
 		}
 		break;
 	case TRANSPORT:
-		printf("Transport");
+		printf("Transport ");
+		break;
+	}
+	printf(" type:");
+	switch (sa->type) {
+	case RTE_SECURITY_ACTION_TYPE_NONE:
+		printf("no-offload ");
+		break;
+	case RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO:
+		printf("inline-crypto-offload ");
+		break;
+	case RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL:
+		printf("inline-protocol-offload ");
+		break;
+	case RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL:
+		printf("lookaside-protocol-offload ");
 		break;
 	}
 	printf("\n");
@@ -716,8 +731,8 @@ sa_create(const char *name, int32_t socket_id)
 	snprintf(s, sizeof(s), "%s_%u", name, socket_id);
 
 	/* Create SA array table */
-	printf("Creating SA context with %u maximum entries\n",
-			IPSEC_SA_MAX_ENTRIES);
+	printf("Creating SA context with %u maximum entries on socket %d\n",
+			IPSEC_SA_MAX_ENTRIES, socket_id);
 
 	mz_size = sizeof(struct sa_ctx);
 	mz = rte_memzone_reserve(s, mz_size, socket_id,
-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-stable] [PATCH 6/6] examples/ipsec-secgw: fix debug in ipsec-secgw.c
       [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
                   ` (4 preceding siblings ...)
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 5/6] examples/ipsec-secgw: fix debug in sa.c Bernard Iremonger
@ 2019-03-06 16:00 ` Bernard Iremonger
  2019-03-07 14:57 ` [dpdk-stable] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto Bernard Iremonger
  2019-03-07 14:57 ` [dpdk-stable] [PATCH v2 2/2] examples/ipsec-secgw/test: fix inline test scripts Bernard Iremonger
  7 siblings, 0 replies; 17+ messages in thread
From: Bernard Iremonger @ 2019-03-06 16:00 UTC (permalink / raw)
  To: dev, konstantin.ananyev, akhil.goyal; +Cc: Bernard Iremonger, stable

Improve debug in ipsec-secgw.c

Fixes: 906257e965b7 ("examples/ipsec-secgw: support IPv6")
Cc: stable@dpdk.org
Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 examples/ipsec-secgw/ipsec-secgw.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 805e6b4..0c3c8f8 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -260,7 +260,8 @@ prepare_one_packet(struct rte_mbuf *pkt, struct ipsec_traffic *t)
 		pkt->l3_len = sizeof(struct ip6_hdr);
 	} else {
 		/* Unknown/Unsupported type, drop the packet */
-		RTE_LOG(ERR, IPSEC, "Unsupported packet type\n");
+		RTE_LOG(ERR, IPSEC, "Unsupported packet type 0x%x\n",
+			rte_be_to_cpu_16(eth->ether_type));
 		rte_pktmbuf_free(pkt);
 	}
 
@@ -984,7 +985,8 @@ main_loop(__attribute__((unused)) void *dummy)
 			socket_ctx[socket_id].session_priv_pool;
 
 	if (qconf->nb_rx_queue == 0) {
-		RTE_LOG(INFO, IPSEC, "lcore %u has nothing to do\n", lcore_id);
+		RTE_LOG(DEBUG, IPSEC, "lcore %u has nothing to do\n",
+			lcore_id);
 		return 0;
 	}
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH 2/6] examples/ipsec-secgw: fix 1st packet dropped patch two
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 2/6] examples/ipsec-secgw: fix 1st packet dropped patch two Bernard Iremonger
@ 2019-03-06 19:39   ` Ananyev, Konstantin
  2019-03-07  9:54     ` Iremonger, Bernard
  0 siblings, 1 reply; 17+ messages in thread
From: Ananyev, Konstantin @ 2019-03-06 19:39 UTC (permalink / raw)
  To: Iremonger, Bernard, dev, akhil.goyal; +Cc: stable


Hi Bernard,

> 
> Call create_inline_session() at initialisition in sa.c
> Call rte_ipsec_session_prepare() in fill_ipsec_session() for inline.

Here and in other places - it probably worth to explain what is the purpose
for  these changes. 
As a side notice, as these series fixes that problem, it probably worse to add a patch
into series that removes the following:

       # to overcome problem with ipsec-secgw for inline mode,
        # when first packet(s) will be always dropped.
        # note that ping will fail here
        ssh ${REMOTE_HOST} ping -c 1 ${LOCAL_IPV4}

from examples/ipsec-secgw/test/(tun|trs)_aesgcm_defs.sh
Konstantin

> 
> Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload")
> Cc: stable@dpdk.org
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>  examples/ipsec-secgw/sa.c | 46 ++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index 414fcd2..7fb1929 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -762,11 +762,13 @@ check_eth_dev_caps(uint16_t portid, uint32_t inbound)
> 
>  static int
>  sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
> -		uint32_t nb_entries, uint32_t inbound)
> +		uint32_t nb_entries, uint32_t inbound,
> +		struct socket_ctx *skt_ctx)
>  {
>  	struct ipsec_sa *sa;
>  	uint32_t i, idx;
>  	uint16_t iv_length, aad_length;
> +	int32_t rc;
> 
>  	/* for ESN upper 32 bits of SQN also need to be part of AAD */
>  	aad_length = (app_sa_prm.enable_esn != 0) ? sizeof(uint32_t) : 0;
> @@ -819,6 +821,17 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
> 
>  			sa->xforms = &sa_ctx->xf[idx].a;
> 
> +			if (sa->type ==
> +				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL ||
> +				sa->type ==
> +				RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
> +				rc = create_inline_session(skt_ctx, sa);
> +				if (rc != 0) {
> +					RTE_LOG(ERR, IPSEC_ESP,
> +						"create_inline_session() failed\n");
> +					return -EINVAL;
> +				}
> +			}
>  			print_one_sa_rule(sa, inbound);
>  		} else {
>  			switch (sa->cipher_algo) {
> @@ -894,16 +907,16 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
> 
>  static inline int
>  sa_out_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
> -		uint32_t nb_entries)
> +		uint32_t nb_entries, struct socket_ctx *skt_ctx)
>  {
> -	return sa_add_rules(sa_ctx, entries, nb_entries, 0);
> +	return sa_add_rules(sa_ctx, entries, nb_entries, 0, skt_ctx);
>  }
> 
>  static inline int
>  sa_in_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
> -		uint32_t nb_entries)
> +		uint32_t nb_entries, struct socket_ctx *skt_ctx)
>  {
> -	return sa_add_rules(sa_ctx, entries, nb_entries, 1);
> +	return sa_add_rules(sa_ctx, entries, nb_entries, 1, skt_ctx);
>  }
> 
>  /*
> @@ -997,10 +1010,12 @@ fill_ipsec_sa_prm(struct rte_ipsec_sa_prm *prm, const struct ipsec_sa *ss,
>  	return 0;
>  }
> 
> -static void
> +static int
>  fill_ipsec_session(struct rte_ipsec_session *ss, struct rte_ipsec_sa *sa,
>  	const struct ipsec_sa *lsa)
>  {
> +	int32_t rc = 0;
> +
>  	ss->sa = sa;
>  	ss->type = lsa->type;
> 
> @@ -1013,6 +1028,17 @@ fill_ipsec_session(struct rte_ipsec_session *ss, struct rte_ipsec_sa *sa,
>  		ss->security.ctx = lsa->security_ctx;
>  		ss->security.ol_flags = lsa->ol_flags;
>  	}
> +
> +	if (ss->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO ||
> +		ss->type == RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
> +		if (ss->security.ses != NULL) {
> +			rc = rte_ipsec_session_prepare(ss);
> +			if (rc != 0)
> +				memset(ss, 0, sizeof(*ss));
> +		}
> +	}
> +
> +	return rc;
>  }
> 
>  /*
> @@ -1047,8 +1073,8 @@ ipsec_sa_init(struct ipsec_sa *lsa, struct rte_ipsec_sa *sa, uint32_t sa_size)
>  	if (rc < 0)
>  		return rc;
> 
> -	fill_ipsec_session(&lsa->ips, sa, lsa);
> -	return 0;
> +	rc = fill_ipsec_session(&lsa->ips, sa, lsa);
> +	return rc;
>  }
> 
>  /*
> @@ -1126,7 +1152,7 @@ sa_init(struct socket_ctx *ctx, int32_t socket_id)
>  				"context %s in socket %d\n", rte_errno,
>  				name, socket_id);
> 
> -		sa_in_add_rules(ctx->sa_in, sa_in, nb_sa_in);
> +		sa_in_add_rules(ctx->sa_in, sa_in, nb_sa_in, ctx);
> 
>  		if (app_sa_prm.enable != 0) {
>  			rc = ipsec_satbl_init(ctx->sa_in, sa_in, nb_sa_in,
> @@ -1146,7 +1172,7 @@ sa_init(struct socket_ctx *ctx, int32_t socket_id)
>  				"context %s in socket %d\n", rte_errno,
>  				name, socket_id);
> 
> -		sa_out_add_rules(ctx->sa_out, sa_out, nb_sa_out);
> +		sa_out_add_rules(ctx->sa_out, sa_out, nb_sa_out, ctx);
> 
>  		if (app_sa_prm.enable != 0) {
>  			rc = ipsec_satbl_init(ctx->sa_out, sa_out, nb_sa_out,
> --
> 2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH 2/6] examples/ipsec-secgw: fix 1st packet dropped patch two
  2019-03-06 19:39   ` Ananyev, Konstantin
@ 2019-03-07  9:54     ` Iremonger, Bernard
  0 siblings, 0 replies; 17+ messages in thread
From: Iremonger, Bernard @ 2019-03-07  9:54 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev, akhil.goyal; +Cc: stable

Hi Konstantin,

<snip>

> Subject: RE: [PATCH 2/6] examples/ipsec-secgw: fix 1st packet dropped patch
> two
> 
> 
> Hi Bernard,
> 
> >
> > Call create_inline_session() at initialisition in sa.c Call
> > rte_ipsec_session_prepare() in fill_ipsec_session() for inline.
> 
> Here and in other places - it probably worth to explain what is the purpose for
> these changes.

I will improve explanation.

> As a side notice, as these series fixes that problem, it probably worse to add a
> patch into series that removes the following:
> 
>        # to overcome problem with ipsec-secgw for inline mode,
>         # when first packet(s) will be always dropped.
>         # note that ping will fail here
>         ssh ${REMOTE_HOST} ping -c 1 ${LOCAL_IPV4}
> 
> from examples/ipsec-secgw/test/(tun|trs)_aesgcm_defs.sh
> Konstantin

I will add a patch to fix the test scripts.

<snip>

Regards,

Bernard.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-stable] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
       [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
                   ` (5 preceding siblings ...)
  2019-03-06 16:00 ` [dpdk-stable] [PATCH 6/6] examples/ipsec-secgw: fix debug in ipsec-secgw.c Bernard Iremonger
@ 2019-03-07 14:57 ` Bernard Iremonger
  2019-03-22 13:18   ` [dpdk-stable] [dpdk-dev] " Akhil Goyal
  2019-03-07 14:57 ` [dpdk-stable] [PATCH v2 2/2] examples/ipsec-secgw/test: fix inline test scripts Bernard Iremonger
  7 siblings, 1 reply; 17+ messages in thread
From: Bernard Iremonger @ 2019-03-07 14:57 UTC (permalink / raw)
  To: dev, konstantin.ananyev, akhil.goyal; +Cc: Bernard Iremonger, stable

Inline crypto installs a flow rule in the NIC. This flow
rule must be installed before the first inbound packet is
received.

The create_session() function installs the flow rule,
create_session() has been refactored into create_inline_session()
and create_lookaside_session(). The create_inline_session() function
uses the socket_ctx data and is now called at initialisation in
sa_add_rules().

The max_session_size() function has been added to calculate memory
requirements.

The cryprodev_init() function has been refactored to drop calls to
rte_mempool_create() and to drop calculation of memory requirements.

The main() function has been refactored to call max_session_size() and
to call session_pool_init() and session_priv_pool_init() earlier.
The ports are started now before adding a flow rule in main().
The sa_init(), sp4_init(), sp6_init() and rt_init() functions are
now called after the ports have been started.

The rte_ipsec_session_prepare() function is called in fill_ipsec_session()
for inline which is called from the ipsec_sa_init() function.

Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload")
Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
Cc: stable@dpdk.org

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 examples/ipsec-secgw/ipsec-secgw.c   | 243 +++++++++++++++++------------------
 examples/ipsec-secgw/ipsec.c         | 123 ++++++++++++------
 examples/ipsec-secgw/ipsec.h         |   5 +-
 examples/ipsec-secgw/ipsec_process.c |   9 +-
 examples/ipsec-secgw/sa.c            |  46 +++++--
 5 files changed, 246 insertions(+), 180 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 8e7cd1b..835cc98 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -1627,7 +1627,7 @@ cryptodevs_init(void)
 	struct rte_cryptodev_config dev_conf;
 	struct rte_cryptodev_qp_conf qp_conf;
 	uint16_t idx, max_nb_qps, qp, i;
-	int16_t cdev_id, port_id;
+	int16_t cdev_id;
 	struct rte_hash_parameters params = { 0 };
 
 	params.entries = CDEV_MAP_ENTRIES;
@@ -1650,45 +1650,6 @@ cryptodevs_init(void)
 
 	printf("lcore/cryptodev/qp mappings:\n");
 
-	uint32_t max_sess_sz = 0, sess_sz;
-	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
-		void *sec_ctx;
-
-		/* Get crypto priv session size */
-		sess_sz = rte_cryptodev_sym_get_private_session_size(cdev_id);
-		if (sess_sz > max_sess_sz)
-			max_sess_sz = sess_sz;
-
-		/*
-		 * If crypto device is security capable, need to check the
-		 * size of security session as well.
-		 */
-
-		/* Get security context of the crypto device */
-		sec_ctx = rte_cryptodev_get_sec_ctx(cdev_id);
-		if (sec_ctx == NULL)
-			continue;
-
-		/* Get size of security session */
-		sess_sz = rte_security_session_get_size(sec_ctx);
-		if (sess_sz > max_sess_sz)
-			max_sess_sz = sess_sz;
-	}
-	RTE_ETH_FOREACH_DEV(port_id) {
-		void *sec_ctx;
-
-		if ((enabled_port_mask & (1 << port_id)) == 0)
-			continue;
-
-		sec_ctx = rte_eth_dev_get_sec_ctx(port_id);
-		if (sec_ctx == NULL)
-			continue;
-
-		sess_sz = rte_security_session_get_size(sec_ctx);
-		if (sess_sz > max_sess_sz)
-			max_sess_sz = sess_sz;
-	}
-
 	idx = 0;
 	for (cdev_id = 0; cdev_id < rte_cryptodev_count(); cdev_id++) {
 		struct rte_cryptodev_info cdev_info;
@@ -1726,45 +1687,6 @@ cryptodevs_init(void)
 				"Device does not support at least %u "
 				"sessions", CDEV_MP_NB_OBJS);
 
-		if (!socket_ctx[dev_conf.socket_id].session_pool) {
-			char mp_name[RTE_MEMPOOL_NAMESIZE];
-			struct rte_mempool *sess_mp;
-
-			snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
-					"sess_mp_%u", dev_conf.socket_id);
-			sess_mp = rte_cryptodev_sym_session_pool_create(
-					mp_name, CDEV_MP_NB_OBJS,
-					0, CDEV_MP_CACHE_SZ, 0,
-					dev_conf.socket_id);
-			socket_ctx[dev_conf.socket_id].session_pool = sess_mp;
-		}
-
-		if (!socket_ctx[dev_conf.socket_id].session_priv_pool) {
-			char mp_name[RTE_MEMPOOL_NAMESIZE];
-			struct rte_mempool *sess_mp;
-
-			snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
-					"sess_mp_priv_%u", dev_conf.socket_id);
-			sess_mp = rte_mempool_create(mp_name,
-					CDEV_MP_NB_OBJS,
-					max_sess_sz,
-					CDEV_MP_CACHE_SZ,
-					0, NULL, NULL, NULL,
-					NULL, dev_conf.socket_id,
-					0);
-			socket_ctx[dev_conf.socket_id].session_priv_pool =
-					sess_mp;
-		}
-
-		if (!socket_ctx[dev_conf.socket_id].session_priv_pool ||
-				!socket_ctx[dev_conf.socket_id].session_pool)
-			rte_exit(EXIT_FAILURE,
-				"Cannot create session pool on socket %d\n",
-				dev_conf.socket_id);
-		else
-			printf("Allocated session pool on socket %d\n",
-					dev_conf.socket_id);
-
 		if (rte_cryptodev_configure(cdev_id, &dev_conf))
 			rte_panic("Failed to initialize cryptodev %u\n",
 					cdev_id);
@@ -1785,38 +1707,6 @@ cryptodevs_init(void)
 					cdev_id);
 	}
 
-	/* create session pools for eth devices that implement security */
-	RTE_ETH_FOREACH_DEV(port_id) {
-		if ((enabled_port_mask & (1 << port_id)) &&
-				rte_eth_dev_get_sec_ctx(port_id)) {
-			int socket_id = rte_eth_dev_socket_id(port_id);
-
-			if (!socket_ctx[socket_id].session_pool) {
-				char mp_name[RTE_MEMPOOL_NAMESIZE];
-				struct rte_mempool *sess_mp;
-
-				snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
-						"sess_mp_%u", socket_id);
-				sess_mp = rte_mempool_create(mp_name,
-						(CDEV_MP_NB_OBJS * 2),
-						max_sess_sz,
-						CDEV_MP_CACHE_SZ,
-						0, NULL, NULL, NULL,
-						NULL, socket_id,
-						0);
-				if (sess_mp == NULL)
-					rte_exit(EXIT_FAILURE,
-						"Cannot create session pool "
-						"on socket %d\n", socket_id);
-				else
-					printf("Allocated session pool "
-						"on socket %d\n", socket_id);
-				socket_ctx[socket_id].session_pool = sess_mp;
-			}
-		}
-	}
-
-
 	printf("\n");
 
 	return 0;
@@ -1982,6 +1872,99 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 	printf("\n");
 }
 
+static size_t
+max_session_size(void)
+{
+	size_t max_sz, sz;
+	void *sec_ctx;
+	int16_t cdev_id, port_id, n;
+
+	max_sz = 0;
+	n =  rte_cryptodev_count();
+	for (cdev_id = 0; cdev_id != n; cdev_id++) {
+		sz = rte_cryptodev_sym_get_private_session_size(cdev_id);
+		if (sz > max_sz)
+			max_sz = sz;
+		/*
+		 * If crypto device is security capable, need to check the
+		 * size of security session as well.
+		 */
+
+		/* Get security context of the crypto device */
+		sec_ctx = rte_cryptodev_get_sec_ctx(cdev_id);
+		if (sec_ctx == NULL)
+			continue;
+
+		/* Get size of security session */
+		sz = rte_security_session_get_size(sec_ctx);
+		if (sz > max_sz)
+			max_sz = sz;
+	}
+
+	RTE_ETH_FOREACH_DEV(port_id) {
+		if ((enabled_port_mask & (1 << port_id)) == 0)
+			continue;
+
+		sec_ctx = rte_eth_dev_get_sec_ctx(port_id);
+		if (sec_ctx == NULL)
+			continue;
+
+		sz = rte_security_session_get_size(sec_ctx);
+		if (sz > max_sz)
+			max_sz = sz;
+	}
+
+	return max_sz;
+}
+
+static void
+session_pool_init(struct socket_ctx *ctx, int32_t socket_id, size_t sess_sz)
+{
+	char mp_name[RTE_MEMPOOL_NAMESIZE];
+	struct rte_mempool *sess_mp;
+
+	snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+			"sess_mp_%u", socket_id);
+	sess_mp = rte_cryptodev_sym_session_pool_create(
+			mp_name, CDEV_MP_NB_OBJS,
+			sess_sz, CDEV_MP_CACHE_SZ, 0,
+			socket_id);
+	ctx->session_pool = sess_mp;
+
+	if (ctx->session_pool == NULL)
+		rte_exit(EXIT_FAILURE,
+			"Cannot init session pool on socket %d\n", socket_id);
+	else
+		printf("Allocated session pool on socket %d\n",	socket_id);
+}
+
+static void
+session_priv_pool_init(struct socket_ctx *ctx, int32_t socket_id,
+	size_t sess_sz)
+{
+	char mp_name[RTE_MEMPOOL_NAMESIZE];
+	struct rte_mempool *sess_mp;
+
+	snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+			"sess_mp_priv_%u", socket_id);
+	sess_mp = rte_mempool_create(mp_name,
+			CDEV_MP_NB_OBJS,
+			sess_sz,
+			CDEV_MP_CACHE_SZ,
+			0, NULL, NULL, NULL,
+			NULL, socket_id,
+			0);
+	ctx->session_priv_pool = sess_mp;
+
+	if (ctx->session_priv_pool == NULL)
+		rte_exit(EXIT_FAILURE,
+			"Cannot init session priv pool on socket %d\n",
+			socket_id);
+	else
+		printf("Allocated session priv pool on socket %d\n",
+			socket_id);
+}
+
 static void
 pool_init(struct socket_ctx *ctx, int32_t socket_id, uint32_t nb_mbuf)
 {
@@ -2062,9 +2045,11 @@ main(int32_t argc, char **argv)
 {
 	int32_t ret;
 	uint32_t lcore_id;
+	uint32_t i;
 	uint8_t socket_id;
 	uint16_t portid;
 	uint64_t req_rx_offloads, req_tx_offloads;
+	size_t sess_sz;
 
 	/* init EAL */
 	ret = rte_eal_init(argc, argv);
@@ -2092,7 +2077,8 @@ main(int32_t argc, char **argv)
 
 	nb_lcores = rte_lcore_count();
 
-	/* Replicate each context per socket */
+	sess_sz = max_session_size();
+
 	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
 		if (rte_lcore_is_enabled(lcore_id) == 0)
 			continue;
@@ -2102,20 +2088,14 @@ main(int32_t argc, char **argv)
 		else
 			socket_id = 0;
 
+		/* mbuf_pool is initialised by the pool_init() function*/
 		if (socket_ctx[socket_id].mbuf_pool)
 			continue;
 
-		/* initilaze SPD */
-		sp4_init(&socket_ctx[socket_id], socket_id);
-
-		sp6_init(&socket_ctx[socket_id], socket_id);
-
-		/* initilaze SAD */
-		sa_init(&socket_ctx[socket_id], socket_id);
-
-		rt_init(&socket_ctx[socket_id], socket_id);
-
 		pool_init(&socket_ctx[socket_id], socket_id, NB_MBUF);
+		session_pool_init(&socket_ctx[socket_id], socket_id, sess_sz);
+		session_priv_pool_init(&socket_ctx[socket_id], socket_id,
+			sess_sz);
 	}
 
 	RTE_ETH_FOREACH_DEV(portid) {
@@ -2133,7 +2113,11 @@ main(int32_t argc, char **argv)
 		if ((enabled_port_mask & (1 << portid)) == 0)
 			continue;
 
-		/* Start device */
+		/*
+		 * Start device
+		 * note: device must be started before a flow rule
+		 * can be installed.
+		 */
 		ret = rte_eth_dev_start(portid);
 		if (ret < 0)
 			rte_exit(EXIT_FAILURE, "rte_eth_dev_start: "
@@ -2151,6 +2135,19 @@ main(int32_t argc, char **argv)
 			RTE_ETH_EVENT_IPSEC, inline_ipsec_event_callback, NULL);
 	}
 
+	/* Replicate each context per socket */
+	for (i = 0; i < NB_SOCKETS && i < rte_socket_count(); i++) {
+		socket_id = rte_socket_id_by_idx(i);
+		if ((socket_ctx[socket_id].mbuf_pool != NULL) &&
+			(socket_ctx[socket_id].sa_in == NULL) &&
+			(socket_ctx[socket_id].sa_out == NULL)) {
+			sa_init(&socket_ctx[socket_id], socket_id);
+			sp4_init(&socket_ctx[socket_id], socket_id);
+			sp6_init(&socket_ctx[socket_id], socket_id);
+			rt_init(&socket_ctx[socket_id], socket_id);
+		}
+	}
+
 	check_all_ports_link_status(enabled_port_mask);
 
 	/* launch per-lcore init on every lcore */
diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index 4352cb8..7090dbe 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -40,7 +40,7 @@ set_ipsec_conf(struct ipsec_sa *sa, struct rte_security_ipsec_xform *ipsec)
 }
 
 int
-create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
+create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 {
 	struct rte_cryptodev_info cdev_info;
 	unsigned long cdev_id_qp = 0;
@@ -108,23 +108,82 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 				"SEC Session init failed: err: %d\n", ret);
 				return -1;
 			}
-		} else if (sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
+		} else if ((sa->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) ||
+				(sa->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)) {
+					RTE_LOG(ERR, IPSEC,
+						"Inline not supported\n");
+					return -1;
+		}
+	} else {
+		sa->crypto_session = rte_cryptodev_sym_session_create(
+				ipsec_ctx->session_pool);
+		rte_cryptodev_sym_session_init(ipsec_ctx->tbl[cdev_id_qp].id,
+				sa->crypto_session, sa->xforms,
+				ipsec_ctx->session_priv_pool);
+
+		rte_cryptodev_info_get(ipsec_ctx->tbl[cdev_id_qp].id,
+				&cdev_info);
+	}
+
+	sa->cdev_id_qp = cdev_id_qp;
+
+	return 0;
+}
+
+int
+create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa)
+{
+	unsigned long cdev_id_qp = 0;
+	int32_t ret = 0;
+	struct rte_security_ctx *sec_ctx;
+
+	RTE_LOG_DP(DEBUG, IPSEC, "Create session for SA spi %u on port %u\n",
+		sa->spi, sa->portid);
+
+	if (sa->type != RTE_SECURITY_ACTION_TYPE_NONE) {
+		struct rte_security_session_conf sess_conf = {
+			.action_type = sa->type,
+			.protocol = RTE_SECURITY_PROTOCOL_IPSEC,
+			{.ipsec = {
+				.spi = sa->spi,
+				.salt = sa->salt,
+				.options = { 0 },
+				.direction = sa->direction,
+				.proto = RTE_SECURITY_IPSEC_SA_PROTO_ESP,
+				.mode = (sa->flags == IP4_TUNNEL ||
+						sa->flags == IP6_TUNNEL) ?
+					RTE_SECURITY_IPSEC_SA_MODE_TUNNEL :
+					RTE_SECURITY_IPSEC_SA_MODE_TRANSPORT,
+			} },
+			.crypto_xform = sa->xforms,
+			.userdata = NULL,
+		};
+
+		if (sa->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
 			struct rte_flow_error err;
-			struct rte_security_ctx *ctx = (struct rte_security_ctx *)
-							rte_eth_dev_get_sec_ctx(
-							sa->portid);
 			const struct rte_security_capability *sec_cap;
 			int ret = 0;
 
-			sa->sec_session = rte_security_session_create(ctx,
-					&sess_conf, ipsec_ctx->session_pool);
+			sec_ctx = (struct rte_security_ctx *)
+							rte_eth_dev_get_sec_ctx(
+							sa->portid);
+			if (sec_ctx == NULL) {
+				RTE_LOG(ERR, IPSEC,
+				" rte_eth_dev_get_sec_ctx failed\n");
+				return -1;
+			}
+
+			sa->sec_session = rte_security_session_create(sec_ctx,
+					&sess_conf, skt_ctx->session_pool);
 			if (sa->sec_session == NULL) {
 				RTE_LOG(ERR, IPSEC,
 				"SEC Session init failed: err: %d\n", ret);
 				return -1;
 			}
 
-			sec_cap = rte_security_capabilities_get(ctx);
+			sec_cap = rte_security_capabilities_get(sec_ctx);
 
 			/* iterate until ESP tunnel*/
 			while (sec_cap->action !=
@@ -147,7 +206,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 			}
 
 			sa->ol_flags = sec_cap->ol_flags;
-			sa->security_ctx = ctx;
+			sa->security_ctx = sec_ctx;
 			sa->pattern[0].type = RTE_FLOW_ITEM_TYPE_ETH;
 
 			sa->pattern[1].type = RTE_FLOW_ITEM_TYPE_IPV4;
@@ -196,7 +255,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 				/* Try RSS. */
 				sa->action[1].type = RTE_FLOW_ACTION_TYPE_RSS;
 				sa->action[1].conf = &action_rss;
-				eth_dev = ctx->device;
+				eth_dev = sec_ctx->device;
 				rte_eth_dev_rss_hash_conf_get(sa->portid,
 							      &rss_conf);
 				for (i = 0, j = 0;
@@ -252,12 +311,12 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 			}
 		} else if (sa->type ==
 				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
-			struct rte_security_ctx *ctx =
-					(struct rte_security_ctx *)
-					rte_eth_dev_get_sec_ctx(sa->portid);
 			const struct rte_security_capability *sec_cap;
 
-			if (ctx == NULL) {
+			sec_ctx = (struct rte_security_ctx *)
+					rte_eth_dev_get_sec_ctx(sa->portid);
+
+			if (sec_ctx == NULL) {
 				RTE_LOG(ERR, IPSEC,
 				"Ethernet device doesn't have security features registered\n");
 				return -1;
@@ -279,15 +338,15 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 
 			sess_conf.userdata = (void *) sa;
 
-			sa->sec_session = rte_security_session_create(ctx,
-					&sess_conf, ipsec_ctx->session_pool);
+			sa->sec_session = rte_security_session_create(sec_ctx,
+					&sess_conf, skt_ctx->session_pool);
 			if (sa->sec_session == NULL) {
 				RTE_LOG(ERR, IPSEC,
 				"SEC Session init failed: err: %d\n", ret);
 				return -1;
 			}
 
-			sec_cap = rte_security_capabilities_get(ctx);
+			sec_cap = rte_security_capabilities_get(sec_ctx);
 
 			if (sec_cap == NULL) {
 				RTE_LOG(ERR, IPSEC,
@@ -316,17 +375,8 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa)
 			}
 
 			sa->ol_flags = sec_cap->ol_flags;
-			sa->security_ctx = ctx;
+			sa->security_ctx = sec_ctx;
 		}
-	} else {
-		sa->crypto_session = rte_cryptodev_sym_session_create(
-				ipsec_ctx->session_pool);
-		rte_cryptodev_sym_session_init(ipsec_ctx->tbl[cdev_id_qp].id,
-				sa->crypto_session, sa->xforms,
-				ipsec_ctx->session_priv_pool);
-
-		rte_cryptodev_info_get(ipsec_ctx->tbl[cdev_id_qp].id,
-				&cdev_info);
 	}
 	sa->cdev_id_qp = cdev_id_qp;
 
@@ -395,7 +445,7 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 			rte_prefetch0(&priv->sym_cop);
 
 			if ((unlikely(sa->sec_session == NULL)) &&
-					create_session(ipsec_ctx, sa)) {
+				create_lookaside_session(ipsec_ctx, sa)) {
 				rte_pktmbuf_free(pkts[i]);
 				continue;
 			}
@@ -414,7 +464,7 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 			rte_prefetch0(&priv->sym_cop);
 
 			if ((unlikely(sa->crypto_session == NULL)) &&
-					create_session(ipsec_ctx, sa)) {
+				create_lookaside_session(ipsec_ctx, sa)) {
 				rte_pktmbuf_free(pkts[i]);
 				continue;
 			}
@@ -429,12 +479,7 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 			}
 			break;
 		case RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL:
-			if ((unlikely(sa->sec_session == NULL)) &&
-					create_session(ipsec_ctx, sa)) {
-				rte_pktmbuf_free(pkts[i]);
-				continue;
-			}
-
+			RTE_ASSERT(sa->sec_session != NULL);
 			ipsec_ctx->ol_pkts[ipsec_ctx->ol_pkts_cnt++] = pkts[i];
 			if (sa->ol_flags & RTE_SECURITY_TX_OLOAD_NEED_MDATA)
 				rte_security_set_pkt_metadata(
@@ -442,17 +487,11 @@ ipsec_enqueue(ipsec_xform_fn xform_func, struct ipsec_ctx *ipsec_ctx,
 						sa->sec_session, pkts[i], NULL);
 			continue;
 		case RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO:
+			RTE_ASSERT(sa->sec_session != NULL);
 			priv->cop.type = RTE_CRYPTO_OP_TYPE_SYMMETRIC;
 			priv->cop.status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
 
 			rte_prefetch0(&priv->sym_cop);
-
-			if ((unlikely(sa->sec_session == NULL)) &&
-					create_session(ipsec_ctx, sa)) {
-				rte_pktmbuf_free(pkts[i]);
-				continue;
-			}
-
 			rte_security_attach_session(&priv->cop,
 					sa->sec_session);
 
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index 99f49d6..db5a5c1 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -306,6 +306,9 @@ void
 enqueue_cop_burst(struct cdev_qp *cqp);
 
 int
-create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa);
+create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa);
+
+int
+create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa);
 
 #endif /* __IPSEC_H__ */
diff --git a/examples/ipsec-secgw/ipsec_process.c b/examples/ipsec-secgw/ipsec_process.c
index e403c46..152a684 100644
--- a/examples/ipsec-secgw/ipsec_process.c
+++ b/examples/ipsec-secgw/ipsec_process.c
@@ -95,22 +95,23 @@ fill_ipsec_session(struct rte_ipsec_session *ss, struct ipsec_ctx *ctx,
 	/* setup crypto section */
 	if (ss->type == RTE_SECURITY_ACTION_TYPE_NONE) {
 		if (sa->crypto_session == NULL) {
-			rc = create_session(ctx, sa);
+			rc = create_lookaside_session(ctx, sa);
 			if (rc != 0)
 				return rc;
 		}
 		ss->crypto.ses = sa->crypto_session;
 	/* setup session action type */
-	} else {
+	} else if (sa->type == RTE_SECURITY_ACTION_TYPE_LOOKASIDE_PROTOCOL) {
 		if (sa->sec_session == NULL) {
-			rc = create_session(ctx, sa);
+			rc = create_lookaside_session(ctx, sa);
 			if (rc != 0)
 				return rc;
 		}
 		ss->security.ses = sa->sec_session;
 		ss->security.ctx = sa->security_ctx;
 		ss->security.ol_flags = sa->ol_flags;
-	}
+	} else
+		RTE_ASSERT(0);
 
 	rc = rte_ipsec_session_prepare(ss);
 	if (rc != 0)
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 414fcd2..7fb1929 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -762,11 +762,13 @@ check_eth_dev_caps(uint16_t portid, uint32_t inbound)
 
 static int
 sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
-		uint32_t nb_entries, uint32_t inbound)
+		uint32_t nb_entries, uint32_t inbound,
+		struct socket_ctx *skt_ctx)
 {
 	struct ipsec_sa *sa;
 	uint32_t i, idx;
 	uint16_t iv_length, aad_length;
+	int32_t rc;
 
 	/* for ESN upper 32 bits of SQN also need to be part of AAD */
 	aad_length = (app_sa_prm.enable_esn != 0) ? sizeof(uint32_t) : 0;
@@ -819,6 +821,17 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 
 			sa->xforms = &sa_ctx->xf[idx].a;
 
+			if (sa->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL ||
+				sa->type ==
+				RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO) {
+				rc = create_inline_session(skt_ctx, sa);
+				if (rc != 0) {
+					RTE_LOG(ERR, IPSEC_ESP,
+						"create_inline_session() failed\n");
+					return -EINVAL;
+				}
+			}
 			print_one_sa_rule(sa, inbound);
 		} else {
 			switch (sa->cipher_algo) {
@@ -894,16 +907,16 @@ sa_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
 
 static inline int
 sa_out_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
-		uint32_t nb_entries)
+		uint32_t nb_entries, struct socket_ctx *skt_ctx)
 {
-	return sa_add_rules(sa_ctx, entries, nb_entries, 0);
+	return sa_add_rules(sa_ctx, entries, nb_entries, 0, skt_ctx);
 }
 
 static inline int
 sa_in_add_rules(struct sa_ctx *sa_ctx, const struct ipsec_sa entries[],
-		uint32_t nb_entries)
+		uint32_t nb_entries, struct socket_ctx *skt_ctx)
 {
-	return sa_add_rules(sa_ctx, entries, nb_entries, 1);
+	return sa_add_rules(sa_ctx, entries, nb_entries, 1, skt_ctx);
 }
 
 /*
@@ -997,10 +1010,12 @@ fill_ipsec_sa_prm(struct rte_ipsec_sa_prm *prm, const struct ipsec_sa *ss,
 	return 0;
 }
 
-static void
+static int
 fill_ipsec_session(struct rte_ipsec_session *ss, struct rte_ipsec_sa *sa,
 	const struct ipsec_sa *lsa)
 {
+	int32_t rc = 0;
+
 	ss->sa = sa;
 	ss->type = lsa->type;
 
@@ -1013,6 +1028,17 @@ fill_ipsec_session(struct rte_ipsec_session *ss, struct rte_ipsec_sa *sa,
 		ss->security.ctx = lsa->security_ctx;
 		ss->security.ol_flags = lsa->ol_flags;
 	}
+
+	if (ss->type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO ||
+		ss->type == RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL) {
+		if (ss->security.ses != NULL) {
+			rc = rte_ipsec_session_prepare(ss);
+			if (rc != 0)
+				memset(ss, 0, sizeof(*ss));
+		}
+	}
+
+	return rc;
 }
 
 /*
@@ -1047,8 +1073,8 @@ ipsec_sa_init(struct ipsec_sa *lsa, struct rte_ipsec_sa *sa, uint32_t sa_size)
 	if (rc < 0)
 		return rc;
 
-	fill_ipsec_session(&lsa->ips, sa, lsa);
-	return 0;
+	rc = fill_ipsec_session(&lsa->ips, sa, lsa);
+	return rc;
 }
 
 /*
@@ -1126,7 +1152,7 @@ sa_init(struct socket_ctx *ctx, int32_t socket_id)
 				"context %s in socket %d\n", rte_errno,
 				name, socket_id);
 
-		sa_in_add_rules(ctx->sa_in, sa_in, nb_sa_in);
+		sa_in_add_rules(ctx->sa_in, sa_in, nb_sa_in, ctx);
 
 		if (app_sa_prm.enable != 0) {
 			rc = ipsec_satbl_init(ctx->sa_in, sa_in, nb_sa_in,
@@ -1146,7 +1172,7 @@ sa_init(struct socket_ctx *ctx, int32_t socket_id)
 				"context %s in socket %d\n", rte_errno,
 				name, socket_id);
 
-		sa_out_add_rules(ctx->sa_out, sa_out, nb_sa_out);
+		sa_out_add_rules(ctx->sa_out, sa_out, nb_sa_out, ctx);
 
 		if (app_sa_prm.enable != 0) {
 			rc = ipsec_satbl_init(ctx->sa_out, sa_out, nb_sa_out,
-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-stable] [PATCH v2 2/2] examples/ipsec-secgw/test: fix inline test scripts
       [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
                   ` (6 preceding siblings ...)
  2019-03-07 14:57 ` [dpdk-stable] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto Bernard Iremonger
@ 2019-03-07 14:57 ` Bernard Iremonger
  7 siblings, 0 replies; 17+ messages in thread
From: Bernard Iremonger @ 2019-03-07 14:57 UTC (permalink / raw)
  To: dev, konstantin.ananyev, akhil.goyal; +Cc: Bernard Iremonger, stable

Remove workaround in tun_aesgcm_defs.sh and trs_aesgcm_defs.sh
to get around the bug where the first inbound packet is dropped
for inline crypto.

Fixes: 929784452094 ("examples/ipsec-secgw: add scripts for functional test")
Cc: stable@dpdk.org

Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
---
 examples/ipsec-secgw/test/trs_aesgcm_defs.sh | 10 ----------
 examples/ipsec-secgw/test/tun_aesgcm_defs.sh | 10 ----------
 2 files changed, 20 deletions(-)

diff --git a/examples/ipsec-secgw/test/trs_aesgcm_defs.sh b/examples/ipsec-secgw/test/trs_aesgcm_defs.sh
index a4d902b..8382d3d 100644
--- a/examples/ipsec-secgw/test/trs_aesgcm_defs.sh
+++ b/examples/ipsec-secgw/test/trs_aesgcm_defs.sh
@@ -33,11 +33,6 @@ aead "rfc4106\(gcm\(aes\)\)" \
 
 	ssh ${REMOTE_HOST} ip xfrm policy list
 	ssh ${REMOTE_HOST} ip xfrm state list
-
-	# to overcome problem with ipsec-secgw for inline mode,
-	# when first packet(s) will be always dropped.
-	# note that ping will fail here
-	ssh ${REMOTE_HOST} ping -c 1 ${LOCAL_IPV4}
 }
 
 config6_remote_xfrm()
@@ -68,9 +63,4 @@ aead "rfc4106\(gcm\(aes\)\)" \
 
 	ssh ${REMOTE_HOST} ip xfrm policy list
 	ssh ${REMOTE_HOST} ip xfrm state list
-
-	# to overcome problem with ipsec-secgw for inline mode,
-	# when first packet(s) will be always dropped.
-	# note that ping will fail here
-	ssh ${REMOTE_HOST} ping -c 1 ${LOCAL_IPV6}
 }
diff --git a/examples/ipsec-secgw/test/tun_aesgcm_defs.sh b/examples/ipsec-secgw/test/tun_aesgcm_defs.sh
index 1764ef6..8ae6532 100644
--- a/examples/ipsec-secgw/test/tun_aesgcm_defs.sh
+++ b/examples/ipsec-secgw/test/tun_aesgcm_defs.sh
@@ -35,11 +35,6 @@ aead "rfc4106\(gcm\(aes\)\)" \
 
 	ssh ${REMOTE_HOST} ip xfrm policy list
 	ssh ${REMOTE_HOST} ip xfrm state list
-
-	# to overcome problem with ipsec-secgw for inline mode,
-	# when first packet(s) will be always dropped.
-	# note that ping will fail here
-	ssh ${REMOTE_HOST} ping -c 1 ${LOCAL_IPV4}
 }
 
 config6_remote_xfrm()
@@ -72,9 +67,4 @@ aead "rfc4106\(gcm\(aes\)\)" \
 
 	ssh ${REMOTE_HOST} ip xfrm policy list
 	ssh ${REMOTE_HOST} ip xfrm state list
-
-	# to overcome problem with ipsec-secgw for inline mode,
-	# when first packet(s) will be always dropped.
-	# note that ping will fail here
-	ssh ${REMOTE_HOST} ping6 -c 1 ${LOCAL_IPV6}
 }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
  2019-03-07 14:57 ` [dpdk-stable] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto Bernard Iremonger
@ 2019-03-22 13:18   ` Akhil Goyal
  2019-03-26 10:22     ` Iremonger, Bernard
  0 siblings, 1 reply; 17+ messages in thread
From: Akhil Goyal @ 2019-03-22 13:18 UTC (permalink / raw)
  To: Bernard Iremonger, dev, konstantin.ananyev; +Cc: stable

Hi Bernard,

On 3/7/2019 8:27 PM, Bernard Iremonger wrote:
> Inline crypto installs a flow rule in the NIC. This flow
> rule must be installed before the first inbound packet is
> received.
>
> The create_session() function installs the flow rule,
> create_session() has been refactored into create_inline_session()
> and create_lookaside_session(). The create_inline_session() function
> uses the socket_ctx data and is now called at initialisation in
> sa_add_rules().
why do we need a separate function for session creation for inline and 
lookaside cases?
Why can't we initialize the sessions on sa_init in both the cases?
>
> The max_session_size() function has been added to calculate memory
> requirements.
>
> The cryprodev_init() function has been refactored to drop calls to
> rte_mempool_create() and to drop calculation of memory requirements.
>
> The main() function has been refactored to call max_session_size() and
> to call session_pool_init() and session_priv_pool_init() earlier.
> The ports are started now before adding a flow rule in main().
> The sa_init(), sp4_init(), sp6_init() and rt_init() functions are
> now called after the ports have been started.
>
> The rte_ipsec_session_prepare() function is called in fill_ipsec_session()
> for inline which is called from the ipsec_sa_init() function.
>
> Fixes: ec17993a145a ("examples/ipsec-secgw: support security offload")
> Fixes: d299106e8e31 ("examples/ipsec-secgw: add IPsec sample application")
> Cc: stable@dpdk.org
>
> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
> ---
>   examples/ipsec-secgw/ipsec-secgw.c   | 243 +++++++++++++++++------------------
>   examples/ipsec-secgw/ipsec.c         | 123 ++++++++++++------
>   examples/ipsec-secgw/ipsec.h         |   5 +-
>   examples/ipsec-secgw/ipsec_process.c |   9 +-
>   examples/ipsec-secgw/sa.c            |  46 +++++--
>   5 files changed, 246 insertions(+), 180 deletions(-)
>
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
  2019-03-22 13:18   ` [dpdk-stable] [dpdk-dev] " Akhil Goyal
@ 2019-03-26 10:22     ` Iremonger, Bernard
  2019-03-26 11:04       ` Akhil Goyal
  0 siblings, 1 reply; 17+ messages in thread
From: Iremonger, Bernard @ 2019-03-26 10:22 UTC (permalink / raw)
  To: Akhil Goyal, dev, Ananyev, Konstantin; +Cc: stable

Hi Akhil,

<snip>
> Subject: Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt
> dropped for inline crypto
> 
> Hi Bernard,
> 
> On 3/7/2019 8:27 PM, Bernard Iremonger wrote:
> > Inline crypto installs a flow rule in the NIC. This flow rule must be
> > installed before the first inbound packet is received.
> >
> > The create_session() function installs the flow rule,
> > create_session() has been refactored into create_inline_session() and
> > create_lookaside_session(). The create_inline_session() function uses
> > the socket_ctx data and is now called at initialisation in
> > sa_add_rules().
> why do we need a separate function for session creation for inline and
> lookaside cases?
> Why can't we initialize the sessions on sa_init in both the cases?

For the create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa) function,
all of the required data is available in the in the skt_ctx variable.
The skt_ctx variable is already setup when sa_init() is called.

For the create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa) function,
the required data is available in the ipsec_ctx variable.

The ipsec_ctx variable is not setup when sa_init() is called.
It is setup in the main_loop() function  when the variable qconf is setup.
The main_loop() function is called after the sa_init() function is called.

I hope this  answers your question

<snip>

Regards,

Bernard.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
  2019-03-26 10:22     ` Iremonger, Bernard
@ 2019-03-26 11:04       ` Akhil Goyal
  2019-03-26 11:41         ` Iremonger, Bernard
  0 siblings, 1 reply; 17+ messages in thread
From: Akhil Goyal @ 2019-03-26 11:04 UTC (permalink / raw)
  To: Iremonger, Bernard, dev, Ananyev, Konstantin; +Cc: stable

Hi Bernard,

On 3/26/2019 3:52 PM, Iremonger, Bernard wrote:
> Hi Akhil,
>
> <snip>
>> Subject: Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt
>> dropped for inline crypto
>>
>> Hi Bernard,
>>
>> On 3/7/2019 8:27 PM, Bernard Iremonger wrote:
>>> Inline crypto installs a flow rule in the NIC. This flow rule must be
>>> installed before the first inbound packet is received.
>>>
>>> The create_session() function installs the flow rule,
>>> create_session() has been refactored into create_inline_session() and
>>> create_lookaside_session(). The create_inline_session() function uses
>>> the socket_ctx data and is now called at initialisation in
>>> sa_add_rules().
>> why do we need a separate function for session creation for inline and
>> lookaside cases?
>> Why can't we initialize the sessions on sa_init in both the cases?
> For the create_inline_session(struct socket_ctx *skt_ctx, struct ipsec_sa *sa) function,
> all of the required data is available in the in the skt_ctx variable.
> The skt_ctx variable is already setup when sa_init() is called.
>
> For the create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa *sa) function,
> the required data is available in the ipsec_ctx variable.
>
> The ipsec_ctx variable is not setup when sa_init() is called.
> It is setup in the main_loop() function  when the variable qconf is setup.
> The main_loop() function is called after the sa_init() function is called.
>
> I hope this  answers your question
Whatever information that is required for session creation is available 
before we call the main loop() in both the cases.
My point is both the sessions(inline/lookaside) can be init at the same 
position, we do not need to have a separate path for them.
If it is not possible in sa_init(), it may be somewhere else before the 
actual data path is started.

The problem with inline processing is that, h/w need to know the SA 
before the first packet is received. So we cannot init the session on 
receive of first packet. However there is no such limitation in case of 
lookaside, it can be initialized anywhere.

-Akhil

>
> <snip>
>
> Regards,
>
> Bernard.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
  2019-03-26 11:04       ` Akhil Goyal
@ 2019-03-26 11:41         ` Iremonger, Bernard
  2019-03-26 11:48           ` Akhil Goyal
  0 siblings, 1 reply; 17+ messages in thread
From: Iremonger, Bernard @ 2019-03-26 11:41 UTC (permalink / raw)
  To: Akhil Goyal, dev, Ananyev, Konstantin; +Cc: stable

Hi Akhil,

<snip>

> >> Subject: Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st
> >> pkt dropped for inline crypto
> >>
> >> Hi Bernard,
> >>
> >> On 3/7/2019 8:27 PM, Bernard Iremonger wrote:
> >>> Inline crypto installs a flow rule in the NIC. This flow rule must
> >>> be installed before the first inbound packet is received.
> >>>
> >>> The create_session() function installs the flow rule,
> >>> create_session() has been refactored into create_inline_session()
> >>> and create_lookaside_session(). The create_inline_session() function
> >>> uses the socket_ctx data and is now called at initialisation in
> >>> sa_add_rules().
> >> why do we need a separate function for session creation for inline
> >> and lookaside cases?
> >> Why can't we initialize the sessions on sa_init in both the cases?
> > For the create_inline_session(struct socket_ctx *skt_ctx, struct
> > ipsec_sa *sa) function, all of the required data is available in the in the
> skt_ctx variable.
> > The skt_ctx variable is already setup when sa_init() is called.
> >
> > For the create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct
> > ipsec_sa *sa) function, the required data is available in the ipsec_ctx
> variable.
> >
> > The ipsec_ctx variable is not setup when sa_init() is called.
> > It is setup in the main_loop() function  when the variable qconf is setup.
> > The main_loop() function is called after the sa_init() function is called.
> >
> > I hope this  answers your question
> Whatever information that is required for session creation is available before
> we call the main loop() in both the cases.
> My point is both the sessions(inline/lookaside) can be init at the same
> position, we do not need to have a separate path for them.
> If it is not possible in sa_init(), it may be somewhere else before the actual
> data path is started.
> 
> The problem with inline processing is that, h/w need to know the SA before
> the first packet is received. So we cannot init the session on receive of first
> packet. However there is no such limitation in case of lookaside, it can be
> initialized anywhere.
> 
> -Akhil

This patch is intended to fix the bug in the inline processing, which is that  the flow rule must be installed, before the first packet is received while leaving the lookaside processing as it was originally.

It is not intended to refactor the lookaside processing.

Regards,

Bernard

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
  2019-03-26 11:41         ` Iremonger, Bernard
@ 2019-03-26 11:48           ` Akhil Goyal
  2019-03-26 12:29             ` Ananyev, Konstantin
  0 siblings, 1 reply; 17+ messages in thread
From: Akhil Goyal @ 2019-03-26 11:48 UTC (permalink / raw)
  To: Iremonger, Bernard, dev, Ananyev, Konstantin; +Cc: stable

Hi Bernard,

On 3/26/2019 5:11 PM, Iremonger, Bernard wrote:
> Hi Akhil,
>
> <snip>
>
>>>> Subject: Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st
>>>> pkt dropped for inline crypto
>>>>
>>>> Hi Bernard,
>>>>
>>>> On 3/7/2019 8:27 PM, Bernard Iremonger wrote:
>>>>> Inline crypto installs a flow rule in the NIC. This flow rule must
>>>>> be installed before the first inbound packet is received.
>>>>>
>>>>> The create_session() function installs the flow rule,
>>>>> create_session() has been refactored into create_inline_session()
>>>>> and create_lookaside_session(). The create_inline_session() function
>>>>> uses the socket_ctx data and is now called at initialisation in
>>>>> sa_add_rules().
>>>> why do we need a separate function for session creation for inline
>>>> and lookaside cases?
>>>> Why can't we initialize the sessions on sa_init in both the cases?
>>> For the create_inline_session(struct socket_ctx *skt_ctx, struct
>>> ipsec_sa *sa) function, all of the required data is available in the in the
>> skt_ctx variable.
>>> The skt_ctx variable is already setup when sa_init() is called.
>>>
>>> For the create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct
>>> ipsec_sa *sa) function, the required data is available in the ipsec_ctx
>> variable.
>>> The ipsec_ctx variable is not setup when sa_init() is called.
>>> It is setup in the main_loop() function  when the variable qconf is setup.
>>> The main_loop() function is called after the sa_init() function is called.
>>>
>>> I hope this  answers your question
>> Whatever information that is required for session creation is available before
>> we call the main loop() in both the cases.
>> My point is both the sessions(inline/lookaside) can be init at the same
>> position, we do not need to have a separate path for them.
>> If it is not possible in sa_init(), it may be somewhere else before the actual
>> data path is started.
>>
>> The problem with inline processing is that, h/w need to know the SA before
>> the first packet is received. So we cannot init the session on receive of first
>> packet. However there is no such limitation in case of lookaside, it can be
>> initialized anywhere.
>>
>> -Akhil
> This patch is intended to fix the bug in the inline processing, which is that  the flow rule must be installed, before the first packet is received while leaving the lookaside processing as it was originally.
>
> It is not intended to refactor the lookaside processing.
The fix provided should not make the code complex and difficult to 
understand when we have the option available to fix that in a simpler 
way. Application is already complex enough(4 action types) and by adding 
separate code for session init at two different places will be difficult 
to maintain in future and will reduce the code readability.

-Akhil

> Regards,
>
> Bernard


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
  2019-03-26 11:48           ` Akhil Goyal
@ 2019-03-26 12:29             ` Ananyev, Konstantin
  2019-03-26 12:55               ` Akhil Goyal
  0 siblings, 1 reply; 17+ messages in thread
From: Ananyev, Konstantin @ 2019-03-26 12:29 UTC (permalink / raw)
  To: Akhil Goyal, Iremonger, Bernard, dev; +Cc: stable


Hi Akhil,

> >
> >>>> Subject: Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st
> >>>> pkt dropped for inline crypto
> >>>>
> >>>> Hi Bernard,
> >>>>
> >>>> On 3/7/2019 8:27 PM, Bernard Iremonger wrote:
> >>>>> Inline crypto installs a flow rule in the NIC. This flow rule must
> >>>>> be installed before the first inbound packet is received.
> >>>>>
> >>>>> The create_session() function installs the flow rule,
> >>>>> create_session() has been refactored into create_inline_session()
> >>>>> and create_lookaside_session(). The create_inline_session() function
> >>>>> uses the socket_ctx data and is now called at initialisation in
> >>>>> sa_add_rules().
> >>>> why do we need a separate function for session creation for inline
> >>>> and lookaside cases?
> >>>> Why can't we initialize the sessions on sa_init in both the cases?
> >>> For the create_inline_session(struct socket_ctx *skt_ctx, struct
> >>> ipsec_sa *sa) function, all of the required data is available in the in the
> >> skt_ctx variable.
> >>> The skt_ctx variable is already setup when sa_init() is called.
> >>>
> >>> For the create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct
> >>> ipsec_sa *sa) function, the required data is available in the ipsec_ctx
> >> variable.
> >>> The ipsec_ctx variable is not setup when sa_init() is called.
> >>> It is setup in the main_loop() function  when the variable qconf is setup.
> >>> The main_loop() function is called after the sa_init() function is called.
> >>>
> >>> I hope this  answers your question
> >> Whatever information that is required for session creation is available before
> >> we call the main loop() in both the cases.
> >> My point is both the sessions(inline/lookaside) can be init at the same
> >> position, we do not need to have a separate path for them.
> >> If it is not possible in sa_init(), it may be somewhere else before the actual
> >> data path is started.
> >>
> >> The problem with inline processing is that, h/w need to know the SA before
> >> the first packet is received. So we cannot init the session on receive of first
> >> packet. However there is no such limitation in case of lookaside, it can be
> >> initialized anywhere.
> >>
> >> -Akhil
> > This patch is intended to fix the bug in the inline processing, which is that  the flow rule must be installed, before the first packet is
> received while leaving the lookaside processing as it was originally.
> >
> > It is not intended to refactor the lookaside processing.
> The fix provided should not make the code complex and difficult to
> understand when we have the option available to fix that in a simpler
> way. Application is already complex enough(4 action types) and by adding
> separate code for session init at two different places will be difficult
> to maintain in future and will reduce the code readability.

It can be changed to do initialization for both security and crypto sessions at sa_init()
time, but it would cause much more changes in the code:
the idea is for each SA go through all available crypto-devices, check their capabilities,
and if they match, we'll invoke init_session() for that SA and device.
That way at the end of sa_init() we'll have crypto-sessions that can be used on any attached
crypto-dev.
As a drawback -  each session might occupy more memory.
Though as I said, it would require more changes (and testing) in init code.
So my thought to have this patch as it is and then add these changes as a separate patch series (phase 2).
What do you think?
Konstantin


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto
  2019-03-26 12:29             ` Ananyev, Konstantin
@ 2019-03-26 12:55               ` Akhil Goyal
  0 siblings, 0 replies; 17+ messages in thread
From: Akhil Goyal @ 2019-03-26 12:55 UTC (permalink / raw)
  To: Ananyev, Konstantin, Iremonger, Bernard, dev; +Cc: stable

Hi Konstantin,

On 3/26/2019 5:59 PM, Ananyev, Konstantin wrote:
> Hi Akhil,
>
>>>>>> Subject: Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st
>>>>>> pkt dropped for inline crypto
>>>>>>
>>>>>> Hi Bernard,
>>>>>>
>>>>>> On 3/7/2019 8:27 PM, Bernard Iremonger wrote:
>>>>>>> Inline crypto installs a flow rule in the NIC. This flow rule must
>>>>>>> be installed before the first inbound packet is received.
>>>>>>>
>>>>>>> The create_session() function installs the flow rule,
>>>>>>> create_session() has been refactored into create_inline_session()
>>>>>>> and create_lookaside_session(). The create_inline_session() function
>>>>>>> uses the socket_ctx data and is now called at initialisation in
>>>>>>> sa_add_rules().
>>>>>> why do we need a separate function for session creation for inline
>>>>>> and lookaside cases?
>>>>>> Why can't we initialize the sessions on sa_init in both the cases?
>>>>> For the create_inline_session(struct socket_ctx *skt_ctx, struct
>>>>> ipsec_sa *sa) function, all of the required data is available in the in the
>>>> skt_ctx variable.
>>>>> The skt_ctx variable is already setup when sa_init() is called.
>>>>>
>>>>> For the create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct
>>>>> ipsec_sa *sa) function, the required data is available in the ipsec_ctx
>>>> variable.
>>>>> The ipsec_ctx variable is not setup when sa_init() is called.
>>>>> It is setup in the main_loop() function  when the variable qconf is setup.
>>>>> The main_loop() function is called after the sa_init() function is called.
>>>>>
>>>>> I hope this  answers your question
>>>> Whatever information that is required for session creation is available before
>>>> we call the main loop() in both the cases.
>>>> My point is both the sessions(inline/lookaside) can be init at the same
>>>> position, we do not need to have a separate path for them.
>>>> If it is not possible in sa_init(), it may be somewhere else before the actual
>>>> data path is started.
>>>>
>>>> The problem with inline processing is that, h/w need to know the SA before
>>>> the first packet is received. So we cannot init the session on receive of first
>>>> packet. However there is no such limitation in case of lookaside, it can be
>>>> initialized anywhere.
>>>>
>>>> -Akhil
>>> This patch is intended to fix the bug in the inline processing, which is that  the flow rule must be installed, before the first packet is
>> received while leaving the lookaside processing as it was originally.
>>> It is not intended to refactor the lookaside processing.
>> The fix provided should not make the code complex and difficult to
>> understand when we have the option available to fix that in a simpler
>> way. Application is already complex enough(4 action types) and by adding
>> separate code for session init at two different places will be difficult
>> to maintain in future and will reduce the code readability.
> It can be changed to do initialization for both security and crypto sessions at sa_init()
> time, but it would cause much more changes in the code:
> the idea is for each SA go through all available crypto-devices, check their capabilities,
> and if they match, we'll invoke init_session() for that SA and device.
> That way at the end of sa_init() we'll have crypto-sessions that can be used on any attached
> crypto-dev.
Agreed, my point is we should have same code path for all action types 
as much as possible to make the user comfortable to port their 
applications over DPDK.
> As a drawback -  each session might occupy more memory.
That is still there for the inline cases.
> Though as I said, it would require more changes (and testing) in init code.
> So my thought to have this patch as it is and then add these changes as a separate patch series (phase 2).
> What do you think?
The code changes are still there in this patch as well. And the testing 
cycle has not started yet for this release.

5 files changed, 246 insertions(+), 180 deletions(-)

If we intend to defer it, we will have similar code movement again. So IMO, it is good that we do the changes in one go.
This is an application code and we can have it merged in RC2 as well.

> Konstantin
>


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-03-26 12:55 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1551888011-27692-1-git-send-email-bernard.iremonger@intel.com>
2019-03-06 16:00 ` [dpdk-stable] [PATCH 1/6] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto Bernard Iremonger
2019-03-06 16:00 ` [dpdk-stable] [PATCH 2/6] examples/ipsec-secgw: fix 1st packet dropped patch two Bernard Iremonger
2019-03-06 19:39   ` Ananyev, Konstantin
2019-03-07  9:54     ` Iremonger, Bernard
2019-03-06 16:00 ` [dpdk-stable] [PATCH 3/6] examples/ipsec-secgw: fix 1st packet dropped patch three Bernard Iremonger
2019-03-06 16:00 ` [dpdk-stable] [PATCH 4/6] examples/ipsec-secgw: fix debug in esp.c Bernard Iremonger
2019-03-06 16:00 ` [dpdk-stable] [PATCH 5/6] examples/ipsec-secgw: fix debug in sa.c Bernard Iremonger
2019-03-06 16:00 ` [dpdk-stable] [PATCH 6/6] examples/ipsec-secgw: fix debug in ipsec-secgw.c Bernard Iremonger
2019-03-07 14:57 ` [dpdk-stable] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st pkt dropped for inline crypto Bernard Iremonger
2019-03-22 13:18   ` [dpdk-stable] [dpdk-dev] " Akhil Goyal
2019-03-26 10:22     ` Iremonger, Bernard
2019-03-26 11:04       ` Akhil Goyal
2019-03-26 11:41         ` Iremonger, Bernard
2019-03-26 11:48           ` Akhil Goyal
2019-03-26 12:29             ` Ananyev, Konstantin
2019-03-26 12:55               ` Akhil Goyal
2019-03-07 14:57 ` [dpdk-stable] [PATCH v2 2/2] examples/ipsec-secgw/test: fix inline test scripts Bernard Iremonger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).