Hi Konstantin,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.anan...@intel.com>
> Sent: Wednesday, January 29, 2020 9:05 PM
> To: Anoob Joseph <ano...@marvell.com>; Akhil Goyal <akhil.go...@nxp.com>;
> Nicolau, Radu <radu.nico...@intel.com>; Thomas Monjalon
> <tho...@monjalon.net>
> Cc: Lukas Bartosik <lbarto...@marvell.com>; Jerin Jacob Kollanukkaran
> <jer...@marvell.com>; Narayana Prasad Raju Athreya
> <pathr...@marvell.com>; Ankur Dwivedi <adwiv...@marvell.com>; Archana
> Muniganti <march...@marvell.com>; Tejasree Kondoj
> <ktejas...@marvell.com>; Vamsi Krishna Attunuru <vattun...@marvell.com>;
> dev@dpdk.org
> Subject: [EXT] RE: [PATCH v2 11/12] examples/ipsec-secgw: add app mode
> worker
>
> External Email
>
> ----------------------------------------------------------------------
> > 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 <ano...@marvell.com>
> > Signed-off-by: Ankur Dwivedi <adwiv...@marvell.com>
> > Signed-off-by: Lukasz Bartosik <lbarto...@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.
[Anoob] Good catch. Will fix in v3.
>
> >
> > #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?
>
> >
> > +#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
[Anoob] Will do in v3.
>
> > +
> > /* Port mask to identify the unprotected ports */ uint32_t
> > unprotected_port_mask;
> >
