[dpdk-dev] [PATCH] net/ixgbe: add security statistics

[dpdk-dev] [PATCH] net/ixgbe: add security statistics

[Radu Nicolau]

Update IXGBE PMD with support for IPsec statistics.

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
---
 drivers/net/ixgbe/ixgbe_ipsec.c        | 244 ++++++++++++++++++++++++++++++++-
 drivers/net/ixgbe/ixgbe_ipsec.h        |  17 ++-
 drivers/net/ixgbe/ixgbe_rxtx.c         |  18 +++
 drivers/net/ixgbe/ixgbe_rxtx.h         |   4 +
 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c |   6 +
 5 files changed, 285 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
index 48f5082..94daf92 100644
--- a/drivers/net/ixgbe/ixgbe_ipsec.c
+++ b/drivers/net/ixgbe/ixgbe_ipsec.c
@@ -9,6 +9,7 @@
 #include <rte_security_driver.h>
 #include <rte_cryptodev.h>
 #include <rte_flow.h>
+#include <rte_hash_crc.h>
 
 #include "base/ixgbe_type.h"
 #include "base/ixgbe_api.h"
@@ -94,6 +95,7 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 			dev->data->dev_private);
 	uint32_t reg_val;
 	int sa_index = -1;
+	struct ixgbe_crypto_sess_htable_key tbl_key = {0};
 
 	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
 		int i, ip_index = -1;
@@ -158,9 +160,14 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 		if (ic_session->dst_ip.type == IPv6) {
 			priv->rx_sa_tbl[sa_index].mode |= IPSRXMOD_IPV6;
 			priv->rx_ip_tbl[ip_index].ip.type = IPv6;
-		} else if (ic_session->dst_ip.type == IPv4)
+			memcpy(&tbl_key.ip.ipv6, &ic_session->dst_ip.ipv6,
+					sizeof(ic_session->dst_ip.ipv6));
+		} else if (ic_session->dst_ip.type == IPv4) {
 			priv->rx_ip_tbl[ip_index].ip.type = IPv4;
+			tbl_key.ip.ipv4 = ic_session->dst_ip.ipv4;
+		}
 
+		tbl_key.spi = priv->rx_sa_tbl[sa_index].spi;
 		priv->rx_sa_tbl[sa_index].used = 1;
 
 		/* write IP table entry*/
@@ -238,6 +245,7 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 
 		priv->tx_sa_tbl[sa_index].spi =
 			rte_cpu_to_be_32(ic_session->spi);
+		tbl_key.spi = priv->tx_sa_tbl[sa_index].spi;
 		priv->tx_sa_tbl[i].used = 1;
 		ic_session->sa_index = sa_index;
 
@@ -264,6 +272,7 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
 		free(key);
 	}
 
+	rte_hash_add_key_data(priv->session_tbl, &tbl_key, ic_session);
 	return 0;
 }
 
@@ -276,6 +285,7 @@ ixgbe_crypto_remove_sa(struct rte_eth_dev *dev,
 			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
 	uint32_t reg_val;
 	int sa_index = -1;
+	struct ixgbe_crypto_sess_htable_key tbl_key = {0};
 
 	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
 		int i, ip_index = -1;
@@ -324,6 +334,13 @@ ixgbe_crypto_remove_sa(struct rte_eth_dev *dev,
 		IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, 0);
 		IXGBE_WAIT_RWRITE;
 		priv->rx_sa_tbl[sa_index].used = 0;
+		if (priv->rx_sa_tbl[sa_index].mode & IPSRXMOD_IPV6) {
+			memcpy(&tbl_key.ip.ipv6, &ic_session->dst_ip.ipv6,
+					sizeof(ic_session->dst_ip.ipv6));
+		} else {
+			tbl_key.ip.ipv4 = ic_session->dst_ip.ipv4;
+		}
+		tbl_key.spi = priv->rx_sa_tbl[sa_index].spi;
 
 		/* If last used then clear the IP table entry*/
 		priv->rx_ip_tbl[ip_index].ref_count--;
@@ -361,8 +378,10 @@ ixgbe_crypto_remove_sa(struct rte_eth_dev *dev,
 		IXGBE_WAIT_TWRITE;
 
 		priv->tx_sa_tbl[sa_index].used = 0;
+		tbl_key.spi = priv->tx_sa_tbl[sa_index].spi;
 	}
 
+	rte_hash_del_key(priv->session_tbl, &tbl_key);
 	return 0;
 }
 
@@ -376,6 +395,8 @@ ixgbe_crypto_create_session(void *device,
 	struct ixgbe_crypto_session *ic_session = NULL;
 	struct rte_crypto_aead_xform *aead_xform;
 	struct rte_eth_conf *dev_conf = &eth_dev->data->dev_conf;
+	struct ixgbe_ipsec *priv =
+			IXGBE_DEV_PRIVATE_TO_IPSEC(eth_dev->data->dev_private);
 
 	if (rte_mempool_get(mempool, (void **)&ic_session)) {
 		PMD_DRV_LOG(ERR, "Cannot get object from ic_session mempool");
@@ -426,6 +447,40 @@ ixgbe_crypto_create_session(void *device,
 		}
 	}
 
+	if (conf->ipsec.options.stats) {
+		ic_session->stats_enabled = 1;
+		priv->per_session_stats_active++;
+	}
+
+	return 0;
+}
+
+static int
+ixgbe_crypto_stats_get(__rte_unused void *device,
+		struct rte_security_session *sess,
+		struct rte_security_stats *stats)
+{
+	struct rte_eth_dev *eth_dev = device;
+	struct ixgbe_ipsec *priv =
+			IXGBE_DEV_PRIVATE_TO_IPSEC(eth_dev->data->dev_private);
+	volatile struct rte_security_ipsec_stats *ixgbe_stats;
+
+	if (sess) {
+		struct ixgbe_crypto_session *ic_session =
+			(struct ixgbe_crypto_session *)
+			get_sec_session_private_data(sess);
+		ixgbe_stats = &ic_session->stats;
+	} else {
+		ixgbe_stats = &priv->stats;
+	}
+
+	stats->ipsec.ipackets = ixgbe_stats->ipackets;
+	stats->ipsec.opackets = ixgbe_stats->opackets;
+	stats->ipsec.ibytes = ixgbe_stats->ibytes;
+	stats->ipsec.obytes = ixgbe_stats->obytes;
+	stats->ipsec.ierrors = ixgbe_stats->ierrors;
+	stats->ipsec.oerrors = ixgbe_stats->oerrors;
+
 	return 0;
 }
 
