[dpdk-dev,v2,10/12] net/ixgbe: enable inline ipsec

  From: Radu Nicolau <radu.nicolau@intel.com>

Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 config/common_base                     |   1 +
 drivers/net/Makefile                   |   2 +-
 drivers/net/ixgbe/Makefile             |   4 +-
 drivers/net/ixgbe/ixgbe_ethdev.c       |  18 +
 drivers/net/ixgbe/ixgbe_ethdev.h       |  10 +-
 drivers/net/ixgbe/ixgbe_flow.c         |  46 +++
 drivers/net/ixgbe/ixgbe_ipsec.c        | 728 +++++++++++++++++++++++++++++++++
 drivers/net/ixgbe/ixgbe_ipsec.h        | 147 +++++++
 drivers/net/ixgbe/ixgbe_rxtx.c         |  63 ++-
 drivers/net/ixgbe/ixgbe_rxtx.h         |   4 +
 drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c |  44 ++
 11 files changed, 1062 insertions(+), 5 deletions(-)
 create mode 100644 drivers/net/ixgbe/ixgbe_ipsec.c
 create mode 100644 drivers/net/ixgbe/ixgbe_ipsec.h
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal
> Sent: Tuesday, October 3, 2017 2:14 PM
> To: dev@dpdk.org
> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; hemant.agrawal@nxp.com;
> Nicolau, Radu <radu.nicolau@intel.com>; borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net;
> sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com; Mcnamara, John <john.mcnamara@intel.com>; olivier.matz@6wind.com
> Subject: [dpdk-dev] [PATCH v2 10/12] net/ixgbe: enable inline ipsec
> 
> From: Radu Nicolau <radu.nicolau@intel.com>
> 
> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>  config/common_base                     |   1 +
>  drivers/net/Makefile                   |   2 +-
>  drivers/net/ixgbe/Makefile             |   4 +-
>  drivers/net/ixgbe/ixgbe_ethdev.c       |  18 +
>  drivers/net/ixgbe/ixgbe_ethdev.h       |  10 +-
>  drivers/net/ixgbe/ixgbe_flow.c         |  46 +++
>  drivers/net/ixgbe/ixgbe_ipsec.c        | 728 +++++++++++++++++++++++++++++++++
>  drivers/net/ixgbe/ixgbe_ipsec.h        | 147 +++++++
>  drivers/net/ixgbe/ixgbe_rxtx.c         |  63 ++-
>  drivers/net/ixgbe/ixgbe_rxtx.h         |   4 +
>  drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c |  44 ++
>  11 files changed, 1062 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/net/ixgbe/ixgbe_ipsec.c
>  create mode 100644 drivers/net/ixgbe/ixgbe_ipsec.h
> 
> diff --git a/config/common_base b/config/common_base
> index 7a1766b..bf6b06f 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -179,6 +179,7 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
>  CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
>  CONFIG_RTE_IXGBE_INC_VECTOR=y
>  CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n
> +CONFIG_RTE_LIBRTE_IXGBE_IPSEC=y
> 
>  #
>  # Compile burst-oriented I40E PMD driver
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index d33c959..8ffbff3 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -66,7 +66,7 @@ DEPDIRS-fm10k = $(core-libs) librte_hash
>  DIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e
>  DEPDIRS-i40e = $(core-libs) librte_hash
>  DIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe
> -DEPDIRS-ixgbe = $(core-libs) librte_hash
> +DEPDIRS-ixgbe = $(core-libs) librte_hash librte_security
>  DIRS-$(CONFIG_RTE_LIBRTE_LIO_PMD) += liquidio
>  DEPDIRS-liquidio = $(core-libs)
>  DIRS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += mlx4
> diff --git a/drivers/net/ixgbe/Makefile b/drivers/net/ixgbe/Makefile
> index 5e57cb3..1180900 100644
> --- a/drivers/net/ixgbe/Makefile
> +++ b/drivers/net/ixgbe/Makefile
> @@ -118,11 +118,13 @@ SRCS-$(CONFIG_RTE_IXGBE_INC_VECTOR) += ixgbe_rxtx_vec_neon.c
>  else
>  SRCS-$(CONFIG_RTE_IXGBE_INC_VECTOR) += ixgbe_rxtx_vec_sse.c
>  endif
> -
>  ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_BYPASS),y)
>  SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_bypass.c
>  SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_82599_bypass.c
>  endif
> +ifeq ($(CONFIG_RTE_LIBRTE_IXGBE_IPSEC),y)
> +SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_ipsec.c
> +endif
>  SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += rte_pmd_ixgbe.c
>  SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_tm.c
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index 22171d8..f75b5eb 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -61,6 +61,7 @@
>  #include <rte_random.h>
>  #include <rte_dev.h>
>  #include <rte_hash_crc.h>
> +#include <rte_security_driver.h>
> 
>  #include "ixgbe_logs.h"
>  #include "base/ixgbe_api.h"
> @@ -1135,6 +1136,10 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
>  	PMD_INIT_FUNC_TRACE();
> 
>  	eth_dev->dev_ops = &ixgbe_eth_dev_ops;
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	rte_security_register(&eth_dev->data->sec_id,
> +			      (void *)eth_dev, &ixgbe_security_ops);
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */

I still wonder do we really need new config macro and
Ifdef it through all ixgbe code?
Can we have it just always on?
If the RX/TX performance suffers a lot we  can have a special
RX/TX functions for ipsec enabled case.    

