Hi Fan & Akhil, > -----Original Message----- > From: Zhang, Roy Fan <roy.fan.zh...@intel.com> > Sent: Friday, May 1, 2020 2:18 PM > > Hi Akhil, > > > -----Original Message----- > > From: dev <dev-boun...@dpdk.org> On Behalf Of Akhil Goyal > > Sent: Wednesday, April 22, 2020 2:44 PM > > To: Coyle, David <david.co...@intel.com>; Doherty, Declan > > <declan.dohe...@intel.com>; Thomas Monjalon > <tho...@monjalon.net>; > > Yigit, Ferruh <ferruh.yi...@intel.com>; Trahe, Fiona > > <fiona.tr...@intel.com> > > Cc: techbo...@dpdk.org; dev@dpdk.org; De Lara Guarch, Pablo > > <pablo.de.lara.gua...@intel.com>; Ryan, Brendan > > <brendan.r...@intel.com>; Hemant Agrawal > <hemant.agra...@nxp.com>; > > Anoob Joseph <ano...@marvell.com>; Ruifeng Wang > > <ruifeng.w...@arm.com>; Liron Himi <lir...@marvell.com>; Nagadheeraj > > Rottela <rnagadhee...@marvell.com>; Srikanth Jampala > > <jsrika...@marvell.com>; Gagandeep Singh <g.si...@nxp.com>; Jay > Zhou > > <jianjay.z...@huawei.com>; Ravi Kumar <ravi1.ku...@amd.com>; > > Richardson, Bruce <bruce.richard...@intel.com>; > > olivier.m...@6wind.com; honnappa.nagaraha...@arm.com; Stephen > > Hemminger <step...@networkplumber.org>; al...@mellanox.com > > Subject: Re: [dpdk-dev] [PATCH v3 0/4] add AESNI-MB rawdev for multi- > > function processing > ... > > Yes, it is preferred, but it should be a union to > > rte_crypto_sym_op/rte_crypto_asym_op. > > Crypto_op->type as RTE_CRYPTO_OP_TYPE_SECURITY and sess_type as > > RTE_CRYPTO_OP_SECURITY_SESSION The size of rte_crypto_op will remain > > as is and there will be no ABI breakage I guess. > > > [Fan: with this way the PMD will have to do rte_crypto_op.type check, and > then look into rte_security_op field, only when it find the security_op type > is > crypto_crc, it will process the security_op data. Would that being too many > reads and checking for a single op? Can we create a new API for rte_security > to process rte_security_ops for Crypto_CRC or future needs?] ...
[DC] If we were to add new enqueue/dequeue APIs to rte_security, then this may cause extra churn and extra paths of code in a customer's application. For the DOCSIS Crypto-CRC use-case which is currently supported by IPSecMB, only the AES-DOCSISBPI cipher algorithm is supported. For these Crypto-CRC ops, they would create rte_security sessions, attach these to rte_security_ops and enqueue/dequeue using the new APIs in rte_security. However, the customer may also be using the legacy DES-DOCSISBPI cipher algorithm for some subscribers, and this algorithm is not supported in the chained Crypto-CRC functionality in IPSecMB (and most likely never will be). So for these the customer would need to create cryptodev sessions, attach these to rte_crypto_ops and enqueue/ dequeue with the cryptodev enq/deq APIs. That is 2 different paths of code now in the application datapath, where some packets in a batch need to be enqueued through rte_security and some need to be enqueued through cryptodev. If rte_crypto_ops are always used and enqueued/dequeued through cryptodev, then the only thing that changes is the type of session that is created and either the security session or the cryptodev session gets attached to the crypto_op. Now, we could add support to rte_security for DES-DOCSISBPI too, but it would not be a combined operation with CRC - it would be a simple cipher operation going through rte_security. But that, to me, does not seem like a good use of rte_security. For DOCSIS Crypto-CRC, we may also want to take advantage of the rte_cryptodev_sym_cpu_crypto_process() API which was added to cryptodev recently to avoid the enqueue/dequeue overhead. A similar API would also then need to be added to rte_security. Taking all of the above into account, I feel keeping the normal cryptodev enqueue/dequeue would be best. Having said all that, we do need to consider performance in the PMD of the extra op type checks. Take aesni_mb PMD as an example. It would need to check rte_crypto_op->type and if it's not RTE_CRYPTO_OP_TYPE_SECURITY, then it can assume it's an RTE_CRYPTO_OP_TYPE_SYMMETRIC op and carry on as normal for existing symmetric operations. Security ops will need some extra parsing but this is new functionality. The impact on existing functionality of the extra checks would certainly need to be tested though, but as all the op data will be in the same cache line, I don't see any major impact. Akhil & Fan (& others), I would be interested to hear your feedback on this. Regards, David > > Regards, > Fan
