Re: [dpdk-dev] [PATCH v5 00/15] add eventmode to ipsec-secgw

[Akhil Goyal <akhil.goyal@nxp.com>]

Hi Anoob,

Please send a release note update as a reply to this mail. I will update it while merging the patchset.

Regards,
Akhil
> 
> Hi Akhil,
> 
> Reminder.
> 
> Do you have any further review comments?
> 
> Thanks,
> Anoob
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Sent: Tuesday, March 3, 2020 11:30 PM
> > To: Lukas Bartosik <lbartosik@marvell.com>; Akhil Goyal
> > <akhil.goyal@nxp.com>; Nicolau, Radu <radu.nicolau@intel.com>; Thomas
> > Monjalon <thomas@monjalon.net>
> > Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Narayana Prasad Raju
> > Athreya <pathreya@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>;
> > Anoob Joseph <anoobj@marvell.com>; Archana Muniganti
> > <marchana@marvell.com>; Tejasree Kondoj <ktejasree@marvell.com>; Vamsi
> > Krishna Attunuru <vattunuru@marvell.com>; dev@dpdk.org
> > Subject: [EXT] RE: [PATCH v5 00/15] add eventmode to ipsec-secgw
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> >
> > >
> > > This series introduces event-mode additions to ipsec-secgw.
> > >
> > > 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
> > > --event-schedule-type: to specify the scheduling type
> > >                  (RTE_SCHED_TYPE_ORDERED/
> > >                   RTE_SCHED_TYPE_ATOMIC/
> > >                   RTE_SCHED_TYPE_PARALLEL)
> > >
> > > Additionally the event mode introduces two modes of processing packets:
> > >
> > > 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.
> > >
> > > The driver mode is selected with existing --single-sa option (used
> > > also by poll mode). When --single-sa option is used in conjution with
> > > event mode then index passed to --single-sa is ignored.
> > >
> > > Example commands to execute ipsec-secgw in various modes on OCTEON
> TX2
> > > platform,
> > >
> > > #Inbound and outbound 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 -f aes-gcm.cfg
> > > --transfer-mode event --event-schedule-type parallel
> > >
> > > #Inbound and outbound driver 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 -f aes-gcm.cfg
> > > --transfer-mode event --event-schedule-type parallel --single-sa 0
> > >
> > > This series adds non burst tx internal port workers only. It provides
> > > infrastructure for non internal port workers, however does not define
> > > any. Also, only inline ipsec protocol mode is supported by the worker threads
> > added.
> > >
> > > Following are planned features,
> > > 1. Add burst mode workers.
> > > 2. Add non internal port workers.
> > > 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.
> > >
> > > For the features that Marvell won't be attempting, new workers can be
> > > introduced by the respective stake holders.
> > >
> > > This series is tested on Marvell OCTEON TX2.
> > > This series is targeted for 20.05 release.
> > >
> > > Changes in v5:
> > > * Rename function check_params() to check_poll_mode_params() and
> > >   check_eh_conf() to check_event_mode_params() in order to make it clear
> > >   what is their purpose.
> > > * Forbid usage of --config option in event mode.
> > > * Replace magic numbers on return with enum values in
> > process_ipsec_ev_inbound()
> > >   and process_ipsec_ev_outbound() functions.
> > > * Add session_priv_pool for both inbound and outbound configuration in
> > >   ipsec_wrkr_non_burst_int_port_app_mode worker.
> > > * Add check of event type in ipsec_wrkr_non_burst_int_port_app_mode
> > worker.
> > > * Update description of --config option in both ipsec-secgw help and
> > documentation.
> > >
> > > Changes in v4:
> > > * Update ipsec-secgw documentation to describe the new options as well as
> > >   event mode support.
> > > * In event mode reserve number of crypto queues equal to number of eth
> > ports
> > >   in order to meet inline protocol offload requirements.
> > > * Add calculate_nb_mbufs() function to calculate number of mbufs in a pool
> > >   and include fragments table size into the calculation.
> > > * Move structures ipsec_xf and ipsec_sad to ipsec.h and remove static
> > keyword
> > >   from sa_out, nb_sa_out, sa_in and nb_sa_in in sa.c.
> > > * Update process_ipsec_ev_inbound(), process_ipsec_ev_outbound(),
> > check_sp()
> > >   and prepare_out_sessions_tbl() functions as a result of changes introduced
> > >   by SAD feature.
> > > * Remove setting sa->cdev_id_qp to 0 in create_inline_session as sa_ctx
> > >   is created with rte_zmalloc.
> > > * Minor cleanup enhancements:
> > > - In eh_set_default_conf_eventdev() function in event_helper.c put
> definition
> > >   of int local vars in one line, remove invalid comment, put
> > >   "eventdev_config->ev_queue_mode =
> RTE_EVENT_QUEUE_CFG_ALL_TYPES"
> > in one line
> > >   instead of two.
> > > - Remove extern "C" from event_helper.h.
> > > - Put local vars in reverse xmas tree order in eh_dev_has_rx_internal_port()
> > and
> > >   eh_dev_has_tx_internal_port() functions in event_helper.c.
> > > - Put #include <rte_bitmap.h> in alphabetical order in ipsec-secgw.c.
> > > - Move "extern volatile bool force_quit" and "#include <stdbool.h>" to ipsec-
> > secgw.h,
> > >   remove #include <rte_hash.h>.
> > > - Remove not needed includes in ipsec_worker.c.
> > > - Remove expired todo from ipsec_worker.h.
> > >
> > > Changes in v3:
> > > * Move eh_conf_init() and eh_conf_uninit() functions to event_helper.c
> > >   including minor rework.
> > > * Rename --schedule-type option to --event-schedule-type.
> > > * Replace macro UNPROTECTED_PORT with static inline function
> > >   is_unprotected_port().
> > > * Move definitions of global variables used by multiple modules
> > >   to .c files and add externs in .h headers.
> > > * Add eh_check_conf() which validates ipsec-secgw configuration
> > >   for event mode.
> > > * Add dynamic calculation of number of buffers in a pool based
> > >   on number of cores, ports and crypto queues.
> > > * Fix segmentation fault in event mode driver worker which happens
> > >   when there are no inline outbound sessions configured.
> > > * Remove change related to updating number of crypto queues
> > >   in cryptodevs_init(). The update of crypto queues will be handled
> > >   in a separate patch.
> > > * Fix compilation error on 32-bit platforms by using userdata instead
> > >   of udata64 from rte_mbuf.
> > >
> > > Changes in v2:
> > > * Remove --process-dir option. Instead use existing unprotected port mask
> > >   option (-u) to decide wheter port handles inbound or outbound traffic.
> > > * Remove --process-mode option. Instead use existing --single-sa option
> > >   to select between app and driver modes.
> > > * Add handling of PKT_RX_SEC_OFFLOAD_FAIL result in app worker thread.
> > > * Fix passing of req_rx_offload flags to create_default_ipsec_flow().
> > > * Move destruction of flows to a location where eth ports are stopped
> > >   and closed.
> > > * Print error and exit when event mode --schedule-type option is used
> > >   in poll mode.
> > > * Reduce number of goto statements replacing them with loop constructs.
> > > * Remove sec_session_fixed table and replace it with locally build
> > >   table in driver worker thread. Table is indexed by port identifier
> > >   and holds first inline session pointer found for a given port.
> > > * Print error and exit when sessions other than inline are configured
> > >   in event mode.
> > > * When number of event queues is less than number of eth ports then
> > >   map all eth ports to one event queue.
> > > * Cleanup and minor improvements in code as suggested by Konstantin
> > >
> > > Ankur Dwivedi (1):
> > >   examples/ipsec-secgw: add default rte flow for inline Rx
> > >
> > > Anoob Joseph (5):
> > >   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
> > >
> > > Lukasz Bartosik (9):
> > >   examples/ipsec-secgw: add routines to launch workers
> > >   examples/ipsec-secgw: add support for internal ports
> > >   examples/ipsec-secgw: add event helper config init/uninit
> > >   examples/ipsec-secgw: add eventmode to ipsec-secgw
> > >   examples/ipsec-secgw: add driver mode worker
> > >   examples/ipsec-secgw: add app mode worker
> > >   examples/ipsec-secgw: make number of buffers dynamic
> > >   doc: add event mode support to ipsec-secgw
> > >   examples/ipsec-secgw: reserve crypto queues in event mode
> > >
> > >  doc/guides/sample_app_ug/ipsec_secgw.rst |  135 ++-
> > >  examples/ipsec-secgw/Makefile            |    2 +
> > >  examples/ipsec-secgw/event_helper.c      | 1812
> > ++++++++++++++++++++++++++++++
> > >  examples/ipsec-secgw/event_helper.h      |  327 ++++++
> > >  examples/ipsec-secgw/ipsec-secgw.c       |  506 +++++++--
> > >  examples/ipsec-secgw/ipsec-secgw.h       |   88 ++
> > >  examples/ipsec-secgw/ipsec.c             |    5 +-
> > >  examples/ipsec-secgw/ipsec.h             |   53 +-
> > >  examples/ipsec-secgw/ipsec_worker.c      |  649 +++++++++++
> > >  examples/ipsec-secgw/ipsec_worker.h      |   41 +
> > >  examples/ipsec-secgw/meson.build         |    6 +-
> > >  examples/ipsec-secgw/sa.c                |   21 +-
> > >  examples/ipsec-secgw/sad.h               |    5 -
> > >  13 files changed, 3516 insertions(+), 134 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
> > >
> > > --
> >
> > Have to say I didn't look extensively on event mode.
> > My primary concern was poll-mode and common code changes.
> > From that perspective - LGTM.
> >
> > Series Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> >
> > > 2.7.4