> 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? 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. > > > > > 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
