Hi Konstantin,

Please see inline.

Thanks,
Anoob

-----Original Message-----
From: Ananyev, Konstantin <redacted>
Sent: Wednesday, January 29, 2020 9:05 PM
To: Anoob Joseph <redacted>; Akhil Goyal <redacted>;
Nicolau, Radu [off-list ref]; Thomas Monjalon
[off-list ref]
Cc: Lukas Bartosik <redacted>; Jerin Jacob Kollanukkaran
[off-list ref]; Narayana Prasad Raju Athreya
[off-list ref]; Ankur Dwivedi [off-list ref]; Archana
Muniganti [off-list ref]; Tejasree Kondoj
[off-list ref]; Vamsi Krishna Attunuru [off-list ref];
dev@dpdk.org
Subject: [EXT] RE: [PATCH v2 11/12] examples/ipsec-secgw: add app mode
worker

External Email

----------------------------------------------------------------------

quoted

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 <redacted>
Signed-off-by: Ankur Dwivedi <redacted>
Signed-off-by: Lukasz Bartosik <redacted>
---
 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,

quoted

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 @@

quoted

static uint16_t nb_txd = IPSEC_SECGW_TX_DESC_DEFAULT;

 #define MTU_TO_FRAMELEN(x)	((x) + RTE_ETHER_HDR_LEN +

RTE_ETHER_CRC_LEN)

quoted

-/* 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.

[Anoob] Good catch. Will fix in v3.
 

quoted

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

[Anoob] I would prefer a static inline function. Shall I make this change in v3?
 

quoted

+#if RTE_BYTE_ORDER != RTE_LITTLE_ENDIAN #define

__BYTES_TO_UINT64(a,

quoted

+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

[Anoob] Will do in v3.
 

quoted

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