> -----Original Message----- > From: Akhil Goyal <gak...@marvell.com> > Sent: Thursday, June 20, 2024 3:40 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, > > > > > -----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. > > Even if it is an optional thing. > After adding the devarg, the user is expected to use the buffers the way your > PMD is expecting. So, this is a restriction. Right?
The devarg is not enabled by default, if user adds the devarg, that means user know what he is doing, and the input is suitable for that optimization. PMD doesn't restrict user must use that hint to handle IPsec case, user will still be able to handle IPsec operation without that devarg. If user has mixed cases, just leave the devarg away, does that make sense? > > What would be the behavior if devarg is set but the buffers are configured the > same way as before? > > > 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. > > I do not understand how it is bringing better compatibility. > > The devarg that is added for ipsec_opt seems redundant. > We should use standard APIs when they are available. > Devargs are added to pass on additional run time configuration which is not > part of standard API set and is specific to a particular PMD. > But in this case, we do have rte_security and rte_ipsec APIs to configure > IPsec > specific requirements. OK, I assume you agreed before that the standard API does not define the memory layout, right? AAD, IV and payload are all defined separately. The API is not affected. Let's align the patch does not break the API. And another reason to have it is due to AES_GCM can also be use as IPsec mechanism(that is what has been merged). What do you think? And again, I still agree with you other API may also be able to achieve that. But that's another topic, for now, we expect mlx5 PMD AES_GCM can serve IPsec with and without the hint(after that patch). Thanks, Suanming
