Re: [dpdk-dev] [PATCH 11/14] examples/ipsec-secgw: add app processing code

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> 
> Add IPsec application processing code for event mode.
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
>  examples/ipsec-secgw/ipsec-secgw.c  | 124 ++++++------------
>  examples/ipsec-secgw/ipsec-secgw.h  |  81 ++++++++++++
>  examples/ipsec-secgw/ipsec.h        |  37 +++---
>  examples/ipsec-secgw/ipsec_worker.c | 242 ++++++++++++++++++++++++++++++++++--
>  examples/ipsec-secgw/ipsec_worker.h |  39 ++++++
>  examples/ipsec-secgw/sa.c           |  11 --
>  6 files changed, 409 insertions(+), 125 deletions(-)
>  create mode 100644 examples/ipsec-secgw/ipsec-secgw.h
>  create mode 100644 examples/ipsec-secgw/ipsec_worker.h
> 
> diff --git a/examples/ipsec-secgw/ipsec-secgw.c b/examples/ipsec-secgw/ipsec-secgw.c
> index c5d95b9..2e7d4d8 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.c
> +++ b/examples/ipsec-secgw/ipsec-secgw.c
> @@ -50,12 +50,11 @@
> 
>  #include "event_helper.h"
>  #include "ipsec.h"
> +#include "ipsec_worker.h"
>  #include "parser.h"
> 
>  volatile bool force_quit;
> 
> -#define RTE_LOGTYPE_IPSEC RTE_LOGTYPE_USER1
> -
>  #define MAX_JUMBO_PKT_LEN  9600
> 
>  #define MEMPOOL_CACHE_SIZE 256
> @@ -70,8 +69,6 @@ volatile bool force_quit;
> 
>  #define BURST_TX_DRAIN_US 100 /* TX drain every ~100us */
> 
> -#define NB_SOCKETS 4
> -
>  /* Configure how many packets ahead to prefetch, when reading packets */
>  #define PREFETCH_OFFSET	3
> 
> @@ -79,8 +76,6 @@ volatile bool force_quit;
> 
>  #define MAX_LCORE_PARAMS 1024
> 
> -#define UNPROTECTED_PORT(port) (unprotected_port_mask & (1 << portid))
> -
>  /*
>   * Configurable number of RX/TX ring descriptors
>   */
> @@ -89,29 +84,6 @@ volatile bool force_quit;
>  static uint16_t nb_rxd = IPSEC_SECGW_RX_DESC_DEFAULT;
>  static uint16_t nb_txd = IPSEC_SECGW_TX_DESC_DEFAULT;
> 
> -#if RTE_BYTE_ORDER != RTE_LITTLE_ENDIAN
> -#define __BYTES_TO_UINT64(a, b, c, d, e, f, g, h) \
> -	(((uint64_t)((a) & 0xff) << 56) | \
> -	((uint64_t)((b) & 0xff) << 48) | \
> -	((uint64_t)((c) & 0xff) << 40) | \
> -	((uint64_t)((d) & 0xff) << 32) | \
> -	((uint64_t)((e) & 0xff) << 24) | \
> -	((uint64_t)((f) & 0xff) << 16) | \
> -	((uint64_t)((g) & 0xff) << 8)  | \
> -	((uint64_t)(h) & 0xff))
> -#else
> -#define __BYTES_TO_UINT64(a, b, c, d, e, f, g, h) \
> -	(((uint64_t)((h) & 0xff) << 56) | \
> -	((uint64_t)((g) & 0xff) << 48) | \
> -	((uint64_t)((f) & 0xff) << 40) | \
> -	((uint64_t)((e) & 0xff) << 32) | \
> -	((uint64_t)((d) & 0xff) << 24) | \
> -	((uint64_t)((c) & 0xff) << 16) | \
> -	((uint64_t)((b) & 0xff) << 8) | \
> -	((uint64_t)(a) & 0xff))
> -#endif
> -#define ETHADDR(a, b, c, d, e, f) (__BYTES_TO_UINT64(a, b, c, d, e, f, 0, 0))
> -
>  #define ETHADDR_TO_UINT64(addr) __BYTES_TO_UINT64( \
>  		(addr)->addr_bytes[0], (addr)->addr_bytes[1], \
>  		(addr)->addr_bytes[2], (addr)->addr_bytes[3], \
> @@ -123,18 +95,6 @@ static uint16_t nb_txd = IPSEC_SECGW_TX_DESC_DEFAULT;
> 
>  #define MTU_TO_FRAMELEN(x)	((x) + RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN)
> 
> -/* port/source ethernet addr and destination ethernet addr */
> -struct ethaddr_info {
> -	uint64_t src, dst;
> -};
> -
> -struct ethaddr_info ethaddr_tbl[RTE_MAX_ETHPORTS] = {
> -	{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x7e, 0x94, 0x9a) },
> -	{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x22, 0xa1, 0xd9) },
> -	{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x08, 0x69, 0x26) },
> -	{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x49, 0x9e, 0xdd) }
> -};
> -
>  struct flow_info flow_info_tbl[RTE_MAX_ETHPORTS];
> 
>  #define CMD_LINE_OPT_CONFIG		"config"
> @@ -192,10 +152,16 @@ static const struct option lgopts[] = {
>  	{NULL, 0, 0, 0}
>  };
> 
> +struct ethaddr_info ethaddr_tbl[RTE_MAX_ETHPORTS] = {
> +	{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x7e, 0x94, 0x9a) },
> +	{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x22, 0xa1, 0xd9) },
> +	{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x08, 0x69, 0x26) },
> +	{ 0, ETHADDR(0x00, 0x16, 0x3e, 0x49, 0x9e, 0xdd) }
> +};
> +
>  /* mask of enabled ports */
>  static uint32_t enabled_port_mask;
>  static uint64_t enabled_cryptodev_mask = UINT64_MAX;
> -static uint32_t unprotected_port_mask;
>  static int32_t promiscuous_on = 1;
>  static int32_t numa_on = 1; /**< NUMA is enabled by default. */
>  static uint32_t nb_lcores;
> @@ -283,8 +249,6 @@ static struct rte_eth_conf port_conf = {
>  	},
>  };
> 
> -static struct socket_ctx socket_ctx[NB_SOCKETS];
> -
>  /*
>   * Determine is multi-segment support required:
>   *  - either frame buffer size is smaller then mtu
> @@ -2828,47 +2792,10 @@ main(int32_t argc, char **argv)
> 
>  		sa_check_offloads(portid, &req_rx_offloads, &req_tx_offloads);
>  		port_init(portid, req_rx_offloads, req_tx_offloads);
> -		/* Create default ipsec flow for the ethernet device */
> -		ret = create_default_ipsec_flow(portid, req_rx_offloads);
> -		if (ret)
> -			printf("Cannot create default flow, err=%d, port=%d\n",
> -					ret, portid);
>  	}
> 
>  	cryptodevs_init();
> 
> -	/* start ports */
> -	RTE_ETH_FOREACH_DEV(portid) {
> -		if ((enabled_port_mask & (1 << portid)) == 0)
> -			continue;
> -
> -		/*
> -		 * Start device
> -		 * note: device must be started before a flow rule
> -		 * can be installed.
> -		 */
> -		ret = rte_eth_dev_start(portid);
> -		if (ret < 0)
> -			rte_exit(EXIT_FAILURE, "rte_eth_dev_start: "
> -					"err=%d, port=%d\n", ret, portid);
> -		/*
> -		 * If enabled, put device in promiscuous mode.
> -		 * This allows IO forwarding mode to forward packets
> -		 * to itself through 2 cross-connected  ports of the
> -		 * target machine.
> -		 */
> -		if (promiscuous_on) {
> -			ret = rte_eth_promiscuous_enable(portid);
> -			if (ret != 0)
> -				rte_exit(EXIT_FAILURE,
> -					"rte_eth_promiscuous_enable: err=%s, port=%d\n",
> -					rte_strerror(-ret), portid);
> -		}
> -
> -		rte_eth_dev_callback_register(portid,
> -			RTE_ETH_EVENT_IPSEC, inline_ipsec_event_callback, NULL);
> -	}
> -
>  	/* fragment reassemble is enabled */
>  	if (frag_tbl_sz != 0) {
>  		ret = reassemble_init();
> @@ -2889,8 +2816,6 @@ main(int32_t argc, char **argv)
>  		}
>  	}
> 
> -	check_all_ports_link_status(enabled_port_mask);
> -
>  	/*
>  	 * Set the enabled port mask in helper config for use by helper
>  	 * sub-system. This will be used while intializing devices using
> @@ -2903,6 +2828,39 @@ main(int32_t argc, char **argv)
>  	if (ret < 0)
>  		rte_exit(EXIT_FAILURE, "eh_devs_init failed, err=%d\n", ret);
> 
> +	/* Create default ipsec flow for each port and start each port */
> +	RTE_ETH_FOREACH_DEV(portid) {
> +		if ((enabled_port_mask & (1 << portid)) == 0)
> +			continue;
> +
> +		ret = create_default_ipsec_flow(portid, req_rx_offloads);

That doesn't look right.
For more than one eth port in the system, req_rx_offloads will be overwritten by that moment.


> +		if (ret)
> +			printf("create_default_ipsec_flow failed, err=%d, "
> +			       "port=%d\n", ret, portid);
> +		/*
> +		 * Start device
> +		 * note: device must be started before a flow rule
> +		 * can be installed.
> +		 */
> +		ret = rte_eth_dev_start(portid);

Moving that piece of code (dev_start) after sa_init() breaks ixgbe inline-crypto support.
As I understand, because configured ipsec flows don't persist dev_start().
At least for ixgbe PMD.
Any reason why to move that code at all?

> +		if (ret < 0)
> +			rte_exit(EXIT_FAILURE, "rte_eth_dev_start: "
> +					"err=%d, port=%d\n", ret, portid);
> +		/*
> +		 * If enabled, put device in promiscuous mode.
> +		 * This allows IO forwarding mode to forward packets
> +		 * to itself through 2 cross-connected  ports of the
> +		 * target machine.
> +		 */
> +		if (promiscuous_on)
> +			rte_eth_promiscuous_enable(portid);
> +
> +		rte_eth_dev_callback_register(portid,
> +			RTE_ETH_EVENT_IPSEC, inline_ipsec_event_callback, NULL);
> +	}
> +
> +	check_all_ports_link_status(enabled_port_mask);
> +
>  	/* launch per-lcore init on every lcore */
>  	rte_eal_mp_remote_launch(ipsec_launch_one_lcore, eh_conf, CALL_MASTER);
>