Re: [dpdk-dev] [PATCH 2/3] cryptodev: add security operation to crypto operation

["Coyle, David" <david.coyle@intel.com>]

Hi Konstantin, see below

> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Tuesday, June 9, 2020 2:23 PM
> 
> 
> >
> >  /** Status of crypto operation */
> > @@ -121,6 +123,13 @@ struct rte_crypto_op {
> >  		struct rte_crypto_asym_op asym[0];
> >  		/**< Asymmetric operation parameters */
> >
> > +#ifdef RTE_LIBRTE_SECURITY
> > +		uint8_t security[0];
> > +		/**< Security operation parameters
> > +		 * - Must be accessed through a rte_security_op pointer
> > +		 */
> > +#endif
> > +
> >  	}; /**< operation specific parameters */  };
> 
> Is there any point to have this extra level of indirection?
> Might be simply:
> 
> enum rte_crypto_op_type {
> 	....
> +	RTE_CRYPTO_OP_TYPE_SEC_DOCSIS,
> };
> ...
> struct rte_crypto_op {
> 	....
> 	__extension__
>         union {
>                 struct rte_crypto_sym_op sym[0];
>                 /**< Symmetric operation parameters */
> 
>                 struct rte_crypto_asym_op asym[0];
>                 /**< Asymmetric operation parameters */
> 
> +	struct rte_security_docsis_op docsis[0];
> 
>         }; /**< operation specific parameters */
> 
> ?
[DC] This was to allow some form of extensibility and not to limit this to just DOCSIS.
If it's felt that having the extra level of indirection is overkill, it can be easily changed.

However, we cannot include a struct of type 'struct rte_security_docsis_op' (or 'struct rte_security_op')
directly here, without creating nasty circular dependency of includes between rte_cryptodev and
rte_security.

I had tried defining an opaque version 'struct rte_security_op' (i.e. no fields within the struct) here
in rte_crypto.h, but the compiler complained that it couldn't determine the size of the struct,
even though it's a zero length array.

That is why I had to use the uint8_t in 'uint8_t security[0];' - I don't like this, but I couldn't find
another way that kept the compiler happy and didn't create a circular dependency.