[dpdk-dev] [PATCH v2 0/5] cryptodev: hide internal structures

[Zhang, Roy Fan]

Hi Kai and Akhil,

Thanks Kai for finding the problem. 

To resolve the seg fault for multi-process and keep the structure private,
Instead  of creating the fp_ops we may ditch making
rte_cryptodev_enqueue_burst() and  rte_cryptodev_dequeue_burst() 
inline, and keep everything else as it was? To me these inlines looks not
very useful on performance wise anyway.

Regards,
Fan

> -----Original Message-----
> From: Ji, Kai <kai.ji at intel.com>
> Sent: Monday, October 11, 2021 6:07 PM
> To: Zhang, Roy Fan <roy.fan.zhang at intel.com>; Akhil Goyal
> <gakhil at marvell.com>; dev at dpdk.org
> Cc: thomas at monjalon.net; david.marchand at redhat.com;
> hemant.agrawal at nxp.com; anoobj at marvell.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>; Trahe, Fiona <fiona.trahe at intel.com>;
> Doherty, Declan <declan.doherty at intel.com>; matan at nvidia.com;
> g.singh at nxp.com; jianjay.zhou at huawei.com; asomalap at amd.com;
> ruifeng.wang at arm.com; Ananyev, Konstantin
> <konstantin.ananyev at intel.com>; Nicolau, Radu <radu.nicolau at intel.com>;
> ajit.khaparde at broadcom.com; rnagadheeraj at marvell.com;
> adwivedi at marvell.com; Power, Ciara <ciara.power at intel.com>
> Subject: RE: [PATCH v2 0/5] cryptodev: hide internal structures
> 
> Hi Akhil,
> 
> Just ran a quick mutli process test against the patch set, unfortunately it
> failed on the secondary process enqueue or dequeue.
> 
> USER1: Configuring vector 0, using session 0
> USER1: Start enqueuing packets on dev 0 qp 0
> USER1: Start dequeuing packets on dev 0 qp 0
> USER1: Enqueuing - Dequeueing -Segmentation fault (core dumped)
> 
> It will happen on any PMD type.
> 
> Regards
> 
> Kai
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces at dpdk.org> On Behalf Of Zhang, Roy Fan
> > Sent: Monday, October 11, 2021 5:03 PM
> > To: Akhil Goyal <gakhil at marvell.com>; dev at dpdk.org
> > Cc: thomas at monjalon.net; david.marchand at redhat.com;
> > hemant.agrawal at nxp.com; anoobj at marvell.com; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch at intel.com>; Trahe, Fiona <fiona.trahe at intel.com>;
> > Doherty, Declan <declan.doherty at intel.com>; matan at nvidia.com;
> > g.singh at nxp.com; jianjay.zhou at huawei.com; asomalap at amd.com;
> > ruifeng.wang at arm.com; Ananyev, Konstantin
> > <konstantin.ananyev at intel.com>; Nicolau, Radu
> <radu.nicolau at intel.com>;
> > ajit.khaparde at broadcom.com; rnagadheeraj at marvell.com;
> > adwivedi at marvell.com; Power, Ciara <ciara.power at intel.com>
> > Subject: Re: [dpdk-dev] [PATCH v2 0/5] cryptodev: hide internal structures
> >
> > Hi Akhil,
> >
> > The approach looks great but we may have to check if it works in multi-
> > process environment - since all enqueue/dequeue handlers are set by
> > primary process the secondary process may not recognize the fp_ops data.
> >
> > We will run a quick test to see if it is true.
> >
> > Regards,
> > Fan
> >
> > > -----Original Message-----
> > > From: Akhil Goyal <gakhil at marvell.com>
> > > Sent: Monday, October 11, 2021 1:43 PM
> > > To: dev at dpdk.org
> > > Cc: thomas at monjalon.net; david.marchand at redhat.com;
> > > hemant.agrawal at nxp.com; anoobj at marvell.com; De Lara Guarch, Pablo
> > > <pablo.de.lara.guarch at intel.com>; Trahe, Fiona
> > > <fiona.trahe at intel.com>; Doherty, Declan <declan.doherty at intel.com>;
> > > matan at nvidia.com; g.singh at nxp.com; Zhang, Roy Fan
> > > <roy.fan.zhang at intel.com>; jianjay.zhou at huawei.com;
> > asomalap at amd.com;
> > > ruifeng.wang at arm.com; Ananyev, Konstantin
> > > <konstantin.ananyev at intel.com>; Nicolau, Radu
> > > <radu.nicolau at intel.com>; ajit.khaparde at broadcom.com;
> > > rnagadheeraj at marvell.com; adwivedi at marvell.com; Power, Ciara
> > > <ciara.power at intel.com>; Akhil Goyal <gakhil at marvell.com>
> > > Subject: [PATCH v2 0/5] cryptodev: hide internal structures
> > >
> > > Structures rte_cryptodev and rte_cryptodev_data are not supposed to
> be
> > > directly used by the application. These are made public as they are
> > > used by inline datapath public APIs.
> > > This patchset, creates a new rte_cryptodev_core.h file which helps in
> > > defining a data structure to hold datapath APIs in a flat array based
> > > on the device identifier which is filled by the PMD.
> > >
> > > Similar series for ethdev and eventdev are also floated on ML.
> > > https://patchwork.dpdk.org/project/dpdk/list/?series=19428
> > > https://patchwork.dpdk.org/project/dpdk/list/?series=19405
> > >
> > > changes in v2: align with the latest versions of above series.
> > >
> > > Akhil Goyal (5):
> > >   cryptodev: separate out internal structures
> > >   cryptodev: allocate max space for internal qp array
> > >   cryptodev: move inline APIs into separate structure
> > >   cryptodev: update fast path APIs to use new flat array
> > >   cryptodev: move device specific structures
> > >
> > >  drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c    |   1 -
> > >  drivers/crypto/ccp/ccp_dev.h                  |   2 +-
> > >  drivers/crypto/cnxk/cn10k_ipsec.c             |   2 +-
> > >  drivers/crypto/cnxk/cn9k_ipsec.c              |   2 +-
> > >  .../crypto/cnxk/cnxk_cryptodev_capabilities.c |   2 +-
> > >  drivers/crypto/cnxk/cnxk_cryptodev_sec.c      |   2 +-
> > >  drivers/crypto/nitrox/nitrox_sym_reqmgr.c     |   2 +-
> > >  drivers/crypto/octeontx/otx_cryptodev.c       |   1 -
> > >  .../crypto/octeontx/otx_cryptodev_hw_access.c |   2 +-
> > >  .../crypto/octeontx/otx_cryptodev_hw_access.h |   2 +-
> > >  drivers/crypto/octeontx/otx_cryptodev_ops.h   |   2 +-
> > >  .../crypto/octeontx2/otx2_cryptodev_mbox.c    |   2 +-
> > >  drivers/crypto/scheduler/scheduler_failover.c |   2 +-
> > >  .../crypto/scheduler/scheduler_multicore.c    |   2 +-
> > >  .../scheduler/scheduler_pkt_size_distr.c      |   2 +-
> > >  .../crypto/scheduler/scheduler_roundrobin.c   |   2 +-
> > >  drivers/event/cnxk/cnxk_eventdev.h            |   2 +-
> > >  drivers/event/dpaa/dpaa_eventdev.c            |   2 +-
> > >  drivers/event/dpaa2/dpaa2_eventdev.c          |   2 +-
> > >  drivers/event/octeontx/ssovf_evdev.c          |   2 +-
> > >  .../event/octeontx2/otx2_evdev_crypto_adptr.c |   2 +-
> > >  lib/cryptodev/cryptodev_pmd.c                 |  51 +++
> > >  lib/cryptodev/cryptodev_pmd.h                 |  82 +++-
> > >  lib/cryptodev/meson.build                     |   4 +-
> > >  lib/cryptodev/rte_cryptodev.c                 |  50 ++-
> > >  lib/cryptodev/rte_cryptodev.h                 | 367 +++++++-----------
> > >  lib/cryptodev/rte_cryptodev_core.h            |  62 +++
> > >  lib/cryptodev/version.map                     |   7 +-
> > >  28 files changed, 398 insertions(+), 265 deletions(-)  create mode
> > > 100644 lib/cryptodev/rte_cryptodev_core.h
> > >
> > > --
> > > 2.25.1