> Hi Akhil, > > +Techboard for guidance > The proposal is accepted in techboard.
Please fix compilation issues reported in CI. > > I agree that common part would be init only but it can scale for > > non-security > > sessions easily. > > Currently, dpdk-test-crypto-perf has a data path framework which prepares > crypto_operations based on session. The proposed application is about > measuring performance when creating and destroying sessions. So it would take > rte_security_conf as the argument and current data path framework would be > completely bypassed. Current dpdk-test-crypto-perf creates mbuf_pool etc and > creates one session per core. This app would need larger pool for sessions and > no pool for crypto_op or mbuf. Moreover, dpdk-test-crypto-perf works on > cryptodev while security-perf can work on rte_ethdevs as well. I still do not > see > any community feedback on whether plugging rte_ethdev init etc in dpdk-test- > crypto-perf is the right thing to do. > > So other than basic eal_init(), I do not see anything common and even in the > long run, this gulf is bound to grow. If the app has to be integrated into > dpdk- > test-crypto-perf, then it will be separate .c & .h files and completely > branch out > after very early init phase. The testing methodology and philosophy would also > be different (for security-perf, we are running all algos supported as there > is no > need for command line parsing of all algos. CL parsing would be added for > protocol features like custom AR window size). DPDK community had earlier > encountered same issue with "test-flow-perf" which could have been integrated > into "test-pmd" in a similar manner. But DPDK community decided to allow > "test-flow-perf" and so the same logic can be applied here as well. > > Thanks, > Anoob > > > -----Original Message----- > > From: Akhil Goyal <gak...@marvell.com> > > Sent: Wednesday, September 28, 2022 12:47 AM > > To: Anoob Joseph <ano...@marvell.com> > > Cc: Aakash Sasidharan <asasidha...@marvell.com>; dev@dpdk.org; > > techbo...@dpdk.org; Jerin Jacob Kollanukkaran <jer...@marvell.com>; > > Thomas Monjalon <tho...@monjalon.net>; Hemant Agrawal > > <hemant.agra...@nxp.com>; Sachin Saxena > > <sachin.sax...@oss.nxp.com>; Ciara Power <ciara.po...@intel.com> > > Subject: RE: [PATCH 0/1] Add security perf application > > > > Hi Anoob, > > > Hi Akhil, > > > > > > Do you have any further comments? > > > > > > 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. > > > > > > Are you planning to add more options is that app? > > if not, then adding just one more option about nb_sess would do trick in > > test-crypto-perf. > > You would just need to add 2 new functions (test_security_session_perf and > > sec_conf_init) in a new .c file in app/test-crypto-perf/ and the > > mempool_init > > is being called from > > cperf_initialize_cryptodev() which we can hook to get the nb_sessions from > > the command line arguments. > > I do not suspect any changes in datapath - so it won't be an issue. > > The point is not about the things being common in the two apps. The point is > > whether we can accommodate in existing app or not. We cannot have too > > many different apps. > > We only introduce apps which are not possible to accommodate in existing > > ones. > > I remember, there was discussion in past about having a new app for testing > > multi-process for crypto. > > But that was dropped as we do not want too many apps. > > I agree that common part would be init only but it can scale for > > non-security > > sessions easily. > > > > Regards, > > Akhil
