RE: [PATCH 0/1] Add security perf application

[Anoob Joseph <anoobj@marvell.com>]

Hi Akhil,

Please see inline.

Thanks,
Anoob

> 
> Hi Anoob,
> > Subject: [PATCH 0/1] Add security perf application
> >
> > Add performance application to test security session create & destroy
> > rates supported by the security enabled cryptodev PMD. The application
> > would create specified number of sessions and captures the time taken
> > for the same before proceeding to destroy of the same. When operating
> > on multi-core, the number of sessions would be evenly distributed
> > across all cores.
> >
> > The application would test with all combinations of cipher & auth
> > algorithms supported by the PMD.
> >
> > The app is similar to 'test-flow-perf' tool which captures the rate at
> > which flow rules can be created and destroyed.
> >
> Is it not good to add this into dpdk-test-crypto-perf?

[Anoob] IMO, It is not good. Following are the reasons,

Dpdk-test-crypto-perf is primarily for capturing crypto operation throughputs. And so the framework allocates minimal number of sessions and the datapath function pointer etc deals with only one session. The entire framework available in that application is for populating crypto_op and mbuf, which is not required for this app. Touching that framework would mean throughput tests would get affected, which I don't think is the right thing to do. And for PMDs like Intel's (which don't have security support), it would be an unnecessary performance drop.

The proposed app currently runs for all supported ciphers while in dpdk-test-crypto-perf, it runs only for a specific algorithm combination. If we want to limit the functionality of the proposed app to match dpdk-test-crypto-perf usage, that also calls for a major rework.

And the only thing that can be reused is probably cryptodev init & queue pair configuration. As you are well aware, security device can be cryptodev or an ethdev. Dpdk-test-crypto-perf doesn't have support for initializing ethdev and rightfully so. Adding this to an already complicated framework will be counter productive in the long run.

> Can we add as a separate .c file, say, cperf_test_sec_session.c in test-crypto-
> perf folder and use the existing framework.

[Anoob] As I mentioned earlier, nothing from the framework can be leveraged for this application. If you insist on not having a new app, then all this can be integrated into dpdk-test-crypto-perf, but that will follow it's own path from very early stage (mempool allocations etc need to happen differently). And it would mean adding more command line options (which is currently at 37) as we add more options for measuring security perf.

> This way we can leverage it for crypto sessions also.
> 
> 
> > Anoob Joseph (1):
> >   app/test-security-perf: add security perf app
> >
> >  MAINTAINERS                                 |   6 +
> >  app/meson.build                             |   1 +
> >  app/test-security-perf/meson.build          |  14 +
> >  app/test-security-perf/test_security_perf.c | 554
> ++++++++++++++++++++
> >  doc/guides/tools/index.rst                  |   1 +
> >  doc/guides/tools/securityperf.rst           |  47 ++
> >  6 files changed, 623 insertions(+)
> >  create mode 100644 app/test-security-perf/meson.build
> >  create mode 100644 app/test-security-perf/test_security_perf.c
> >  create mode 100644 doc/guides/tools/securityperf.rst
> >
> > --
> > 2.25.1