>  	eth_dev->rx_pkt_burst = &ixgbe_recv_pkts;
>  	eth_dev->tx_pkt_burst = &ixgbe_xmit_pkts;
>  	eth_dev->tx_pkt_prepare = &ixgbe_prep_pkts;
> @@ -1165,6 +1170,9 @@ eth_ixgbe_dev_init(struct rte_eth_dev *eth_dev)
> 
>  	rte_eth_copy_pci_info(eth_dev, pci_dev);
>  	eth_dev->data->dev_flags |= RTE_ETH_DEV_DETACHABLE;
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	eth_dev->data->dev_flags |= RTE_ETH_DEV_SECURITY;
> +#endif /*RTE_LIBRTE_IXGBE_IPSEC*/
> 
>  	/* Vendor and Device ID need to be set before init of shared code */
>  	hw->device_id = pci_dev->id.device_id;
> @@ -1401,6 +1409,10 @@ eth_ixgbe_dev_uninit(struct rte_eth_dev *eth_dev)
>  	/* Remove all Traffic Manager configuration */
>  	ixgbe_tm_conf_uninit(eth_dev);
> 
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	rte_security_unregister(eth_dev->data->sec_id);
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> +
>  	return 0;
>  }
> 
> @@ -3665,6 +3677,12 @@ ixgbe_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
>  	    hw->mac.type == ixgbe_mac_X550EM_a)
>  		dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM;
> 
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	dev_info->rx_offload_capa |= DEV_RX_OFFLOAD_SECURITY;
> +	dev_info->tx_offload_capa |= DEV_TX_OFFLOAD_SECURITY |
> +			DEV_TX_OFFLOAD_SEC_NEED_MDATA;
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> +
>  	dev_info->default_rxconf = (struct rte_eth_rxconf) {
>  		.rx_thresh = {
>  			.pthresh = IXGBE_DEFAULT_RX_PTHRESH,
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index caa50c8..d1a84e2 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -38,6 +38,9 @@
>  #include "base/ixgbe_dcb_82599.h"
>  #include "base/ixgbe_dcb_82598.h"
>  #include "ixgbe_bypass.h"
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +#include "ixgbe_ipsec.h"
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
>  #include <rte_time.h>
>  #include <rte_hash.h>
>  #include <rte_pci.h>
> @@ -529,7 +532,9 @@ struct ixgbe_adapter {
>  	struct ixgbe_filter_info    filter;
>  	struct ixgbe_l2_tn_info     l2_tn;
>  	struct ixgbe_bw_conf        bw_conf;
> -
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	struct ixgbe_ipsec          ipsec;
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
>  	bool rx_bulk_alloc_allowed;
>  	bool rx_vec_allowed;
>  	struct rte_timecounter      systime_tc;
> @@ -586,6 +591,9 @@ struct ixgbe_adapter {
>  #define IXGBE_DEV_PRIVATE_TO_TM_CONF(adapter) \
>  	(&((struct ixgbe_adapter *)adapter)->tm_conf)
> 
> +#define IXGBE_DEV_PRIVATE_TO_IPSEC(adapter)\
> +	(&((struct ixgbe_adapter *)adapter)->ipsec)
> +
>  /*
>   * RX/TX function prototypes
>   */
> diff --git a/drivers/net/ixgbe/ixgbe_flow.c b/drivers/net/ixgbe/ixgbe_flow.c
> index d679608..d450161 100644
> --- a/drivers/net/ixgbe/ixgbe_flow.c
> +++ b/drivers/net/ixgbe/ixgbe_flow.c
> @@ -142,6 +142,10 @@ const struct rte_flow_action *next_no_void_action(
>   * END
>   * other members in mask and spec should set to 0x00.
>   * item->last should be NULL.
> + *
> + * Special case when RTE_LIBRTE_IXGBE_IPSEC is enabled and the
> + * flow action is security.
> + *
>   */
>  static int
>  cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
> @@ -181,6 +185,48 @@ cons_parse_ntuple_filter(const struct rte_flow_attr *attr,
>  		return -rte_errno;
>  	}
> 
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	/**
> +	 *  special case for RTE_FLOW_ACTION_TYPE_SECURITY
> +	 *  Inline IPsec
> +	 */
> +	act = next_no_void_action(actions, NULL);
> +	if (act->type == RTE_FLOW_ACTION_TYPE_SECURITY) {
> +		const void *conf = act->conf;
> +		/* check if the next not void item is END */
> +		act = next_no_void_action(actions, act);
> +		if (act->type != RTE_FLOW_ACTION_TYPE_END) {
> +			memset(filter, 0, sizeof(struct rte_eth_ntuple_filter));
> +			rte_flow_error_set(error, EINVAL,
> +				RTE_FLOW_ERROR_TYPE_ACTION,
> +				act, "Not supported action.");
> +			return -rte_errno;
> +		}
> +
> +		/* get the IP pattern*/
> +		item = next_no_void_pattern(pattern, NULL);
> +		while (item->type != RTE_FLOW_ITEM_TYPE_IPV4 &&
> +				item->type != RTE_FLOW_ITEM_TYPE_IPV6) {
> +			if (item->last ||
> +					item->type == RTE_FLOW_ITEM_TYPE_END) {
> +				rte_flow_error_set(error, EINVAL,
> +					RTE_FLOW_ERROR_TYPE_ITEM,
> +					item, "IP pattern missing.");
> +				return -rte_errno;
> +			}
> +			item = next_no_void_pattern(pattern, item);
> +		}
> +
> +		filter->priority = (uint16_t)attr->priority;
> +		if (attr->priority < IXGBE_MIN_N_TUPLE_PRIO ||
> +				attr->priority > IXGBE_MAX_N_TUPLE_PRIO)
> +			filter->priority = 1;
> +
> +		return ixgbe_crypto_add_ingress_sa_from_flow(conf, item->spec,
> +					item->type == RTE_FLOW_ITEM_TYPE_IPV6);
> +	}
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> +
>  	/* the first not void item can be MAC or IPv4 */
>  	item = next_no_void_pattern(pattern, NULL);
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ipsec.c b/drivers/net/ixgbe/ixgbe_ipsec.c
> new file mode 100644
> index 0000000..178f16b
> --- /dev/null
> +++ b/drivers/net/ixgbe/ixgbe_ipsec.c
> @@ -0,0 +1,728 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <rte_ethdev.h>
> +#include <rte_ethdev_pci.h>
> +#include <rte_ip.h>
> +#include <rte_jhash.h>
> +#include <rte_security_driver.h>
> +#include <rte_cryptodev.h>
> +#include <rte_flow.h>
> +
> +#include "base/ixgbe_type.h"
> +#include "base/ixgbe_api.h"
> +#include "ixgbe_ethdev.h"
> +#include "ixgbe_ipsec.h"
> +
> +
> +#define IXGBE_WAIT_RW(__reg, __rw)					\
> +{									\
> +	int cnt = 100;							\
> +	IXGBE_WRITE_REG(hw, (__reg), reg);				\
> +	while (((IXGBE_READ_REG(hw, (__reg))) & (__rw)) && (cnt--))	\
> +		rte_delay_us(1);					\
> +}

Looks usefull.
Probably worth to add cnt as a parameter and put the macro (or even better inline func)
Inside base/ixgbe_osdep.h.

> +#define IXGBE_WAIT_RREAD  IXGBE_WAIT_RW(IXGBE_IPSRXIDX, IPSRXIDX_READ)
> +#define IXGBE_WAIT_RWRITE IXGBE_WAIT_RW(IXGBE_IPSRXIDX, IPSRXIDX_WRITE)
> +#define IXGBE_WAIT_TREAD  IXGBE_WAIT_RW(IXGBE_IPSTXIDX, IPSRXIDX_READ)
> +#define IXGBE_WAIT_TWRITE IXGBE_WAIT_RW(IXGBE_IPSTXIDX, IPSRXIDX_WRITE)
> +
> +#define CMP_IP(a, b) (\
> +	(a).ipv6[0] == (b).ipv6[0] && \
> +	(a).ipv6[1] == (b).ipv6[1] && \
> +	(a).ipv6[2] == (b).ipv6[2] && \
> +	(a).ipv6[3] == (b).ipv6[3])
> +
> +
> +static void
> +ixgbe_crypto_clear_ipsec_tables(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	int i = 0;
> +
> +	/* clear Rx IP table*/
> +	for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
> +		uint16_t index = i << 3;
> +		uint32_t reg = IPSRXIDX_WRITE | IPSRXIDX_TABLE_IP | index;
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3), 0);
> +		IXGBE_WAIT_RWRITE;
> +	}
> +
> +	/* clear Rx SPI and Rx/Tx SA tables*/
> +	for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
> +		uint32_t index = i << 3;
> +		uint32_t reg = IPSRXIDX_WRITE | IPSRXIDX_TABLE_SPI | index;
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, 0);
> +		IXGBE_WAIT_RWRITE;
> +		reg = IPSRXIDX_WRITE | IPSRXIDX_TABLE_KEY | index;
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, 0);
> +		IXGBE_WAIT_RWRITE;
> +		reg = IPSRXIDX_WRITE | index;
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, 0);
> +		IXGBE_WAIT_TWRITE;
> +	}
> +}
> +
> +static int
> +ixgbe_crypto_add_sa(struct ixgbe_crypto_session *ic_session)
> +{
> +	struct rte_eth_dev *dev = ic_session->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;
> +	int sa_index = -1;
> +
> +	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
> +		int i, ip_index = -1;
> +
> +		/* Find a match in the IP table*/
> +		for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
> +			if (CMP_IP(priv->rx_ip_tbl[i].ip,
> +				   ic_session->dst_ip)) {
> +				ip_index = i;
> +				break;
> +			}
> +		}
> +		/* If no match, find a free entry in the IP table*/
> +		if (ip_index < 0) {
> +			for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
> +				if (priv->rx_ip_tbl[i].ref_count == 0) {
> +					ip_index = i;
> +					break;
> +				}
> +			}
> +		}
> +
> +		/* Fail if no match and no free entries*/
> +		if (ip_index < 0) {
> +			PMD_DRV_LOG(ERR,
> +				    "No free entry left in the Rx IP table\n");
> +			return -1;
> +		}
> +
> +		/* Find a free entry in the SA table*/
> +		for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
> +			if (priv->rx_sa_tbl[i].used == 0) {
> +				sa_index = i;
> +				break;
> +			}
> +		}
> +		/* Fail if no free entries*/
> +		if (sa_index < 0) {
> +			PMD_DRV_LOG(ERR,
> +				    "No free entry left in the Rx SA table\n");
> +			return -1;
> +		}
> +
> +		priv->rx_ip_tbl[ip_index].ip.ipv6[0] =
> +				ic_session->dst_ip.ipv6[0];
> +		priv->rx_ip_tbl[ip_index].ip.ipv6[1] =
> +				ic_session->dst_ip.ipv6[1];
> +		priv->rx_ip_tbl[ip_index].ip.ipv6[2] =
> +				ic_session->dst_ip.ipv6[2];
> +		priv->rx_ip_tbl[ip_index].ip.ipv6[3] =
> +				ic_session->dst_ip.ipv6[3];
> +		priv->rx_ip_tbl[ip_index].ref_count++;
> +
> +		priv->rx_sa_tbl[sa_index].spi =
> +			rte_cpu_to_be_32(ic_session->spi);
> +		priv->rx_sa_tbl[sa_index].ip_index = ip_index;
> +		priv->rx_sa_tbl[sa_index].key[3] =
> +			rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[0]);
> +		priv->rx_sa_tbl[sa_index].key[2] =
> +			rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[4]);
> +		priv->rx_sa_tbl[sa_index].key[1] =
> +			rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[8]);
> +		priv->rx_sa_tbl[sa_index].key[0] =
> +			rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[12]);
> +		priv->rx_sa_tbl[sa_index].salt =
> +			rte_cpu_to_be_32(ic_session->salt);
> +		priv->rx_sa_tbl[sa_index].mode = IPSRXMOD_VALID;
> +		if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION)
> +			priv->rx_sa_tbl[sa_index].mode |=
> +					(IPSRXMOD_PROTO | IPSRXMOD_DECRYPT);
> +		if (ic_session->dst_ip.type == IPv6)
> +			priv->rx_sa_tbl[sa_index].mode |= IPSRXMOD_IPV6;
> +		priv->rx_sa_tbl[sa_index].used = 1;
> +
> +		/* write IP table entry*/
> +		reg = IPSRXIDX_RX_EN | IPSRXIDX_WRITE |
> +				IPSRXIDX_TABLE_IP | (ip_index << 3);
> +		if (priv->rx_ip_tbl[ip_index].ip.type == IPv4) {
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0), 0);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1), 0);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2), 0);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3),
> +					priv->rx_ip_tbl[ip_index].ip.ipv4);
> +		} else {
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0),
> +					priv->rx_ip_tbl[ip_index].ip.ipv6[0]);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1),
> +					priv->rx_ip_tbl[ip_index].ip.ipv6[1]);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2),
> +					priv->rx_ip_tbl[ip_index].ip.ipv6[2]);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3),
> +					priv->rx_ip_tbl[ip_index].ip.ipv6[3]);
> +		}
> +		IXGBE_WAIT_RWRITE;
> +
> +		/* write SPI table entry*/
> +		reg = IPSRXIDX_RX_EN | IPSRXIDX_WRITE |
> +				IPSRXIDX_TABLE_SPI | (sa_index << 3);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI,
> +				priv->rx_sa_tbl[sa_index].spi);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX,
> +				priv->rx_sa_tbl[sa_index].ip_index);
> +		IXGBE_WAIT_RWRITE;
> +
> +		/* write Key table entry*/
> +		reg = IPSRXIDX_RX_EN | IPSRXIDX_WRITE |
> +				IPSRXIDX_TABLE_KEY | (sa_index << 3);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0),
> +				priv->rx_sa_tbl[sa_index].key[0]);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1),
> +				priv->rx_sa_tbl[sa_index].key[1]);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2),
> +				priv->rx_sa_tbl[sa_index].key[2]);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3),
> +				priv->rx_sa_tbl[sa_index].key[3]);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT,
> +				priv->rx_sa_tbl[sa_index].salt);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD,
> +				priv->rx_sa_tbl[sa_index].mode);
> +		IXGBE_WAIT_RWRITE;
> +
> +	} else { /* sess->dir == RTE_CRYPTO_OUTBOUND */
> +		int i;
> +
> +		/* Find a free entry in the SA table*/
> +		for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
> +			if (priv->tx_sa_tbl[i].used == 0) {
> +				sa_index = i;
> +				break;
> +			}
> +		}
> +		/* Fail if no free entries*/
> +		if (sa_index < 0) {
> +			PMD_DRV_LOG(ERR,
> +				    "No free entry left in the Tx SA table\n");
> +			return -1;
> +		}
> +
> +		priv->tx_sa_tbl[sa_index].spi =
> +			rte_cpu_to_be_32(ic_session->spi);
> +		priv->tx_sa_tbl[sa_index].key[3] =
> +			rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[0]);
> +		priv->tx_sa_tbl[sa_index].key[2] =
> +			rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[4]);
> +		priv->tx_sa_tbl[sa_index].key[1] =
> +			rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[8]);
> +		priv->tx_sa_tbl[sa_index].key[0] =
> +			rte_cpu_to_be_32(*(uint32_t *)&ic_session->key[12]);
> +		priv->tx_sa_tbl[sa_index].salt =
> +			rte_cpu_to_be_32(ic_session->salt);
> +
> +		reg = IPSRXIDX_RX_EN | IPSRXIDX_WRITE | (sa_index << 3);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0),
> +				priv->tx_sa_tbl[sa_index].key[0]);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1),
> +				priv->tx_sa_tbl[sa_index].key[1]);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2),
> +				priv->tx_sa_tbl[sa_index].key[2]);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3),
> +				priv->tx_sa_tbl[sa_index].key[3]);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT,
> +				priv->tx_sa_tbl[sa_index].salt);
> +		IXGBE_WAIT_TWRITE;
> +
> +		priv->tx_sa_tbl[i].used = 1;
> +		ic_session->sa_index = sa_index;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +ixgbe_crypto_remove_sa(struct rte_eth_dev *dev,
> +		       struct ixgbe_crypto_session *ic_session)
> +{
> +	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;
> +	int sa_index = -1;
> +
> +	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
> +		int i, ip_index = -1;
> +
> +		/* Find a match in the IP table*/
> +		for (i = 0; i < IPSEC_MAX_RX_IP_COUNT; i++) {
> +			if (CMP_IP(priv->rx_ip_tbl[i].ip, ic_session->dst_ip)) {
> +				ip_index = i;
> +				break;
> +			}
> +		}
> +
> +		/* Fail if no match*/
> +		if (ip_index < 0) {
> +			PMD_DRV_LOG(ERR,
> +				    "Entry not found in the Rx IP table\n");
> +			return -1;
> +		}
> +
> +		/* Find a free entry in the SA table*/
> +		for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
> +			if (priv->rx_sa_tbl[i].spi ==
> +				  rte_cpu_to_be_32(ic_session->spi)) {
> +				sa_index = i;
> +				break;
> +			}
> +		}
> +		/* Fail if no match*/
> +		if (sa_index < 0) {
> +			PMD_DRV_LOG(ERR,
> +				    "Entry not found in the Rx SA table\n");
> +			return -1;
> +		}
> +
> +		/* Disable and clear Rx SPI and key table table entryes*/
> +		reg = IPSRXIDX_WRITE | IPSRXIDX_TABLE_SPI | (sa_index << 3);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSPI, 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPIDX, 0);
> +		IXGBE_WAIT_RWRITE;
> +		reg = IPSRXIDX_WRITE | IPSRXIDX_TABLE_KEY | (sa_index << 3);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(0), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(1), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(2), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXKEY(3), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXSALT, 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSRXMOD, 0);
> +		IXGBE_WAIT_RWRITE;
> +		priv->rx_sa_tbl[sa_index].used = 0;
> +
> +		/* If last used then clear the IP table entry*/
> +		priv->rx_ip_tbl[ip_index].ref_count--;
> +		if (priv->rx_ip_tbl[ip_index].ref_count == 0) {
> +			reg = IPSRXIDX_WRITE | IPSRXIDX_TABLE_IP |
> +					(ip_index << 3);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(0), 0);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(1), 0);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(2), 0);
> +			IXGBE_WRITE_REG(hw, IXGBE_IPSRXIPADDR(3), 0);
> +		}
> +	} else { /* session->dir == RTE_CRYPTO_OUTBOUND */
> +		int i;
> +
> +		/* Find a match in the SA table*/
> +		for (i = 0; i < IPSEC_MAX_SA_COUNT; i++) {
> +			if (priv->tx_sa_tbl[i].spi ==
> +				    rte_cpu_to_be_32(ic_session->spi)) {
> +				sa_index = i;
> +				break;
> +			}
> +		}
> +		/* Fail if no match entries*/
> +		if (sa_index < 0) {
> +			PMD_DRV_LOG(ERR,
> +				    "Entry not found in the Tx SA table\n");
> +			return -1;
> +		}
> +		reg = IPSRXIDX_WRITE | (sa_index << 3);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(0), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(1), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(2), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXKEY(3), 0);
> +		IXGBE_WRITE_REG(hw, IXGBE_IPSTXSALT, 0);
> +		IXGBE_WAIT_TWRITE;
> +
> +		priv->tx_sa_tbl[sa_index].used = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +ixgbe_crypto_create_session(void *device,
> +		struct rte_security_session_conf *conf,
> +		struct rte_security_session *session,
> +		struct rte_mempool *mempool)
> +{
> +	struct rte_eth_dev *eth_dev = (struct rte_eth_dev *)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;
> +
> +	if (rte_mempool_get(mempool, (void **)&ic_session)) {
> +		PMD_DRV_LOG(ERR, "Cannot get object from ic_session mempool");
> +		return -ENOMEM;
> +	}
> +
> +	if (conf->crypto_xform->type != RTE_CRYPTO_SYM_XFORM_AEAD ||
> +			conf->crypto_xform->aead.algo !=
> +					RTE_CRYPTO_AEAD_AES_GCM) {
> +		PMD_DRV_LOG(ERR, "Unsupported crypto transformation mode\n");
> +		return -ENOTSUP;
> +	}
> +	aead_xform = &conf->crypto_xform->aead;
> +
> +	if (conf->ipsec.direction == RTE_SECURITY_IPSEC_SA_DIR_INGRESS) {
> +		if (dev_conf->rxmode.enable_sec) {
> +			ic_session->op = IXGBE_OP_AUTHENTICATED_DECRYPTION;
> +		} else {
> +			PMD_DRV_LOG(ERR, "IPsec decryption not enabled\n");
> +			return -ENOTSUP;
> +		}
> +	} else {
> +		if (dev_conf->txmode.enable_sec) {
> +			ic_session->op = IXGBE_OP_AUTHENTICATED_ENCRYPTION;
> +		} else {
> +			PMD_DRV_LOG(ERR, "IPsec encryption not enabled\n");
> +			return -ENOTSUP;
> +		}
> +	}
> +
> +	ic_session->key = aead_xform->key.data;
> +	memcpy(&ic_session->salt,
> +	       &aead_xform->key.data[aead_xform->key.length], 4);
> +	ic_session->spi = conf->ipsec.spi;
> +	ic_session->dev = eth_dev;
> +
> +	set_sec_session_private_data(session, ic_session);
> +
> +	if (ic_session->op == IXGBE_OP_AUTHENTICATED_ENCRYPTION) {
> +		if (ixgbe_crypto_add_sa(ic_session)) {
> +			PMD_DRV_LOG(ERR, "Failed to add SA\n");
> +			return -EPERM;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +ixgbe_crypto_remove_session(void *device,
> +		struct rte_security_session *session)
> +{
> +	struct rte_eth_dev *eth_dev = device;
> +	struct ixgbe_crypto_session *ic_session =
> +		(struct ixgbe_crypto_session *)
> +		get_sec_session_private_data(session);
> +	struct rte_mempool *mempool = rte_mempool_from_obj(ic_session);
> +
> +	if (eth_dev != ic_session->dev) {
> +		PMD_DRV_LOG(ERR, "Session not bound to this device\n");
> +		return -ENODEV;
> +	}
> +
> +	if (ixgbe_crypto_remove_sa(eth_dev, ic_session)) {
> +		PMD_DRV_LOG(ERR, "Failed to remove session\n");
> +		return -EFAULT;
> +	}
> +
> +	rte_mempool_put(mempool, (void *)ic_session);
> +
> +	return 0;
> +}
> +
> +static int
> +ixgbe_crypto_update_mb(void *device __rte_unused,
> +		struct rte_security_session *session,
> +		       struct rte_mbuf *m, void *params __rte_unused)
> +{
> +	struct ixgbe_crypto_session *ic_session =
> +			get_sec_session_private_data(session);
> +	if (ic_session->op == IXGBE_OP_AUTHENTICATED_ENCRYPTION) {
> +		struct ixgbe_crypto_tx_desc_metadata *mdata =
> +			(struct ixgbe_crypto_tx_desc_metadata *)&m->udata64;
> +		mdata->enc = 1;
> +		mdata->sa_idx = ic_session->sa_index;
> +		mdata->pad_len = *rte_pktmbuf_mtod_offset(m,
> +			uint8_t *, rte_pktmbuf_pkt_len(m) - 18) + 18;
> +	}
> +	return 0;
> +}
> +
> +struct rte_cryptodev_capabilities aes_gmac_crypto_capabilities[] = {
> +	{	/* AES GMAC (128-bit) */
> +		.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
> +		{.sym = {
> +			.xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
> +			{.auth = {
> +				.algo = RTE_CRYPTO_AUTH_AES_GMAC,
> +				.block_size = 16,
> +				.key_size = {
> +					.min = 16,
> +					.max = 16,
> +					.increment = 0
> +				},
> +				.digest_size = {
> +					.min = 12,
> +					.max = 12,
> +					.increment = 0
> +				},
> +				.iv_size = {
> +					.min = 12,
> +					.max = 12,
> +					.increment = 0
> +				}
> +			}, }
> +		}, }
> +	},
> +	{
> +		.op = RTE_CRYPTO_OP_TYPE_UNDEFINED,
> +		{.sym = {
> +			.xform_type = RTE_CRYPTO_SYM_XFORM_NOT_SPECIFIED
> +		}, }
> +	},
> +};
> +
> +struct rte_cryptodev_capabilities aes_gcm_gmac_crypto_capabilities[] = {
> +	{	/* AES GMAC (128-bit) */
> +		.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
> +		{.sym = {
> +			.xform_type = RTE_CRYPTO_SYM_XFORM_AUTH,
> +			{.auth = {
> +				.algo = RTE_CRYPTO_AUTH_AES_GMAC,
> +				.block_size = 16,
> +				.key_size = {
> +					.min = 16,
> +					.max = 16,
> +					.increment = 0
> +				},
> +				.digest_size = {
> +					.min = 12,
> +					.max = 12,
> +					.increment = 0
> +				},
> +				.iv_size = {
> +					.min = 12,
> +					.max = 12,
> +					.increment = 0
> +				}
> +			}, }
> +		}, }
> +	},
> +	{	/* AES GCM (128-bit) */
> +		.op = RTE_CRYPTO_OP_TYPE_SYMMETRIC,
> +		{.sym = {
> +			.xform_type = RTE_CRYPTO_SYM_XFORM_AEAD,
> +			{.aead = {
> +				.algo = RTE_CRYPTO_AEAD_AES_GCM,
> +				.block_size = 16,
> +				.key_size = {
> +					.min = 16,
> +					.max = 16,
> +					.increment = 0
> +				},
> +				.digest_size = {
> +					.min = 8,
> +					.max = 16,
> +					.increment = 4
> +				},
> +				.aad_size = {
> +					.min = 0,
> +					.max = 65535,
> +					.increment = 1
> +				},
> +				.iv_size = {
> +					.min = 12,
> +					.max = 12,
> +					.increment = 0
> +				}
> +			}, }
> +		}, }
> +	},
> +	{
> +		.op = RTE_CRYPTO_OP_TYPE_UNDEFINED,
> +		{.sym = {
> +			.xform_type = RTE_CRYPTO_SYM_XFORM_NOT_SPECIFIED
> +		}, }
> +	},
> +};
> +
> +static const struct rte_security_capability ixgbe_security_capabilities[] = {
> +	{ /* IPsec Inline Crypto ESP Transport Egress */
> +		.action = RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO,
> +		.protocol = RTE_SECURITY_PROTOCOL_IPSEC,
> +		.ipsec = {
> +			.proto = RTE_SECURITY_IPSEC_SA_PROTO_ESP,
> +			.mode = RTE_SECURITY_IPSEC_SA_MODE_TRANSPORT,
> +			.direction = RTE_SECURITY_IPSEC_SA_DIR_EGRESS,
> +			.options = { 0 }
> +		},
> +		.crypto_capabilities = aes_gcm_gmac_crypto_capabilities
> +	},
> +	{ /* IPsec Inline Crypto ESP Transport Ingress */
> +		.action = RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO,
> +		.protocol = RTE_SECURITY_PROTOCOL_IPSEC,
> +		.ipsec = {
> +			.proto = RTE_SECURITY_IPSEC_SA_PROTO_ESP,
> +			.mode = RTE_SECURITY_IPSEC_SA_MODE_TRANSPORT,
> +			.direction = RTE_SECURITY_IPSEC_SA_DIR_INGRESS,
> +			.options = { 0 }
> +		},
> +		.crypto_capabilities = aes_gcm_gmac_crypto_capabilities
> +	},
> +	{ /* IPsec Inline Crypto ESP Tunnel Egress */
> +		.action = RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO,
> +		.protocol = RTE_SECURITY_PROTOCOL_IPSEC,
> +		.ipsec = {
> +			.proto = RTE_SECURITY_IPSEC_SA_PROTO_ESP,
> +			.mode = RTE_SECURITY_IPSEC_SA_MODE_TUNNEL,
> +			.direction = RTE_SECURITY_IPSEC_SA_DIR_EGRESS,
> +			.options = { 0 }
> +		},
> +		.crypto_capabilities = aes_gcm_gmac_crypto_capabilities
> +	},
> +	{ /* IPsec Inline Crypto ESP Tunnel Ingress */
> +		.action = RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO,
> +		.protocol = RTE_SECURITY_PROTOCOL_IPSEC,
> +		.ipsec = {
> +			.proto = RTE_SECURITY_IPSEC_SA_PROTO_ESP,
> +			.mode = RTE_SECURITY_IPSEC_SA_MODE_TUNNEL,
> +			.direction = RTE_SECURITY_IPSEC_SA_DIR_INGRESS,
> +			.options = { 0 }
> +		},
> +		.crypto_capabilities = aes_gcm_gmac_crypto_capabilities
> +	},
> +	{
> +		.action = RTE_SECURITY_ACTION_TYPE_NONE
> +	}
> +};
> +
> +static const struct rte_security_capability *
> +ixgbe_crypto_capabilities_get(void *device __rte_unused)
> +{
> +	return ixgbe_security_capabilities;
> +}
> +
> +
> +int
> +ixgbe_crypto_enable_ipsec(struct rte_eth_dev *dev)
> +{
> +	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	uint32_t reg;
> +
> +
> +	/* Set IXGBE_SECTXBUFFAF to 0x15 as required in the datasheet*/
> +	IXGBE_WRITE_REG(hw, IXGBE_SECTXBUFFAF, 0x15);
> +
> +	/* IFG needs to be set to 3 when we are using security. Otherwise a Tx
> +	 * hang will occur with heavy traffic.
> +	 */
> +	reg = IXGBE_READ_REG(hw, IXGBE_SECTXMINIFG);
> +	reg = (reg & 0xFFFFFFF0) | 0x3;
> +	IXGBE_WRITE_REG(hw, IXGBE_SECTXMINIFG, reg);
> +
> +	reg  = IXGBE_READ_REG(hw, IXGBE_HLREG0);
> +	reg |= IXGBE_HLREG0_TXCRCEN | IXGBE_HLREG0_RXCRCSTRP;
> +	IXGBE_WRITE_REG(hw, IXGBE_HLREG0, reg);
> +
> +	if (dev->data->dev_conf.rxmode.enable_sec) {
> +		IXGBE_WRITE_REG(hw, IXGBE_SECRXCTRL, 0);
> +		reg = IXGBE_READ_REG(hw, IXGBE_SECRXCTRL);
> +		if (reg != 0) {
> +			PMD_DRV_LOG(ERR, "Error enabling Rx Crypto");
> +			return -1;
> +		}
> +	}
> +	if (dev->data->dev_conf.txmode.enable_sec) {
> +		IXGBE_WRITE_REG(hw, IXGBE_SECTXCTRL,
> +				IXGBE_SECTXCTRL_STORE_FORWARD);
> +		reg = IXGBE_READ_REG(hw, IXGBE_SECTXCTRL);
> +		if (reg != IXGBE_SECTXCTRL_STORE_FORWARD) {
> +			PMD_DRV_LOG(ERR, "Error enabling Rx Crypto");
> +			return -1;
> +		}
> +	}
> +
> +	ixgbe_crypto_clear_ipsec_tables(dev);
> +
> +	return 0;
> +}
> +
> +int
> +ixgbe_crypto_add_ingress_sa_from_flow(const void *sess,
> +				      const void *ip_spec,
> +				      uint8_t is_ipv6)
> +{
> +	struct ixgbe_crypto_session *ic_session
> +		= get_sec_session_private_data(sess);
> +
> +	if (ic_session->op == IXGBE_OP_AUTHENTICATED_DECRYPTION) {
> +		if (is_ipv6) {
> +			const struct rte_flow_item_ipv6 *ipv6 = ip_spec;
> +			ic_session->src_ip.type = IPv6;
> +			ic_session->dst_ip.type = IPv6;
> +			rte_memcpy(ic_session->src_ip.ipv6,
> +				   ipv6->hdr.src_addr, 16);
> +			rte_memcpy(ic_session->dst_ip.ipv6,
> +				   ipv6->hdr.dst_addr, 16);
> +		} else {
> +			const struct rte_flow_item_ipv4 *ipv4 = ip_spec;
> +			ic_session->src_ip.type = IPv4;
> +			ic_session->dst_ip.type = IPv4;
> +			ic_session->src_ip.ipv4 = ipv4->hdr.src_addr;
> +			ic_session->dst_ip.ipv4 = ipv4->hdr.dst_addr;
> +		}
> +		return ixgbe_crypto_add_sa(ic_session);
> +	}
> +
> +	return 0;
> +}
> +
> +
> +struct rte_security_ops ixgbe_security_ops = {
> +	.session_create = ixgbe_crypto_create_session,
> +	.session_update = NULL,
> +	.session_stats_get = NULL,
> +	.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
> new file mode 100644
> index 0000000..524bfaf
> --- /dev/null
> +++ b/drivers/net/ixgbe/ixgbe_ipsec.h
> @@ -0,0 +1,147 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2017 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef IXGBE_IPSEC_H_
> +#define IXGBE_IPSEC_H_
> +
> +#include <rte_security.h>
> +
> +#define IPSRXIDX_RX_EN                                    0x00000001
> +#define IPSRXIDX_TABLE_IP                                 0x00000002
> +#define IPSRXIDX_TABLE_SPI                                0x00000004
> +#define IPSRXIDX_TABLE_KEY                                0x00000006
> +#define IPSRXIDX_WRITE                                    0x80000000
> +#define IPSRXIDX_READ                                     0x40000000
> +#define IPSRXMOD_VALID                                    0x00000001
> +#define IPSRXMOD_PROTO                                    0x00000004
> +#define IPSRXMOD_DECRYPT                                  0x00000008
> +#define IPSRXMOD_IPV6                                     0x00000010
> +#define IXGBE_ADVTXD_POPTS_IPSEC                          0x00000400
> +#define IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP                 0x00002000
> +#define IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN               0x00004000
> +#define IXGBE_RXDADV_IPSEC_STATUS_SECP                    0x00020000
> +#define IXGBE_RXDADV_IPSEC_ERROR_BIT_MASK                 0x18000000
> +#define IXGBE_RXDADV_IPSEC_ERROR_INVALID_PROTOCOL         0x08000000
> +#define IXGBE_RXDADV_IPSEC_ERROR_INVALID_LENGTH           0x10000000
> +#define IXGBE_RXDADV_IPSEC_ERROR_AUTHENTICATION_FAILED    0x18000000
> +
> +#define IPSEC_MAX_RX_IP_COUNT           128
> +#define IPSEC_MAX_SA_COUNT              1024
> +
> +enum ixgbe_operation {
> +	IXGBE_OP_AUTHENTICATED_ENCRYPTION,
> +	IXGBE_OP_AUTHENTICATED_DECRYPTION
> +};
> +
> +enum ixgbe_gcm_key {
> +	IXGBE_GCM_KEY_128,
> +	IXGBE_GCM_KEY_256
> +};
> +
> +/**
> + * Generic IP address structure
> + * TODO: Find better location for this rte_net.h possibly.
> + **/
> +struct ipaddr {
> +	enum ipaddr_type {
> +		IPv4,
> +		IPv6
> +	} type;
> +	/**< IP Address Type - IPv4/IPv6 */
> +
> +	union {
> +		uint32_t ipv4;
> +		uint32_t ipv6[4];
> +	};
> +};
> +
> +/** inline crypto crypto private session structure */
> +struct ixgbe_crypto_session {
> +	enum ixgbe_operation op;
> +	uint8_t *key;
> +	uint32_t salt;
> +	uint32_t sa_index;
> +	uint32_t spi;
> +	struct ipaddr src_ip;
> +	struct ipaddr dst_ip;
> +	struct rte_eth_dev *dev;
> +} __rte_cache_aligned;
> +
> +struct ixgbe_crypto_rx_ip_table {
> +	struct ipaddr ip;
> +	uint16_t ref_count;
> +};
> +struct ixgbe_crypto_rx_sa_table {
> +	uint32_t spi;
> +	uint32_t ip_index;
> +	uint32_t key[4];
> +	uint32_t salt;
> +	uint8_t  mode;
> +	uint8_t  used;
> +};
> +
> +struct ixgbe_crypto_tx_sa_table {
> +	uint32_t spi;
> +	uint32_t key[4];
> +	uint32_t salt;
> +	uint8_t  used;
> +};
> +
> +struct ixgbe_crypto_tx_desc_metadata {
> +	union {
> +		uint64_t data;
> +		struct {
> +			  uint32_t sa_idx;
> +			  uint8_t pad_len;
> +			  uint8_t enc;
> +		};
> +	};
> +};
> +
> +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];
> +};
> +
> +extern struct rte_security_ops ixgbe_security_ops;
> +
> +
> +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);
> +
> +
> +
> +#endif /*IXGBE_IPSEC_H_*/
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
> index 64bff25..9499ecb 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.c
> @@ -93,6 +93,7 @@
>  		PKT_TX_TCP_SEG |		 \
>  		PKT_TX_MACSEC |			 \
>  		PKT_TX_OUTER_IP_CKSUM |		 \
> +		PKT_TX_SEC_OFFLOAD |	 \
>  		IXGBE_TX_IEEE1588_TMST)
> 
>  #define IXGBE_TX_OFFLOAD_NOTSUP_MASK \
> @@ -395,7 +396,8 @@ ixgbe_xmit_pkts_vec(void *tx_queue, struct rte_mbuf **tx_pkts,
>  static inline void
>  ixgbe_set_xmit_ctx(struct ixgbe_tx_queue *txq,
>  		volatile struct ixgbe_adv_tx_context_desc *ctx_txd,
> -		uint64_t ol_flags, union ixgbe_tx_offload tx_offload)
> +		uint64_t ol_flags, union ixgbe_tx_offload tx_offload,
> +		__rte_unused struct rte_mbuf *mb)
>  {
>  	uint32_t type_tucmd_mlhl;
>  	uint32_t mss_l4len_idx = 0;
> @@ -479,6 +481,20 @@ ixgbe_set_xmit_ctx(struct ixgbe_tx_queue *txq,
>  		seqnum_seed |= tx_offload.l2_len
>  			       << IXGBE_ADVTXD_TUNNEL_LEN;
>  	}
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	if (mb->ol_flags & PKT_TX_SEC_OFFLOAD) {
> +		struct ixgbe_crypto_tx_desc_metadata *mdata =
> +				(struct ixgbe_crypto_tx_desc_metadata *)
> +				&mb->udata64;
> +		seqnum_seed |=
> +			(IXGBE_ADVTXD_IPSEC_SA_INDEX_MASK & mdata->sa_idx);
> +		type_tucmd_mlhl |= mdata->enc ?
> +				(IXGBE_ADVTXD_TUCMD_IPSEC_TYPE_ESP |
> +				IXGBE_ADVTXD_TUCMD_IPSEC_ENCRYPT_EN) : 0;
> +		type_tucmd_mlhl |=
> +			(mdata->pad_len & IXGBE_ADVTXD_IPSEC_ESP_LEN_MASK);
> +	}
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> 
>  	txq->ctx_cache[ctx_idx].flags = ol_flags;
>  	txq->ctx_cache[ctx_idx].tx_offload.data[0]  =
> @@ -657,6 +673,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  	uint32_t ctx = 0;
>  	uint32_t new_ctx;
>  	union ixgbe_tx_offload tx_offload;
> +	__rte_unused struct ixgbe_crypto_tx_desc_metadata *ipsec_mdata;
> 
>  	tx_offload.data[0] = 0;
>  	tx_offload.data[1] = 0;
> @@ -685,6 +702,12 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  		 */
>  		ol_flags = tx_pkt->ol_flags;
> 
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +		ipsec_mdata = (struct ixgbe_crypto_tx_desc_metadata *)
> +				&tx_pkt->udata64;
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> +
> +
>  		/* If hardware offload required */
>  		tx_ol_req = ol_flags & IXGBE_TX_OFFLOAD_MASK;
>  		if (tx_ol_req) {
> @@ -695,6 +718,12 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  			tx_offload.tso_segsz = tx_pkt->tso_segsz;
>  			tx_offload.outer_l2_len = tx_pkt->outer_l2_len;
>  			tx_offload.outer_l3_len = tx_pkt->outer_l3_len;
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +			if (ol_flags & PKT_TX_SEC_OFFLOAD) {
> +				tx_offload.sa_idx = ipsec_mdata->sa_idx;
> +				tx_offload.sec_pad_len = ipsec_mdata->pad_len;
> +			}
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> 
>  			/* If new context need be built or reuse the exist ctx. */
>  			ctx = what_advctx_update(txq, tx_ol_req,
> @@ -855,7 +884,7 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  				}
> 
>  				ixgbe_set_xmit_ctx(txq, ctx_txd, tx_ol_req,
> -					tx_offload);
> +					tx_offload, tx_pkt);
> 
>  				txe->last_id = tx_last;
>  				tx_id = txe->next_id;
> @@ -872,7 +901,13 @@ ixgbe_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  			olinfo_status |= ctx << IXGBE_ADVTXD_IDX_SHIFT;
>  		}
> 
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +		olinfo_status |= ((pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT) |
> +				(((ol_flags & PKT_TX_SEC_OFFLOAD) != 0) *
> +						IXGBE_ADVTXD_POPTS_IPSEC));
> +#else /* RTE_LIBRTE_IXGBE_IPSEC */
>  		olinfo_status |= (pkt_len << IXGBE_ADVTXD_PAYLEN_SHIFT);
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> 
>  		m_seg = tx_pkt;
>  		do {
> @@ -1447,6 +1482,14 @@ rx_desc_error_to_pkt_flags(uint32_t rx_status)
>  		pkt_flags |= PKT_RX_EIP_CKSUM_BAD;
>  	}
> 
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	if (rx_status & IXGBE_RXD_STAT_SECP) {
> +		pkt_flags |= PKT_RX_SEC_OFFLOAD;
> +		if (rx_status & IXGBE_RXDADV_LNKSEC_ERROR_BAD_SIG)
> +			pkt_flags |= PKT_RX_SEC_OFFLOAD_FAILED;
> +	}
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> +
>  	return pkt_flags;
>  }
> 
> @@ -4981,6 +5024,22 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>  			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
>  		ixgbe_setup_loopback_link_82599(hw);

