Hi Konstantin,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: dev <dev-boun...@dpdk.org> On Behalf Of Ananyev, Konstantin
> Sent: Monday, January 6, 2020 9:15 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: Re: [dpdk-dev] [PATCH 09/14] examples/ipsec-secgw: add
> eventmode to ipsec-secgw
>
>
> > > > Add eventmode support to ipsec-secgw. This uses event helper to
> > > > setup and use the eventmode capabilities. Add driver inbound worker.
> > > >
> > > > Example command:
> > > > ./ipsec-secgw -c 0x1 -w 0002:02:00.0,ipsec_in_max_spi=100 -w
> > > > 0002:07:00.0 -w 0002:0e:00.0 -w 0002:10:00.1 -- -P -p 0x3 -u 0x1
> > > > --config "(0,0,0),(1,0,0)" -f a-aes-gcm-msa.cfg --transfer-mode 1
> > > > --schedule-type 2 --process-mode drv --process-dir in
> > > >
> > > > Signed-off-by: Anoob Joseph <ano...@marvell.com>
> > > > Signed-off-by: Lukasz Bartosik <lbarto...@marvell.com>
> > > > ---
> > > > examples/ipsec-secgw/Makefile | 1 +
> > > > examples/ipsec-secgw/event_helper.c | 3 +
> > > > examples/ipsec-secgw/event_helper.h | 26 +++
> > > > examples/ipsec-secgw/ipsec-secgw.c | 344
> > > +++++++++++++++++++++++++++++++++++-
> > > > examples/ipsec-secgw/ipsec.h | 7 +
> > > > examples/ipsec-secgw/ipsec_worker.c | 180 +++++++++++++++++++
> > > > examples/ipsec-secgw/meson.build | 2 +-
> > > > 7 files changed, 555 insertions(+), 8 deletions(-) create mode
> > > > 100644 examples/ipsec-secgw/ipsec_worker.c
> > > >
> > > > diff --git a/examples/ipsec-secgw/Makefile
> > > > b/examples/ipsec-secgw/Makefile index 09e3c5a..f6fd94c 100644
> > > > --- a/examples/ipsec-secgw/Makefile
> > > > +++ b/examples/ipsec-secgw/Makefile
> > > > @@ -15,6 +15,7 @@ SRCS-y += sa.c
> > > > SRCS-y += rt.c
> > > > SRCS-y += ipsec_process.c
> > > > SRCS-y += ipsec-secgw.c
> > > > +SRCS-y += ipsec_worker.c
> > > > SRCS-y += event_helper.c
> > > >
> > > > CFLAGS += -gdwarf-2
> > > > diff --git a/examples/ipsec-secgw/event_helper.c
> > > > b/examples/ipsec-secgw/event_helper.c
> > > > index 6549875..44f997d 100644
> > > > --- a/examples/ipsec-secgw/event_helper.c
> > > > +++ b/examples/ipsec-secgw/event_helper.c
> > > > @@ -984,6 +984,9 @@ eh_find_worker(uint32_t lcore_id, struct
> > > > eh_conf
> > > *conf,
> > > > else
> > > > curr_conf.cap.burst = EH_RX_TYPE_NON_BURST;
> > > >
> > > > + curr_conf.cap.ipsec_mode = conf->ipsec_mode;
> > > > + curr_conf.cap.ipsec_dir = conf->ipsec_dir;
> > > > +
> > > > /* Parse the passed list and see if we have matching
> > > > capabilities */
> > > >
> > > > /* Initialize the pointer used to traverse the list */ diff
> > > > --git a/examples/ipsec-secgw/event_helper.h
> > > > b/examples/ipsec-secgw/event_helper.h
> > > > index 2895dfa..07849b0 100644
> > > > --- a/examples/ipsec-secgw/event_helper.h
> > > > +++ b/examples/ipsec-secgw/event_helper.h
> > > > @@ -74,6 +74,22 @@ enum eh_tx_types {
> > > > EH_TX_TYPE_NO_INTERNAL_PORT
> > > > };
> > > >
> > > > +/**
> > > > + * Event mode ipsec mode types
> > > > + */
> > > > +enum eh_ipsec_mode_types {
> > > > + EH_IPSEC_MODE_TYPE_APP = 0,
> > > > + EH_IPSEC_MODE_TYPE_DRIVER
> > > > +};
> > > > +
> > > > +/**
> > > > + * Event mode ipsec direction types */ enum eh_ipsec_dir_types {
> > > > + EH_IPSEC_DIR_TYPE_OUTBOUND = 0,
> > > > + EH_IPSEC_DIR_TYPE_INBOUND,
> > > > +};
> > > > +
> > > > /* Event dev params */
> > > > struct eventdev_params {
> > > > uint8_t eventdev_id;
> > > > @@ -183,6 +199,12 @@ struct eh_conf {
> > > > */
> > > > void *mode_params;
> > > > /**< Mode specific parameters */
> > > > +
> > > > + /** Application specific params */
> > > > + enum eh_ipsec_mode_types ipsec_mode;
> > > > + /**< Mode of ipsec run */
> > > > + enum eh_ipsec_dir_types ipsec_dir;
> > > > + /**< Direction of ipsec processing */
> > > > };
> > > >
> > > > /* Workers registered by the application */ @@ -194,6 +216,10 @@
> > > > struct eh_app_worker_params {
> > > > /**< Specify status of rx type burst */
> > > > uint64_t tx_internal_port : 1;
> > > > /**< Specify whether tx internal port is
> > > > available */
> > > > + uint64_t ipsec_mode : 1;
> > > > + /**< Specify ipsec processing level */
> > > > + uint64_t ipsec_dir : 1;
> > > > + /**< Specify direction of ipsec */
> > > > };
> > > > uint64_t u64;
> > > > } cap;
> > > > diff --git a/examples/ipsec-secgw/ipsec-secgw.c
> > > > b/examples/ipsec-secgw/ipsec-secgw.c
> > > > index 7506922..c5d95b9 100644
> > > > --- a/examples/ipsec-secgw/ipsec-secgw.c
> > > > +++ b/examples/ipsec-secgw/ipsec-secgw.c
> > > > @@ -2,6 +2,7 @@
> > > > * Copyright(c) 2016 Intel Corporation
> > > > */
> > > >
> > > > +#include <stdbool.h>
> > > > #include <stdio.h>
> > > > #include <stdlib.h>
> > > > #include <stdint.h>
> > > > @@ -14,6 +15,7 @@
> > > > #include <sys/queue.h>
> > > > #include <stdarg.h>
> > > > #include <errno.h>
> > > > +#include <signal.h>
> > > > #include <getopt.h>
> > > >
> > > > #include <rte_common.h>
> > > > @@ -41,12 +43,17 @@
> > > > #include <rte_jhash.h>
> > > > #include <rte_cryptodev.h>
> > > > #include <rte_security.h>
> > > > +#include <rte_bitmap.h>
> > > > +#include <rte_eventdev.h>
> > > > #include <rte_ip.h>
> > > > #include <rte_ip_frag.h>
> > > >
> > > > +#include "event_helper.h"
> > > > #include "ipsec.h"
> > > > #include "parser.h"
> > > >
> > > > +volatile bool force_quit;
> > > > +
> > > > #define RTE_LOGTYPE_IPSEC RTE_LOGTYPE_USER1
> > > >
> > > > #define MAX_JUMBO_PKT_LEN 9600
> > > > @@ -133,12 +140,21 @@ struct flow_info
> > > flow_info_tbl[RTE_MAX_ETHPORTS];
> > > > #define CMD_LINE_OPT_CONFIG "config"
> > > > #define CMD_LINE_OPT_SINGLE_SA "single-sa"
> > > > #define CMD_LINE_OPT_CRYPTODEV_MASK "cryptodev_mask"
> > > > +#define CMD_LINE_OPT_TRANSFER_MODE "transfer-mode"
> > > > +#define CMD_LINE_OPT_SCHEDULE_TYPE "schedule-type"
> > > > +#define CMD_LINE_OPT_IPSEC_MODE "process-mode"
> > > > +#define CMD_LINE_OPT_IPSEC_DIR "process-dir"
> > > > #define CMD_LINE_OPT_RX_OFFLOAD "rxoffload"
> > > > #define CMD_LINE_OPT_TX_OFFLOAD "txoffload"
> > > > #define CMD_LINE_OPT_REASSEMBLE "reassemble"
> > > > #define CMD_LINE_OPT_MTU "mtu"
> > > > #define CMD_LINE_OPT_FRAG_TTL "frag-ttl"
> > > >
> > > > +#define CMD_LINE_ARG_APP "app"
> > > > +#define CMD_LINE_ARG_DRV "drv"
> > > > +#define CMD_LINE_ARG_INB "in"
> > > > +#define CMD_LINE_ARG_OUT "out"
> > > > +
> > > > enum {
> > > > /* long options mapped to a short option */
> > > >
> > > > @@ -149,7 +165,11 @@ enum {
> > > > CMD_LINE_OPT_CONFIG_NUM,
> > > > CMD_LINE_OPT_SINGLE_SA_NUM,
> > > > CMD_LINE_OPT_CRYPTODEV_MASK_NUM,
> > > > + CMD_LINE_OPT_TRANSFER_MODE_NUM,
> > > > + CMD_LINE_OPT_SCHEDULE_TYPE_NUM,
> > > > CMD_LINE_OPT_RX_OFFLOAD_NUM,
> > > > + CMD_LINE_OPT_IPSEC_MODE_NUM,
> > > > + CMD_LINE_OPT_IPSEC_DIR_NUM,
> > > > CMD_LINE_OPT_TX_OFFLOAD_NUM,
> > > > CMD_LINE_OPT_REASSEMBLE_NUM,
> > > > CMD_LINE_OPT_MTU_NUM,
> > > > @@ -160,6 +180,10 @@ static const struct option lgopts[] = {
> > > > {CMD_LINE_OPT_CONFIG, 1, 0, CMD_LINE_OPT_CONFIG_NUM},
> > > > {CMD_LINE_OPT_SINGLE_SA, 1, 0,
> > > CMD_LINE_OPT_SINGLE_SA_NUM},
> > > > {CMD_LINE_OPT_CRYPTODEV_MASK, 1, 0,
> > > > CMD_LINE_OPT_CRYPTODEV_MASK_NUM},
> > > > + {CMD_LINE_OPT_TRANSFER_MODE, 1, 0,
> > > CMD_LINE_OPT_TRANSFER_MODE_NUM},
> > > > + {CMD_LINE_OPT_SCHEDULE_TYPE, 1, 0,
> > > CMD_LINE_OPT_SCHEDULE_TYPE_NUM},
> > > > + {CMD_LINE_OPT_IPSEC_MODE, 1, 0,
> > > CMD_LINE_OPT_IPSEC_MODE_NUM},
> > > > + {CMD_LINE_OPT_IPSEC_DIR, 1, 0,
> > > CMD_LINE_OPT_IPSEC_DIR_NUM},
> > > > {CMD_LINE_OPT_RX_OFFLOAD, 1, 0,
> > > CMD_LINE_OPT_RX_OFFLOAD_NUM},
> > > > {CMD_LINE_OPT_TX_OFFLOAD, 1, 0,
> > > CMD_LINE_OPT_TX_OFFLOAD_NUM},
> > > > {CMD_LINE_OPT_REASSEMBLE, 1, 0,
> > > CMD_LINE_OPT_REASSEMBLE_NUM}, @@
> > > > -1094,8 +1118,8 @@ drain_outbound_crypto_queues(const struct
> > > > lcore_conf *qconf, }
> > > >
> > > > /* main processing loop */
> > > > -static int32_t
> > > > -main_loop(__attribute__((unused)) void *dummy)
> > > > +void
> > > > +ipsec_poll_mode_worker(void)
> > > > {
> > > > struct rte_mbuf *pkts[MAX_PKT_BURST];
> > > > uint32_t lcore_id;
> > > > @@ -1137,7 +1161,7 @@ main_loop(__attribute__((unused)) void
> > > *dummy)
> > > > if (qconf->nb_rx_queue == 0) {
> > > > RTE_LOG(DEBUG, IPSEC, "lcore %u has nothing to do\n",
> > > > lcore_id);
> > > > - return 0;
> > > > + return;
> > > > }
> > > >
> > > > RTE_LOG(INFO, IPSEC, "entering main loop on lcore %u\n",
> > > > lcore_id); @@ -1150,7 +1174,7 @@
> main_loop(__attribute__((unused))
> > > > void
> > > *dummy)
> > > > lcore_id, portid, queueid);
> > > > }
> > > >
> > > > - while (1) {
> > > > + while (!force_quit) {
> > > > cur_tsc = rte_rdtsc();
> > > >
> > > > /* TX queue buffer drain */
> > > > @@ -1277,6 +1301,10 @@ print_usage(const char *prgname)
> > > > " --config (port,queue,lcore)[,(port,queue,lcore)]"
> > > > " [--single-sa SAIDX]"
> > > > " [--cryptodev_mask MASK]"
> > > > + " [--transfer-mode MODE]"
> > > > + " [--schedule-type TYPE]"
> > > > + " [--process-mode MODE]"
> > > > + " [--process-dir DIR]"
> > > > " [--" CMD_LINE_OPT_RX_OFFLOAD " RX_OFFLOAD_MASK]"
> > > > " [--" CMD_LINE_OPT_TX_OFFLOAD " TX_OFFLOAD_MASK]"
> > > > " [--" CMD_LINE_OPT_REASSEMBLE "
> > > REASSEMBLE_TABLE_SIZE]"
> > > > @@ -1298,6 +1326,22 @@ print_usage(const char *prgname)
> > > > " bypassing the SP\n"
> > > > " --cryptodev_mask MASK: Hexadecimal bitmask of the
> > > crypto\n"
> > > > " devices to configure\n"
> > > > + " --transfer-mode MODE\n"
> > > > + " 0: Packet transfer via polling
> > > > (default)\n"
> > > > + " 1: Packet transfer via eventdev\n"
> > > > + " --schedule-type TYPE queue schedule type, used only
> > > when\n"
> > > > + " transfer mode is set to
> > > > eventdev\n"
> > > > + " 0: Ordered (default)\n"
> > > > + " 1: Atomic\n"
> > > > + " 2: Parallel\n"
> > >
> > > For last two, why not something huma-readable?
> > > I.E. == --tranfer-mode=(poll|event) or so.
> > > Same for schedule-type.
> >
> > [Anoob] Will do so in v2.
> >
> > >
> > > > + " --process-mode MODE processing mode, used only
> > > when\n"
> > > > + " transfer mode is set to
> > > > eventdev\n"
> > > > + " \"app\" : application mode (default)\n"
> > > > + " \"drv\" : driver mode\n"
> > > > + " --process-dir DIR processing direction, used only
> > > > when\n"
> > > > + " transfer mode is set to eventdev\n"
> > > > + " \"out\" : outbound (default)\n"
> > > > + " \"in\" : inbound\n"
> > >
> > > Hmm and why in eventdev mode it is not possible to handle both
> > > inbound and outbound traffic?
> > > Where is the limitation: eventdev framework/PMD/ipsec-secgw?
> >
> > [Anoob] It's not a limitation of any of the nodes. The current
> > ipsec-segcw has a data path check of port to determine whether inbound
> or outbound processing need to be done.
> > In case of poll-mode, we have specific cores polling fixed eth port &
> > queue. So the extra check involved doesn't cost much.
>
>
> > But in case of event-mode, we will have both inbound & outbound packets
> ending up on same core.
>
> For poll mode we can have one core handling several ports.
> Some of them could be inbound, other outbound, so it is a switch based on
> port value.
> My thought was that the same switch based on port_id can be done in
> event-mode too.
> But might be I am missing something here.
[Anoob] Yes. You are right. Even in poll mode the same bidirectional processing
on same core is possible.
>
> > So the penalty of running inbound &
> > outbound at the same time (and relying on data path check) is more in
> > case of event mode. For inline ipsec implementation, this impact isn't that
> much and we were able to minimize the perf degradation to 1%. I would
> expect lookaside crypto/protocol to have higher impacts.
> >
> > Said that, I'm okay with removing the extra option and retaining the
> > current behavior. If you think single instance of ipsec-secgw should work
> bidirectional, I can make the required changes and see the perf impact.
>
> I think it would be good if event-mode could work in bi-directional way (as
> poll mode does), but will leave final decision to you and other guys more
> familiar with event-dev details.
[Anoob] Agreed. I'll have this reworked to have one thread.
>
> >
> > >
> > > > " --" CMD_LINE_OPT_RX_OFFLOAD
> > > > ": bitmask of the RX HW offload capabilities to
> > > > enable/use\n"
> > > > " (DEV_RX_OFFLOAD_*)\n"
> > > > @@ -1433,7 +1477,89 @@ print_app_sa_prm(const struct app_sa_prm
> > > *prm)
> > > > }
> > > >
> > > > static int32_t
> > > > -parse_args(int32_t argc, char **argv)
> > > > +eh_parse_decimal(const char *str) {
> > > > + unsigned long num;
> > > > + char *end = NULL;
> > > > +
> > > > + num = strtoul(str, &end, 10);
> > > > + if ((str[0] == '\0') || (end == NULL) || (*end != '\0'))
> > > > + return -EINVAL;
> > > > +
> > > > + return num;
> > > > +}
> > >
> > > There already exists parse_decimal(), why to create a dup?
> >
> > [Anoob] Will this in v2.
> >
> > >
> > > > +
> > > > +static int
> > > > +parse_transfer_mode(struct eh_conf *conf, const char *optarg) {
> > > > + int32_t parsed_dec;
> > > > +
> > > > + parsed_dec = eh_parse_decimal(optarg);
> > > > + if (parsed_dec != EH_PKT_TRANSFER_MODE_POLL &&
> > > > + parsed_dec != EH_PKT_TRANSFER_MODE_EVENT) {
> > > > + printf("Unsupported packet transfer mode");
> > > > + return -EINVAL;
> > > > + }
> > > > + conf->mode = parsed_dec;
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +parse_schedule_type(struct eh_conf *conf, const char *optarg) {
> > > > + struct eventmode_conf *em_conf = NULL;
> > > > + int32_t parsed_dec;
> > > > +
> > > > + parsed_dec = eh_parse_decimal(optarg);
> > > > + if (parsed_dec != RTE_SCHED_TYPE_ORDERED &&
> > > > + parsed_dec != RTE_SCHED_TYPE_ATOMIC &&
> > > > + parsed_dec != RTE_SCHED_TYPE_PARALLEL)
> > > > + return -EINVAL;
> > > > +
> > > > + /* Get eventmode conf */
> > > > + em_conf = (struct eventmode_conf *)(conf->mode_params);
> > > > +
> > > > + em_conf->ext_params.sched_type = parsed_dec;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +parse_ipsec_mode(struct eh_conf *conf, const char *optarg) {
> > > > + if (!strncmp(CMD_LINE_ARG_APP, optarg,
> > > strlen(CMD_LINE_ARG_APP)) &&
> > > > + strlen(optarg) == strlen(CMD_LINE_ARG_APP))
> > >
> > > Ugh, that's an ugly construction, why not just:
> > > if (strcmp(CMD_LINE_ARG_APP, optarg) == 0) ?
> >
> > [Anoob] Will fix this in v2.
> >
> > >
> > > > + conf->ipsec_mode = EH_IPSEC_MODE_TYPE_APP;
> > > > + else if (!strncmp(CMD_LINE_ARG_DRV, optarg,
> > > strlen(CMD_LINE_ARG_DRV)) &&
> > > > + strlen(optarg) == strlen(CMD_LINE_ARG_DRV))
> > > > + conf->ipsec_mode = EH_IPSEC_MODE_TYPE_DRIVER;
> > > > + else {
> > > > + printf("Unsupported ipsec mode\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int
> > > > +parse_ipsec_dir(struct eh_conf *conf, const char *optarg) {
> > > > + if (!strncmp(CMD_LINE_ARG_INB, optarg,
> > > strlen(CMD_LINE_ARG_INB)) &&
> > > > + strlen(optarg) == strlen(CMD_LINE_ARG_INB))
> > > > + conf->ipsec_dir = EH_IPSEC_DIR_TYPE_INBOUND;
> > > > + else if (!strncmp(CMD_LINE_ARG_OUT, optarg,
> > > strlen(CMD_LINE_ARG_OUT)) &&
> > > > + strlen(optarg) == strlen(CMD_LINE_ARG_OUT))
> > > > + conf->ipsec_dir = EH_IPSEC_DIR_TYPE_OUTBOUND;
> > > > + else {
> > > > + printf("Unsupported ipsec direction\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +static int32_t
> > > > +parse_args(int32_t argc, char **argv, struct eh_conf *eh_conf)
> > > > {
> > > > int opt;
> > > > int64_t ret;
> > > > @@ -1536,6 +1662,43 @@ parse_args(int32_t argc, char **argv)
> > > > /* else */
> > > > enabled_cryptodev_mask = ret;
> > > > break;
> > > > +
> > > > + case CMD_LINE_OPT_TRANSFER_MODE_NUM:
> > > > + ret = parse_transfer_mode(eh_conf, optarg);
> > > > + if (ret < 0) {
> > > > + printf("Invalid packet transfer
> > > > mode\n");
> > > > + print_usage(prgname);
> > > > + return -1;
> > > > + }
> > > > + break;
> > > > +
> > > > + case CMD_LINE_OPT_SCHEDULE_TYPE_NUM:
> > > > + ret = parse_schedule_type(eh_conf, optarg);
> > > > + if (ret < 0) {
> > > > + printf("Invalid queue schedule type\n");
> > > > + print_usage(prgname);
> > > > + return -1;
> > > > + }
> > > > + break;
> > > > +
> > > > + case CMD_LINE_OPT_IPSEC_MODE_NUM:
> > > > + ret = parse_ipsec_mode(eh_conf, optarg);
> > > > + if (ret < 0) {
> > > > + printf("Invalid ipsec mode\n");
> > > > + print_usage(prgname);
> > > > + return -1;
> > > > + }
> > > > + break;
> > > > +
> > > > + case CMD_LINE_OPT_IPSEC_DIR_NUM:
> > > > + ret = parse_ipsec_dir(eh_conf, optarg);
> > > > + if (ret < 0) {
> > > > + printf("Invalid ipsec direction\n");
> > > > + print_usage(prgname);
> > > > + return -1;
> > > > + }
> > > > + break;
> > > > +
> > > > case CMD_LINE_OPT_RX_OFFLOAD_NUM:
> > > > ret = parse_mask(optarg, &dev_rx_offload);
> > > > if (ret != 0) {
> > > > @@ -2457,6 +2620,132 @@ create_default_ipsec_flow(uint16_t
> > > > port_id,
> > > uint64_t rx_offloads)
> > > > return ret;
> > > > }
> > > >
> > > > +static struct eh_conf *
> > > > +eh_conf_init(void)
> > > > +{
> > > > + struct eventmode_conf *em_conf = NULL;
> > > > + struct eh_conf *conf = NULL;
> > > > + unsigned int eth_core_id;
> > > > + uint32_t nb_bytes;
> > > > + void *mem = NULL;
> > > > +
> > > > + /* Allocate memory for config */
> > > > + conf = calloc(1, sizeof(struct eh_conf));
> > > > + if (conf == NULL) {
> > > > + printf("Failed to allocate memory for eventmode helper
> > > conf");
> > > > + goto err;
> > > > + }
> > > > +
> > > > + /* Set default conf */
> > > > +
> > > > + /* Packet transfer mode: poll */
> > > > + conf->mode = EH_PKT_TRANSFER_MODE_POLL;
> > > > + conf->ipsec_mode = EH_IPSEC_MODE_TYPE_APP;
> > > > + conf->ipsec_dir = EH_IPSEC_DIR_TYPE_OUTBOUND;
> > > > +
> > > > + /* Keep all ethernet ports enabled by default */
> > > > + conf->eth_portmask = -1;
> > > > +
> > > > + /* Allocate memory for event mode params */
> > > > + conf->mode_params =
> > > > + calloc(1, sizeof(struct eventmode_conf));
> > > > + if (conf->mode_params == NULL) {
> > > > + printf("Failed to allocate memory for event mode
> > > > params");
> > > > + goto err;
> > > > + }
> > > > +
> > > > + /* Get eventmode conf */
> > > > + em_conf = (struct eventmode_conf *)(conf->mode_params);
> > > > +
> > > > + /* Allocate and initialize bitmap for eth cores */
> > > > + nb_bytes = rte_bitmap_get_memory_footprint(RTE_MAX_LCORE);
> > > > + if (!nb_bytes) {
> > > > + printf("Failed to get bitmap footprint");
> > > > + goto err;
> > > > + }
> > > > +
> > > > + mem = rte_zmalloc("event-helper-ethcore-bitmap", nb_bytes,
> > > > + RTE_CACHE_LINE_SIZE);
> > > > + if (!mem) {
> > > > + printf("Failed to allocate memory for eth cores
> > > > bitmap\n");
> > > > + goto err;
> > > > + }
> > > > +
> > > > + em_conf->eth_core_mask = rte_bitmap_init(RTE_MAX_LCORE,
> > > mem, nb_bytes);
> > > > + if (!em_conf->eth_core_mask) {
> > > > + printf("Failed to initialize bitmap");
> > > > + goto err;
> > > > + }
> > > > +
> > > > + /* Schedule type: ordered */
> > > > + em_conf->ext_params.sched_type = RTE_SCHED_TYPE_ORDERED;
> > > > +
> > > > + /* Set two cores as eth cores for Rx & Tx */
> > > > +
> > > > + /* Use first core other than master core as Rx core */
> > > > + eth_core_id = rte_get_next_lcore(0, /* curr core */
> > > > + 1, /* skip master core */
> > > > + 0 /* wrap */);
> > > > +
> > > > + rte_bitmap_set(em_conf->eth_core_mask, eth_core_id);
> > > > +
> > > > + /* Use next core as Tx core */
> > > > + eth_core_id = rte_get_next_lcore(eth_core_id, /* curr core
> > > */
> > > > + 1, /* skip master
> > > > core */
> > > > + 0 /* wrap */);
> > > > +
> > > > + rte_bitmap_set(em_conf->eth_core_mask, eth_core_id);
> > > > +
> > > > + return conf;
> > > > +err:
> > > > + rte_free(mem);
> > > > + free(em_conf);
> > > > + free(conf);
> > > > + return NULL;
> > > > +}
> > > > +
> > > > +static void
> > > > +eh_conf_uninit(struct eh_conf *conf) {
> > > > + struct eventmode_conf *em_conf = NULL;
> > > > +
> > > > + /* Get eventmode conf */
> > > > + em_conf = (struct eventmode_conf *)(conf->mode_params);
> > > > +
> > > > + /* Free evenmode configuration memory */
> > > > + rte_free(em_conf->eth_core_mask);
> > > > + free(em_conf);
> > > > + free(conf);
> > > > +}
> > > > +
> > > > +static void
> > > > +signal_handler(int signum)
> > > > +{
> > > > + if (signum == SIGINT || signum == SIGTERM) {
> > > > + uint16_t port_id;
> > > > + printf("\n\nSignal %d received, preparing to exit...\n",
> > > > + signum);
> > > > + force_quit = true;
> > > > +
> > > > + /* Destroy the default ipsec flow */
> > > > + RTE_ETH_FOREACH_DEV(port_id) {
> > > > + if ((enabled_port_mask & (1 << port_id)) == 0)
> > > > + continue;
> > > > + if (flow_info_tbl[port_id].rx_def_flow) {
> > > > + struct rte_flow_error err;
> > > > + int ret;
> > >
> > > As we are going to call dev_stop(), etc. at force_quit below, is
> > > there any reason to call rte_flow_destroy() here?
> > > Just curious.
> >
> > [Anoob] dev_stop() should clear all the rte_flow entries. But doing it
> > from the app as a good citizen. 😊
> >
> > I can remove it since the same is not done for SA specific rte_flows created
> for inline crypto.
>
> No need to remove.
> My question was just stylish one:
> why not to do it at the same place where dev_stop()/dev_close() is done, to
> have everything in one place.
[Anoob] I misunderstood your query. Will have it moved close to dev_stop() etc.
>
> >
> > >
> > > > + ret = rte_flow_destroy(port_id,
> > > > +
> > > > flow_info_tbl[port_id].rx_def_flow,
> > > > + &err);
> > > > + if (ret)
> > > > + RTE_LOG(ERR, IPSEC,
> > > > + "Failed to destroy flow for
> > > > port %u, "
> > > > + "err msg: %s\n", port_id,
> > > err.message);
> > > > + }
> > > > + }
> > > > + }
> > > > +}
> > > > +
> > > > int32_t
> > > > main(int32_t argc, char **argv)
> > > > {
> > > > @@ -2466,6 +2755,7 @@ main(int32_t argc, char **argv)
> > > > uint8_t socket_id;
> > > > uint16_t portid;
> > > > uint64_t req_rx_offloads, req_tx_offloads;
> > > > + struct eh_conf *eh_conf = NULL;
> > > > size_t sess_sz;
> > > >
> > > > /* init EAL */
> > > > @@ -2475,8 +2765,17 @@ main(int32_t argc, char **argv)
> > > > argc -= ret;
> > > > argv += ret;
> > > >
> > > > + force_quit = false;
> > > > + signal(SIGINT, signal_handler);
> > > > + signal(SIGTERM, signal_handler);
> > > > +
> > > > + /* initialize event helper configuration */
> > > > + eh_conf = eh_conf_init();
> > > > + if (eh_conf == NULL)
> > > > + rte_exit(EXIT_FAILURE, "Failed to init event helper
> > > > config");
> > > > +
> > > > /* parse application arguments (after the EAL ones) */
> > > > - ret = parse_args(argc, argv);
> > > > + ret = parse_args(argc, argv, eh_conf);
> > > > if (ret < 0)
> > > > rte_exit(EXIT_FAILURE, "Invalid parameters\n");
> > > >
> > > > @@ -2592,12 +2891,43 @@ 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
> > > > + * helper sub-system.
> > > > + */
> > > > + eh_conf->eth_portmask = enabled_port_mask;
> > > > +
> > > > + /* Initialize eventmode components */
> > > > + ret = eh_devs_init(eh_conf);
> > > > + if (ret < 0)
> > > > + rte_exit(EXIT_FAILURE, "eh_devs_init failed, err=%d\n",
> > > ret);
> > > > +
> > > > /* launch per-lcore init on every lcore */
> > > > - rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
> > > > + rte_eal_mp_remote_launch(ipsec_launch_one_lcore, eh_conf,
> > > > +CALL_MASTER);
> > > > +
> > > > RTE_LCORE_FOREACH_SLAVE(lcore_id) {
> > > > if (rte_eal_wait_lcore(lcore_id) < 0)
> > > > return -1;
> > > > }
> > > >
> > > > + /* Uninitialize eventmode components */
> > > > + ret = eh_devs_uninit(eh_conf);
> > > > + if (ret < 0)
> > > > + rte_exit(EXIT_FAILURE, "eh_devs_uninit failed,
> > > > err=%d\n",
> > > ret);
> > > > +
> > > > + /* Free eventmode configuration memory */
> > > > + eh_conf_uninit(eh_conf);
> > > > +
> > > > + RTE_ETH_FOREACH_DEV(portid) {
> > > > + if ((enabled_port_mask & (1 << portid)) == 0)
> > > > + continue;
> > > > + printf("Closing port %d...", portid);
> > > > + rte_eth_dev_stop(portid);
> > > > + rte_eth_dev_close(portid);
> > > > + printf(" Done\n");
> > > > + }
> > > > + printf("Bye...\n");
> > > > +
> > > > return 0;
> > > > }
