Re: [dpdk-dev] [PATCH v2 11/12] examples/ipsec-secgw: add app mode worker

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

> Add application inbound/outbound worker thread and
> IPsec application processing code for event mode.
> 
> Exampple ipsec-secgw command in app mode:
> ipsec-secgw -w 0002:02:00.0,ipsec_in_max_spi=128
> -w 0002:03:00.0,ipsec_in_max_spi=128 -w 0002:0e:00.0 -w 0002:10:00.1
> --log-level=8 -c 0x1 -- -P -p 0x3 -u 0x1 --config "(1,0,0),(0,0,0)"
> -f aes-gcm.cfg --transfer-mode event --schedule-type parallel
> 
> Signed-off-by: Anoob Joseph <anoobj@marvell.com>
> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
> Signed-off-by: Lukasz Bartosik <lbartosik@marvell.com>
> ---
>  examples/ipsec-secgw/ipsec-secgw.c  |  45 +---
>  examples/ipsec-secgw/ipsec-secgw.h  |  69 ++++++
>  examples/ipsec-secgw/ipsec.h        |  22 --
>  examples/ipsec-secgw/ipsec_worker.c | 418 +++++++++++++++++++++++++++++++++++-
>  examples/ipsec-secgw/ipsec_worker.h |  39 ++++
>  5 files changed, 533 insertions(+), 60 deletions(-)
>  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 86215fb..7d844bb 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
> @@ -85,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], \
> @@ -119,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"
> @@ -183,6 +147,13 @@ 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;
> diff --git a/examples/ipsec-secgw/ipsec-secgw.h b/examples/ipsec-secgw/ipsec-secgw.h
> index 5b19e29..926ce5d 100644
> --- a/examples/ipsec-secgw/ipsec-secgw.h
> +++ b/examples/ipsec-secgw/ipsec-secgw.h
> @@ -4,10 +4,79 @@
>  #ifndef _IPSEC_SECGW_H_
>  #define _IPSEC_SECGW_H_
> 
> +#include <rte_hash.h>
> +
> +#define NB_SOCKETS 4
> +
> +#define MAX_PKT_BURST 32
> +
> +#define RTE_LOGTYPE_IPSEC RTE_LOGTYPE_USER1
> +
>  #define NB_SOCKETS 4

Duplicate, NB_SOCKETS already defined, see above.

> 
>  #define UNPROTECTED_PORT(portid) (unprotected_port_mask & (1 << portid))

As you are moving it anyway probably a good time to put portid param in (),
or even make it a static inline function.

> 
> +#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))
> +
> +struct traffic_type {
> +	const uint8_t *data[MAX_PKT_BURST * 2];
> +	struct rte_mbuf *pkts[MAX_PKT_BURST * 2];
> +	void *saptr[MAX_PKT_BURST * 2];
> +	uint32_t res[MAX_PKT_BURST * 2];
> +	uint32_t num;
> +};
> +
> +struct ipsec_traffic {
> +	struct traffic_type ipsec;
> +	struct traffic_type ip4;
> +	struct traffic_type ip6;
> +};
> +
> +/* Fields optimized for devices without burst */
> +struct traffic_type_nb {
> +	const uint8_t *data;
> +	struct rte_mbuf *pkt;
> +	uint32_t res;
> +	uint32_t num;
> +};
> +
> +struct ipsec_traffic_nb {
> +	struct traffic_type_nb ipsec;
> +	struct traffic_type_nb ip4;
> +	struct traffic_type_nb ip6;
> +};
> +
> +/* port/source ethernet addr and destination ethernet addr */
> +struct ethaddr_info {
> +	uint64_t src, dst;
> +};
> +
> +struct ethaddr_info ethaddr_tbl[RTE_MAX_ETHPORTS];
> +
> +/* TODO: All var definitions need to be part of a .c file */

Seems like that TODO wasn't done :)
Probably a good thing to add extern for all global vars declarations here,
and keep actual definitions in ipsec-secgw.c.
Same story for:
+struct socket_ctx socket_ctx[NB_SOCKETS];
in ipsec.h

> +
>  /* Port mask to identify the unprotected ports */
>  uint32_t unprotected_port_mask;
>