As I can see from the datasheet LRO and IPsec are mutually exclusive,
plus IPsec requires hw crc strip enabled.
I think you need add extra checks regarding that in ixgbe_dev_rx_init() or so.
Another thing - probably need to update ixgbe_set_tx_function() to
select full-featured TX func when  txmode.enable_sec is on.

> 
> +	if (dev->data->dev_conf.rxmode.enable_sec ||
> +			dev->data->dev_conf.txmode.enable_sec) {
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +		ret = ixgbe_crypto_enable_ipsec(dev);
> +		if (ret != 0) {
> +			PMD_DRV_LOG(ERR,
> +				    "ixgbe_crypto_enable_ipsec fails with %d.",
> +				    ret);
> +			return ret;
> +		}
> +#else
> +		PMD_DRV_LOG(ERR, "Inline IPsec not enabled");
> +		return -ENOTSUP;
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> +	}
> +
>  	return 0;
>  }
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
> index 85feb0b..c73e457 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx.h
> +++ b/drivers/net/ixgbe/ixgbe_rxtx.h
> @@ -183,6 +183,10 @@ union ixgbe_tx_offload {
>  		/* fields for TX offloading of tunnels */
>  		uint64_t outer_l3_len:8; /**< Outer L3 (IP) Hdr Length. */
>  		uint64_t outer_l2_len:8; /**< Outer L2 (MAC) Hdr Length. */
> +
> +		/* inline ipsec related*/
> +		uint64_t sa_idx:8;	/**< TX SA database entry index */
> +		uint64_t sec_pad_len:4;	/**< padding length */
>  	};
>  };
> 
> diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> index e704a7f..8bec4fe 100644
> --- a/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> +++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c
> @@ -128,6 +128,10 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
>  {
>  	__m128i ptype0, ptype1, vtag0, vtag1, csum;
>  	__m128i rearm0, rearm1, rearm2, rearm3;
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	__m128i sterr0, sterr1, sterr2, sterr3;
> +	__m128i tmp1, tmp2, tmp3, tmp4;
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> 
>  	/* mask everything except rss type */
>  	const __m128i rsstype_msk = _mm_set_epi16(
> @@ -174,6 +178,23 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
>  		0, PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t), 0,
>  		PKT_RX_L4_CKSUM_GOOD >> sizeof(uint8_t));
> 
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	const __m128i ipsec_sterr_msk = _mm_set_epi32(
> +		0, IXGBE_RXD_STAT_SECP | IXGBE_RXDADV_LNKSEC_ERROR_BAD_SIG,
> +		0, 0);
> +	const __m128i ipsec_proc_msk  = _mm_set_epi32(
> +		0, IXGBE_RXD_STAT_SECP, 0, 0);
> +	const __m128i ipsec_err_flag  = _mm_set_epi32(
> +		0, PKT_RX_SEC_OFFLOAD_FAILED | PKT_RX_SEC_OFFLOAD,
> +		0, 0);
> +	const __m128i ipsec_proc_flag = _mm_set_epi32(
> +		0, PKT_RX_SEC_OFFLOAD, 0, 0);
> +	sterr0 = _mm_and_si128(descs[0], ipsec_sterr_msk);
> +	sterr1 = _mm_and_si128(descs[1], ipsec_sterr_msk);
> +	sterr2 = _mm_and_si128(descs[2], ipsec_sterr_msk);
> +	sterr3 = _mm_and_si128(descs[3], ipsec_sterr_msk);
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> +
>  	ptype0 = _mm_unpacklo_epi16(descs[0], descs[1]);
>  	ptype1 = _mm_unpacklo_epi16(descs[2], descs[3]);
>  	vtag0 = _mm_unpackhi_epi16(descs[0], descs[1]);
> @@ -221,6 +242,29 @@ desc_to_olflags_v(__m128i descs[4], __m128i mbuf_init, uint8_t vlan_flags,
>  	rearm2 = _mm_blend_epi16(mbuf_init, _mm_slli_si128(vtag1, 4), 0x10);
>  	rearm3 = _mm_blend_epi16(mbuf_init, _mm_slli_si128(vtag1, 2), 0x10);
> 
> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
> +	tmp1 = _mm_cmpeq_epi32(sterr0, ipsec_sterr_msk);
> +	tmp2 = _mm_cmpeq_epi32(sterr0, ipsec_proc_msk);
> +	tmp3 = _mm_cmpeq_epi32(sterr1, ipsec_sterr_msk);
> +	tmp4 = _mm_cmpeq_epi32(sterr1, ipsec_proc_msk);
> +	sterr0 = _mm_or_si128(_mm_and_si128(tmp1, ipsec_err_flag),
> +				_mm_and_si128(tmp2, ipsec_proc_flag));
> +	sterr1 = _mm_or_si128(_mm_and_si128(tmp3, ipsec_err_flag),
> +				_mm_and_si128(tmp4, ipsec_proc_flag));
> +	tmp1 = _mm_cmpeq_epi32(sterr2, ipsec_sterr_msk);
> +	tmp2 = _mm_cmpeq_epi32(sterr2, ipsec_proc_msk);
> +	tmp3 = _mm_cmpeq_epi32(sterr3, ipsec_sterr_msk);
> +	tmp4 = _mm_cmpeq_epi32(sterr3, ipsec_proc_msk);
> +	sterr2 = _mm_or_si128(_mm_and_si128(tmp1, ipsec_err_flag),
> +				_mm_and_si128(tmp2, ipsec_proc_flag));
> +	sterr3 = _mm_or_si128(_mm_and_si128(tmp3, ipsec_err_flag),
> +				_mm_and_si128(tmp4, ipsec_proc_flag));
> +	rearm0 = _mm_or_si128(rearm0, sterr0);
> +	rearm1 = _mm_or_si128(rearm1, sterr1);
> +	rearm2 = _mm_or_si128(rearm2, sterr2);
> +	rearm3 = _mm_or_si128(rearm3, sterr3);
> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> +
>  	/* write the rearm data and the olflags in one write */
>  	RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, ol_flags) !=
>  			offsetof(struct rte_mbuf, rearm_data) + 8);
> --
> 2.9.3
  