@@ -444,6 +499,8 @@ ixgbe_crypto_remove_session(void *device,
 		(struct ixgbe_crypto_session *)
 		get_sec_session_private_data(session);
 	struct rte_mempool *mempool = rte_mempool_from_obj(ic_session);
+	struct ixgbe_ipsec *priv =
+			IXGBE_DEV_PRIVATE_TO_IPSEC(eth_dev->data->dev_private);
 
 	if (eth_dev != ic_session->dev) {
 		PMD_DRV_LOG(ERR, "Session not bound to this device\n");
@@ -455,11 +512,44 @@ ixgbe_crypto_remove_session(void *device,
 		return -EFAULT;
 	}
 
+	if (ic_session->stats_enabled && priv->per_session_stats_active > 0)
+		priv->per_session_stats_active--;
+
 	rte_mempool_put(mempool, (void *)ic_session);
 
 	return 0;
 }
 
+static int
+ixgbe_crypto_update_session(void *device,
+		struct rte_security_session *session,
+		struct rte_security_session_conf *conf)
+{
+	struct rte_eth_dev *eth_dev = device;
+	struct ixgbe_crypto_session *ic_session =
+		(struct ixgbe_crypto_session *)
+		get_sec_session_private_data(session);
+	struct ixgbe_ipsec *priv =
+			IXGBE_DEV_PRIVATE_TO_IPSEC(eth_dev->data->dev_private);
+
+	if (eth_dev != ic_session->dev) {
+		PMD_DRV_LOG(ERR, "Session not bound to this device\n");
+		return -ENODEV;
+	}
+
+	/* Enable/disable per session stats */
+	if (ic_session->stats_enabled && !conf->ipsec.options.stats) {
+		ic_session->stats_enabled = 0;
+		if (priv->per_session_stats_active > 0)
+			priv->per_session_stats_active--;
+	} else if (!ic_session->stats_enabled && conf->ipsec.options.stats) {
+		ic_session->stats_enabled = 1;
+		priv->per_session_stats_active++;
+	}
+
+	return 0;
+}
+
 static inline uint8_t
 ixgbe_crypto_compute_pad_len(struct rte_mbuf *m)
 {
@@ -624,6 +714,8 @@ int
 ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev)
 {
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct ixgbe_ipsec *priv =
+			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
 	uint32_t reg;
 	uint64_t rx_offloads;
 	uint64_t tx_offloads;
@@ -676,6 +768,23 @@ ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev)
 
 	ixgbe_crypto_clear_ipsec_tables(dev);
 
+	if (priv->session_tbl == NULL) {
+		char session_tbl_hash_name[RTE_HASH_NAMESIZE];
+		struct rte_hash_parameters params = {
+			.name = session_tbl_hash_name,
+			.entries = IPSEC_MAX_SA_COUNT * 2,
+			.key_len = sizeof(struct ixgbe_crypto_sess_htable_key),
+			.hash_func = rte_hash_crc,
+			.socket_id = rte_socket_id(),
+		};
+		snprintf(session_tbl_hash_name, RTE_HASH_NAMESIZE,
+			 "session_tbl_hash_%s", dev->device->name);
+
+		priv->session_tbl = rte_hash_create(&params);
+	} else {
+		rte_hash_reset(priv->session_tbl);
+	}
+
 	return 0;
 }
 
@@ -709,11 +818,140 @@ ixgbe_crypto_add_ingress_sa_from_flow(const void *sess,
 	return 0;
 }
 
