> > > > > > > > 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. > > > > Devarg is optional and not enabled by default. That is clear to me at the > > first > > place. > > > > The point is PMD devarg can dictate the behavior of PMD and NOT the user. > > The standard APIs are the ones which user must adhere to. > > > > You cannot expect a user to change its code when it wants to use the devarg > > optimization. > > Sorry, it is my bad missing the background introduction here. In fact, the > motivation to implement that feature is not to request user to build the > memory > in that order. > Opsitely, after the first UMR version, we noticed that user may have IPsec as > input and that input can be optimized by that hint. > Since the API does not reject user's IPsec input, so finally we decided to > add that > hint to optimize the IPsec input case. > I agree we can implement based on other API and force user to use other API, > but > if the current API does not reject user's IPsec input, I assume it is worth to > optimize that. What do you think? > > > > > > 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? > > Yes it does not for crypto APIs. > > > > But if we use rte_security and rte_ipsec APIs, format of packets should be > > as > > per IPsec requirement. > > That is implicit for the application to improve performance. > > I always agree with rte_security and rte_ipsec APIs, and this is what we want > to > expand next. but as replied above, the hint does not conflicts with these > API, we > can have both supported finally. > User can choose anyway he wants since current API does not rejects the IPsec > inputs. What do you think? > > > > > > 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? > > > > That is supported and tested using ipsec-secgw as well as dpdk-test > > application. > > But adding a PMD devarg for explicitly supporting IPsec is not required. > > User cannot be dictated by the PMD devarg to align its buffers. > > Just to be more accurate, in fact our current PMD has supported IPsec already > via > UMR's way. This series is an optimization, not newly supporting IPsec, but to > optimize the IPsec input case. > And like I replied above, it is not the patches invites the layout and > require user to > have such buffers, but the truth is we want to better serve the IPsec input > come > with that layout case in the real world as API does not reject that IPsec > input.
Ok got it. But if we set the devarg for a mlx5 device, it will be a hint to the PMD for all the flows that it will process. Right? And there may be an application use case where it has 2 separate flows simultaneously - IPsec and non-IPsec. Then how would PMD handle that? Will it assume contiguous memory for non-IPsec case as well? How will PMD identify between IPsec and non-IPsec case? > > > > > > 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