Thanks for reviewing!

Some comments inline.


On 10/5/2017 6:55 PM, Ananyev, Konstantin wrote:
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal
>> Sent: Tuesday, October 3, 2017 2:14 PM
>> To: dev@dpdk.org
>> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; hemant.agrawal@nxp.com;
>> Nicolau, Radu <radu.nicolau@intel.com>; borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net;
>> sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com; Mcnamara, John <john.mcnamara@intel.com>; olivier.matz@6wind.com
>> Subject: [dpdk-dev] [PATCH v2 10/12] net/ixgbe: enable inline ipsec
>>
>> From: Radu Nicolau <radu.nicolau@intel.com>
>>
>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> ---
>> <snip>
>>
>>   	eth_dev->dev_ops = &ixgbe_eth_dev_ops;
>> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
>> +	rte_security_register(&eth_dev->data->sec_id,
>> +			      (void *)eth_dev, &ixgbe_security_ops);
>> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
> I still wonder do we really need new config macro and
> Ifdef it through all ixgbe code?
> Can we have it just always on?
> If the RX/TX performance suffers a lot we  can have a special
> RX/TX functions for ipsec enabled case.
I only put it there in case there is a performance degradation, but I'm 
fairly certain that there is none.
So if you think that's best I will remove it, but just in case that 
there is a performance degradation for non-ipsec traffic it will provide 
a quick way to turn the feature off.
>
>> <snip>
>> +#include "base/ixgbe_type.h"
>> +#include "base/ixgbe_api.h"
>> +#include "ixgbe_ethdev.h"
>> +#include "ixgbe_ipsec.h"
>> +
>> +
>> +#define IXGBE_WAIT_RW(__reg, __rw)					\
>> +{									\
>> +	int cnt = 100;							\
>> +	IXGBE_WRITE_REG(hw, (__reg), reg);				\
>> +	while (((IXGBE_READ_REG(hw, (__reg))) & (__rw)) && (cnt--))	\
>> +		rte_delay_us(1);					\
>> +}
> Looks usefull.
> Probably worth to add cnt as a parameter and put the macro (or even better inline func)
> Inside base/ixgbe_osdep.h.
First let me explain why I've put it there: in the datasheet it is 
stated that after the software requests a write the hw will perform the 
write and afterwards clear the write bit (7.12.9.2.1).
My understanding is that I need to wait for the write bit to clear until 
I request another subsequent write (and there are multiple writes into 
multiple tables in succession for setting up the Rx SA).
I added the cnt because I wasn't comfortable with a potentially endless 
loop...
So if you think that this will be useful in other places I will move it 
as you say.
>
>> <snip>
>>   }
>>
>> @@ -4981,6 +5024,22 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>>   			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
>>   		ixgbe_setup_loopback_link_82599(hw);
> As I can see from the datasheet LRO and IPsec are mutually exclusive,
> plus IPsec requires hw crc strip enabled.
> I think you need add extra checks regarding that in ixgbe_dev_rx_init() or so.
> Another thing - probably need to update ixgbe_set_tx_function() to
> select full-featured TX func when  txmode.enable_sec is on.
I will look into it.
  

