Hi Konstantin,
> Hi lads,
>
> >
> > Hi Akhil,
> >
> > Thanks again for the review!
> > To summarize, the following places to be changed for v10.
> >
> > 1. Documentation update and reviewed internally in Intel first.
> > 2. Add the missing comments to the structure.
> > 3. Change the name "dp_service" to "raw_dp" to all APIs and documentation.
> > 4. Change the structure
> > struct rte_crypto_sym_vec {
> > /** array of SGL vectors */
> > struct rte_crypto_sgl *sgl;
> >
> > union {
> > /** Supposed to be used with CPU crypto API call. */
> > struct {
> > /** array of pointers to IV */
> > void **iv;
> > /** array of pointers to AAD */
> > void **aad;
> > /** array of pointers to digest */
> > void **digest;
> > } cpu_crypto;
> > /** Supposed to be used with HW raw crypto API call. */
> > struct {
> > /** array of pointers to cipher IV */
> > void **cipher_iv_ptr;
> > /** array of IOVA addresses to cipher IV */
> > rte_iova_t *cipher_iv_iova;
> > /** array of pointers to auth IV */
> > void **auth_iv_ptr;
> > /** array of IOVA addresses to auth IV */
> > rte_iova_t *auth_iv_iova;
> > /** array of pointers to digest */
> > void **digest_ptr;
> > /** array of IOVA addresses to digest */
> > rte_iova_t *digest_iova;
> > } hw_chain;
> > /** Supposed to be used with HW raw crypto API call. */
> > struct {
> > /** array of pointers to AEAD IV */
> > void **iv_ptr;
> > /** array of IOVA addresses to AEAD IV */
> > rte_iova_t *iv_iova;
> > /** array of pointers to AAD */
> > void **aad_ptr;
> > /** array of IOVA addresses to AAD */
> > rte_iova_t *aad_iova;
> > /** array of pointers to digest */
> > void **digest_ptr;
> > /** array of IOVA addresses to digest */
> > rte_iova_t *digest_iova;
> > } hw_aead;
> > };
> >
> > /**
> > * array of statuses for each operation:
> > * - 0 on success
> > * - errno on error
> > */
> > int32_t *status;
> > /** number of operations to perform */
> > uint32_t num;
> > };
>
>
> As I understand you just need to add pointers to iova[] for iv, aad and
> digest,
> correct?
> If so, why not simply:
>
> struct rte_va_iova_ptr {
> void *va;
> rte_iova_t *iova;
> };
>
> struct rte_crypto_sym_vec {
> /** array of SGL vectors */
> struct rte_crypto_sgl *sgl;
> /** array of pointers to IV */
> struct rte_va_iova_ptr iv;
> /** array of pointers to AAD */
> struct rte_va_iova_ptr aad;
> /** array of pointers to digest */
> struct rte_va_iova_ptr digest;
> /**
> * array of statuses for each operation:
> * - 0 on success
> * - errno on error
> */
> int32_t *status;
> /** number of operations to perform */
> uint32_t num;
> };
>
> BTW, it would be both ABI and API breakage,
> though all functions using this struct are marked as experimental,
> plus it is an LTS release, so it seems to be ok.
> Though I think it needs to be flagged in RN.
This is a good suggestion. This will make some changes in the cpu-crypto
support as well
And should be a separate patch.
We can take the API and ABI breakage in this release. That is not an issue.
>
> Another option obviously - introduce completely new structure for it
> and leave existing one unaffected.
>
This will create some duplicate code. Would not prefer that.
> >
> > 5. Remove enum rte_crypto_dp_service, let the PMDs using the session private
> data to decide function handler.
> > 6. Remove is_update parameter.
> >
> > The main point that is uncertain is the existance of "submit_single".
> > I am ok to remove "submit_single" function. In VPP we can use
> rte_cryptodev_dp_sym_submit_vec() with vec.num=1 each time to avoid
> > double looping.
> > But we have to put the rte_cryptodev_dp_sym_submit_vec() as an inline
> function - this will cause the API not traced in version map.
> >
> > Any ideas?
> >
> > Regards,
> > Fan
> >
