Hi Konstantin, Thanks for the review. Please see inline.
Thanks, Anoob > -----Original Message----- > From: Ananyev, Konstantin <konstantin.anan...@intel.com> > Sent: Wednesday, October 16, 2019 6:33 PM > To: Anoob Joseph <ano...@marvell.com>; Akhil Goyal > <akhil.go...@nxp.com>; Nicolau, Radu <radu.nico...@intel.com> > Cc: Thomas Monjalon <tho...@monjalon.net>; Jerin Jacob Kollanukkaran > <jer...@marvell.com>; Narayana Prasad Raju Athreya > <pathr...@marvell.com>; Lukas Bartosik <lbarto...@marvell.com>; > dev@dpdk.org > Subject: [EXT] RE: [dpdk-dev] [RFC PATCH 00/13] add eventmode to ipsec- > secgw > > External Email > > ---------------------------------------------------------------------- > > > > This series introduces event-mode additions to ipsec-secgw. This > > effort is based on the proposed changes for l2fwd-event and the > > additions in l3fwd for event support. > > > > With this series, ipsec-secgw would be able to run in eventmode. The > > worker thread (executing loop) would be receiving events and would be > > submitting it back to the eventdev after the processing. This way, > > multicore scaling and h/w assisted scheduling is achieved by making > > use of the eventdev capabilities. > > > > Since the underlying event device would be having varying > > capabilities, the worker thread could be drafted differently to maximize > performance. > > This series introduces usage of multiple worker threads, among which > > the one to be used will be determined by the operating conditions and > > the underlying device capabilities. > > > > For example, if an event device - eth device pair has Tx internal > > port, then application can do tx_adapter_enqueue() instead of regular > > event_enqueue(). So a thread making an assumption that the device pair > > has internal port will not be the right solution for another pair. The > > infrastructure added with these patches aims to help application to > > have multiple worker threads, there by extracting maximum performance > > from every device without affecting existing paths/use cases. > > > > The eventmode configuration is predefined. All packets reaching one > > eth port will hit one event queue. All event queues will be mapped to > > all event ports. So all cores will be able to receive traffic from all > > ports. > > When schedule_type is set as RTE_SCHED_TYPE_ORDERED/ATOMIC, event > > device will ensure the ordering. Ordering would be lost when tried in > PARALLEL. > > > > Following command line options are introduced, > > > > --transfer-mode: to choose between poll mode & event mode > > --schedule-type: to specify the scheduling type > > (RTE_SCHED_TYPE_ORDERED/ > > RTE_SCHED_TYPE_ATOMIC/ > > RTE_SCHED_TYPE_PARALLEL) > > --process-dir: outbound/inbound > > --process-mode: app mode /driver mode > > > > The two s/w config options added to ipsec-secgw can be used in > > benchmarking h/w performance, > > > > I didn't look at the actual code (yet), just cover letter, few quick questions > below. > > > 1. process-dir : states whether the direction is outbound/inbound. > > This option aims to avoid an unnecessary check of determining whether > > inbound/outbound processing need to be done on the packet. For each > > option a different light weight worker thread would be executed. > > Bur right now app can do both inbound and outbound simultaneously on the > same core. > I presume the default behavior will be preserved? [Anoob] The existing behavior with poll mode thread is not touched. The current changes are only applicable when launched in event-mode. > > > > > 2. process-mode: states whether the application has to run in driver > > mode or app mode. > > > > Driver-mode: This mode will have bare minimum changes in the application > > to support ipsec. There woudn't be any lookup etc done in > > the application. And for inline-protocol use case, the > > thread would resemble l2fwd as the ipsec processing would be > > done entirely in the h/w. This mode can be used to benchmark > > the raw performance of the h/w. All the application side > > steps (like lookup) can be redone based on the requirement > > of the end user. Hence the need for a mode which would > > report the raw performance. > > > > App-mode: This mode will have all the features currently implemented > with > > ipsec-secgw (non librte_ipsec mode). All the lookups etc > > would follow the existing methods and would report numbers > > that can be compared against regular ipsec-secgw benchmark > > numbers. > > > > Example commands to execute ipsec-secgw in various modes on > OCTEONTX2 > > platform, > > > > #Inbound driver mode > > ./ipsec-secgw -w 0002:02:00.0,nb_ipsec_in_sa=128 -w > > 0002:03:00.0,nb_ipsec_in_sa=128 -w 0002:04:00.0,nb_ipsec_in_sa=128 -w > > 0002:07:00.0,nb_ipsec_in_sa=128 -w 0002:0e:00.0 -w 0002:10:00.1 > > --log-level=8 -c 0x7 – -P -p 0xf --config > > "(0,0,0),(1,0,0),(2,0,0),(3,0,0)" -f dpdk_internal/100g_4.3.cfg > > --transfer-mode 1 --schedule-type 2 --process-mode 1 --process-dir 1 > > For all these parameters, I think better to use names (app/driver, etc.) > instead of raw numbers. [Anoob] Agreed. Will make this change when we submit the actual patches. > > > > > #Inbound app mode > > ./ipsec-secgw -w 0002:02:00.0,nb_ipsec_in_sa=128 -w > > 0002:03:00.0,nb_ipsec_in_sa=128 -w 0002:04:00.0,nb_ipsec_in_sa=128 -w > > 0002:07:00.0,nb_ipsec_in_sa=128 -w 0002:0e:00.0 -w 0002:10:00.1 > > --log-level=8 -c 0x3f – -P -p 0xf --config > > "(0,0,0),(1,0,0),(2,0,0),(3,0,0)" - f dpdk_internal/100g_4.3.cfg > > --transfer-mode 1 --schedule-type 2 --process-mode 0 --process-dir 1 > > > > #Outbound driver mode > > ./ipsec-secgw -w 0002:02:00.0 -w 0002:03:00.0 -w 0002:04:00.0 -w > > 0002:07:00.0 -w 0002:0e:00.0 -w 0002:10:00.1 --log-level=8 -c 0x1f – - > > P -p 0xf --config "(0,0,0),(1,0,0),(2,0,0),(3,0,0)" -f > > a-aes-gcm-new.cfg --transfer-mode 1 --schedule-type 2 --process-mode 1 > > --process-dir 0 > > > > #Outbound app mode > > ./ipsec-secgw -w 0002:02:00.0 -w 0002:03:00.0 -w 0002:04:00.0 -w > > 0002:07:00.0 -w 0002:0e:00.0 -w 0002:10:00.1 --log-level=8 -c 0x7f – - > > P -p 0xf --config "(0,0,0),(1,0,0),(2,0,0),(3,0,0)" -f > > a-aes-gcm-new.cfg --transfer-mode 1 --schedule-type 2 --process-mode 0 > > --process-dir 0 > > > > This series is targeted for next release (20.02). This series doesn't > > introduce any library change. > > By 'library change' you mean that this new event-mode will be supported > only by legacy code-path or ...? [Anoob] All the changes are confined to 'examples/ipsec-secgw' directory. Right now, the worker threads make use of the existing routines in non-librte_ipsec mode. > > >And the decision to add eventmode additions in ipsec-secgw was > >approved by the Tech Board. > > > > Following are missing in the RFC. Will add it when sending patches. > > 1. Documentation. > > 2. More cleanup is needed. There are options that are added so that future > > expansion is not hindered. Need inputs from the community if there is > use > > case for them. > > > > Following are planned features, > > 1. Add burst mode workers. > > 2. Add non tx internal port worker. > > 3. Verify support for Rx core (the support is added but lack of h/w to > verify). > > 4. Add lookaside protocol support. > > > > Following are features that Marvell won't be attempting. > > 1. Inline crypto support. > > 2. Lookaside crypto support. > > Ok so what mode is supported right now with this RFC? [Anoob] Inline protocol support is added with the RFC. > > > > > For the features that Marvell won't be attempting, new workers can be > > introduced by the respective stake holders. > > > > Anoob Joseph (13): > > examples/ipsec-secgw: add framework for eventmode helper > > examples/ipsec-secgw: add eventdev port-lcore link > > examples/ipsec-secgw: add Rx adapter support > > examples/ipsec-secgw: add Tx adapter support > > examples/ipsec-secgw: add routines to display config > > examples/ipsec-secgw: add routines to launch workers > > examples/ipsec-secgw: add support for internal ports > > examples/ipsec-secgw: add eventmode to ipsec-secgw > > examples/ipsec-secgw: add app inbound worker > > examples/ipsec-secgw: add app processing code > > examples/ipsec-secgw: add driver outbound worker > > examples/ipsec-secgw: add app outbound worker > > examples/ipsec-secgw: add cmd line option for bufs > > > > examples/ipsec-secgw/Makefile | 2 + > > examples/ipsec-secgw/event_helper.c | 1757 > > +++++++++++++++++++++++++++++++++++ > > examples/ipsec-secgw/event_helper.h | 334 +++++++ > > examples/ipsec-secgw/ipsec-secgw.c | 436 +++++++-- > > examples/ipsec-secgw/ipsec-secgw.h | 81 ++ > > examples/ipsec-secgw/ipsec.c | 4 + > > examples/ipsec-secgw/ipsec.h | 30 +- > > examples/ipsec-secgw/ipsec_worker.c | 766 +++++++++++++++ > > examples/ipsec-secgw/ipsec_worker.h | 39 + > > examples/ipsec-secgw/meson.build | 4 +- > > examples/ipsec-secgw/sa.c | 11 - > > 11 files changed, 3360 insertions(+), 104 deletions(-) create mode > > 100644 examples/ipsec-secgw/event_helper.c > > create mode 100644 examples/ipsec-secgw/event_helper.h > > create mode 100644 examples/ipsec-secgw/ipsec-secgw.h > > create mode 100644 examples/ipsec-secgw/ipsec_worker.c > > create mode 100644 examples/ipsec-secgw/ipsec_worker.h > > > > -- > > 2.7.4