DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 1/4] examples/ipsec-secgw: update error prints to data path log
@ 2022-02-06 14:30 Nithin Dabilpuram
  2022-02-06 14:30 ` [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-06 14:30 UTC (permalink / raw)
  To: jerinj, Radu Nicolau, Akhil Goyal; +Cc: dev, Nithin Dabilpuram

Update error prints in data path to RTE_LOG_DP().
Error prints in fast path are not good for performance
as they slow down the application when few bad packets are
received.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 examples/ipsec-secgw/ipsec_worker.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec_worker.c b/examples/ipsec-secgw/ipsec_worker.c
index 7419e85..e9493c5 100644
--- a/examples/ipsec-secgw/ipsec_worker.c
+++ b/examples/ipsec-secgw/ipsec_worker.c
@@ -332,7 +332,8 @@ process_ipsec_ev_inbound(struct ipsec_ctx *ctx, struct route_table *rt,
 		break;
 
 	default:
-		RTE_LOG(ERR, IPSEC, "Unsupported packet type = %d\n", type);
+		RTE_LOG_DP(DEBUG, IPSEC_ESP, "Unsupported packet type = %d\n",
+			   type);
 		goto drop_pkt_and_exit;
 	}
 
@@ -570,7 +571,8 @@ classify_pkt(struct rte_mbuf *pkt, struct ipsec_traffic *t)
 		t->ip6.pkts[(t->ip6.num)++] = pkt;
 		break;
 	default:
-		RTE_LOG(ERR, IPSEC, "Unsupported packet type = %d\n", type);
+		RTE_LOG_DP(DEBUG, IPSEC_ESP, "Unsupported packet type = %d\n",
+			   type);
 		free_pkts(&pkt, 1);
 		break;
 	}
-- 
2.8.4


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

* [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-06 14:30 [PATCH 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
@ 2022-02-06 14:30 ` Nithin Dabilpuram
  2022-02-07  9:52   ` Ananyev, Konstantin
  2022-02-06 14:30 ` [PATCH 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-06 14:30 UTC (permalink / raw)
  To: jerinj, Radu Nicolau, Akhil Goyal; +Cc: dev, Nithin Dabilpuram

Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
In other cases such as Tx Inline protocol offload, checksum computation
is implicitly taken care by HW. The advantage of having only necessary
offloads enabled is that Tx burst function can be as light as possible.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 examples/ipsec-secgw/ipsec-secgw.c | 3 ---
 examples/ipsec-secgw/sa.c          | 9 +++++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 21abc0d..d8a9bfa 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 		local_port_conf.txmode.offloads |=
 			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
 
-	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
-		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
-
 	printf("port %u configuring rx_offloads=0x%" PRIx64
 		", tx_offloads=0x%" PRIx64 "\n",
 		portid, local_port_conf.rxmode.offloads,
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 1839ac7..b878a48 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
 				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
 				&& rule->portid == port_id) {
 			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
+
+			/* Checksum offload is not needed for inline protocol as
+			 * all processing for Outbound IPSec packets will be
+			 * implicitly taken care and for non-IPSec packets,
+			 * there is no need of IPv4 Checksum offload.
+			 */
+			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
+				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+
 			if (rule->mss)
 				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
 		}
-- 
2.8.4


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

* [PATCH 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode
  2022-02-06 14:30 [PATCH 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
  2022-02-06 14:30 ` [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
@ 2022-02-06 14:30 ` Nithin Dabilpuram
  2022-02-06 14:30 ` [PATCH 4/4] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-06 14:30 UTC (permalink / raw)
  To: jerinj, Radu Nicolau, Akhil Goyal; +Cc: dev, Nithin Dabilpuram, schalla, stable

Fix packet processing to skip after mbuf is freed instead of
touching and Tx'ing it.

Also free vector event buffer in event worker when after processing
there is no pkt to be enqueued to Tx adapter.

Fixes: 86738ebe1e3d ("examples/ipsec-secgw: support event vector")
Cc: schalla@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 examples/ipsec-secgw/ipsec_worker.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec_worker.c b/examples/ipsec-secgw/ipsec_worker.c
index e9493c5..8639426 100644
--- a/examples/ipsec-secgw/ipsec_worker.c
+++ b/examples/ipsec-secgw/ipsec_worker.c
@@ -205,12 +205,16 @@ check_sp_sa_bulk(struct sp_ctx *sp, struct sa_ctx *sa_ctx,
 			ip->pkts[j++] = m;
 		else {
 			sa = *(struct ipsec_sa **)rte_security_dynfield(m);
-			if (sa == NULL)
+			if (sa == NULL) {
 				free_pkts(&m, 1);
+				continue;
+			}
 
 			/* SPI on the packet should match with the one in SA */
-			if (unlikely(sa->spi != sa_ctx->sa[res - 1].spi))
+			if (unlikely(sa->spi != sa_ctx->sa[res - 1].spi)) {
 				free_pkts(&m, 1);
+				continue;
+			}
 
 			ip->pkts[j++] = m;
 		}
@@ -536,6 +540,7 @@ ipsec_ev_route_pkts(struct rte_event_vector *vec, struct route_table *rt,
 				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)) {
 				RTE_LOG(ERR, IPSEC, "SA type not supported\n");
 				free_pkts(&pkt, 1);
+				continue;
 			}
 			rte_security_set_pkt_metadata(sess->security.ctx,
 						sess->security.ses, pkt, NULL);
@@ -695,11 +700,13 @@ ipsec_ev_vector_process(struct lcore_conf_ev_tx_int_port_wrkr *lconf,
 		ret = process_ipsec_ev_outbound_vector(&lconf->outbound,
 						       &lconf->rt, vec);
 
-	if (ret > 0) {
+	if (likely(ret > 0)) {
 		vec->nb_elem = ret;
 		rte_event_eth_tx_adapter_enqueue(links[0].eventdev_id,
 						 links[0].event_port_id,
 						 ev, 1, 0);
+	} else {
+		rte_mempool_put(rte_mempool_from_obj(vec), vec);
 	}
 }
 
@@ -720,6 +727,8 @@ ipsec_ev_vector_drv_mode_process(struct eh_event_link_info *links,
 		rte_event_eth_tx_adapter_enqueue(links[0].eventdev_id,
 						 links[0].event_port_id,
 						 ev, 1, 0);
+	else
+		rte_mempool_put(rte_mempool_from_obj(vec), vec);
 }
 
 /*
-- 
2.8.4


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

* [PATCH 4/4] examples/ipsec-secgw: add per port pool and vector pool size
  2022-02-06 14:30 [PATCH 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
  2022-02-06 14:30 ` [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
  2022-02-06 14:30 ` [PATCH 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
@ 2022-02-06 14:30 ` Nithin Dabilpuram
  2022-02-07  6:26 ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
  2022-02-23  9:53 ` [PATCH v3 1/3] " Nithin Dabilpuram
  4 siblings, 0 replies; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-06 14:30 UTC (permalink / raw)
  To: jerinj, Radu Nicolau, Akhil Goyal; +Cc: dev, Nithin Dabilpuram

Add support to enable per port packet pool and also override
vector pool size from command line args. This is useful
on some HW to tune performance based on usecase.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 examples/ipsec-secgw/event_helper.c | 17 ++++++--
 examples/ipsec-secgw/event_helper.h |  2 +
 examples/ipsec-secgw/ipsec-secgw.c  | 80 ++++++++++++++++++++++++++++---------
 examples/ipsec-secgw/ipsec-secgw.h  |  2 +
 examples/ipsec-secgw/ipsec.h        |  2 +-
 5 files changed, 80 insertions(+), 23 deletions(-)

diff --git a/examples/ipsec-secgw/event_helper.c b/examples/ipsec-secgw/event_helper.c
index 8947e41..172ab8e 100644
--- a/examples/ipsec-secgw/event_helper.c
+++ b/examples/ipsec-secgw/event_helper.c
@@ -792,8 +792,8 @@ eh_rx_adapter_configure(struct eventmode_conf *em_conf,
 	uint32_t service_id, socket_id, nb_elem;
 	struct rte_mempool *vector_pool = NULL;
 	uint32_t lcore_id = rte_lcore_id();
+	int ret, portid, nb_ports = 0;
 	uint8_t eventdev_id;
-	int ret;
 	int j;
 
 	/* Get event dev ID */
@@ -806,10 +806,21 @@ eh_rx_adapter_configure(struct eventmode_conf *em_conf,
 		return ret;
 	}
 
+	RTE_ETH_FOREACH_DEV(portid)
+		if ((em_conf->eth_portmask & (1 << portid)))
+			nb_ports++;
+
 	if (em_conf->ext_params.event_vector) {
 		socket_id = rte_lcore_to_socket_id(lcore_id);
-		nb_elem = (nb_bufs_in_pool / em_conf->ext_params.vector_size)
-			  + 1;
+
+		if (em_conf->vector_pool_sz) {
+			nb_elem = em_conf->vector_pool_sz;
+		} else {
+			nb_elem = (nb_bufs_in_pool /
+				   em_conf->ext_params.vector_size) + 1;
+			if (per_port_pool)
+				nb_elem = nb_ports * nb_elem;
+		}
 
 		vector_pool = rte_event_vector_pool_create(
 			"vector_pool", nb_elem, 0,
diff --git a/examples/ipsec-secgw/event_helper.h b/examples/ipsec-secgw/event_helper.h
index 5be6c62..f3cbe57 100644
--- a/examples/ipsec-secgw/event_helper.h
+++ b/examples/ipsec-secgw/event_helper.h
@@ -183,6 +183,8 @@ struct eventmode_conf {
 		/**< 64 bit field to specify extended params */
 	uint64_t vector_tmo_ns;
 		/**< Max vector timeout in nanoseconds */
+	uint64_t vector_pool_sz;
+		/**< Vector pool size */
 };
 
 /**
diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index d8a9bfa..96da776 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -118,6 +118,8 @@ struct flow_info flow_info_tbl[RTE_MAX_ETHPORTS];
 #define CMD_LINE_OPT_EVENT_VECTOR	"event-vector"
 #define CMD_LINE_OPT_VECTOR_SIZE	"vector-size"
 #define CMD_LINE_OPT_VECTOR_TIMEOUT	"vector-tmo"
+#define CMD_LINE_OPT_VECTOR_POOL_SZ	"vector-pool-sz"
+#define CMD_LINE_OPT_PER_PORT_POOL	"per-port-pool"
 
 #define CMD_LINE_ARG_EVENT	"event"
 #define CMD_LINE_ARG_POLL	"poll"
@@ -145,6 +147,8 @@ enum {
 	CMD_LINE_OPT_EVENT_VECTOR_NUM,
 	CMD_LINE_OPT_VECTOR_SIZE_NUM,
 	CMD_LINE_OPT_VECTOR_TIMEOUT_NUM,
+	CMD_LINE_OPT_VECTOR_POOL_SZ_NUM,
+	CMD_LINE_OPT_PER_PORT_POOL_NUM,
 };
 
 static const struct option lgopts[] = {
@@ -161,6 +165,8 @@ static const struct option lgopts[] = {
 	{CMD_LINE_OPT_EVENT_VECTOR, 0, 0, CMD_LINE_OPT_EVENT_VECTOR_NUM},
 	{CMD_LINE_OPT_VECTOR_SIZE, 1, 0, CMD_LINE_OPT_VECTOR_SIZE_NUM},
 	{CMD_LINE_OPT_VECTOR_TIMEOUT, 1, 0, CMD_LINE_OPT_VECTOR_TIMEOUT_NUM},
+	{CMD_LINE_OPT_VECTOR_POOL_SZ, 1, 0, CMD_LINE_OPT_VECTOR_POOL_SZ_NUM},
+	{CMD_LINE_OPT_PER_PORT_POOL, 0, 0, CMD_LINE_OPT_PER_PORT_POOL_NUM},
 	{NULL, 0, 0, 0}
 };
 
@@ -234,7 +240,6 @@ struct lcore_conf {
 	struct rt_ctx *rt6_ctx;
 	struct {
 		struct rte_ip_frag_tbl *tbl;
-		struct rte_mempool *pool_dir;
 		struct rte_mempool *pool_indir;
 		struct rte_ip_frag_death_row dr;
 	} frag;
@@ -262,6 +267,8 @@ static struct rte_eth_conf port_conf = {
 
 struct socket_ctx socket_ctx[NB_SOCKETS];
 
+bool per_port_pool;
+
 /*
  * Determine is multi-segment support required:
  *  - either frame buffer size is smaller then mtu
@@ -630,12 +637,10 @@ send_fragment_packet(struct lcore_conf *qconf, struct rte_mbuf *m,
 
 	if (proto == IPPROTO_IP)
 		rc = rte_ipv4_fragment_packet(m, tbl->m_table + len,
-			n, mtu_size, qconf->frag.pool_dir,
-			qconf->frag.pool_indir);
+			n, mtu_size, m->pool, qconf->frag.pool_indir);
 	else
 		rc = rte_ipv6_fragment_packet(m, tbl->m_table + len,
-			n, mtu_size, qconf->frag.pool_dir,
-			qconf->frag.pool_indir);
+			n, mtu_size, m->pool, qconf->frag.pool_indir);
 
 	if (rc >= 0)
 		len += rc;
@@ -1256,7 +1261,6 @@ ipsec_poll_mode_worker(void)
 	qconf->outbound.session_pool = socket_ctx[socket_id].session_pool;
 	qconf->outbound.session_priv_pool =
 			socket_ctx[socket_id].session_priv_pool;
-	qconf->frag.pool_dir = socket_ctx[socket_id].mbuf_pool;
 	qconf->frag.pool_indir = socket_ctx[socket_id].mbuf_pool_indir;
 
 	rc = ipsec_sad_lcore_cache_init(app_sa_prm.cache_sz);
@@ -1511,6 +1515,9 @@ print_usage(const char *prgname)
 		"  --vector-size Max vector size (default value: 16)\n"
 		"  --vector-tmo Max vector timeout in nanoseconds"
 		"    (default value: 102400)\n"
+		"  --" CMD_LINE_OPT_PER_PORT_POOL " Enable per port mbuf pool\n"
+		"  --" CMD_LINE_OPT_VECTOR_POOL_SZ " Vector pool size\n"
+		"                    (default value is based on mbuf count)\n"
 		"\n",
 		prgname);
 }
@@ -1894,6 +1901,15 @@ parse_args(int32_t argc, char **argv, struct eh_conf *eh_conf)
 			em_conf = eh_conf->mode_params;
 			em_conf->vector_tmo_ns = ret;
 			break;
+		case CMD_LINE_OPT_VECTOR_POOL_SZ_NUM:
+			ret = parse_decimal(optarg);
+
+			em_conf = eh_conf->mode_params;
+			em_conf->vector_pool_sz = ret;
+			break;
+		case CMD_LINE_OPT_PER_PORT_POOL_NUM:
+			per_port_pool = 1;
+			break;
 		default:
 			print_usage(prgname);
 			return -1;
@@ -2378,6 +2394,7 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 		/* init RX queues */
 		for (queue = 0; queue < qconf->nb_rx_queue; ++queue) {
 			struct rte_eth_rxconf rxq_conf;
+			struct rte_mempool *pool;
 
 			if (portid != qconf->rx_queue_list[queue].port_id)
 				continue;
@@ -2389,9 +2406,14 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 
 			rxq_conf = dev_info.default_rxconf;
 			rxq_conf.offloads = local_port_conf.rxmode.offloads;
+
+			if (per_port_pool)
+				pool = socket_ctx[socket_id].mbuf_pool[portid];
+			else
+				pool = socket_ctx[socket_id].mbuf_pool[0];
+
 			ret = rte_eth_rx_queue_setup(portid, rx_queueid,
-					nb_rxd,	socket_id, &rxq_conf,
-					socket_ctx[socket_id].mbuf_pool);
+					nb_rxd,	socket_id, &rxq_conf, pool);
 			if (ret < 0)
 				rte_exit(EXIT_FAILURE,
 					"rte_eth_rx_queue_setup: err=%d, "
@@ -2504,28 +2526,37 @@ session_priv_pool_init(struct socket_ctx *ctx, int32_t socket_id,
 }
 
 static void
-pool_init(struct socket_ctx *ctx, int32_t socket_id, uint32_t nb_mbuf)
+pool_init(struct socket_ctx *ctx, int32_t socket_id, int portid,
+	  uint32_t nb_mbuf)
 {
 	char s[64];
 	int32_t ms;
 
-	snprintf(s, sizeof(s), "mbuf_pool_%d", socket_id);
-	ctx->mbuf_pool = rte_pktmbuf_pool_create(s, nb_mbuf,
-			MEMPOOL_CACHE_SIZE, ipsec_metadata_size(),
-			frame_buf_size, socket_id);
+
+	/* mbuf_pool is initialised by the pool_init() function*/
+	if (socket_ctx[socket_id].mbuf_pool[portid])
+		return;
+
+	snprintf(s, sizeof(s), "mbuf_pool_%d_%d", socket_id, portid);
+	ctx->mbuf_pool[portid] = rte_pktmbuf_pool_create(s, nb_mbuf,
+							 MEMPOOL_CACHE_SIZE,
+							 ipsec_metadata_size(),
+							 frame_buf_size,
+							 socket_id);
 
 	/*
 	 * if multi-segment support is enabled, then create a pool
-	 * for indirect mbufs.
+	 * for indirect mbufs. This is not per-port but global.
 	 */
 	ms = multi_seg_required();
-	if (ms != 0) {
+	if (ms != 0 && !ctx->mbuf_pool_indir) {
 		snprintf(s, sizeof(s), "mbuf_pool_indir_%d", socket_id);
 		ctx->mbuf_pool_indir = rte_pktmbuf_pool_create(s, nb_mbuf,
 			MEMPOOL_CACHE_SIZE, 0, 0, socket_id);
 	}
 
-	if (ctx->mbuf_pool == NULL || (ms != 0 && ctx->mbuf_pool_indir == NULL))
+	if (ctx->mbuf_pool[portid] == NULL ||
+	    (ms != 0 && ctx->mbuf_pool_indir == NULL))
 		rte_exit(EXIT_FAILURE, "Cannot init mbuf pool on socket %d\n",
 				socket_id);
 	else
@@ -3338,11 +3369,22 @@ 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)
+		if (per_port_pool) {
+			RTE_ETH_FOREACH_DEV(portid) {
+				if ((enabled_port_mask & (1 << portid)) == 0)
+					continue;
+
+				pool_init(&socket_ctx[socket_id], socket_id,
+					  portid, nb_bufs_in_pool);
+			}
+		} else {
+			pool_init(&socket_ctx[socket_id], socket_id, 0,
+				  nb_bufs_in_pool);
+		}
+
+		if (socket_ctx[socket_id].session_pool)
 			continue;
 
-		pool_init(&socket_ctx[socket_id], socket_id, nb_bufs_in_pool);
 		session_pool_init(&socket_ctx[socket_id], socket_id, sess_sz);
 		session_priv_pool_init(&socket_ctx[socket_id], socket_id,
 			sess_sz);
diff --git a/examples/ipsec-secgw/ipsec-secgw.h b/examples/ipsec-secgw/ipsec-secgw.h
index ac4fa5e..24f11ad 100644
--- a/examples/ipsec-secgw/ipsec-secgw.h
+++ b/examples/ipsec-secgw/ipsec-secgw.h
@@ -134,6 +134,8 @@ extern volatile bool force_quit;
 
 extern uint32_t nb_bufs_in_pool;
 
+extern bool per_port_pool;
+
 static inline uint8_t
 is_unprotected_port(uint16_t port_id)
 {
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index bc87b1a..ccfde8e 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -248,7 +248,7 @@ struct socket_ctx {
 	struct sp_ctx *sp_ip6_out;
 	struct rt_ctx *rt_ip4;
 	struct rt_ctx *rt_ip6;
-	struct rte_mempool *mbuf_pool;
+	struct rte_mempool *mbuf_pool[RTE_MAX_ETHPORTS];
 	struct rte_mempool *mbuf_pool_indir;
 	struct rte_mempool *session_pool;
 	struct rte_mempool *session_priv_pool;
-- 
2.8.4


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

* [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log
  2022-02-06 14:30 [PATCH 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
                   ` (2 preceding siblings ...)
  2022-02-06 14:30 ` [PATCH 4/4] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
@ 2022-02-07  6:26 ` Nithin Dabilpuram
  2022-02-07  6:26   ` [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
                     ` (3 more replies)
  2022-02-23  9:53 ` [PATCH v3 1/3] " Nithin Dabilpuram
  4 siblings, 4 replies; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-07  6:26 UTC (permalink / raw)
  To: jerinj, Radu Nicolau, Akhil Goyal; +Cc: dev, Nithin Dabilpuram

Update error prints in data path to RTE_LOG_DP().
Error prints in fast path are not good for performance
as they slow down the application when few bad packets are
received.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---

v2:
- Fixed issue with warning in patch 4/4 by checking for session pool
  initialization instead of mbuf_pool as now mbuf pool is per port.

 examples/ipsec-secgw/ipsec_worker.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec_worker.c b/examples/ipsec-secgw/ipsec_worker.c
index 7419e85..e9493c5 100644
--- a/examples/ipsec-secgw/ipsec_worker.c
+++ b/examples/ipsec-secgw/ipsec_worker.c
@@ -332,7 +332,8 @@ process_ipsec_ev_inbound(struct ipsec_ctx *ctx, struct route_table *rt,
 		break;
 
 	default:
-		RTE_LOG(ERR, IPSEC, "Unsupported packet type = %d\n", type);
+		RTE_LOG_DP(DEBUG, IPSEC_ESP, "Unsupported packet type = %d\n",
+			   type);
 		goto drop_pkt_and_exit;
 	}
 
@@ -570,7 +571,8 @@ classify_pkt(struct rte_mbuf *pkt, struct ipsec_traffic *t)
 		t->ip6.pkts[(t->ip6.num)++] = pkt;
 		break;
 	default:
-		RTE_LOG(ERR, IPSEC, "Unsupported packet type = %d\n", type);
+		RTE_LOG_DP(DEBUG, IPSEC_ESP, "Unsupported packet type = %d\n",
+			   type);
 		free_pkts(&pkt, 1);
 		break;
 	}
-- 
2.8.4


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

* [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-07  6:26 ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
@ 2022-02-07  6:26   ` Nithin Dabilpuram
  2022-02-17 18:12     ` Akhil Goyal
  2022-02-17 19:22     ` Ananyev, Konstantin
  2022-02-07  6:26   ` [PATCH v2 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
                     ` (2 subsequent siblings)
  3 siblings, 2 replies; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-07  6:26 UTC (permalink / raw)
  To: jerinj, Radu Nicolau, Akhil Goyal; +Cc: dev, Nithin Dabilpuram

Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
In other cases such as Tx Inline protocol offload, checksum computation
is implicitly taken care by HW. The advantage of having only necessary
offloads enabled is that Tx burst function can be as light as possible.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 examples/ipsec-secgw/ipsec-secgw.c | 3 ---
 examples/ipsec-secgw/sa.c          | 9 +++++++++
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 21abc0d..d8a9bfa 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 		local_port_conf.txmode.offloads |=
 			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
 
-	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
-		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
-
 	printf("port %u configuring rx_offloads=0x%" PRIx64
 		", tx_offloads=0x%" PRIx64 "\n",
 		portid, local_port_conf.rxmode.offloads,
diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
index 1839ac7..b878a48 100644
--- a/examples/ipsec-secgw/sa.c
+++ b/examples/ipsec-secgw/sa.c
@@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
 				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
 				&& rule->portid == port_id) {
 			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
+
+			/* Checksum offload is not needed for inline protocol as
+			 * all processing for Outbound IPSec packets will be
+			 * implicitly taken care and for non-IPSec packets,
+			 * there is no need of IPv4 Checksum offload.
+			 */
+			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
+				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
+
 			if (rule->mss)
 				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
 		}
-- 
2.8.4


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

* [PATCH v2 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode
  2022-02-07  6:26 ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
  2022-02-07  6:26   ` [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
@ 2022-02-07  6:26   ` Nithin Dabilpuram
  2022-02-17 18:12     ` Akhil Goyal
  2022-02-07  6:26   ` [PATCH v2 4/4] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
  2022-02-17 18:11   ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Akhil Goyal
  3 siblings, 1 reply; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-07  6:26 UTC (permalink / raw)
  To: jerinj, Radu Nicolau, Akhil Goyal; +Cc: dev, Nithin Dabilpuram, schalla, stable

Fix packet processing to skip after mbuf is freed instead of
touching and Tx'ing it.

Also free vector event buffer in event worker when after processing
there is no pkt to be enqueued to Tx adapter.

Fixes: 86738ebe1e3d ("examples/ipsec-secgw: support event vector")
Cc: schalla@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 examples/ipsec-secgw/ipsec_worker.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec_worker.c b/examples/ipsec-secgw/ipsec_worker.c
index e9493c5..8639426 100644
--- a/examples/ipsec-secgw/ipsec_worker.c
+++ b/examples/ipsec-secgw/ipsec_worker.c
@@ -205,12 +205,16 @@ check_sp_sa_bulk(struct sp_ctx *sp, struct sa_ctx *sa_ctx,
 			ip->pkts[j++] = m;
 		else {
 			sa = *(struct ipsec_sa **)rte_security_dynfield(m);
-			if (sa == NULL)
+			if (sa == NULL) {
 				free_pkts(&m, 1);
+				continue;
+			}
 
 			/* SPI on the packet should match with the one in SA */
-			if (unlikely(sa->spi != sa_ctx->sa[res - 1].spi))
+			if (unlikely(sa->spi != sa_ctx->sa[res - 1].spi)) {
 				free_pkts(&m, 1);
+				continue;
+			}
 
 			ip->pkts[j++] = m;
 		}
@@ -536,6 +540,7 @@ ipsec_ev_route_pkts(struct rte_event_vector *vec, struct route_table *rt,
 				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)) {
 				RTE_LOG(ERR, IPSEC, "SA type not supported\n");
 				free_pkts(&pkt, 1);
+				continue;
 			}
 			rte_security_set_pkt_metadata(sess->security.ctx,
 						sess->security.ses, pkt, NULL);
@@ -695,11 +700,13 @@ ipsec_ev_vector_process(struct lcore_conf_ev_tx_int_port_wrkr *lconf,
 		ret = process_ipsec_ev_outbound_vector(&lconf->outbound,
 						       &lconf->rt, vec);
 
-	if (ret > 0) {
+	if (likely(ret > 0)) {
 		vec->nb_elem = ret;
 		rte_event_eth_tx_adapter_enqueue(links[0].eventdev_id,
 						 links[0].event_port_id,
 						 ev, 1, 0);
+	} else {
+		rte_mempool_put(rte_mempool_from_obj(vec), vec);
 	}
 }
 
@@ -720,6 +727,8 @@ ipsec_ev_vector_drv_mode_process(struct eh_event_link_info *links,
 		rte_event_eth_tx_adapter_enqueue(links[0].eventdev_id,
 						 links[0].event_port_id,
 						 ev, 1, 0);
+	else
+		rte_mempool_put(rte_mempool_from_obj(vec), vec);
 }
 
 /*
-- 
2.8.4


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

* [PATCH v2 4/4] examples/ipsec-secgw: add per port pool and vector pool size
  2022-02-07  6:26 ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
  2022-02-07  6:26   ` [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
  2022-02-07  6:26   ` [PATCH v2 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
@ 2022-02-07  6:26   ` Nithin Dabilpuram
  2022-02-17 18:13     ` Akhil Goyal
  2022-02-17 18:11   ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Akhil Goyal
  3 siblings, 1 reply; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-07  6:26 UTC (permalink / raw)
  To: jerinj, Radu Nicolau, Akhil Goyal; +Cc: dev, Nithin Dabilpuram

Add support to enable per port packet pool and also override
vector pool size from command line args. This is useful
on some HW to tune performance based on usecase.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
 examples/ipsec-secgw/event_helper.c | 17 ++++++--
 examples/ipsec-secgw/event_helper.h |  2 +
 examples/ipsec-secgw/ipsec-secgw.c  | 82 ++++++++++++++++++++++++++++---------
 examples/ipsec-secgw/ipsec-secgw.h  |  2 +
 examples/ipsec-secgw/ipsec.h        |  2 +-
 5 files changed, 81 insertions(+), 24 deletions(-)

diff --git a/examples/ipsec-secgw/event_helper.c b/examples/ipsec-secgw/event_helper.c
index 8947e41..172ab8e 100644
--- a/examples/ipsec-secgw/event_helper.c
+++ b/examples/ipsec-secgw/event_helper.c
@@ -792,8 +792,8 @@ eh_rx_adapter_configure(struct eventmode_conf *em_conf,
 	uint32_t service_id, socket_id, nb_elem;
 	struct rte_mempool *vector_pool = NULL;
 	uint32_t lcore_id = rte_lcore_id();
+	int ret, portid, nb_ports = 0;
 	uint8_t eventdev_id;
-	int ret;
 	int j;
 
 	/* Get event dev ID */
@@ -806,10 +806,21 @@ eh_rx_adapter_configure(struct eventmode_conf *em_conf,
 		return ret;
 	}
 
+	RTE_ETH_FOREACH_DEV(portid)
+		if ((em_conf->eth_portmask & (1 << portid)))
+			nb_ports++;
+
 	if (em_conf->ext_params.event_vector) {
 		socket_id = rte_lcore_to_socket_id(lcore_id);
-		nb_elem = (nb_bufs_in_pool / em_conf->ext_params.vector_size)
-			  + 1;
+
+		if (em_conf->vector_pool_sz) {
+			nb_elem = em_conf->vector_pool_sz;
+		} else {
+			nb_elem = (nb_bufs_in_pool /
+				   em_conf->ext_params.vector_size) + 1;
+			if (per_port_pool)
+				nb_elem = nb_ports * nb_elem;
+		}
 
 		vector_pool = rte_event_vector_pool_create(
 			"vector_pool", nb_elem, 0,
diff --git a/examples/ipsec-secgw/event_helper.h b/examples/ipsec-secgw/event_helper.h
index 5be6c62..f3cbe57 100644
--- a/examples/ipsec-secgw/event_helper.h
+++ b/examples/ipsec-secgw/event_helper.h
@@ -183,6 +183,8 @@ struct eventmode_conf {
 		/**< 64 bit field to specify extended params */
 	uint64_t vector_tmo_ns;
 		/**< Max vector timeout in nanoseconds */
+	uint64_t vector_pool_sz;
+		/**< Vector pool size */
 };
 
 /**
diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index d8a9bfa..2f3ebea 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -118,6 +118,8 @@ struct flow_info flow_info_tbl[RTE_MAX_ETHPORTS];
 #define CMD_LINE_OPT_EVENT_VECTOR	"event-vector"
 #define CMD_LINE_OPT_VECTOR_SIZE	"vector-size"
 #define CMD_LINE_OPT_VECTOR_TIMEOUT	"vector-tmo"
+#define CMD_LINE_OPT_VECTOR_POOL_SZ	"vector-pool-sz"
+#define CMD_LINE_OPT_PER_PORT_POOL	"per-port-pool"
 
 #define CMD_LINE_ARG_EVENT	"event"
 #define CMD_LINE_ARG_POLL	"poll"
@@ -145,6 +147,8 @@ enum {
 	CMD_LINE_OPT_EVENT_VECTOR_NUM,
 	CMD_LINE_OPT_VECTOR_SIZE_NUM,
 	CMD_LINE_OPT_VECTOR_TIMEOUT_NUM,
+	CMD_LINE_OPT_VECTOR_POOL_SZ_NUM,
+	CMD_LINE_OPT_PER_PORT_POOL_NUM,
 };
 
 static const struct option lgopts[] = {
@@ -161,6 +165,8 @@ static const struct option lgopts[] = {
 	{CMD_LINE_OPT_EVENT_VECTOR, 0, 0, CMD_LINE_OPT_EVENT_VECTOR_NUM},
 	{CMD_LINE_OPT_VECTOR_SIZE, 1, 0, CMD_LINE_OPT_VECTOR_SIZE_NUM},
 	{CMD_LINE_OPT_VECTOR_TIMEOUT, 1, 0, CMD_LINE_OPT_VECTOR_TIMEOUT_NUM},
+	{CMD_LINE_OPT_VECTOR_POOL_SZ, 1, 0, CMD_LINE_OPT_VECTOR_POOL_SZ_NUM},
+	{CMD_LINE_OPT_PER_PORT_POOL, 0, 0, CMD_LINE_OPT_PER_PORT_POOL_NUM},
 	{NULL, 0, 0, 0}
 };
 
@@ -234,7 +240,6 @@ struct lcore_conf {
 	struct rt_ctx *rt6_ctx;
 	struct {
 		struct rte_ip_frag_tbl *tbl;
-		struct rte_mempool *pool_dir;
 		struct rte_mempool *pool_indir;
 		struct rte_ip_frag_death_row dr;
 	} frag;
@@ -262,6 +267,8 @@ static struct rte_eth_conf port_conf = {
 
 struct socket_ctx socket_ctx[NB_SOCKETS];
 
+bool per_port_pool;
+
 /*
  * Determine is multi-segment support required:
  *  - either frame buffer size is smaller then mtu
@@ -630,12 +637,10 @@ send_fragment_packet(struct lcore_conf *qconf, struct rte_mbuf *m,
 
 	if (proto == IPPROTO_IP)
 		rc = rte_ipv4_fragment_packet(m, tbl->m_table + len,
-			n, mtu_size, qconf->frag.pool_dir,
-			qconf->frag.pool_indir);
+			n, mtu_size, m->pool, qconf->frag.pool_indir);
 	else
 		rc = rte_ipv6_fragment_packet(m, tbl->m_table + len,
-			n, mtu_size, qconf->frag.pool_dir,
-			qconf->frag.pool_indir);
+			n, mtu_size, m->pool, qconf->frag.pool_indir);
 
 	if (rc >= 0)
 		len += rc;
@@ -1256,7 +1261,6 @@ ipsec_poll_mode_worker(void)
 	qconf->outbound.session_pool = socket_ctx[socket_id].session_pool;
 	qconf->outbound.session_priv_pool =
 			socket_ctx[socket_id].session_priv_pool;
-	qconf->frag.pool_dir = socket_ctx[socket_id].mbuf_pool;
 	qconf->frag.pool_indir = socket_ctx[socket_id].mbuf_pool_indir;
 
 	rc = ipsec_sad_lcore_cache_init(app_sa_prm.cache_sz);
@@ -1511,6 +1515,9 @@ print_usage(const char *prgname)
 		"  --vector-size Max vector size (default value: 16)\n"
 		"  --vector-tmo Max vector timeout in nanoseconds"
 		"    (default value: 102400)\n"
+		"  --" CMD_LINE_OPT_PER_PORT_POOL " Enable per port mbuf pool\n"
+		"  --" CMD_LINE_OPT_VECTOR_POOL_SZ " Vector pool size\n"
+		"                    (default value is based on mbuf count)\n"
 		"\n",
 		prgname);
 }
@@ -1894,6 +1901,15 @@ parse_args(int32_t argc, char **argv, struct eh_conf *eh_conf)
 			em_conf = eh_conf->mode_params;
 			em_conf->vector_tmo_ns = ret;
 			break;
+		case CMD_LINE_OPT_VECTOR_POOL_SZ_NUM:
+			ret = parse_decimal(optarg);
+
+			em_conf = eh_conf->mode_params;
+			em_conf->vector_pool_sz = ret;
+			break;
+		case CMD_LINE_OPT_PER_PORT_POOL_NUM:
+			per_port_pool = 1;
+			break;
 		default:
 			print_usage(prgname);
 			return -1;
@@ -2378,6 +2394,7 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 		/* init RX queues */
 		for (queue = 0; queue < qconf->nb_rx_queue; ++queue) {
 			struct rte_eth_rxconf rxq_conf;
+			struct rte_mempool *pool;
 
 			if (portid != qconf->rx_queue_list[queue].port_id)
 				continue;
@@ -2389,9 +2406,14 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 
 			rxq_conf = dev_info.default_rxconf;
 			rxq_conf.offloads = local_port_conf.rxmode.offloads;
+
+			if (per_port_pool)
+				pool = socket_ctx[socket_id].mbuf_pool[portid];
+			else
+				pool = socket_ctx[socket_id].mbuf_pool[0];
+
 			ret = rte_eth_rx_queue_setup(portid, rx_queueid,
-					nb_rxd,	socket_id, &rxq_conf,
-					socket_ctx[socket_id].mbuf_pool);
+					nb_rxd,	socket_id, &rxq_conf, pool);
 			if (ret < 0)
 				rte_exit(EXIT_FAILURE,
 					"rte_eth_rx_queue_setup: err=%d, "
@@ -2504,28 +2526,37 @@ session_priv_pool_init(struct socket_ctx *ctx, int32_t socket_id,
 }
 
 static void
-pool_init(struct socket_ctx *ctx, int32_t socket_id, uint32_t nb_mbuf)
+pool_init(struct socket_ctx *ctx, int32_t socket_id, int portid,
+	  uint32_t nb_mbuf)
 {
 	char s[64];
 	int32_t ms;
 
-	snprintf(s, sizeof(s), "mbuf_pool_%d", socket_id);
-	ctx->mbuf_pool = rte_pktmbuf_pool_create(s, nb_mbuf,
-			MEMPOOL_CACHE_SIZE, ipsec_metadata_size(),
-			frame_buf_size, socket_id);
+
+	/* mbuf_pool is initialised by the pool_init() function*/
+	if (socket_ctx[socket_id].mbuf_pool[portid])
+		return;
+
+	snprintf(s, sizeof(s), "mbuf_pool_%d_%d", socket_id, portid);
+	ctx->mbuf_pool[portid] = rte_pktmbuf_pool_create(s, nb_mbuf,
+							 MEMPOOL_CACHE_SIZE,
+							 ipsec_metadata_size(),
+							 frame_buf_size,
+							 socket_id);
 
 	/*
 	 * if multi-segment support is enabled, then create a pool
-	 * for indirect mbufs.
+	 * for indirect mbufs. This is not per-port but global.
 	 */
 	ms = multi_seg_required();
-	if (ms != 0) {
+	if (ms != 0 && !ctx->mbuf_pool_indir) {
 		snprintf(s, sizeof(s), "mbuf_pool_indir_%d", socket_id);
 		ctx->mbuf_pool_indir = rte_pktmbuf_pool_create(s, nb_mbuf,
 			MEMPOOL_CACHE_SIZE, 0, 0, socket_id);
 	}
 
-	if (ctx->mbuf_pool == NULL || (ms != 0 && ctx->mbuf_pool_indir == NULL))
+	if (ctx->mbuf_pool[portid] == NULL ||
+	    (ms != 0 && ctx->mbuf_pool_indir == NULL))
 		rte_exit(EXIT_FAILURE, "Cannot init mbuf pool on socket %d\n",
 				socket_id);
 	else
@@ -3338,11 +3369,22 @@ 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)
+		if (per_port_pool) {
+			RTE_ETH_FOREACH_DEV(portid) {
+				if ((enabled_port_mask & (1 << portid)) == 0)
+					continue;
+
+				pool_init(&socket_ctx[socket_id], socket_id,
+					  portid, nb_bufs_in_pool);
+			}
+		} else {
+			pool_init(&socket_ctx[socket_id], socket_id, 0,
+				  nb_bufs_in_pool);
+		}
+
+		if (socket_ctx[socket_id].session_pool)
 			continue;
 
-		pool_init(&socket_ctx[socket_id], socket_id, nb_bufs_in_pool);
 		session_pool_init(&socket_ctx[socket_id], socket_id, sess_sz);
 		session_priv_pool_init(&socket_ctx[socket_id], socket_id,
 			sess_sz);
@@ -3415,7 +3457,7 @@ main(int32_t argc, char **argv)
 	/* 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) &&
+		if ((socket_ctx[socket_id].session_pool != NULL) &&
 			(socket_ctx[socket_id].sa_in == NULL) &&
 			(socket_ctx[socket_id].sa_out == NULL)) {
 			sa_init(&socket_ctx[socket_id], socket_id);
diff --git a/examples/ipsec-secgw/ipsec-secgw.h b/examples/ipsec-secgw/ipsec-secgw.h
index ac4fa5e..24f11ad 100644
--- a/examples/ipsec-secgw/ipsec-secgw.h
+++ b/examples/ipsec-secgw/ipsec-secgw.h
@@ -134,6 +134,8 @@ extern volatile bool force_quit;
 
 extern uint32_t nb_bufs_in_pool;
 
+extern bool per_port_pool;
+
 static inline uint8_t
 is_unprotected_port(uint16_t port_id)
 {
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index bc87b1a..ccfde8e 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -248,7 +248,7 @@ struct socket_ctx {
 	struct sp_ctx *sp_ip6_out;
 	struct rt_ctx *rt_ip4;
 	struct rt_ctx *rt_ip6;
-	struct rte_mempool *mbuf_pool;
+	struct rte_mempool *mbuf_pool[RTE_MAX_ETHPORTS];
 	struct rte_mempool *mbuf_pool_indir;
 	struct rte_mempool *session_pool;
 	struct rte_mempool *session_priv_pool;
-- 
2.8.4


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

* RE: [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-06 14:30 ` [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
@ 2022-02-07  9:52   ` Ananyev, Konstantin
  2022-02-07 14:15     ` Nithin Kumar Dabilpuram
  0 siblings, 1 reply; 22+ messages in thread
From: Ananyev, Konstantin @ 2022-02-07  9:52 UTC (permalink / raw)
  To: Nithin Dabilpuram, jerinj, Nicolau, Radu, Akhil Goyal; +Cc: dev


> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
> In other cases such as Tx Inline protocol offload, checksum computation
> is implicitly taken care by HW.

Why is that?
These is two separate HW offload and user has to enable each of them explicitly.
Also we can TX clear-text traffic.

> The advantage of having only necessary
> offloads enabled is that Tx burst function can be as light as possible.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>  examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>  examples/ipsec-secgw/sa.c          | 9 +++++++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index 21abc0d..d8a9bfa 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
>  		local_port_conf.txmode.offloads |=
>  			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
> 
> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> -
>  	printf("port %u configuring rx_offloads=0x%" PRIx64
>  		", tx_offloads=0x%" PRIx64 "\n",
>  		portid, local_port_conf.rxmode.offloads,
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index 1839ac7..b878a48 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
>  				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>  				&& rule->portid == port_id) {
>  			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
> +
> +			/* Checksum offload is not needed for inline protocol as
> +			 * all processing for Outbound IPSec packets will be
> +			 * implicitly taken care and for non-IPSec packets,
> +			 * there is no need of IPv4 Checksum offload.
> +			 */
> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> +
>  			if (rule->mss)
>  				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>  		}
> --
> 2.8.4


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

* Re: [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-07  9:52   ` Ananyev, Konstantin
@ 2022-02-07 14:15     ` Nithin Kumar Dabilpuram
  2022-02-17 19:17       ` Ananyev, Konstantin
  0 siblings, 1 reply; 22+ messages in thread
From: Nithin Kumar Dabilpuram @ 2022-02-07 14:15 UTC (permalink / raw)
  To: dev, konstantin.ananyev; +Cc: radu.nicolau, gakhil



On 2/7/22 3:22 PM, Ananyev, Konstantin wrote:
> 
>> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
>> In other cases such as Tx Inline protocol offload, checksum computation
>> is implicitly taken care by HW.
> 
> Why is that?
> These is two separate HW offload and user has to enable each of them explicitly.


In Inline IPSec protocol offload, the complete tunnel header for tunnel 
mode is updated by HW/PMD. So it doesn't have any dependency on 
RTE_ETH_TX_OFFLOAD_IPV4_CKSUM as there is no valid l2_len/l3_len yet in 
the mbuf. Similarly in case of Transport mode, the IPv4 header is 
updated by HW itself for next proto and hence the offsets and all can 
vary based on the HW implementation.

Hence my thought was for Inline IPsec protocol offload, there is no need 
to explicitly say that RTE_ETH_TX_OFFLOAD_IPV4_CKSUM is enabled and need 
not provide ol_flags RTE_MBUF_F_TX_IP_CKSUM and l2_len and l3_len which 
might not be correct in prepare_tx_pkt().

 >* RTE_MBUF_F_TX_IP_CKSUM.
 > *  - fill the mbuf offload information: l2_len, l3_len
(Ex: Tunnel header being inserted is IPv6 while inner header is IPv4.

For inline crypto I agree, the packet content is all in place except for 
plain text->cipher text translation so l2/l3 offsets are valid.

 > Also we can TX clear-text traffic.
Ok, I agree that we can have clear-text traffic. We are already handling 
ipv4 checksum in SW in case Tx offload doesn't have IPv4 Checksum 
offload enabled. And for clear text traffic I think that is not needed
as well as we are not updating ttl.

My overall intention was to have lighter Tx burst function for Inline 
IPsec protocol offload as mainly IPsec traffic and not plain traffic is 
primary use case for ipsec-secgw.



> 
>> The advantage of having only necessary
>> offloads enabled is that Tx burst function can be as light as possible.
>>
>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>> ---
>>   examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>>   examples/ipsec-secgw/sa.c          | 9 +++++++++
>>   2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
>> index 21abc0d..d8a9bfa 100644
>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
>>   		local_port_conf.txmode.offloads |=
>>   			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>>
>> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
>> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>> -
>>   	printf("port %u configuring rx_offloads=0x%" PRIx64
>>   		", tx_offloads=0x%" PRIx64 "\n",
>>   		portid, local_port_conf.rxmode.offloads,
>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>> index 1839ac7..b878a48 100644
>> --- a/examples/ipsec-secgw/sa.c
>> +++ b/examples/ipsec-secgw/sa.c
>> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
>>   				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>>   				&& rule->portid == port_id) {
>>   			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
>> +
>> +			/* Checksum offload is not needed for inline protocol as
>> +			 * all processing for Outbound IPSec packets will be
>> +			 * implicitly taken care and for non-IPSec packets,
>> +			 * there is no need of IPv4 Checksum offload.
>> +			 */
>> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
>> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>> +
>>   			if (rule->mss)
>>   				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>>   		}
>> --
>> 2.8.4
> 

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

* RE: [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log
  2022-02-07  6:26 ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
                     ` (2 preceding siblings ...)
  2022-02-07  6:26   ` [PATCH v2 4/4] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
@ 2022-02-17 18:11   ` Akhil Goyal
  3 siblings, 0 replies; 22+ messages in thread
From: Akhil Goyal @ 2022-02-17 18:11 UTC (permalink / raw)
  To: Nithin Kumar Dabilpuram, Jerin Jacob Kollanukkaran, Radu Nicolau
  Cc: dev, Nithin Kumar Dabilpuram

> 
> Update error prints in data path to RTE_LOG_DP().
> Error prints in fast path are not good for performance
> as they slow down the application when few bad packets are
> received.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
Acked-by: Akhil Goyal <gakhil@marvell.com>

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

* RE: [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-07  6:26   ` [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
@ 2022-02-17 18:12     ` Akhil Goyal
  2022-02-17 19:22     ` Ananyev, Konstantin
  1 sibling, 0 replies; 22+ messages in thread
From: Akhil Goyal @ 2022-02-17 18:12 UTC (permalink / raw)
  To: Nithin Kumar Dabilpuram, Jerin Jacob Kollanukkaran, Radu Nicolau
  Cc: dev, Nithin Kumar Dabilpuram

> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
> In other cases such as Tx Inline protocol offload, checksum computation
> is implicitly taken care by HW. The advantage of having only necessary
> offloads enabled is that Tx burst function can be as light as possible.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
Acked-by: Akhil Goyal <gakhil@marvell.com>

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

* RE: [PATCH v2 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode
  2022-02-07  6:26   ` [PATCH v2 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
@ 2022-02-17 18:12     ` Akhil Goyal
  0 siblings, 0 replies; 22+ messages in thread
From: Akhil Goyal @ 2022-02-17 18:12 UTC (permalink / raw)
  To: Nithin Kumar Dabilpuram, Jerin Jacob Kollanukkaran, Radu Nicolau
  Cc: dev, Nithin Kumar Dabilpuram, Srujana Challa, stable

> Fix packet processing to skip after mbuf is freed instead of
> touching and Tx'ing it.
> 
> Also free vector event buffer in event worker when after processing
> there is no pkt to be enqueued to Tx adapter.
> 
> Fixes: 86738ebe1e3d ("examples/ipsec-secgw: support event vector")
> Cc: schalla@marvell.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>

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

* RE: [PATCH v2 4/4] examples/ipsec-secgw: add per port pool and vector pool size
  2022-02-07  6:26   ` [PATCH v2 4/4] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
@ 2022-02-17 18:13     ` Akhil Goyal
  0 siblings, 0 replies; 22+ messages in thread
From: Akhil Goyal @ 2022-02-17 18:13 UTC (permalink / raw)
  To: Nithin Kumar Dabilpuram, Jerin Jacob Kollanukkaran, Radu Nicolau
  Cc: dev, Nithin Kumar Dabilpuram

> Add support to enable per port packet pool and also override
> vector pool size from command line args. This is useful
> on some HW to tune performance based on usecase.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>  examples/ipsec-secgw/event_helper.c | 17 ++++++--
>  examples/ipsec-secgw/event_helper.h |  2 +
>  examples/ipsec-secgw/ipsec-secgw.c  | 82 ++++++++++++++++++++++++++++--
> -------
>  examples/ipsec-secgw/ipsec-secgw.h  |  2 +
>  examples/ipsec-secgw/ipsec.h        |  2 +-
>  5 files changed, 81 insertions(+), 24 deletions(-)

Documentation update missing for the new options added.
Apart from that,
Acked-by: Akhil Goyal <gakhil@marvell.com>

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

* RE: [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-07 14:15     ` Nithin Kumar Dabilpuram
@ 2022-02-17 19:17       ` Ananyev, Konstantin
  2022-02-18 13:58         ` Nithin Kumar Dabilpuram
  0 siblings, 1 reply; 22+ messages in thread
From: Ananyev, Konstantin @ 2022-02-17 19:17 UTC (permalink / raw)
  To: Nithin Kumar Dabilpuram, dev; +Cc: Nicolau, Radu, gakhil


> >> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
> >> In other cases such as Tx Inline protocol offload, checksum computation
> >> is implicitly taken care by HW.
> >
> > Why is that?
> > These is two separate HW offload and user has to enable each of them explicitly.
> 
> 
> In Inline IPSec protocol offload, the complete tunnel header for tunnel
> mode is updated by HW/PMD. So it doesn't have any dependency on
> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM as there is no valid l2_len/l3_len yet in
> the mbuf. Similarly in case of Transport mode, the IPv4 header is
> updated by HW itself for next proto and hence the offsets and all can
> vary based on the HW implementation.
> 
> Hence my thought was for Inline IPsec protocol offload, there is no need
> to explicitly say that RTE_ETH_TX_OFFLOAD_IPV4_CKSUM is enabled and need
> not provide ol_flags RTE_MBUF_F_TX_IP_CKSUM and l2_len and l3_len which
> might not be correct in prepare_tx_pkt().
> 
>  >* RTE_MBUF_F_TX_IP_CKSUM.
>  > *  - fill the mbuf offload information: l2_len, l3_len
> (Ex: Tunnel header being inserted is IPv6 while inner header is IPv4.
> 
> For inline crypto I agree, the packet content is all in place except for
> plain text->cipher text translation so l2/l3 offsets are valid.

Ok, but apart from inline modes we also have lookaside modes.
Why to disable ip cksum for them?

> 
>  > Also we can TX clear-text traffic.
> Ok, I agree that we can have clear-text traffic. We are already handling
> ipv4 checksum in SW in case Tx offload doesn't have IPv4 Checksum
> offload enabled. And for clear text traffic I think that is not needed
> as well as we are not updating ttl.

As I remember we always recalculate ip cksum if RTE_MBUF_F_TX_IP_CKSUM
is not set:

static inline void
prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port,
                const struct lcore_conf *qconf)
{
        struct ip *ip;
        struct rte_ether_hdr *ethhdr;

        ip = rte_pktmbuf_mtod(pkt, struct ip *);

        ethhdr = (struct rte_ether_hdr *)
                rte_pktmbuf_prepend(pkt, RTE_ETHER_HDR_LEN);

        if (ip->ip_v == IPVERSION) {
                pkt->ol_flags |= qconf->outbound.ipv4_offloads;
                pkt->l3_len = sizeof(struct ip);
                pkt->l2_len = RTE_ETHER_HDR_LEN;

                ip->ip_sum = 0;

                /* calculate IPv4 cksum in SW */
                if ((pkt->ol_flags & RTE_MBUF_F_TX_IP_CKSUM) == 0)
                        ip->ip_sum = rte_ipv4_cksum((struct rte_ipv4_hdr *)ip);

...

> 
> My overall intention was to have lighter Tx burst function for Inline
> IPsec protocol offload as mainly IPsec traffic and not plain traffic is
> primary use case for ipsec-secgw.
> 
> 
> 
> >
> >> The advantage of having only necessary
> >> offloads enabled is that Tx burst function can be as light as possible.
> >>
> >> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >> ---
> >>   examples/ipsec-secgw/ipsec-secgw.c | 3 ---
> >>   examples/ipsec-secgw/sa.c          | 9 +++++++++
> >>   2 files changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> >> index 21abc0d..d8a9bfa 100644
> >> --- a/examples/ipsec-secgw/ipsec-secgw.c
> >> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> >> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
> >>   		local_port_conf.txmode.offloads |=
> >>   			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
> >>
> >> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
> >> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> >> -
> >>   	printf("port %u configuring rx_offloads=0x%" PRIx64
> >>   		", tx_offloads=0x%" PRIx64 "\n",
> >>   		portid, local_port_conf.rxmode.offloads,
> >> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> >> index 1839ac7..b878a48 100644
> >> --- a/examples/ipsec-secgw/sa.c
> >> +++ b/examples/ipsec-secgw/sa.c
> >> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
> >>   				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
> >>   				&& rule->portid == port_id) {
> >>   			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
> >> +
> >> +			/* Checksum offload is not needed for inline protocol as
> >> +			 * all processing for Outbound IPSec packets will be
> >> +			 * implicitly taken care and for non-IPSec packets,
> >> +			 * there is no need of IPv4 Checksum offload.
> >> +			 */
> >> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> >> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> >> +
> >>   			if (rule->mss)
> >>   				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
> >>   		}
> >> --
> >> 2.8.4
> >

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

* RE: [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-07  6:26   ` [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
  2022-02-17 18:12     ` Akhil Goyal
@ 2022-02-17 19:22     ` Ananyev, Konstantin
  1 sibling, 0 replies; 22+ messages in thread
From: Ananyev, Konstantin @ 2022-02-17 19:22 UTC (permalink / raw)
  To: Nithin Dabilpuram, jerinj, Nicolau, Radu, Akhil Goyal; +Cc: dev



> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
> In other cases such as Tx Inline protocol offload, checksum computation
> is implicitly taken care by HW. The advantage of having only necessary
> offloads enabled is that Tx burst function can be as light as possible.

I am still not sure this is a right thing to do.
Could you explain what will happen for lookaside modes?
Would they always fall-back to SW cksum calculation?

> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
>  examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>  examples/ipsec-secgw/sa.c          | 9 +++++++++
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index 21abc0d..d8a9bfa 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
>  		local_port_conf.txmode.offloads |=
>  			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
> 
> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> -
>  	printf("port %u configuring rx_offloads=0x%" PRIx64
>  		", tx_offloads=0x%" PRIx64 "\n",
>  		portid, local_port_conf.rxmode.offloads,
> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
> index 1839ac7..b878a48 100644
> --- a/examples/ipsec-secgw/sa.c
> +++ b/examples/ipsec-secgw/sa.c
> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
>  				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>  				&& rule->portid == port_id) {
>  			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
> +
> +			/* Checksum offload is not needed for inline protocol as
> +			 * all processing for Outbound IPSec packets will be
> +			 * implicitly taken care and for non-IPSec packets,
> +			 * there is no need of IPv4 Checksum offload.
> +			 */
> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
> +
>  			if (rule->mss)
>  				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>  		}
> --
> 2.8.4


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

* Re: [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-17 19:17       ` Ananyev, Konstantin
@ 2022-02-18 13:58         ` Nithin Kumar Dabilpuram
  2022-02-23  9:58           ` Nithin Kumar Dabilpuram
  0 siblings, 1 reply; 22+ messages in thread
From: Nithin Kumar Dabilpuram @ 2022-02-18 13:58 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev; +Cc: Nicolau, Radu, gakhil



On 2/18/22 12:47 AM, Ananyev, Konstantin wrote:
> 
>>>> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
>>>> In other cases such as Tx Inline protocol offload, checksum computation
>>>> is implicitly taken care by HW.
>>>
>>> Why is that?
>>> These is two separate HW offload and user has to enable each of them explicitly.
>>
>>
>> In Inline IPSec protocol offload, the complete tunnel header for tunnel
>> mode is updated by HW/PMD. So it doesn't have any dependency on
>> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM as there is no valid l2_len/l3_len yet in
>> the mbuf. Similarly in case of Transport mode, the IPv4 header is
>> updated by HW itself for next proto and hence the offsets and all can
>> vary based on the HW implementation.
>>
>> Hence my thought was for Inline IPsec protocol offload, there is no need
>> to explicitly say that RTE_ETH_TX_OFFLOAD_IPV4_CKSUM is enabled and need
>> not provide ol_flags RTE_MBUF_F_TX_IP_CKSUM and l2_len and l3_len which
>> might not be correct in prepare_tx_pkt().
>>
>>   >* RTE_MBUF_F_TX_IP_CKSUM.
>>   > *  - fill the mbuf offload information: l2_len, l3_len
>> (Ex: Tunnel header being inserted is IPv6 while inner header is IPv4.
>>
>> For inline crypto I agree, the packet content is all in place except for
>> plain text->cipher text translation so l2/l3 offsets are valid.
> 
> Ok, but apart from inline modes we also have lookaside modes.
> Why to disable ip cksum for them?

Ack, I missed that case. I'll make the change to enable Tx checksum 
offload enabled even for Lookaside crypto.

> 
>>
>>   > Also we can TX clear-text traffic.
>> Ok, I agree that we can have clear-text traffic. We are already handling
>> ipv4 checksum in SW in case Tx offload doesn't have IPv4 Checksum
>> offload enabled. And for clear text traffic I think that is not needed
>> as well as we are not updating ttl.
> 
> As I remember we always recalculate ip cksum if RTE_MBUF_F_TX_IP_CKSUM
> is not set:
> 
> static inline void
> prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port,
>                  const struct lcore_conf *qconf)
> {
>          struct ip *ip;
>          struct rte_ether_hdr *ethhdr;
> 
>          ip = rte_pktmbuf_mtod(pkt, struct ip *);
> 
>          ethhdr = (struct rte_ether_hdr *)
>                  rte_pktmbuf_prepend(pkt, RTE_ETHER_HDR_LEN);
> 
>          if (ip->ip_v == IPVERSION) {
>                  pkt->ol_flags |= qconf->outbound.ipv4_offloads;
>                  pkt->l3_len = sizeof(struct ip);
>                  pkt->l2_len = RTE_ETHER_HDR_LEN;
> 
>                  ip->ip_sum = 0;
> 
>                  /* calculate IPv4 cksum in SW */
>                  if ((pkt->ol_flags & RTE_MBUF_F_TX_IP_CKSUM) == 0)
>                          ip->ip_sum = rte_ipv4_cksum((struct rte_ipv4_hdr *)ip);
> 
> ...
> 

I'm working on another series to restructure fast path based on 
different mode. As part of that, I think we can avoid checksum 
recalculation in cases of inline protocol offload.

>>
>> My overall intention was to have lighter Tx burst function for Inline
>> IPsec protocol offload as mainly IPsec traffic and not plain traffic is
>> primary use case for ipsec-secgw.
>>
>>
>>
>>>
>>>> The advantage of having only necessary
>>>> offloads enabled is that Tx burst function can be as light as possible.
>>>>
>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>> ---
>>>>    examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>>>>    examples/ipsec-secgw/sa.c          | 9 +++++++++
>>>>    2 files changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
>>>> index 21abc0d..d8a9bfa 100644
>>>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>>>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>>>> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
>>>>    		local_port_conf.txmode.offloads |=
>>>>    			RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>>>>
>>>> -	if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
>>>> -		local_port_conf.txmode.offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>>>> -
>>>>    	printf("port %u configuring rx_offloads=0x%" PRIx64
>>>>    		", tx_offloads=0x%" PRIx64 "\n",
>>>>    		portid, local_port_conf.rxmode.offloads,
>>>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>>>> index 1839ac7..b878a48 100644
>>>> --- a/examples/ipsec-secgw/sa.c
>>>> +++ b/examples/ipsec-secgw/sa.c
>>>> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t *rx_offloads,
>>>>    				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>>>>    				&& rule->portid == port_id) {
>>>>    			*tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
>>>> +
>>>> +			/* Checksum offload is not needed for inline protocol as
>>>> +			 * all processing for Outbound IPSec packets will be
>>>> +			 * implicitly taken care and for non-IPSec packets,
>>>> +			 * there is no need of IPv4 Checksum offload.
>>>> +			 */
>>>> +			if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
>>>> +				*tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>>>> +
>>>>    			if (rule->mss)
>>>>    				*tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>>>>    		}
>>>> --
>>>> 2.8.4
>>>

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

* [PATCH v3 1/3] examples/ipsec-secgw: update error prints to data path log
  2022-02-06 14:30 [PATCH 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
                   ` (3 preceding siblings ...)
  2022-02-07  6:26 ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
@ 2022-02-23  9:53 ` Nithin Dabilpuram
  2022-02-23  9:53   ` [PATCH v3 2/3] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
                     ` (2 more replies)
  4 siblings, 3 replies; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-23  9:53 UTC (permalink / raw)
  To: Radu Nicolau, Akhil Goyal
  Cc: dev, jerinj, konstantin.ananyev, Nithin Dabilpuram

Update error prints in data path to RTE_LOG_DP().
Error prints in fast path are not good for performance
as they slow down the application when few bad packets are
received.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>
---

v2:
- Fixed issue with warning in patch 4/4 by checking for session pool
  initialization instead of mbuf_pool as now mbuf pool is per port.

v3:
- Removed patch 2/4 from this series. Will send it with other series
  that adds seperate worker thread for ipsec-secgw if all SA's are from
  inline protocol.
- Added documentation for patch 4/4.

 examples/ipsec-secgw/ipsec_worker.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec_worker.c b/examples/ipsec-secgw/ipsec_worker.c
index 7419e85..e9493c5 100644
--- a/examples/ipsec-secgw/ipsec_worker.c
+++ b/examples/ipsec-secgw/ipsec_worker.c
@@ -332,7 +332,8 @@ process_ipsec_ev_inbound(struct ipsec_ctx *ctx, struct route_table *rt,
 		break;
 
 	default:
-		RTE_LOG(ERR, IPSEC, "Unsupported packet type = %d\n", type);
+		RTE_LOG_DP(DEBUG, IPSEC_ESP, "Unsupported packet type = %d\n",
+			   type);
 		goto drop_pkt_and_exit;
 	}
 
@@ -570,7 +571,8 @@ classify_pkt(struct rte_mbuf *pkt, struct ipsec_traffic *t)
 		t->ip6.pkts[(t->ip6.num)++] = pkt;
 		break;
 	default:
-		RTE_LOG(ERR, IPSEC, "Unsupported packet type = %d\n", type);
+		RTE_LOG_DP(DEBUG, IPSEC_ESP, "Unsupported packet type = %d\n",
+			   type);
 		free_pkts(&pkt, 1);
 		break;
 	}
-- 
2.8.4


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

* [PATCH v3 2/3] examples/ipsec-secgw: fix buffer free logic in vector mode
  2022-02-23  9:53 ` [PATCH v3 1/3] " Nithin Dabilpuram
@ 2022-02-23  9:53   ` Nithin Dabilpuram
  2022-02-23  9:53   ` [PATCH v3 3/3] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
  2022-02-23 10:48   ` [PATCH v3 1/3] examples/ipsec-secgw: update error prints to data path log Akhil Goyal
  2 siblings, 0 replies; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-23  9:53 UTC (permalink / raw)
  To: Radu Nicolau, Akhil Goyal
  Cc: dev, jerinj, konstantin.ananyev, Nithin Dabilpuram, schalla, stable

Fix packet processing to skip after mbuf is freed instead of
touching and Tx'ing it.

Also free vector event buffer in event worker when after processing
there is no pkt to be enqueued to Tx adapter.

Fixes: 86738ebe1e3d ("examples/ipsec-secgw: support event vector")
Cc: schalla@marvell.com
Cc: stable@dpdk.org

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>
---
 examples/ipsec-secgw/ipsec_worker.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/examples/ipsec-secgw/ipsec_worker.c b/examples/ipsec-secgw/ipsec_worker.c
index e9493c5..8639426 100644
--- a/examples/ipsec-secgw/ipsec_worker.c
+++ b/examples/ipsec-secgw/ipsec_worker.c
@@ -205,12 +205,16 @@ check_sp_sa_bulk(struct sp_ctx *sp, struct sa_ctx *sa_ctx,
 			ip->pkts[j++] = m;
 		else {
 			sa = *(struct ipsec_sa **)rte_security_dynfield(m);
-			if (sa == NULL)
+			if (sa == NULL) {
 				free_pkts(&m, 1);
+				continue;
+			}
 
 			/* SPI on the packet should match with the one in SA */
-			if (unlikely(sa->spi != sa_ctx->sa[res - 1].spi))
+			if (unlikely(sa->spi != sa_ctx->sa[res - 1].spi)) {
 				free_pkts(&m, 1);
+				continue;
+			}
 
 			ip->pkts[j++] = m;
 		}
@@ -536,6 +540,7 @@ ipsec_ev_route_pkts(struct rte_event_vector *vec, struct route_table *rt,
 				RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)) {
 				RTE_LOG(ERR, IPSEC, "SA type not supported\n");
 				free_pkts(&pkt, 1);
+				continue;
 			}
 			rte_security_set_pkt_metadata(sess->security.ctx,
 						sess->security.ses, pkt, NULL);
@@ -695,11 +700,13 @@ ipsec_ev_vector_process(struct lcore_conf_ev_tx_int_port_wrkr *lconf,
 		ret = process_ipsec_ev_outbound_vector(&lconf->outbound,
 						       &lconf->rt, vec);
 
-	if (ret > 0) {
+	if (likely(ret > 0)) {
 		vec->nb_elem = ret;
 		rte_event_eth_tx_adapter_enqueue(links[0].eventdev_id,
 						 links[0].event_port_id,
 						 ev, 1, 0);
+	} else {
+		rte_mempool_put(rte_mempool_from_obj(vec), vec);
 	}
 }
 
@@ -720,6 +727,8 @@ ipsec_ev_vector_drv_mode_process(struct eh_event_link_info *links,
 		rte_event_eth_tx_adapter_enqueue(links[0].eventdev_id,
 						 links[0].event_port_id,
 						 ev, 1, 0);
+	else
+		rte_mempool_put(rte_mempool_from_obj(vec), vec);
 }
 
 /*
-- 
2.8.4


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

* [PATCH v3 3/3] examples/ipsec-secgw: add per port pool and vector pool size
  2022-02-23  9:53 ` [PATCH v3 1/3] " Nithin Dabilpuram
  2022-02-23  9:53   ` [PATCH v3 2/3] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
@ 2022-02-23  9:53   ` Nithin Dabilpuram
  2022-02-23 10:48   ` [PATCH v3 1/3] examples/ipsec-secgw: update error prints to data path log Akhil Goyal
  2 siblings, 0 replies; 22+ messages in thread
From: Nithin Dabilpuram @ 2022-02-23  9:53 UTC (permalink / raw)
  To: Radu Nicolau, Akhil Goyal
  Cc: dev, jerinj, konstantin.ananyev, Nithin Dabilpuram

Add support to enable per port packet pool and also override
vector pool size from command line args. This is useful
on some HW to tune performance based on usecase.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
Acked-by: Akhil Goyal <gakhil@marvell.com>
---
 doc/guides/sample_app_ug/ipsec_secgw.rst |  7 +++
 examples/ipsec-secgw/event_helper.c      | 17 +++++--
 examples/ipsec-secgw/event_helper.h      |  2 +
 examples/ipsec-secgw/ipsec-secgw.c       | 82 ++++++++++++++++++++++++--------
 examples/ipsec-secgw/ipsec-secgw.h       |  2 +
 examples/ipsec-secgw/ipsec.h             |  2 +-
 6 files changed, 88 insertions(+), 24 deletions(-)

diff --git a/doc/guides/sample_app_ug/ipsec_secgw.rst b/doc/guides/sample_app_ug/ipsec_secgw.rst
index c53ee7c..d93acf0 100644
--- a/doc/guides/sample_app_ug/ipsec_secgw.rst
+++ b/doc/guides/sample_app_ug/ipsec_secgw.rst
@@ -249,6 +249,13 @@ Where:
     Should be lower for low number of reassembly buckets.
     Valid values: from 1 ns to 10 s. Default value: 10000000 (10 s).
 
+*   ``--per-port-pool``: Enable per ethdev port pktmbuf pool.
+     By default one packet mbuf pool per socket is created and configured
+     via Rx queue setup.
+
+*   ``--vector-pool-sz``: Number of buffers in vector pool.
+    By default, vector pool size depeneds on packet pool size
+    and size of each vector.
 
 The mapping of lcores to port/queues is similar to other l3fwd applications.
 
diff --git a/examples/ipsec-secgw/event_helper.c b/examples/ipsec-secgw/event_helper.c
index 8947e41..172ab8e 100644
--- a/examples/ipsec-secgw/event_helper.c
+++ b/examples/ipsec-secgw/event_helper.c
@@ -792,8 +792,8 @@ eh_rx_adapter_configure(struct eventmode_conf *em_conf,
 	uint32_t service_id, socket_id, nb_elem;
 	struct rte_mempool *vector_pool = NULL;
 	uint32_t lcore_id = rte_lcore_id();
+	int ret, portid, nb_ports = 0;
 	uint8_t eventdev_id;
-	int ret;
 	int j;
 
 	/* Get event dev ID */
@@ -806,10 +806,21 @@ eh_rx_adapter_configure(struct eventmode_conf *em_conf,
 		return ret;
 	}
 
+	RTE_ETH_FOREACH_DEV(portid)
+		if ((em_conf->eth_portmask & (1 << portid)))
+			nb_ports++;
+
 	if (em_conf->ext_params.event_vector) {
 		socket_id = rte_lcore_to_socket_id(lcore_id);
-		nb_elem = (nb_bufs_in_pool / em_conf->ext_params.vector_size)
-			  + 1;
+
+		if (em_conf->vector_pool_sz) {
+			nb_elem = em_conf->vector_pool_sz;
+		} else {
+			nb_elem = (nb_bufs_in_pool /
+				   em_conf->ext_params.vector_size) + 1;
+			if (per_port_pool)
+				nb_elem = nb_ports * nb_elem;
+		}
 
 		vector_pool = rte_event_vector_pool_create(
 			"vector_pool", nb_elem, 0,
diff --git a/examples/ipsec-secgw/event_helper.h b/examples/ipsec-secgw/event_helper.h
index 5be6c62..f3cbe57 100644
--- a/examples/ipsec-secgw/event_helper.h
+++ b/examples/ipsec-secgw/event_helper.h
@@ -183,6 +183,8 @@ struct eventmode_conf {
 		/**< 64 bit field to specify extended params */
 	uint64_t vector_tmo_ns;
 		/**< Max vector timeout in nanoseconds */
+	uint64_t vector_pool_sz;
+		/**< Vector pool size */
 };
 
 /**
diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
index 9de1c6d..42b5081 100644
--- a/examples/ipsec-secgw/ipsec-secgw.c
+++ b/examples/ipsec-secgw/ipsec-secgw.c
@@ -118,6 +118,8 @@ struct flow_info flow_info_tbl[RTE_MAX_ETHPORTS];
 #define CMD_LINE_OPT_EVENT_VECTOR	"event-vector"
 #define CMD_LINE_OPT_VECTOR_SIZE	"vector-size"
 #define CMD_LINE_OPT_VECTOR_TIMEOUT	"vector-tmo"
+#define CMD_LINE_OPT_VECTOR_POOL_SZ	"vector-pool-sz"
+#define CMD_LINE_OPT_PER_PORT_POOL	"per-port-pool"
 
 #define CMD_LINE_ARG_EVENT	"event"
 #define CMD_LINE_ARG_POLL	"poll"
@@ -145,6 +147,8 @@ enum {
 	CMD_LINE_OPT_EVENT_VECTOR_NUM,
 	CMD_LINE_OPT_VECTOR_SIZE_NUM,
 	CMD_LINE_OPT_VECTOR_TIMEOUT_NUM,
+	CMD_LINE_OPT_VECTOR_POOL_SZ_NUM,
+	CMD_LINE_OPT_PER_PORT_POOL_NUM,
 };
 
 static const struct option lgopts[] = {
@@ -161,6 +165,8 @@ static const struct option lgopts[] = {
 	{CMD_LINE_OPT_EVENT_VECTOR, 0, 0, CMD_LINE_OPT_EVENT_VECTOR_NUM},
 	{CMD_LINE_OPT_VECTOR_SIZE, 1, 0, CMD_LINE_OPT_VECTOR_SIZE_NUM},
 	{CMD_LINE_OPT_VECTOR_TIMEOUT, 1, 0, CMD_LINE_OPT_VECTOR_TIMEOUT_NUM},
+	{CMD_LINE_OPT_VECTOR_POOL_SZ, 1, 0, CMD_LINE_OPT_VECTOR_POOL_SZ_NUM},
+	{CMD_LINE_OPT_PER_PORT_POOL, 0, 0, CMD_LINE_OPT_PER_PORT_POOL_NUM},
 	{NULL, 0, 0, 0}
 };
 
@@ -234,7 +240,6 @@ struct lcore_conf {
 	struct rt_ctx *rt6_ctx;
 	struct {
 		struct rte_ip_frag_tbl *tbl;
-		struct rte_mempool *pool_dir;
 		struct rte_mempool *pool_indir;
 		struct rte_ip_frag_death_row dr;
 	} frag;
@@ -262,6 +267,8 @@ static struct rte_eth_conf port_conf = {
 
 struct socket_ctx socket_ctx[NB_SOCKETS];
 
+bool per_port_pool;
+
 /*
  * Determine is multi-segment support required:
  *  - either frame buffer size is smaller then mtu
@@ -630,12 +637,10 @@ send_fragment_packet(struct lcore_conf *qconf, struct rte_mbuf *m,
 
 	if (proto == IPPROTO_IP)
 		rc = rte_ipv4_fragment_packet(m, tbl->m_table + len,
-			n, mtu_size, qconf->frag.pool_dir,
-			qconf->frag.pool_indir);
+			n, mtu_size, m->pool, qconf->frag.pool_indir);
 	else
 		rc = rte_ipv6_fragment_packet(m, tbl->m_table + len,
-			n, mtu_size, qconf->frag.pool_dir,
-			qconf->frag.pool_indir);
+			n, mtu_size, m->pool, qconf->frag.pool_indir);
 
 	if (rc >= 0)
 		len += rc;
@@ -1256,7 +1261,6 @@ ipsec_poll_mode_worker(void)
 	qconf->outbound.session_pool = socket_ctx[socket_id].session_pool;
 	qconf->outbound.session_priv_pool =
 			socket_ctx[socket_id].session_priv_pool;
-	qconf->frag.pool_dir = socket_ctx[socket_id].mbuf_pool;
 	qconf->frag.pool_indir = socket_ctx[socket_id].mbuf_pool_indir;
 
 	rc = ipsec_sad_lcore_cache_init(app_sa_prm.cache_sz);
@@ -1511,6 +1515,9 @@ print_usage(const char *prgname)
 		"  --vector-size Max vector size (default value: 16)\n"
 		"  --vector-tmo Max vector timeout in nanoseconds"
 		"    (default value: 102400)\n"
+		"  --" CMD_LINE_OPT_PER_PORT_POOL " Enable per port mbuf pool\n"
+		"  --" CMD_LINE_OPT_VECTOR_POOL_SZ " Vector pool size\n"
+		"                    (default value is based on mbuf count)\n"
 		"\n",
 		prgname);
 }
@@ -1894,6 +1901,15 @@ parse_args(int32_t argc, char **argv, struct eh_conf *eh_conf)
 			em_conf = eh_conf->mode_params;
 			em_conf->vector_tmo_ns = ret;
 			break;
+		case CMD_LINE_OPT_VECTOR_POOL_SZ_NUM:
+			ret = parse_decimal(optarg);
+
+			em_conf = eh_conf->mode_params;
+			em_conf->vector_pool_sz = ret;
+			break;
+		case CMD_LINE_OPT_PER_PORT_POOL_NUM:
+			per_port_pool = 1;
+			break;
 		default:
 			print_usage(prgname);
 			return -1;
@@ -2381,6 +2397,7 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 		/* init RX queues */
 		for (queue = 0; queue < qconf->nb_rx_queue; ++queue) {
 			struct rte_eth_rxconf rxq_conf;
+			struct rte_mempool *pool;
 
 			if (portid != qconf->rx_queue_list[queue].port_id)
 				continue;
@@ -2392,9 +2409,14 @@ port_init(uint16_t portid, uint64_t req_rx_offloads, uint64_t req_tx_offloads)
 
 			rxq_conf = dev_info.default_rxconf;
 			rxq_conf.offloads = local_port_conf.rxmode.offloads;
+
+			if (per_port_pool)
+				pool = socket_ctx[socket_id].mbuf_pool[portid];
+			else
+				pool = socket_ctx[socket_id].mbuf_pool[0];
+
 			ret = rte_eth_rx_queue_setup(portid, rx_queueid,
-					nb_rxd,	socket_id, &rxq_conf,
-					socket_ctx[socket_id].mbuf_pool);
+					nb_rxd,	socket_id, &rxq_conf, pool);
 			if (ret < 0)
 				rte_exit(EXIT_FAILURE,
 					"rte_eth_rx_queue_setup: err=%d, "
@@ -2507,28 +2529,37 @@ session_priv_pool_init(struct socket_ctx *ctx, int32_t socket_id,
 }
 
 static void
-pool_init(struct socket_ctx *ctx, int32_t socket_id, uint32_t nb_mbuf)
+pool_init(struct socket_ctx *ctx, int32_t socket_id, int portid,
+	  uint32_t nb_mbuf)
 {
 	char s[64];
 	int32_t ms;
 
-	snprintf(s, sizeof(s), "mbuf_pool_%d", socket_id);
-	ctx->mbuf_pool = rte_pktmbuf_pool_create(s, nb_mbuf,
-			MEMPOOL_CACHE_SIZE, ipsec_metadata_size(),
-			frame_buf_size, socket_id);
+
+	/* mbuf_pool is initialised by the pool_init() function*/
+	if (socket_ctx[socket_id].mbuf_pool[portid])
+		return;
+
+	snprintf(s, sizeof(s), "mbuf_pool_%d_%d", socket_id, portid);
+	ctx->mbuf_pool[portid] = rte_pktmbuf_pool_create(s, nb_mbuf,
+							 MEMPOOL_CACHE_SIZE,
+							 ipsec_metadata_size(),
+							 frame_buf_size,
+							 socket_id);
 
 	/*
 	 * if multi-segment support is enabled, then create a pool
-	 * for indirect mbufs.
+	 * for indirect mbufs. This is not per-port but global.
 	 */
 	ms = multi_seg_required();
-	if (ms != 0) {
+	if (ms != 0 && !ctx->mbuf_pool_indir) {
 		snprintf(s, sizeof(s), "mbuf_pool_indir_%d", socket_id);
 		ctx->mbuf_pool_indir = rte_pktmbuf_pool_create(s, nb_mbuf,
 			MEMPOOL_CACHE_SIZE, 0, 0, socket_id);
 	}
 
-	if (ctx->mbuf_pool == NULL || (ms != 0 && ctx->mbuf_pool_indir == NULL))
+	if (ctx->mbuf_pool[portid] == NULL ||
+	    (ms != 0 && ctx->mbuf_pool_indir == NULL))
 		rte_exit(EXIT_FAILURE, "Cannot init mbuf pool on socket %d\n",
 				socket_id);
 	else
@@ -3344,11 +3375,22 @@ 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)
+		if (per_port_pool) {
+			RTE_ETH_FOREACH_DEV(portid) {
+				if ((enabled_port_mask & (1 << portid)) == 0)
+					continue;
+
+				pool_init(&socket_ctx[socket_id], socket_id,
+					  portid, nb_bufs_in_pool);
+			}
+		} else {
+			pool_init(&socket_ctx[socket_id], socket_id, 0,
+				  nb_bufs_in_pool);
+		}
+
+		if (socket_ctx[socket_id].session_pool)
 			continue;
 
-		pool_init(&socket_ctx[socket_id], socket_id, nb_bufs_in_pool);
 		session_pool_init(&socket_ctx[socket_id], socket_id, sess_sz);
 		session_priv_pool_init(&socket_ctx[socket_id], socket_id,
 			sess_sz);
@@ -3421,7 +3463,7 @@ main(int32_t argc, char **argv)
 	/* 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) &&
+		if ((socket_ctx[socket_id].session_pool != NULL) &&
 			(socket_ctx[socket_id].sa_in == NULL) &&
 			(socket_ctx[socket_id].sa_out == NULL)) {
 			sa_init(&socket_ctx[socket_id], socket_id);
diff --git a/examples/ipsec-secgw/ipsec-secgw.h b/examples/ipsec-secgw/ipsec-secgw.h
index ac4fa5e..24f11ad 100644
--- a/examples/ipsec-secgw/ipsec-secgw.h
+++ b/examples/ipsec-secgw/ipsec-secgw.h
@@ -134,6 +134,8 @@ extern volatile bool force_quit;
 
 extern uint32_t nb_bufs_in_pool;
 
+extern bool per_port_pool;
+
 static inline uint8_t
 is_unprotected_port(uint16_t port_id)
 {
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index bc87b1a..ccfde8e 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -248,7 +248,7 @@ struct socket_ctx {
 	struct sp_ctx *sp_ip6_out;
 	struct rt_ctx *rt_ip4;
 	struct rt_ctx *rt_ip6;
-	struct rte_mempool *mbuf_pool;
+	struct rte_mempool *mbuf_pool[RTE_MAX_ETHPORTS];
 	struct rte_mempool *mbuf_pool_indir;
 	struct rte_mempool *session_pool;
 	struct rte_mempool *session_priv_pool;
-- 
2.8.4


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

* Re: [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline
  2022-02-18 13:58         ` Nithin Kumar Dabilpuram
@ 2022-02-23  9:58           ` Nithin Kumar Dabilpuram
  0 siblings, 0 replies; 22+ messages in thread
From: Nithin Kumar Dabilpuram @ 2022-02-23  9:58 UTC (permalink / raw)
  To: Ananyev, Konstantin, dev; +Cc: Nicolau, Radu, gakhil



On 2/18/22 7:28 PM, Nithin Kumar Dabilpuram wrote:
> 
> 
> On 2/18/22 12:47 AM, Ananyev, Konstantin wrote:
>>
>>>>> Enable Tx IPv4 checksum offload only when Tx inline crypto is needed.
>>>>> In other cases such as Tx Inline protocol offload, checksum 
>>>>> computation
>>>>> is implicitly taken care by HW.
>>>>
>>>> Why is that?
>>>> These is two separate HW offload and user has to enable each of them 
>>>> explicitly.
>>>
>>>
>>> In Inline IPSec protocol offload, the complete tunnel header for tunnel
>>> mode is updated by HW/PMD. So it doesn't have any dependency on
>>> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM as there is no valid l2_len/l3_len yet in
>>> the mbuf. Similarly in case of Transport mode, the IPv4 header is
>>> updated by HW itself for next proto and hence the offsets and all can
>>> vary based on the HW implementation.
>>>
>>> Hence my thought was for Inline IPsec protocol offload, there is no need
>>> to explicitly say that RTE_ETH_TX_OFFLOAD_IPV4_CKSUM is enabled and need
>>> not provide ol_flags RTE_MBUF_F_TX_IP_CKSUM and l2_len and l3_len which
>>> might not be correct in prepare_tx_pkt().
>>>
>>>   >* RTE_MBUF_F_TX_IP_CKSUM.
>>>   > *  - fill the mbuf offload information: l2_len, l3_len
>>> (Ex: Tunnel header being inserted is IPv6 while inner header is IPv4.
>>>
>>> For inline crypto I agree, the packet content is all in place except for
>>> plain text->cipher text translation so l2/l3 offsets are valid.
>>
>> Ok, but apart from inline modes we also have lookaside modes.
>> Why to disable ip cksum for them?
> 
> Ack, I missed that case. I'll make the change to enable Tx checksum 
> offload enabled even for Lookaside crypto.
> 
>>
>>>
>>>   > Also we can TX clear-text traffic.
>>> Ok, I agree that we can have clear-text traffic. We are already handling
>>> ipv4 checksum in SW in case Tx offload doesn't have IPv4 Checksum
>>> offload enabled. And for clear text traffic I think that is not needed
>>> as well as we are not updating ttl.
>>
>> As I remember we always recalculate ip cksum if RTE_MBUF_F_TX_IP_CKSUM
>> is not set:
>>
>> static inline void
>> prepare_tx_pkt(struct rte_mbuf *pkt, uint16_t port,
>>                  const struct lcore_conf *qconf)
>> {
>>          struct ip *ip;
>>          struct rte_ether_hdr *ethhdr;
>>
>>          ip = rte_pktmbuf_mtod(pkt, struct ip *);
>>
>>          ethhdr = (struct rte_ether_hdr *)
>>                  rte_pktmbuf_prepend(pkt, RTE_ETHER_HDR_LEN);
>>
>>          if (ip->ip_v == IPVERSION) {
>>                  pkt->ol_flags |= qconf->outbound.ipv4_offloads;
>>                  pkt->l3_len = sizeof(struct ip);
>>                  pkt->l2_len = RTE_ETHER_HDR_LEN;
>>
>>                  ip->ip_sum = 0;
>>
>>                  /* calculate IPv4 cksum in SW */
>>                  if ((pkt->ol_flags & RTE_MBUF_F_TX_IP_CKSUM) == 0)
>>                          ip->ip_sum = rte_ipv4_cksum((struct 
>> rte_ipv4_hdr *)ip);
>>
>> ...
>>
> 
> I'm working on another series to restructure fast path based on 
> different mode. As part of that, I think we can avoid checksum 
> recalculation in cases of inline protocol offload.

I removed this patch from v3 of this series. I'll included it in other 
series that I'm working on to have separate worker thread when all
SA's are of particular mode. There we can avoid checksum recalculation
completely both in SW and HW.

Thanks
Nithin

> 
>>>
>>> My overall intention was to have lighter Tx burst function for Inline
>>> IPsec protocol offload as mainly IPsec traffic and not plain traffic is
>>> primary use case for ipsec-secgw.
>>>
>>>
>>>
>>>>
>>>>> The advantage of having only necessary
>>>>> offloads enabled is that Tx burst function can be as light as 
>>>>> possible.
>>>>>
>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>> ---
>>>>>    examples/ipsec-secgw/ipsec-secgw.c | 3 ---
>>>>>    examples/ipsec-secgw/sa.c          | 9 +++++++++
>>>>>    2 files changed, 9 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c 
>>>>> b/examples/ipsec-secgw/ipsec-secgw.c
>>>>> index 21abc0d..d8a9bfa 100644
>>>>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>>>>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>>>>> @@ -2314,9 +2314,6 @@ port_init(uint16_t portid, uint64_t 
>>>>> req_rx_offloads, uint64_t req_tx_offloads)
>>>>>            local_port_conf.txmode.offloads |=
>>>>>                RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>>>>>
>>>>> -    if (dev_info.tx_offload_capa & RTE_ETH_TX_OFFLOAD_IPV4_CKSUM)
>>>>> -        local_port_conf.txmode.offloads |= 
>>>>> RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>>>>> -
>>>>>        printf("port %u configuring rx_offloads=0x%" PRIx64
>>>>>            ", tx_offloads=0x%" PRIx64 "\n",
>>>>>            portid, local_port_conf.rxmode.offloads,
>>>>> diff --git a/examples/ipsec-secgw/sa.c b/examples/ipsec-secgw/sa.c
>>>>> index 1839ac7..b878a48 100644
>>>>> --- a/examples/ipsec-secgw/sa.c
>>>>> +++ b/examples/ipsec-secgw/sa.c
>>>>> @@ -1790,6 +1790,15 @@ sa_check_offloads(uint16_t port_id, uint64_t 
>>>>> *rx_offloads,
>>>>>                    RTE_SECURITY_ACTION_TYPE_INLINE_PROTOCOL)
>>>>>                    && rule->portid == port_id) {
>>>>>                *tx_offloads |= RTE_ETH_TX_OFFLOAD_SECURITY;
>>>>> +
>>>>> +            /* Checksum offload is not needed for inline protocol as
>>>>> +             * all processing for Outbound IPSec packets will be
>>>>> +             * implicitly taken care and for non-IPSec packets,
>>>>> +             * there is no need of IPv4 Checksum offload.
>>>>> +             */
>>>>> +            if (rule_type == RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO)
>>>>> +                *tx_offloads |= RTE_ETH_TX_OFFLOAD_IPV4_CKSUM;
>>>>> +
>>>>>                if (rule->mss)
>>>>>                    *tx_offloads |= RTE_ETH_TX_OFFLOAD_TCP_TSO;
>>>>>            }
>>>>> -- 
>>>>> 2.8.4
>>>>

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

* RE: [PATCH v3 1/3] examples/ipsec-secgw: update error prints to data path log
  2022-02-23  9:53 ` [PATCH v3 1/3] " Nithin Dabilpuram
  2022-02-23  9:53   ` [PATCH v3 2/3] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
  2022-02-23  9:53   ` [PATCH v3 3/3] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
@ 2022-02-23 10:48   ` Akhil Goyal
  2 siblings, 0 replies; 22+ messages in thread
From: Akhil Goyal @ 2022-02-23 10:48 UTC (permalink / raw)
  To: Nithin Kumar Dabilpuram, Radu Nicolau
  Cc: dev, Jerin Jacob Kollanukkaran, konstantin.ananyev,
	Nithin Kumar Dabilpuram

> Update error prints in data path to RTE_LOG_DP().
> Error prints in fast path are not good for performance
> as they slow down the application when few bad packets are
> received.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> Acked-by: Akhil Goyal <gakhil@marvell.com>
> ---
Series Applied to dpdk-next-crypto

Thanks.

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

end of thread, other threads:[~2022-02-23 10:48 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-06 14:30 [PATCH 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
2022-02-06 14:30 ` [PATCH 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
2022-02-07  9:52   ` Ananyev, Konstantin
2022-02-07 14:15     ` Nithin Kumar Dabilpuram
2022-02-17 19:17       ` Ananyev, Konstantin
2022-02-18 13:58         ` Nithin Kumar Dabilpuram
2022-02-23  9:58           ` Nithin Kumar Dabilpuram
2022-02-06 14:30 ` [PATCH 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
2022-02-06 14:30 ` [PATCH 4/4] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
2022-02-07  6:26 ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Nithin Dabilpuram
2022-02-07  6:26   ` [PATCH v2 2/4] examples/ipsec-secgw: disable Tx chksum offload for inline Nithin Dabilpuram
2022-02-17 18:12     ` Akhil Goyal
2022-02-17 19:22     ` Ananyev, Konstantin
2022-02-07  6:26   ` [PATCH v2 3/4] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
2022-02-17 18:12     ` Akhil Goyal
2022-02-07  6:26   ` [PATCH v2 4/4] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
2022-02-17 18:13     ` Akhil Goyal
2022-02-17 18:11   ` [PATCH v2 1/4] examples/ipsec-secgw: update error prints to data path log Akhil Goyal
2022-02-23  9:53 ` [PATCH v3 1/3] " Nithin Dabilpuram
2022-02-23  9:53   ` [PATCH v3 2/3] examples/ipsec-secgw: fix buffer free logic in vector mode Nithin Dabilpuram
2022-02-23  9:53   ` [PATCH v3 3/3] examples/ipsec-secgw: add per port pool and vector pool size Nithin Dabilpuram
2022-02-23 10:48   ` [PATCH v3 1/3] examples/ipsec-secgw: update error prints to data path log Akhil Goyal

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).