[v3,0/5] app: add multi process crypto application

  Due to increasing interest in multi process support for crypto PMDs
new test app can be added so in overview we can say that:

The Multi-process Crypto application is a simple application that
allows to run crypto related operations in a multiple process environment. It
builds on the EAL primary/secondary process infrastructure.

v3:
- split into multiple patches
- refactored parts of code

Arek Kusztal (5):
  app: add muli process crypto application
  app/mp_crypto: add device configuration functions
  app/mp_crypto: add function to allocatie mempools
  app/mp_crypto: add enqueue-dequeue functions
  doc: add documentation for multi process crypto app

 app/Makefile                           |    1 +
 app/meson.build                        |    3 +-
 app/test-mp-crypto/Makefile            |   15 +
 app/test-mp-crypto/main.c              | 1141 ++++++++++++++++++++++++++++++++
 app/test-mp-crypto/meson.build         |    9 +
 app/test-mp-crypto/mp_crypto.c         |  136 ++++
 app/test-mp-crypto/mp_crypto.h         |  226 +++++++
 app/test-mp-crypto/mp_crypto_ipc.c     |   32 +
 app/test-mp-crypto/mp_crypto_parser.c  |  493 ++++++++++++++
 app/test-mp-crypto/mp_crypto_parser.h  |  148 +++++
 app/test-mp-crypto/mp_crypto_vectors.c |  174 +++++
 app/test-mp-crypto/mp_crypto_vectors.h |   66 ++
 doc/guides/tools/index.rst             |    1 +
 doc/guides/tools/mp_crypto.rst         |  151 +++++
 14 files changed, 2595 insertions(+), 1 deletion(-)
 create mode 100644 app/test-mp-crypto/Makefile
 create mode 100644 app/test-mp-crypto/main.c
 create mode 100644 app/test-mp-crypto/meson.build
 create mode 100644 app/test-mp-crypto/mp_crypto.c
 create mode 100644 app/test-mp-crypto/mp_crypto.h
 create mode 100644 app/test-mp-crypto/mp_crypto_ipc.c
 create mode 100644 app/test-mp-crypto/mp_crypto_parser.c
 create mode 100644 app/test-mp-crypto/mp_crypto_parser.h
 create mode 100644 app/test-mp-crypto/mp_crypto_vectors.c
 create mode 100644 app/test-mp-crypto/mp_crypto_vectors.h
 create mode 100644 doc/guides/tools/mp_crypto.rst
  

Hi Arek,
> 
> Due to increasing interest in multi process support for crypto PMDs
> new test app can be added so in overview we can say that:
> 
> The Multi-process Crypto application is a simple application that
> allows to run crypto related operations in a multiple process environment. It
> builds on the EAL primary/secondary process infrastructure.
> 
> v3:
> - split into multiple patches
> - refactored parts of code

Looking at the comments on this patchset, I believe the app may not
be ready by RC2 which falls at end of this week for crypto tree.
I have not started review of the application yet. 
I see it getting deferred for the next release if it is not added in RC3.

Regards,
Akhil
  

15/07/2020 20:26, Akhil Goyal:
> Hi Arek,
> > 
> > Due to increasing interest in multi process support for crypto PMDs
> > new test app can be added so in overview we can say that:
> > 
> > The Multi-process Crypto application is a simple application that
> > allows to run crypto related operations in a multiple process environment. It
> > builds on the EAL primary/secondary process infrastructure.
> > 
> > v3:
> > - split into multiple patches
> > - refactored parts of code
> 
> Looking at the comments on this patchset, I believe the app may not
> be ready by RC2 which falls at end of this week for crypto tree.
> I have not started review of the application yet. 
> I see it getting deferred for the next release if it is not added in RC3.

In general applications are approved by the technical board.
This app was not discussed at all.
And I think it will be rejected,
because I don't see the value of adding a test application just for
multi-process usage.
  

Hi Thomas,
> 
> 15/07/2020 20:26, Akhil Goyal:
> > Hi Arek,
> > >
> > > Due to increasing interest in multi process support for crypto PMDs
> > > new test app can be added so in overview we can say that:
> > >
> > > The Multi-process Crypto application is a simple application that
> > > allows to run crypto related operations in a multiple process environment. It
> > > builds on the EAL primary/secondary process infrastructure.
> > >
> > > v3:
> > > - split into multiple patches
> > > - refactored parts of code
> >
> > Looking at the comments on this patchset, I believe the app may not
> > be ready by RC2 which falls at end of this week for crypto tree.
> > I have not started review of the application yet.
> > I see it getting deferred for the next release if it is not added in RC3.
> 
> In general applications are approved by the technical board.
> This app was not discussed at all.
> And I think it will be rejected,
> because I don't see the value of adding a test application just for
> multi-process usage.
> 
Is this a recent change that a new application need to be discussed in techboard
first? I see that test-sad was added without discussion in the techboard.