+void
+ixgbe_crypto_update_rx_stats(struct ixgbe_ipsec *ixgbe_ipsec,
+			     struct rte_mbuf **mbufs,
+			     uint16_t count)
+{
+	uint16_t i;
+	uint32_t ipackets = 0, ibytes = 0, ierrors = 0;
+
+	if (ixgbe_ipsec->per_session_stats_active) {
+		struct ip *ip;
+		struct rte_esp_hdr *esp;
+		struct ixgbe_crypto_session *sess;
+		struct ixgbe_crypto_sess_htable_key tbl_key = {0};
+
+		for (i = 0; i < count; i++) {
+			if ((mbufs[i]->ol_flags & PKT_RX_SEC_OFFLOAD) == 0)
+				continue;
+			ip = rte_pktmbuf_mtod_offset(mbufs[i],
+					struct ip *,
+					sizeof(struct rte_ether_hdr));
+			if (ip->ip_v == IPVERSION) {
+				tbl_key.ip.ipv4 = ip->ip_dst.s_addr;
+				esp = rte_pktmbuf_mtod_offset(mbufs[i],
+						struct rte_esp_hdr *,
+						sizeof(struct rte_ether_hdr) +
+						ip->ip_hl * 4);
+			} else {
+				struct ip6_hdr *ip6 = (struct ip6_hdr *)ip;
+				memcpy(&tbl_key.ip.ipv6, &ip6->ip6_dst, 16);
+				esp = rte_pktmbuf_mtod_offset(mbufs[i],
+						struct rte_esp_hdr *,
+						sizeof(struct rte_ether_hdr) +
+						sizeof(struct ip6_hdr));
+			}
+			tbl_key.spi = esp->spi;
+			if (rte_hash_lookup_data(ixgbe_ipsec->session_tbl,
+					&tbl_key, (void **)&sess) < 0)
+				sess = NULL;
+			if (mbufs[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) {
+				if (sess && sess->stats_enabled)
+					sess->stats.ierrors++;
+				ierrors++;
+			} else {
+				if (sess && sess->stats_enabled) {
+					sess->stats.ipackets++;
+					sess->stats.ibytes +=
+						rte_pktmbuf_pkt_len(mbufs[i]);
+				}
+				ipackets++;
+				ibytes += rte_pktmbuf_pkt_len(mbufs[i]);
+			}
+		}
+	} else { /* Global stats only */
+		for (i = 0; i < count; i++) {
+			if (mbufs[i]->ol_flags & PKT_RX_SEC_OFFLOAD) {
+				if (mbufs[i]->ol_flags &
+						PKT_RX_SEC_OFFLOAD_FAILED) {
+					ierrors++;
+				} else {
+					ipackets++;
+					ibytes += rte_pktmbuf_pkt_len(mbufs[i]);
+				}
+			}
+		}
+	}
+
+	/* Update global stats */
+	ixgbe_ipsec->stats.ipackets += ipackets;
+	ixgbe_ipsec->stats.ibytes += ibytes;
+	ixgbe_ipsec->stats.ierrors += ierrors;
+}
+
+void
+ixgbe_crypto_update_tx_stats(struct ixgbe_ipsec *ixgbe_ipsec,
+			     struct rte_mbuf **mbufs,
+			     uint16_t count)
+{
+	uint16_t i;
+	uint32_t opackets = 0, obytes = 0;
+
+	if (ixgbe_ipsec->per_session_stats_active) {
+		struct ip *ip;
+		struct rte_esp_hdr *esp;
+		struct ixgbe_crypto_session *sess;
+		struct ixgbe_crypto_sess_htable_key tbl_key = {0};
+
+		for (i = 0; i < count; i++) {
+			if ((mbufs[i]->ol_flags & PKT_TX_SEC_OFFLOAD) == 0)
+				continue;
+			ip = rte_pktmbuf_mtod_offset(mbufs[i],
+					struct ip *,
+					sizeof(struct rte_ether_hdr));
+			if (ip->ip_v == IPVERSION) {
+				esp = rte_pktmbuf_mtod_offset(mbufs[i],
+						struct rte_esp_hdr *,
+						sizeof(struct rte_ether_hdr) +
+						ip->ip_hl * 4);
+			} else {
+				esp = rte_pktmbuf_mtod_offset(mbufs[i],
+						struct rte_esp_hdr *,
+						sizeof(struct rte_ether_hdr) +
+						sizeof(struct ip6_hdr));
+			}
+			tbl_key.spi = esp->spi;
+			if (rte_hash_lookup_data(ixgbe_ipsec->session_tbl,
+					&tbl_key, (void **)&sess) < 0)
+				sess = NULL;
+			if (sess && sess->stats_enabled) {
+				sess->stats.opackets++;
+				sess->stats.obytes +=
+					rte_pktmbuf_pkt_len(mbufs[i]);
+			}
+			opackets++;
+			obytes += rte_pktmbuf_pkt_len(mbufs[i]);
+		}
+	} else { /* Global stats only */
+		for (i = 0; i < count; i++) {
+			if (mbufs[i]->ol_flags & PKT_RX_SEC_OFFLOAD) {
+				opackets++;
+				obytes += rte_pktmbuf_pkt_len(mbufs[i]);
+			}
+		}
+	}
+
+	/* Update global stats */
+	ixgbe_ipsec->stats.opackets += opackets;
+	ixgbe_ipsec->stats.obytes += obytes;
+}
+
 static struct rte_security_ops ixgbe_security_ops = {
 	.session_create = ixgbe_crypto_create_session,
-	.session_update = NULL,
+	.session_update = ixgbe_crypto_update_session,
 	.session_get_size = ixgbe_crypto_session_get_size,
-	.session_stats_get = NULL,
+	.session_stats_get = ixgbe_crypto_stats_get,
 	.session_destroy = ixgbe_crypto_remove_session,
 	.set_pkt_metadata = ixgbe_crypto_update_mb,
 	.capabilities_get = ixgbe_crypto_capabilities_get
diff --git a/drivers/net/ixgbe/ixgbe_ipsec.h b/drivers/net/ixgbe/ixgbe_ipsec.h
index e218c0a..a4ff8b6 100644
--- a/drivers/net/ixgbe/ixgbe_ipsec.h
+++ b/drivers/net/ixgbe/ixgbe_ipsec.h
@@ -70,6 +70,8 @@ struct ixgbe_crypto_session {
 	struct ipaddr src_ip;
 	struct ipaddr dst_ip;
 	struct rte_eth_dev *dev;
+	volatile struct rte_security_ipsec_stats stats;
+	uint8_t stats_enabled;
 } __rte_cache_aligned;
 
 struct ixgbe_crypto_rx_ip_table {
@@ -100,10 +102,18 @@ union ixgbe_crypto_tx_desc_md {
 	};
 };
 
+struct ixgbe_crypto_sess_htable_key {
+	uint32_t spi;
+	struct ipaddr ip;
+};
+
 struct ixgbe_ipsec {
 	struct ixgbe_crypto_rx_ip_table rx_ip_tbl[IPSEC_MAX_RX_IP_COUNT];
 	struct ixgbe_crypto_rx_sa_table rx_sa_tbl[IPSEC_MAX_SA_COUNT];
 	struct ixgbe_crypto_tx_sa_table tx_sa_tbl[IPSEC_MAX_SA_COUNT];
+	volatile struct rte_security_ipsec_stats stats;
+	volatile uint16_t per_session_stats_active;
+	struct rte_hash *session_tbl;
 };
 
 
@@ -112,7 +122,12 @@ int ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev);
 int ixgbe_crypto_add_ingress_sa_from_flow(const void *sess,
 					  const void *ip_spec,
 					  uint8_t is_ipv6);
-
+void ixgbe_crypto_update_rx_stats(struct ixgbe_ipsec *ixgbe_ipsec,
+				  struct rte_mbuf **mbufs,
+				  uint16_t count);
+void ixgbe_crypto_update_tx_stats(struct ixgbe_ipsec *ixgbe_ipsec,
+				  struct rte_mbuf **mbufs,
+				  uint16_t count);
 
 
 #endif /*IXGBE_IPSEC_H_*/
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index edcfa60..cde012f 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -956,6 +956,12 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	IXGBE_PCI_REG_WRITE_RELAXED(txq->tdt_reg_addr, tx_id);
 	txq->tx_tail = tx_id;
 
+#ifdef RTE_LIBRTE_SECURITY
+		if (unlikely(txq->using_ipsec))
+			ixgbe_crypto_update_tx_stats(txq->ixgbe_ipsec,
+					&tx_pkts[-nb_tx], nb_tx);
+#endif
+
 	return nb_tx;
 }
 