> -----Original Message-----

> From: Nicolau, Radu

> Sent: Friday, October 6, 2017 10:18 AM

> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>; dev@dpdk.org

> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; hemant.agrawal@nxp.com;

> borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net; sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com;

> Mcnamara, John <john.mcnamara@intel.com>; olivier.matz@6wind.com

> Subject: Re: [dpdk-dev] [PATCH v2 10/12] net/ixgbe: enable inline ipsec

> 

> Thanks for reviewing!

> 

> Some comments inline.

> 

> 

> On 10/5/2017 6:55 PM, Ananyev, Konstantin wrote:

> >

> >> -----Original Message-----

> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal

> >> Sent: Tuesday, October 3, 2017 2:14 PM

> >> To: dev@dpdk.org

> >> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;

> hemant.agrawal@nxp.com;

> >> Nicolau, Radu <radu.nicolau@intel.com>; borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net;

> >> sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com; Mcnamara, John <john.mcnamara@intel.com>; olivier.matz@6wind.com

> >> Subject: [dpdk-dev] [PATCH v2 10/12] net/ixgbe: enable inline ipsec

> >>

> >> From: Radu Nicolau <radu.nicolau@intel.com>

> >>

> >> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>

> >> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

> >> ---

> >> <snip>

> >>

> >>   	eth_dev->dev_ops = &ixgbe_eth_dev_ops;