I see this application as a useful tool to test the readiness of a driver to be used
in a multi process environment. If app is not a correct place to host it, should it be
added in examples/multi_process/. I also suggested that in v2 but it makes more 
sense in app as it is a unit test application which does not have any relevance as
standalone application as crypto may not be used standalone without ethernet
for multi process scenario.
My first preference was to modify l2fwd-crypto to be used as a multi process proof
Application but it also make sense to have a unit test application to verify standalone
crypto PMDs.
Open for comments from other crypto PMD owners.

Regards,
Akhil
  

15/07/2020 21:25, Akhil Goyal:
> Hi Thomas,
> > 
> > 15/07/2020 20:26, Akhil Goyal:
> > > Hi Arek,
> > > >
> > > > Due to increasing interest in multi process support for crypto PMDs
> > > > new test app can be added so in overview we can say that:
> > > >
> > > > The Multi-process Crypto application is a simple application that
> > > > allows to run crypto related operations in a multiple process environment. It
> > > > builds on the EAL primary/secondary process infrastructure.
> > > >
> > > > v3:
> > > > - split into multiple patches
> > > > - refactored parts of code
> > >
> > > Looking at the comments on this patchset, I believe the app may not
> > > be ready by RC2 which falls at end of this week for crypto tree.
> > > I have not started review of the application yet.
> > > I see it getting deferred for the next release if it is not added in RC3.
> > 
> > In general applications are approved by the technical board.
> > This app was not discussed at all.
> > And I think it will be rejected,
> > because I don't see the value of adding a test application just for
> > multi-process usage.
> > 
> Is this a recent change that a new application need to be discussed in techboard
> first? I see that test-sad was added without discussion in the techboard.

We started having such discussion this year (maybe a bit earlier),
because there are too many applications, and some examples were removed.

> I see this application as a useful tool to test the readiness of a driver to be used
> in a multi process environment. If app is not a correct place to host it, should it be
> added in examples/multi_process/. I also suggested that in v2 but it makes more 
> sense in app as it is a unit test application which does not have any relevance as
> standalone application as crypto may not be used standalone without ethernet
> for multi process scenario.
> My first preference was to modify l2fwd-crypto to be used as a multi process proof
> Application but it also make sense to have a unit test application to verify standalone
> crypto PMDs.
> Open for comments from other crypto PMD owners.

I agree it looks like unit tests.
Can it be added to app/test/test_cryptodev* ?
  

> > I see this application as a useful tool to test the readiness of a driver to be
> used
> > in a multi process environment. If app is not a correct place to host it, should it
> be
> > added in examples/multi_process/. I also suggested that in v2 but it makes
> more
> > sense in app as it is a unit test application which does not have any relevance
> as
> > standalone application as crypto may not be used standalone without ethernet
> > for multi process scenario.
> > My first preference was to modify l2fwd-crypto to be used as a multi process
> proof
> > Application but it also make sense to have a unit test application to verify
> standalone
> > crypto PMDs.
> > Open for comments from other crypto PMD owners.
> 
> I agree it looks like unit tests.
> Can it be added to app/test/test_cryptodev* ?
> 

Running two instances of test application will be a challenge I guess.
If it can be done, I think all the cases covered in test app other than crypto would
be affected/tested.

Test-crypto-perf can be a better option but it may defeat the purpose of test-crypto-perf.
Best would be to make l2fwd-crypto compliant with multi process.
But still I am ok to have a unit test application.
  

15/07/2020 22:15, Akhil Goyal:
> > > I see this application as a useful tool to test the readiness of a driver to be
> > used
> > > in a multi process environment. If app is not a correct place to host it, should it
> > be
> > > added in examples/multi_process/. I also suggested that in v2 but it makes
> > more
> > > sense in app as it is a unit test application which does not have any relevance
> > as
> > > standalone application as crypto may not be used standalone without ethernet
> > > for multi process scenario.
> > > My first preference was to modify l2fwd-crypto to be used as a multi process
> > proof
> > > Application but it also make sense to have a unit test application to verify
> > standalone
> > > crypto PMDs.
> > > Open for comments from other crypto PMD owners.
> > 
> > I agree it looks like unit tests.
> > Can it be added to app/test/test_cryptodev* ?
> > 
> 
> Running two instances of test application will be a challenge I guess.
> If it can be done, I think all the cases covered in test app other than crypto would
> be affected/tested.