@@ -1643,6 +1649,12 @@ ixgbe_rx_fill_from_stage(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	rxq->rx_nb_avail = (uint16_t)(rxq->rx_nb_avail - nb_pkts);
 	rxq->rx_next_avail = (uint16_t)(rxq->rx_next_avail + nb_pkts);
 
+#ifdef RTE_LIBRTE_SECURITY
+	if (unlikely(rxq->using_ipsec))
+		ixgbe_crypto_update_rx_stats(rxq->ixgbe_ipsec,
+				rx_pkts, nb_pkts);
+#endif
+
 	return nb_pkts;
 }
 
@@ -2618,6 +2630,9 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,
 #ifdef RTE_LIBRTE_SECURITY
 	txq->using_ipsec = !!(dev->data->dev_conf.txmode.offloads &
 			DEV_TX_OFFLOAD_SECURITY);
+	if (txq->using_ipsec)
+		txq->ixgbe_ipsec =
+			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
 #endif
 
 	/*
@@ -4730,6 +4745,9 @@ ixgbe_set_rx_function(struct rte_eth_dev *dev)
 #ifdef RTE_LIBRTE_SECURITY
 		rxq->using_ipsec = !!(dev->data->dev_conf.rxmode.offloads &
 				DEV_RX_OFFLOAD_SECURITY);
+		if (rxq->using_ipsec)
+			rxq->ixgbe_ipsec =
+			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
 #endif
 	}
 }
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 505d344..b5f130d 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -114,6 +114,8 @@ struct ixgbe_rx_queue {
 #ifdef RTE_LIBRTE_SECURITY
 	uint8_t            using_ipsec;
 	/**< indicates that IPsec RX feature is in use */
+	struct ixgbe_ipsec *ixgbe_ipsec;
+	/**< IXGBE IPsec internals */
 #endif
 #ifdef RTE_IXGBE_INC_VECTOR
 	uint16_t            rxrearm_nb;     /**< number of remaining to be re-armed */
@@ -231,6 +233,8 @@ struct ixgbe_tx_queue {
 #ifdef RTE_LIBRTE_SECURITY
 	uint8_t		    using_ipsec;
 	/**< indicates that IPsec TX feature is in use */
+	struct ixgbe_ipsec *ixgbe_ipsec;
+	/**< IXGBE IPsec internals */
 #endif
 };
 
diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
index 599ba30..b92d1a9 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
@@ -553,6 +553,12 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
 	rxq->rx_tail = (uint16_t)(rxq->rx_tail & (rxq->nb_rx_desc - 1));
 	rxq->rxrearm_nb = (uint16_t)(rxq->rxrearm_nb + nb_pkts_recd);
 
+#ifdef RTE_LIBRTE_SECURITY
+		if (unlikely(use_ipsec))
+			ixgbe_crypto_update_rx_stats(rxq->ixgbe_ipsec,
+					rx_pkts, nb_pkts_recd);
+#endif
+
 	return nb_pkts_recd;
 }
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] net/ixgbe: add security statistics

[Ananyev, Konstantin]

Hi Radu,

> 
> Update IXGBE PMD with support for IPsec statistics.

The proposed approach - have a new hash table per device,
plus parse  each packet on RX and TX and do hash lookup
seems way too heavy to put it into PMD.
Wonder why we need to do that?
If HW doesn't provide such statistic info, why not to leave it
to the upper layer to collect/maintain?
In many cases SW will already have that infrastructure
and I don't see to put all these heavy-weight operations
into ixgbe rx/tx path.    

Konstantin

> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> ---
>  drivers/net/ixgbe/ixgbe_ipsec.c        | 244 ++++++++++++++++++++++++++++++++-
>  drivers/net/ixgbe/ixgbe_ipsec.h        |  17 ++-
>  drivers/net/ixgbe/ixgbe_rxtx.c         |  18 +++
>  drivers/net/ixgbe/ixgbe_rxtx.h         |   4 +
>  drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c |   6 +
>  5 files changed, 285 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
> index 48f5082..94daf92 100644
> --- a/drivers/net/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ixgbe/ixgbe_ipsec.c
> @@ -9,6 +9,7 @@
>  #include <rte_security_driver.h>
>  #include <rte_cryptodev.h>
>  #include <rte_flow.h>
> +#include <rte_hash_crc.h>
> 
>  #include "base/ixgbe_type.h"
>  #include "base/ixgbe_api.h"
> @@ -94,6 +95,7 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
>  			dev->data->dev_private);
>  	uint32_t reg_val;
>  	int sa_index = -1;
> +	struct ixgbe_crypto_sess_htable_key tbl_key = {0};
> 
>  	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
>  		int i, ip_index = -1;
> @@ -158,9 +160,14 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
>  		if (ic_session->dst_ip.type == IPv6) {
>  			priv->rx_sa_tbl[sa_index].mode |= IPSRXMOD_IPV6;
>  			priv->rx_ip_tbl[ip_index].ip.type = IPv6;
> -		} else if (ic_session->dst_ip.type == IPv4)
> +			memcpy(&tbl_key.ip.ipv6, &ic_session->dst_ip.ipv6,
> +					sizeof(ic_session->dst_ip.ipv6));
> +		} else if (ic_session->dst_ip.type == IPv4) {
>  			priv->rx_ip_tbl[ip_index].ip.type = IPv4;
> +			tbl_key.ip.ipv4 = ic_session->dst_ip.ipv4;
> +		}
> 
> +		tbl_key.spi = priv->rx_sa_tbl[sa_index].spi;
>  		priv->rx_sa_tbl[sa_index].used = 1;
> 
>  		/* write IP table entry*/
> @@ -238,6 +245,7 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
> 
>  		priv->tx_sa_tbl[sa_index].spi =
>  			rte_cpu_to_be_32(ic_session->spi);
> +		tbl_key.spi = priv->tx_sa_tbl[sa_index].spi;
>  		priv->tx_sa_tbl[i].used = 1;
>  		ic_session->sa_index = sa_index;
> 
> @@ -264,6 +272,7 @@ ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
>  		free(key);
>  	}
> 
> +	rte_hash_add_key_data(priv->session_tbl, &tbl_key, ic_session);
>  	return 0;
>  }
> 
> @@ -276,6 +285,7 @@ ixgbe_crypto_remove_sa(struct rte_eth_dev *dev,
>  			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
>  	uint32_t reg_val;
>  	int sa_index = -1;
> +	struct ixgbe_crypto_sess_htable_key tbl_key = {0};
> 
>  	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
>  		int i, ip_index = -1;
> @@ -324,6 +334,13 @@ ixgbe_crypto_remove_sa(struct rte_eth_dev *dev,
>  		IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, 0);
>  		IXGBE_WAIT_RWRITE;
>  		priv->rx_sa_tbl[sa_index].used = 0;
> +		if (priv->rx_sa_tbl[sa_index].mode & IPSRXMOD_IPV6) {
> +			memcpy(&tbl_key.ip.ipv6, &ic_session->dst_ip.ipv6,
> +					sizeof(ic_session->dst_ip.ipv6));
> +		} else {
> +			tbl_key.ip.ipv4 = ic_session->dst_ip.ipv4;
> +		}
> +		tbl_key.spi = priv->rx_sa_tbl[sa_index].spi;
> 
>  		/* If last used then clear the IP table entry*/
>  		priv->rx_ip_tbl[ip_index].ref_count--;
> @@ -361,8 +378,10 @@ ixgbe_crypto_remove_sa(struct rte_eth_dev *dev,
>  		IXGBE_WAIT_TWRITE;
> 
>  		priv->tx_sa_tbl[sa_index].used = 0;
> +		tbl_key.spi = priv->tx_sa_tbl[sa_index].spi;
>  	}
> 
> +	rte_hash_del_key(priv->session_tbl, &tbl_key);
>  	return 0;
>  }
> 
> @@ -376,6 +395,8 @@ ixgbe_crypto_create_session(void *device,
>  	struct ixgbe_crypto_session *ic_session = NULL;
>  	struct rte_crypto_aead_xform *aead_xform;
>  	struct rte_eth_conf *dev_conf = &eth_dev->data->dev_conf;
> +	struct ixgbe_ipsec *priv =
> +			IXGBE_DEV_PRIVATE_TO_IPSEC(eth_dev->data->dev_private);
> 
>  	if (rte_mempool_get(mempool, (void **)&ic_session)) {
>  		PMD_DRV_LOG(ERR, "Cannot get object from ic_session mempool");
> @@ -426,6 +447,40 @@ ixgbe_crypto_create_session(void *device,
>  		}
>  	}
> 
> +	if (conf->ipsec.options.stats) {
> +		ic_session->stats_enabled = 1;
> +		priv->per_session_stats_active++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +ixgbe_crypto_stats_get(__rte_unused void *device,
> +		struct rte_security_session *sess,
> +		struct rte_security_stats *stats)
> +{
> +	struct rte_eth_dev *eth_dev = device;
> +	struct ixgbe_ipsec *priv =
> +			IXGBE_DEV_PRIVATE_TO_IPSEC(eth_dev->data->dev_private);
> +	volatile struct rte_security_ipsec_stats *ixgbe_stats;
> +
> +	if (sess) {
> +		struct ixgbe_crypto_session *ic_session =
> +			(struct ixgbe_crypto_session *)
> +			get_sec_session_private_data(sess);
> +		ixgbe_stats = &ic_session->stats;
> +	} else {
> +		ixgbe_stats = &priv->stats;
> +	}
> +
> +	stats->ipsec.ipackets = ixgbe_stats->ipackets;
> +	stats->ipsec.opackets = ixgbe_stats->opackets;
> +	stats->ipsec.ibytes = ixgbe_stats->ibytes;
> +	stats->ipsec.obytes = ixgbe_stats->obytes;
> +	stats->ipsec.ierrors = ixgbe_stats->ierrors;
> +	stats->ipsec.oerrors = ixgbe_stats->oerrors;
> +
>  	return 0;
>  }
> 
> @@ -444,6 +499,8 @@ ixgbe_crypto_remove_session(void *device,
>  		(struct ixgbe_crypto_session *)
>  		get_sec_session_private_data(session);
>  	struct rte_mempool *mempool = rte_mempool_from_obj(ic_session);
> +	struct ixgbe_ipsec *priv =
> +			IXGBE_DEV_PRIVATE_TO_IPSEC(eth_dev->data->dev_private);
> 
>  	if (eth_dev != ic_session->dev) {
>  		PMD_DRV_LOG(ERR, "Session not bound to this device\n");
> @@ -455,11 +512,44 @@ ixgbe_crypto_remove_session(void *device,
>  		return -EFAULT;
>  	}
> 
> +	if (ic_session->stats_enabled && priv->per_session_stats_active > 0)
> +		priv->per_session_stats_active--;
> +
>  	rte_mempool_put(mempool, (void *)ic_session);
> 
>  	return 0;
>  }
> 
> +static int
> +ixgbe_crypto_update_session(void *device,
> +		struct rte_security_session *session,
> +		struct rte_security_session_conf *conf)
> +{
> +	struct rte_eth_dev *eth_dev = device;
> +	struct ixgbe_crypto_session *ic_session =
> +		(struct ixgbe_crypto_session *)
> +		get_sec_session_private_data(session);
> +	struct ixgbe_ipsec *priv =
> +			IXGBE_DEV_PRIVATE_TO_IPSEC(eth_dev->data->dev_private);
> +
> +	if (eth_dev != ic_session->dev) {
> +		PMD_DRV_LOG(ERR, "Session not bound to this device\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Enable/disable per session stats */
> +	if (ic_session->stats_enabled && !conf->ipsec.options.stats) {
> +		ic_session->stats_enabled = 0;
> +		if (priv->per_session_stats_active > 0)
> +			priv->per_session_stats_active--;
> +	} else if (!ic_session->stats_enabled && conf->ipsec.options.stats) {
> +		ic_session->stats_enabled = 1;
> +		priv->per_session_stats_active++;
> +	}
> +
> +	return 0;
> +}
> +
>  static inline uint8_t
>  ixgbe_crypto_compute_pad_len(struct rte_mbuf *m)
>  {
> @@ -624,6 +714,8 @@ int
>  ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev)
>  {
>  	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	struct ixgbe_ipsec *priv =
> +			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
>  	uint32_t reg;
>  	uint64_t rx_offloads;
>  	uint64_t tx_offloads;
> @@ -676,6 +768,23 @@ ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev)
> 
>  	ixgbe_crypto_clear_ipsec_tables(dev);
> 
> +	if (priv->session_tbl == NULL) {
> +		char session_tbl_hash_name[RTE_HASH_NAMESIZE];
> +		struct rte_hash_parameters params = {
> +			.name = session_tbl_hash_name,
> +			.entries = IPSEC_MAX_SA_COUNT * 2,
> +			.key_len = sizeof(struct ixgbe_crypto_sess_htable_key),
> +			.hash_func = rte_hash_crc,
> +			.socket_id = rte_socket_id(),
> +		};
> +		snprintf(session_tbl_hash_name, RTE_HASH_NAMESIZE,
> +			 "session_tbl_hash_%s", dev->device->name);
> +
> +		priv->session_tbl = rte_hash_create(&params);
> +	} else {
> +		rte_hash_reset(priv->session_tbl);
> +	}
> +
>  	return 0;
>  }
> 
> @@ -709,11 +818,140 @@ ixgbe_crypto_add_ingress_sa_from_flow(const void *sess,
>  	return 0;
>  }
> 
> +void
> +ixgbe_crypto_update_rx_stats(struct ixgbe_ipsec *ixgbe_ipsec,
> +			     struct rte_mbuf **mbufs,
> +			     uint16_t count)
> +{
> +	uint16_t i;
> +	uint32_t ipackets = 0, ibytes = 0, ierrors = 0;
> +
> +	if (ixgbe_ipsec->per_session_stats_active) {
> +		struct ip *ip;
> +		struct rte_esp_hdr *esp;
> +		struct ixgbe_crypto_session *sess;
> +		struct ixgbe_crypto_sess_htable_key tbl_key = {0};
> +
> +		for (i = 0; i < count; i++) {
> +			if ((mbufs[i]->ol_flags & PKT_RX_SEC_OFFLOAD) == 0)
> +				continue;
> +			ip = rte_pktmbuf_mtod_offset(mbufs[i],
> +					struct ip *,
> +					sizeof(struct rte_ether_hdr));
> +			if (ip->ip_v == IPVERSION) {
> +				tbl_key.ip.ipv4 = ip->ip_dst.s_addr;
> +				esp = rte_pktmbuf_mtod_offset(mbufs[i],
> +						struct rte_esp_hdr *,
> +						sizeof(struct rte_ether_hdr) +
> +						ip->ip_hl * 4);
> +			} else {
> +				struct ip6_hdr *ip6 = (struct ip6_hdr *)ip;
> +				memcpy(&tbl_key.ip.ipv6, &ip6->ip6_dst, 16);
> +				esp = rte_pktmbuf_mtod_offset(mbufs[i],
> +						struct rte_esp_hdr *,
> +						sizeof(struct rte_ether_hdr) +
> +						sizeof(struct ip6_hdr));
> +			}
> +			tbl_key.spi = esp->spi;
> +			if (rte_hash_lookup_data(ixgbe_ipsec->session_tbl,
> +					&tbl_key, (void **)&sess) < 0)
> +				sess = NULL;
> +			if (mbufs[i]->ol_flags & PKT_RX_SEC_OFFLOAD_FAILED) {
> +				if (sess && sess->stats_enabled)
> +					sess->stats.ierrors++;
> +				ierrors++;
> +			} else {
> +				if (sess && sess->stats_enabled) {
> +					sess->stats.ipackets++;
> +					sess->stats.ibytes +=
> +						rte_pktmbuf_pkt_len(mbufs[i]);
> +				}
> +				ipackets++;
> +				ibytes += rte_pktmbuf_pkt_len(mbufs[i]);
> +			}
> +		}
> +	} else { /* Global stats only */
> +		for (i = 0; i < count; i++) {
> +			if (mbufs[i]->ol_flags & PKT_RX_SEC_OFFLOAD) {
> +				if (mbufs[i]->ol_flags &
> +						PKT_RX_SEC_OFFLOAD_FAILED) {
> +					ierrors++;
> +				} else {
> +					ipackets++;
> +					ibytes += rte_pktmbuf_pkt_len(mbufs[i]);
> +				}
> +			}
> +		}
> +	}
> +
> +	/* Update global stats */
> +	ixgbe_ipsec->stats.ipackets += ipackets;
> +	ixgbe_ipsec->stats.ibytes += ibytes;
> +	ixgbe_ipsec->stats.ierrors += ierrors;
> +}
> +
> +void
> +ixgbe_crypto_update_tx_stats(struct ixgbe_ipsec *ixgbe_ipsec,
> +			     struct rte_mbuf **mbufs,
> +			     uint16_t count)
> +{
> +	uint16_t i;
> +	uint32_t opackets = 0, obytes = 0;
> +
> +	if (ixgbe_ipsec->per_session_stats_active) {
> +		struct ip *ip;
> +		struct rte_esp_hdr *esp;
> +		struct ixgbe_crypto_session *sess;
> +		struct ixgbe_crypto_sess_htable_key tbl_key = {0};
> +
> +		for (i = 0; i < count; i++) {
> +			if ((mbufs[i]->ol_flags & PKT_TX_SEC_OFFLOAD) == 0)
> +				continue;
> +			ip = rte_pktmbuf_mtod_offset(mbufs[i],
> +					struct ip *,
> +					sizeof(struct rte_ether_hdr));
> +			if (ip->ip_v == IPVERSION) {
> +				esp = rte_pktmbuf_mtod_offset(mbufs[i],
> +						struct rte_esp_hdr *,
> +						sizeof(struct rte_ether_hdr) +
> +						ip->ip_hl * 4);
> +			} else {
> +				esp = rte_pktmbuf_mtod_offset(mbufs[i],
> +						struct rte_esp_hdr *,
> +						sizeof(struct rte_ether_hdr) +
> +						sizeof(struct ip6_hdr));
> +			}
> +			tbl_key.spi = esp->spi;
> +			if (rte_hash_lookup_data(ixgbe_ipsec->session_tbl,
> +					&tbl_key, (void **)&sess) < 0)
> +				sess = NULL;
> +			if (sess && sess->stats_enabled) {
> +				sess->stats.opackets++;
> +				sess->stats.obytes +=
> +					rte_pktmbuf_pkt_len(mbufs[i]);
> +			}
> +			opackets++;
> +			obytes += rte_pktmbuf_pkt_len(mbufs[i]);
> +		}
> +	} else { /* Global stats only */
> +		for (i = 0; i < count; i++) {
> +			if (mbufs[i]->ol_flags & PKT_RX_SEC_OFFLOAD) {
> +				opackets++;
> +				obytes += rte_pktmbuf_pkt_len(mbufs[i]);
> +			}
> +		}
> +	}
> +
> +	/* Update global stats */
> +	ixgbe_ipsec->stats.opackets += opackets;
> +	ixgbe_ipsec->stats.obytes += obytes;
> +}
> +
>  static struct rte_security_ops ixgbe_security_ops = {
>  	.session_create = ixgbe_crypto_create_session,
> -	.session_update = NULL,
> +	.session_update = ixgbe_crypto_update_session,
>  	.session_get_size = ixgbe_crypto_session_get_size,
> -	.session_stats_get = NULL,
> +	.session_stats_get = ixgbe_crypto_stats_get,
>  	.session_destroy = ixgbe_crypto_remove_session,
>  	.set_pkt_metadata = ixgbe_crypto_update_mb,
>  	.capabilities_get = ixgbe_crypto_capabilities_get
> diff --git a/drivers/net/ixgbe/ixgbe_ipsec.h b/drivers/net/ixgbe/ixgbe_ipsec.h
> index e218c0a..a4ff8b6 100644
> --- a/drivers/net/ixgbe/ixgbe_ipsec.h
> +++ b/drivers/net/ixgbe/ixgbe_ipsec.h
> @@ -70,6 +70,8 @@ struct ixgbe_crypto_session {
>  	struct ipaddr src_ip;
>  	struct ipaddr dst_ip;
>  	struct rte_eth_dev *dev;
> +	volatile struct rte_security_ipsec_stats stats;
> +	uint8_t stats_enabled;
>  } __rte_cache_aligned;
> 
>  struct ixgbe_crypto_rx_ip_table {
> @@ -100,10 +102,18 @@ union ixgbe_crypto_tx_desc_md {
>  	};
>  };
> 
> +struct ixgbe_crypto_sess_htable_key {
> +	uint32_t spi;
> +	struct ipaddr ip;
> +};
> +
>  struct ixgbe_ipsec {
>  	struct ixgbe_crypto_rx_ip_table rx_ip_tbl[IPSEC_MAX_RX_IP_COUNT];
>  	struct ixgbe_crypto_rx_sa_table rx_sa_tbl[IPSEC_MAX_SA_COUNT];
>  	struct ixgbe_crypto_tx_sa_table tx_sa_tbl[IPSEC_MAX_SA_COUNT];
> +	volatile struct rte_security_ipsec_stats stats;
> +	volatile uint16_t per_session_stats_active;
> +	struct rte_hash *session_tbl;
>  };
> 
> 
> @@ -112,7 +122,12 @@ int ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev);
>  int ixgbe_crypto_add_ingress_sa_from_flow(const void *sess,
>  					  const void *ip_spec,
>  					  uint8_t is_ipv6);
> -
> +void ixgbe_crypto_update_rx_stats(struct ixgbe_ipsec *ixgbe_ipsec,
> +				  struct rte_mbuf **mbufs,
> +				  uint16_t count);
> +void ixgbe_crypto_update_tx_stats(struct ixgbe_ipsec *ixgbe_ipsec,
> +				  struct rte_mbuf **mbufs,
> +				  uint16_t count);
> 
> 
>  #endif /*IXGBE_IPSEC_H_*/
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index edcfa60..cde012f 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -956,6 +956,12 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  	IXGBE_PCI_REG_WRITE_RELAXED(txq->tdt_reg_addr, tx_id);
>  	txq->tx_tail = tx_id;
> 
> +#ifdef RTE_LIBRTE_SECURITY
> +		if (unlikely(txq->using_ipsec))
> +			ixgbe_crypto_update_tx_stats(txq->ixgbe_ipsec,
> +					&tx_pkts[-nb_tx], nb_tx);
> +#endif
> +
>  	return nb_tx;
>  }
> 
> @@ -1643,6 +1649,12 @@ ixgbe_rx_fill_from_stage(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
>  	rxq->rx_nb_avail = (uint16_t)(rxq->rx_nb_avail - nb_pkts);
>  	rxq->rx_next_avail = (uint16_t)(rxq->rx_next_avail + nb_pkts);
> 
> +#ifdef RTE_LIBRTE_SECURITY
> +	if (unlikely(rxq->using_ipsec))
> +		ixgbe_crypto_update_rx_stats(rxq->ixgbe_ipsec,
> +				rx_pkts, nb_pkts);
> +#endif
> +
>  	return nb_pkts;
>  }
> 
> @@ -2618,6 +2630,9 @@ ixgbe_dev_tx_queue_setup(struct rte_eth_dev *dev,
>  #ifdef RTE_LIBRTE_SECURITY
>  	txq->using_ipsec = !!(dev->data->dev_conf.txmode.offloads &
>  			DEV_TX_OFFLOAD_SECURITY);
> +	if (txq->using_ipsec)
> +		txq->ixgbe_ipsec =
> +			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
>  #endif
> 
>  	/*
> @@ -4730,6 +4745,9 @@ ixgbe_set_rx_function(struct rte_eth_dev *dev)
>  #ifdef RTE_LIBRTE_SECURITY
>  		rxq->using_ipsec = !!(dev->data->dev_conf.rxmode.offloads &
>  				DEV_RX_OFFLOAD_SECURITY);
> +		if (rxq->using_ipsec)
> +			rxq->ixgbe_ipsec =
> +			IXGBE_DEV_PRIVATE_TO_IPSEC(dev->data->dev_private);
>  #endif
>  	}
>  }
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
> index 505d344..b5f130d 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> @@ -114,6 +114,8 @@ struct ixgbe_rx_queue {
>  #ifdef RTE_LIBRTE_SECURITY
>  	uint8_t            using_ipsec;
>  	/**< indicates that IPsec RX feature is in use */
> +	struct ixgbe_ipsec *ixgbe_ipsec;
> +	/**< IXGBE IPsec internals */
>  #endif
>  #ifdef RTE_IXGBE_INC_VECTOR
>  	uint16_t            rxrearm_nb;     /**< number of remaining to be re-armed */
> @@ -231,6 +233,8 @@ struct ixgbe_tx_queue {
>  #ifdef RTE_LIBRTE_SECURITY
>  	uint8_t		    using_ipsec;
>  	/**< indicates that IPsec TX feature is in use */
> +	struct ixgbe_ipsec *ixgbe_ipsec;
> +	/**< IXGBE IPsec internals */
>  #endif
>  };
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> index 599ba30..b92d1a9 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> @@ -553,6 +553,12 @@ _recv_raw_pkts_vec(struct ixgbe_rx_queue *rxq, struct rte_mbuf **rx_pkts,
>  	rxq->rx_tail = (uint16_t)(rxq->rx_tail & (rxq->nb_rx_desc - 1));
>  	rxq->rxrearm_nb = (uint16_t)(rxq->rxrearm_nb + nb_pkts_recd);
> 
> +#ifdef RTE_LIBRTE_SECURITY
> +		if (unlikely(use_ipsec))
> +			ixgbe_crypto_update_rx_stats(rxq->ixgbe_ipsec,
> +					rx_pkts, nb_pkts_recd);
> +#endif
> +
>  	return nb_pkts_recd;
>  }
> 
> --
> 2.7.4

Re: [dpdk-dev] [PATCH] net/ixgbe: add security statistics

[Nicolau, Radu]

Hi,

On 9/8/2019 12:45 PM, Ananyev, Konstantin wrote:
> Hi Radu,
>
>> Update IXGBE PMD with support for IPsec statistics.
> The proposed approach - have a new hash table per device,
> plus parse  each packet on RX and TX and do hash lookup
> seems way too heavy to put it into PMD.

It is indeed heavy, but it's disabled by default and it only uses the 
heavy section when the app is enabling per-session statistics

> Wonder why we need to do that?
> If HW doesn't provide such statistic info, why not to leave it
> to the upper layer to collect/maintain?
> In many cases SW will already have that infrastructure
> and I don't see to put all these heavy-weight operations
> into ixgbe rx/tx path.

The reason is to use the rte_security API consistently and take 
advantage of the hardware backed statistics when they are available.

>
> Konstantin

Re: [dpdk-dev] [PATCH] net/ixgbe: add security statistics

[Ananyev, Konstantin]

Hi Radu,

> >
> >> Update IXGBE PMD with support for IPsec statistics.
> > The proposed approach - have a new hash table per device,
> > plus parse  each packet on RX and TX and do hash lookup
> > seems way too heavy to put it into PMD.
> 
> It is indeed heavy, but it's disabled by default and it only uses the
> heavy section when the app is enabling per-session statistics

As I can read  the code, disruption will be much more significant - 
even if user will enable statistics just for one SA (which might never be used),
that will affect whole device: all RX (or all TX) queues will experience a slowdown.
Even RX/TX for queues without any IPsec packets will be affected
(though not that severe as ones with mix of IPsec and non-IPsec or pure IPsec packets).   

> 
> > Wonder why we need to do that?
> > If HW doesn't provide such statistic info, why not to leave it
> > to the upper layer to collect/maintain?
> > In many cases SW will already have that infrastructure
> > and I don't see to put all these heavy-weight operations
> > into ixgbe rx/tx path.
> 
> The reason is to use the rte_security API consistently and take
> advantage of the hardware backed statistics when they are available.

AFAIK, right now in rte_security API there is no option to request statistics per session,
and struct rte_security_ipsec_stats is literally empty.
That's an extension you proposed by the patch:
http://patches.dpdk.org/patch/58462/
But as I can see it is not backed up by any real HW implementation.
Yours current patch provides pure SW implementation,
and I don't see absolutely no reason to push it into the ixgbe PMD.
Same statistics could be collected by dozen different and probably more effective ways without affecting the actual ixgbe driver
(RX/TX callback or  new option in librte_ipsec or new function that would be called by app directly or ...).
So my suggestion let's postpone introducing new API here till we'll have real HW that supports it.

Konstantin