> >> +#ifdef RTE_LIBRTE_IXGBE_IPSEC

> >> +	rte_security_register(&eth_dev->data->sec_id,

> >> +			      (void *)eth_dev, &ixgbe_security_ops);

> >> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */

> > I still wonder do we really need new config macro and

> > Ifdef it through all ixgbe code?

> > Can we have it just always on?

> > If the RX/TX performance suffers a lot we  can have a special

> > RX/TX functions for ipsec enabled case.

> I only put it there in case there is a performance degradation, but I'm

> fairly certain that there is none.

> So if you think that's best I will remove it, but just in case that

> there is a performance degradation for non-ipsec traffic it will provide

> a quick way to turn the feature off.


My position is let's remove the macro in any way.
If the testing will show no performance degradation - let's have ipsec
non-ipsec path together.
It the testing will show noticeable perfoamce degradation - let's
Have a special rx/tx function for ipsec enabled.
In that case users will still have an option to use ipsec if needed,
and can switch it on/off at runtime.  

> >

> >> <snip>

> >> +#include "base/ixgbe_type.h"

> >> +#include "base/ixgbe_api.h"

> >> +#include "ixgbe_ethdev.h"

> >> +#include "ixgbe_ipsec.h"

> >> +

> >> +

> >> +#define IXGBE_WAIT_RW(__reg, __rw)					\

