Hi Akhil, Any other suggestions regarding the series?
BR, Suanming > -----Original Message----- > From: Suanming Mou <suanmi...@nvidia.com> > Sent: Friday, June 14, 2024 5:32 PM > To: Akhil Goyal <gak...@marvell.com>; Matan Azrad <ma...@nvidia.com> > Cc: dev@dpdk.org > Subject: RE: [EXTERNAL] [PATCH v2 1/2] crypto/mlx5: optimize AES-GCM > IPsec operation > > Hi, > > > -----Original Message----- > > From: Akhil Goyal <gak...@marvell.com> > > Sent: Friday, June 14, 2024 5:07 PM > > To: Suanming Mou <suanmi...@nvidia.com>; Matan Azrad > > <ma...@nvidia.com> > > Cc: dev@dpdk.org > > Subject: RE: [EXTERNAL] [PATCH v2 1/2] crypto/mlx5: optimize AES-GCM > > IPsec operation > > > > > Hi Akhil, > > > > > > > -----Original Message----- > > > > From: Akhil Goyal <gak...@marvell.com> > > > > Sent: Friday, June 14, 2024 2:49 PM > > > > To: Suanming Mou <suanmi...@nvidia.com>; Matan Azrad > > > > <ma...@nvidia.com> > > > > Cc: dev@dpdk.org > > > > Subject: RE: [EXTERNAL] [PATCH v2 1/2] crypto/mlx5: optimize > > > > AES-GCM IPsec operation > > > > > > > > > To optimize AES-GCM IPsec operation within crypto/mlx5, the DPDK > > > > > API typically supplies AES_GCM AAD/Payload/Digest in separate > > > > > locations, potentially disrupting their contiguous layout. In > > > > > cases where the memory layout fails to meet hardware (HW) > > > > > requirements, an UMR WQE is initiated ahead of the GCM's GGA WQE > > > > > to establish a continuous AAD/Payload/Digest virtual memory > > > > > space for the > > HW MMU. > > > > > > > > > > For IPsec scenarios, where the memory layout consistently > > > > > adheres to the fixed order of AAD/IV/Payload/Digest, directly > > > > > shrinking memory for AAD proves more efficient than preparing a > > > > > UMR WQE. To address this, a new devarg "crypto_mode" with mode > > > > > "ipsec_opt" is introduced in the commit, offering an > > > > > optimization hint specifically for IPsec cases. When enabled, > > > > > the PMD copies AAD directly before Payload in the enqueue_burst > > > > > function instead of employing the UMR WQE. Subsequently, in the > > > > > dequeue_burst function, the overridden IV before Payload is > > > > > restored from the GGA WQE. It's crucial for users to avoid > > > > > utilizing the input mbuf data during > > processing. > > > > > > > > This seems very specific to mlx5 and is not as per the > > > > expectations of cryptodev APIs. > > > > > > > > It seems you are asking to alter the user application to > > > > accommodate this to support IPsec. > > > > > > > > Cryptodev APIs are for generic crypto processing of data as > > > > defined in rte_crypto_op. > > > > With your proposed changes, it seems the behavior of the crypto > > > > APIs will be different in case of mlx5 which I believe is not correct. > > > > > > > > Is it not possible for you to use rte_security IPsec path? > > > > > > > > > > Sorry I don't understand why that conflicts the API, IIUC crypto API > > > only just defines the AAD/Payload/Digest in struct > > > rte_crypto_sym_op, but not restrict the sequence, and > > > AAD/Payload/Digest may come from > > difference memory space. > > > Am I missing something here? > > > > Yes you are correct that there is no restriction there. > > > > > The input requirements from mlx5 HW is AAD/Payload/Digest sequence, > > > if the input memory of AAD/Payload/Digest does not meet the > > > requirements, PMD will try to combine the memory address space with > > > UMR WQE as that commit does by software shrink. > > > > And here, you are adding a restriction for IPsec case. > > I believe you need a way to identify IPsec case with non-ipsec case in data > path. > > For that, instead of using a devarg(which is a specific case for > > mlx5), you can use generic rte_security session with action type > > RTE_SECURITY_ACTION_TYPE_NONE. > > Just to emphasize, this is not a restriction, we don't restrict user must use > that > devarg for IPSEC case. > The way to identify or apply that optimization is user's devarg of > "ipsec_opt". > Without that hint from devarg, pmd will work in UMR mode to combine the > memory addresses. > I agree move to other API will also make the hint work. But if providing one > hint devarg here does not conflict the API and bring better compatibility, it > does not hurt. > > > > > You may also benefit from rte_ipsec library APIs and test framework, > > for processing of protocol specific things which are specifically > > written for RTE_SECURITY_ACTION_TYPE_NONE case. > And again, thanks for the suggestion, I assume we will also consider > supporting that next for rte_security as well if possible, to provide more > choice for user. > > > > > > And the most important thing is that "ipsec_opt" is not mandatory, > > > only if user have such case of layout and allows that optimization > > > happens. Otherwise, the existing UMR WQE will still be the default > > > behavior > > here. > > > > > With this new devarg which application would you be using for testing? > > I do not see a patch for application changes for the layout that you > > are mentioning. > IIRC we used test-crypto-perf with headroom proper configured mbuf to > verify. > If you think another new arg is worth to be added to test-crypto-perf to build > everything, I can also send another application patch to support that > later.(but > sorry due to effort limitation maybe not happened soon in that series). > > Thanks, > Suanming