Right

> Test-crypto-perf can be a better option but it may defeat the purpose of test-crypto-perf.
> Best would be to make l2fwd-crypto compliant with multi process.

Yes, probably a good idea.

> But still I am ok to have a unit test application.
  

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: środa, 15 lipca 2020 22:16
> To: Thomas Monjalon <thomas@monjalon.net>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>;
> techboard@dpdk.org; Anoob Joseph <anoobj@marvell.com>; Somalapuram,
> Amaranath <Amaranath.Somalapuram@amd.com>; Ankur Dwivedi
> <adwivedi@marvell.com>; ruifeng.wang@arm.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Nagadheeraj Rottela
> <rnagadheeraj@marvell.com>
> Subject: RE: [PATCH v3 0/5] app: add multi process crypto application
> 
> > > I see this application as a useful tool to test the readiness of a
> > > driver to be
> > used
> > > in a multi process environment. If app is not a correct place to
> > > host it, should it
> > be
> > > added in examples/multi_process/. I also suggested that in v2 but it
> > > makes
> > more
> > > sense in app as it is a unit test application which does not have
> > > any relevance
> > as
> > > standalone application as crypto may not be used standalone without
> > > ethernet for multi process scenario.
> > > My first preference was to modify l2fwd-crypto to be used as a multi
> > > process
> > proof
> > > Application but it also make sense to have a unit test application
> > > to verify
> > standalone
> > > crypto PMDs.
> > > Open for comments from other crypto PMD owners.
> >
> > I agree it looks like unit tests.
> > Can it be added to app/test/test_cryptodev* ?
> >
> 
> Running two instances of test application will be a challenge I guess.
> If it can be done, I think all the cases covered in test app other than crypto
> would be affected/tested.
> 
> Test-crypto-perf can be a better option but it may defeat the purpose of test-
> crypto-perf.
> Best would be to make l2fwd-crypto compliant with multi process.
> But still I am ok to have a unit test application.


[Arek] - Having this as a test would be much more handy as it does not need any additional infrastructure like traffic generators. Just only crypto accelerator and can work out of the box.

Just thinking primary-secondary testing was not getting enough attention by now, for example:
Until patch bus/pci: fix UIO resource access from secondary process  Apr 24 2020
dev->mem_resource[i].addr was not set -> so basically configuration from secondary was rather difficult without PCI BAR address. 

Plus having option to test queue pairs configured in different processes or checking enqueue/dequeue from different processes to one queue pair would be handy even for regression testing.

> 
>
  

Hi Akhil,

We have decided that we will not be upstreaming this patchset in this release (I have changed its status to 'deferred').

Regards,
Arek

> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: środa, 15 lipca 2020 22:16
> To: Thomas Monjalon <thomas@monjalon.net>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>
> Cc: dev@dpdk.org; Trahe, Fiona <fiona.trahe@intel.com>;
> techboard@dpdk.org; Anoob Joseph <anoobj@marvell.com>; Somalapuram,
> Amaranath <Amaranath.Somalapuram@amd.com>; Ankur Dwivedi
> <adwivedi@marvell.com>; ruifeng.wang@arm.com; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Nagadheeraj Rottela
> <rnagadheeraj@marvell.com>
> Subject: RE: [PATCH v3 0/5] app: add multi process crypto application
> 
> > > I see this application as a useful tool to test the readiness of a
> > > driver to be
> > used
> > > in a multi process environment. If app is not a correct place to
> > > host it, should it
> > be
> > > added in examples/multi_process/. I also suggested that in v2 but it
> > > makes
> > more
> > > sense in app as it is a unit test application which does not have
> > > any relevance
> > as
> > > standalone application as crypto may not be used standalone without
> > > ethernet for multi process scenario.
> > > My first preference was to modify l2fwd-crypto to be used as a multi
> > > process
> > proof
> > > Application but it also make sense to have a unit test application
> > > to verify
> > standalone
> > > crypto PMDs.
> > > Open for comments from other crypto PMD owners.
> >
> > I agree it looks like unit tests.
> > Can it be added to app/test/test_cryptodev* ?
> >
> 
> Running two instances of test application will be a challenge I guess.
> If it can be done, I think all the cases covered in test app other than crypto
> would be affected/tested.
> 
> Test-crypto-perf can be a better option but it may defeat the purpose of test-
> crypto-perf.
> Best would be to make l2fwd-crypto compliant with multi process.
> But still I am ok to have a unit test application.
> 
>