> >> +{									\

> >> +	int cnt = 100;							\

> >> +	IXGBE_WRITE_REG(hw, (__reg), reg);				\

> >> +	while (((IXGBE_READ_REG(hw, (__reg))) & (__rw)) && (cnt--))	\

> >> +		rte_delay_us(1);					\

> >> +}

> > Looks usefull.

> > Probably worth to add cnt as a parameter and put the macro (or even better inline func)

> > Inside base/ixgbe_osdep.h.

> First let me explain why I've put it there: in the datasheet it is

> stated that after the software requests a write the hw will perform the

> write and afterwards clear the write bit (7.12.9.2.1).


I think I understand what are you doing here.
You write to the HW register and then poll on that register till HW indicate that the operation completed.
In fact that's not the only place where you have to do same thing.
Let say at dev_rxtx_start():
rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
                rxdctl |= IXGBE_RXDCTL_ENABLE;
                IXGBE_WRITE_REG(hw, IXGBE_RXDCTL(rxq->reg_idx), rxdctl);

                /* Wait until RX Enable ready */
                poll_ms = RTE_IXGBE_REGISTER_POLL_WAIT_10_MS;
                do {
                        rte_delay_ms(1);
                        rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
                } while (--poll_ms && !(rxdctl & IXGBE_RXDCTL_ENABLE));
                if (!poll_ms)
                        PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d",
                                     rx_queue_id);
