Re: [dpdk-dev] [EXT] Re: [PATCH v2 00/15] add OCTEONTX2 inline IPsec support

[Anoob Joseph <anoobj@marvell.com>]

Hi Akhil,

Shall I send v3 with the commit header updates that Jerin suggested?

Thanks,
Anoob

> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, January 22, 2020 7:48 PM
> To: Akhil Goyal <akhil.goyal@nxp.com>
> Cc: Anoob Joseph <anoobj@marvell.com>; Declan Doherty
> <declan.doherty@intel.com>; Thomas Monjalon <thomas@monjalon.net>; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>; Narayana Prasad Raju Athreya
> <pathreya@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankumark@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavatula@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>;
> Archana Muniganti <marchana@marvell.com>; Tejasree Kondoj
> <ktejasree@marvell.com>; Vamsi Krishna Attunuru <vattunuru@marvell.com>;
> Lukas Bartosik <lbartosik@marvell.com>; dpdk-dev <dev@dpdk.org>
> Subject: [EXT] Re: [dpdk-dev] [PATCH v2 00/15] add OCTEONTX2 inline IPsec
> support
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Wed, Jan 22, 2020 at 6:26 PM Akhil Goyal <akhil.goyal@nxp.com> wrote:
> >
> > > > >
> > > > > Hi Jerin,
> > > > >
> > > > > Will  do the suggested change (RX/rx-> Rx & TX/tx->Tx). Do you
> > > > > want me to trim the headline as well?
> > > > >
> > > >
> > > > Hi Anoob,
> > > >
> > > > > @Akhil, did you get a chance to review the series? Do you have
> > > > > any comments on the patches?
> > > > >
> > > >
> > > > You are adding inline ipsec support to ethernet device and not a
> > > > crypto device.
> > > > These patches should not be part of crypto PMD. There will be
> > > > cyclic dependency Between ethernet device and crypto device which
> > > > can be easily avoided.
> > >
> > > [Anoob] We have plans to use lookaside protocol to handle the "fallback"
> > > session. And that involves session sharing between inline and
> > > lookaside protocol offloads. Also, though the feature is exposed as
> > > a feature of ethdev, on our platform, it's the crypto block which
> > > primarily implements the feature. And so, if the code is moved to
> > > ethdev dir, there would be lot of code duplication. The idea is to have all
> security related code in one place.
> > >
> > > Also, the PMDs don't have any calls to each other. The communication
> > > between the two happens via common. The crypto dev PMD will register
> > > the required security ops to a common structure and ethdev would get
> > > it from there. So there won't be an issue of build dependency.
> > >
> >
> > - The code that need to be duplicated can be moved to drivers/common/
> 
> I would like to keep the common code that is common to all the coprocessors.
> 
> Moreover, there are logistic issues in that case where
> a) drivers/common/octeontx2/ going through master repo. So we will be
> creating unnecessary dependency with that 'master' tree.
> b) crypto and ethdev work is done by different teams so we would like to make
> responsibly clear wrt the review and ownership.
> 
> I would like to keep the security/crypto-related code to driver/crypto and hook
> to driver/net/octeontx2 with required functionalities over the driver/common
> using a few function pointers to remove the cyclic build dependency.
> 
> Considering there is no cyclic build and shared library dependency now, Can we
> make forward progress with the existing scheme?
> 
> > - You may not need to include rte_ethdev.h inside drivers/crypto
> > - otx2_sec_eth_ctx_create should be part of ethdev and you would need
> > similar API for crypto device as well when you would support the fallback
> session support.
> > So that would go in crypto. Session creation code may be common and
> > can go in drivers/common.
> > - You would have separate security_ctx for both eth device and crypto
> > device and that should In net device and crypto device separately. Similarly
> security->ops should be different in both of them.
> > However if they may have same session creation code and that can go in
> common.
> >
> > > >
> > > >
> > > > > > >
> > > > > > > This series adds inline IPsec support in OCTEONTX2 PMD.
> > > > > > >
> > > > > > > In the inbound path, rte_flow framework need to be used to
> > > > > > > configure the NPC block, which does the h/w lookup. The
> > > > > > > packets would get processed by the crypto block and would
> > > > > > > submit to the scheduling block, SSO. So inline IPsec mode
> > > > > > > can be enabled only when traffic is received via event device using Rx
> adapter.
> > > > > > >
> > > > > > > In the outbound path, the core would submit to the crypto
> > > > > > > block and the crypto block would submit the packet for Tx internally.
> > > > > >
> > > > > >
> > > > > > Please fix following check-git-log.sh issues.
> > > > > >
> > > > > > Wrong headline lowercase:
> > > > > >         net/octeontx2: add inline ipsec rx path changes
> > > > > >         drivers/octeontx2: add sec in compiler optimized RX
> > > > > > fastpath
> > > > framework
> > > > > >         drivers/octeontx2: add sec in compiler optimized TX
> > > > > > fastpath
> > > > framework
> > > > > >         crypto/octeontx2: add inline tx path changes Headline too long:
> > > > > >         drivers/octeontx2: add sec in compiler optimized RX
> > > > > > fastpath
> > > > framework
> > > > > >         drivers/octeontx2: add sec in compiler optimized TX
> > > > > > fastpath
> > > > framework
> > > > > >         crypto/octeontx2: sync inline tag type cfg with Rx
> > > > > > adapter configuration
> > > > > >
> > > > > > Changing to Rx and Tx will fix most of the issues.
> > > > > >
> > > > > >
> > > > > >
> > > > > > > v2:
> > > > > > > * Minimized additions to common/octeontx2
> > > > > > > * Updated release notes
> > > > > > > * Renamed otx2_is_ethdev to otx2_ethdev_is_sec_capable
> > > > > > >
> > > > > > > Ankur Dwivedi (3):
> > > > > > >   crypto/octeontx2: add eth security capabilities
> > > > > > >   crypto/octeontx2: add datapath ops in eth security ctx
> > > > > > >   crypto/octeontx2: add inline tx path changes
> > > > > > >
> > > > > > > Anoob Joseph (4):
> > > > > > >   common/octeontx2: add CPT LF mbox for inline inbound
> > > > > > >   crypto/octeontx2: create eth security ctx
> > > > > > >   crypto/octeontx2: enable CPT to share QP with ethdev
> > > > > > >   crypto/octeontx2: add eth security session operations
> > > > > > >
> > > > > > > Archana Muniganti (3):
> > > > > > >   crypto/octeontx2: add lookup mem changes to hold sa indices
> > > > > > >   drivers/octeontx2: add sec in compiler optimized RX
> > > > > > > fastpath
> > > > framework
> > > > > > >   drivers/octeontx2: add sec in compiler optimized TX
> > > > > > > fastpath framework
> > > > > > >
> > > > > > > Tejasree Kondoj (3):
> > > > > > >   crypto/octeontx2: configure for inline IPsec
> > > > > > >   crypto/octeontx2: add security in eth dev configure
> > > > > > >   net/octeontx2: add inline ipsec rx path changes
> > > > > > >
> > > > > > > Vamsi Attunuru (2):
> > > > > > >   common/octeontx2: add routine to check if sec capable otx2
> > > > > > >   crypto/octeontx2: sync inline tag type cfg with Rx adapter
> > > > > > >     configuration
> > > > > > >
> > > > > > >  doc/guides/nics/octeontx2.rst                      |  20 +
> > > > > > >  doc/guides/rel_notes/release_20_02.rst             |   9 +
> > > > > > >  drivers/common/octeontx2/otx2_common.c             |  22 +
> > > > > > >  drivers/common/octeontx2/otx2_common.h             |  22 +
> > > > > > >  drivers/common/octeontx2/otx2_mbox.h               |   7 +
> > > > > > >  .../octeontx2/rte_common_octeontx2_version.map     |   3 +
> > > > > > >  drivers/crypto/octeontx2/Makefile                  |   7 +-
> > > > > > >  drivers/crypto/octeontx2/meson.build               |   7 +-
> > > > > > >  drivers/crypto/octeontx2/otx2_cryptodev.c          |   8 +
> > > > > > >  .../crypto/octeontx2/otx2_cryptodev_hw_access.h    |  22 +-
> > > > > > >  drivers/crypto/octeontx2/otx2_cryptodev_mbox.c     |  53 ++
> > > > > > >  drivers/crypto/octeontx2/otx2_cryptodev_mbox.h     |   7 +
> > > > > > >  drivers/crypto/octeontx2/otx2_cryptodev_ops.c      |  56 ++
> > > > > > >  drivers/crypto/octeontx2/otx2_cryptodev_qp.h       |  35 +
> > > > > > >  drivers/crypto/octeontx2/otx2_ipsec_fp.h           | 348 +++++++++
> > > > > > >  drivers/crypto/octeontx2/otx2_security.c           | 870
> > > > > > +++++++++++++++++++++
> > > > > > >  drivers/crypto/octeontx2/otx2_security.h           | 158 ++++
> > > > > > >  drivers/crypto/octeontx2/otx2_security_tx.h        | 175 +++++
> > > > > > >  drivers/event/octeontx2/Makefile                   |   1 +
> > > > > > >  drivers/event/octeontx2/meson.build                |   5 +-
> > > > > > >  drivers/event/octeontx2/otx2_evdev.c               | 170 ++--
> > > > > > >  drivers/event/octeontx2/otx2_evdev.h               |   4 +-
> > > > > > >  drivers/event/octeontx2/otx2_worker.c              |   6 +-
> > > > > > >  drivers/event/octeontx2/otx2_worker.h              |   6 +
> > > > > > >  drivers/event/octeontx2/otx2_worker_dual.c         |   6 +-
> > > > > > >  drivers/net/octeontx2/Makefile                     |   1 +
> > > > > > >  drivers/net/octeontx2/meson.build                  |   3 +
> > > > > > >  drivers/net/octeontx2/otx2_ethdev.c                |  46 +-
> > > > > > >  drivers/net/octeontx2/otx2_ethdev.h                |   2 +
> > > > > > >  drivers/net/octeontx2/otx2_ethdev_devargs.c        |  19 +
> > > > > > >  drivers/net/octeontx2/otx2_flow.c                  |  26 +
> > > > > > >  drivers/net/octeontx2/otx2_lookup.c                |  11 +-
> > > > > > >  drivers/net/octeontx2/otx2_rx.c                    |  27 +-
> > > > > > >  drivers/net/octeontx2/otx2_rx.h                    | 377 ++++++---
> > > > > > >  drivers/net/octeontx2/otx2_tx.c                    |  29 +-
> > > > > > >  drivers/net/octeontx2/otx2_tx.h                    | 271 +++++--
> > > > > > >  36 files changed, 2556 insertions(+), 283 deletions(-)
> > > > > > > create mode
> > > > > > > 100644 drivers/crypto/octeontx2/otx2_cryptodev_qp.h
> > > > > > >  create mode 100644 drivers/crypto/octeontx2/otx2_ipsec_fp.h
> > > > > > >  create mode 100644 drivers/crypto/octeontx2/otx2_security.c
> > > > > > >  create mode 100644 drivers/crypto/octeontx2/otx2_security.h
> > > > > > >  create mode 100644
> > > > > > > drivers/crypto/octeontx2/otx2_security_tx.h
> > > > > > >
> > > > > > > --
> > > > > > > 2.7.4
> > > > > > >