So my thought was that common macro(inline function will be useful here.
Though I think we have to get rid of implicit parameters.
Konstantin

> My understanding is that I need to wait for the write bit to clear until

> I request another subsequent write (and there are multiple writes into

> multiple tables in succession for setting up the Rx SA).

> I added the cnt because I wasn't comfortable with a potentially endless

> loop...

> So if you think that this will be useful in other places I will move it

> as you say.

> >

> >> <snip>

> >>   }

> >>

> >> @@ -4981,6 +5024,22 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)

> >>   			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)

> >>   		ixgbe_setup_loopback_link_82599(hw);

> > As I can see from the datasheet LRO and IPsec are mutually exclusive,

> > plus IPsec requires hw crc strip enabled.

> > I think you need add extra checks regarding that in ixgbe_dev_rx_init() or so.

> > Another thing - probably need to update ixgbe_set_tx_function() to

> > select full-featured TX func when  txmode.enable_sec is on.

> I will look into it.
  

Hi,

Next iteration will have the macro removed and the register write then 
polling macro reworked and moved to ixgbe_osdep.h

Regards,

Radu


On 10/6/2017 7:33 PM, Ananyev, Konstantin wrote:
>
>> -----Original Message-----
>> From: Nicolau, Radu
>> Sent: Friday, October 6, 2017 10:18 AM
>> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>; dev@dpdk.org
>> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; hemant.agrawal@nxp.com;
>> borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net; sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com;
>> Mcnamara, John <john.mcnamara@intel.com>; olivier.matz@6wind.com
>> Subject: Re: [dpdk-dev] [PATCH v2 10/12] net/ixgbe: enable inline ipsec
>>
>> Thanks for reviewing!
>>
>> Some comments inline.
>>
>>
>> On 10/5/2017 6:55 PM, Ananyev, Konstantin wrote:
>>>> -----Original Message-----
>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal
>>>> Sent: Tuesday, October 3, 2017 2:14 PM
>>>> To: dev@dpdk.org
>>>> Cc: Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> hemant.agrawal@nxp.com;
>>>> Nicolau, Radu <radu.nicolau@intel.com>; borisp@mellanox.com; aviadye@mellanox.com; thomas@monjalon.net;
>>>> sandeep.malik@nxp.com; jerin.jacob@caviumnetworks.com; Mcnamara, John <john.mcnamara@intel.com>; olivier.matz@6wind.com
>>>> Subject: [dpdk-dev] [PATCH v2 10/12] net/ixgbe: enable inline ipsec
>>>>
>>>> From: Radu Nicolau <radu.nicolau@intel.com>
>>>>
>>>> Signed-off-by: Radu Nicolau <radu.nicolau@intel.com>
>>>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>>>> ---
>>>> <snip>
>>>>
>>>>    	eth_dev->dev_ops = &ixgbe_eth_dev_ops;
>>>> +#ifdef RTE_LIBRTE_IXGBE_IPSEC
>>>> +	rte_security_register(&eth_dev->data->sec_id,
>>>> +			      (void *)eth_dev, &ixgbe_security_ops);
>>>> +#endif /* RTE_LIBRTE_IXGBE_IPSEC */
>>> I still wonder do we really need new config macro and
>>> Ifdef it through all ixgbe code?
>>> Can we have it just always on?
>>> If the RX/TX performance suffers a lot we  can have a special
>>> RX/TX functions for ipsec enabled case.
>> I only put it there in case there is a performance degradation, but I'm
>> fairly certain that there is none.
>> So if you think that's best I will remove it, but just in case that
>> there is a performance degradation for non-ipsec traffic it will provide
>> a quick way to turn the feature off.
> My position is let's remove the macro in any way.
> If the testing will show no performance degradation - let's have ipsec
> non-ipsec path together.
> It the testing will show noticeable perfoamce degradation - let's
> Have a special rx/tx function for ipsec enabled.
> In that case users will still have an option to use ipsec if needed,
> and can switch it on/off at runtime.
>
>>>> <snip>
>>>> +#include "base/ixgbe_type.h"
>>>> +#include "base/ixgbe_api.h"
>>>> +#include "ixgbe_ethdev.h"
>>>> +#include "ixgbe_ipsec.h"
>>>> +
>>>> +
>>>> +#define IXGBE_WAIT_RW(__reg, __rw)					\
>>>> +{									\
>>>> +	int cnt = 100;							\
>>>> +	IXGBE_WRITE_REG(hw, (__reg), reg);				\
>>>> +	while (((IXGBE_READ_REG(hw, (__reg))) & (__rw)) && (cnt--))	\
>>>> +		rte_delay_us(1);					\
>>>> +}
>>> Looks usefull.
>>> Probably worth to add cnt as a parameter and put the macro (or even better inline func)
>>> Inside base/ixgbe_osdep.h.
>> First let me explain why I've put it there: in the datasheet it is
>> stated that after the software requests a write the hw will perform the
>> write and afterwards clear the write bit (7.12.9.2.1).
> I think I understand what are you doing here.
> You write to the HW register and then poll on that register till HW indicate that the operation completed.
> In fact that's not the only place where you have to do same thing.
> Let say at dev_rxtx_start():
> rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
>                  rxdctl |= IXGBE_RXDCTL_ENABLE;
>                  IXGBE_WRITE_REG(hw, IXGBE_RXDCTL(rxq->reg_idx), rxdctl);
>
>                  /* Wait until RX Enable ready */
>                  poll_ms = RTE_IXGBE_REGISTER_POLL_WAIT_10_MS;
>                  do {
>                          rte_delay_ms(1);
>                          rxdctl = IXGBE_READ_REG(hw, IXGBE_RXDCTL(rxq->reg_idx));
>                  } while (--poll_ms && !(rxdctl & IXGBE_RXDCTL_ENABLE));
>                  if (!poll_ms)
>                          PMD_INIT_LOG(ERR, "Could not enable Rx Queue %d",
>                                       rx_queue_id);
> So my thought was that common macro(inline function will be useful here.
> Though I think we have to get rid of implicit parameters.
> Konstantin
>
>> My understanding is that I need to wait for the write bit to clear until
>> I request another subsequent write (and there are multiple writes into
>> multiple tables in succession for setting up the Rx SA).
>> I added the cnt because I wasn't comfortable with a potentially endless
>> loop...
>> So if you think that this will be useful in other places I will move it
>> as you say.
>>>> <snip>
>>>>    }
>>>>
>>>> @@ -4981,6 +5024,22 @@ ixgbe_dev_rxtx_start(struct rte_eth_dev *dev)
>>>>    			dev->data->dev_conf.lpbk_mode == IXGBE_LPBK_82599_TX_RX)
>>>>    		ixgbe_setup_loopback_link_82599(hw);
>>> As I can see from the datasheet LRO and IPsec are mutually exclusive,
>>> plus IPsec requires hw crc strip enabled.
>>> I think you need add extra checks regarding that in ixgbe_dev_rx_init() or so.
>>> Another thing - probably need to update ixgbe_set_tx_function() to
>>> select full-featured TX func when  txmode.enable_sec is on.
>> I will look into it.
>