Re: [dpdk-dev] [PATCH v2 6/9] app/procinfo: add code for debug crypto

[Stephen Hemminger <stephen@networkplumber.org>]

On Sat, 27 Oct 2018 04:42:19 +0000
"Varghese, Vipin" <vipin.varghese@intel.com> wrote:

> Hi,
> 
> <snipped>
> 
> > ---
> > +		struct rte_cryptodev_info dev_info = {0};
> > +		struct rte_cryptodev_stats stats = {0};
> > +
> > 
> > Memset for initialization as mentioned in other patch.
> >   
> 
> Yes, I will correct the same as certain compiler flag combination will treat this as incorrect use.
> 
> > +
> > +#define DSP_CRYPTO_FLAG(x) do { \
> > +printf("  - feature flags\n"); \
> > +printf("\t  -- symmetric (%c) asymmetric (%c)" \ " symmetric operation
> > +chaining (%c)\n", \ (x & RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO)?'y':'n', \
> > +(x & RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO)?'y':'n', \ (x &
> > +RTE_CRYPTODEV_FF_SYM_OPERATION_CHAINING)?'y':'n'); \ printf("\t  -- CPU
> > +SSE (%c) AVX (%c) AVX2 (%c) AVX512 (%c)\n", \ (x &
> > +RTE_CRYPTODEV_FF_CPU_SSE)?'y':'n', \ (x &
> > +RTE_CRYPTODEV_FF_CPU_AVX)?'y':'n', \ (x &
> > +RTE_CRYPTODEV_FF_CPU_AVX2)?'y':'n', \ (x &
> > +RTE_CRYPTODEV_FF_CPU_AVX512)?'y':'n'); \ printf("\t  -- Acclerate CPU
> > +AESNI (%c) HW (%c)\n", \ (x & RTE_CRYPTODEV_FF_CPU_AESNI)?'y':'n', \ (x
> > +& RTE_CRYPTODEV_FF_HW_ACCELERATED)?'y':'n'); \ printf("\t  -- INLINE
> > +(%c)\n", \ (x & RTE_CRYPTODEV_FF_SECURITY)?'y':'n'); \ printf("\t  --
> > +ARM NEON (%c) CE (%c)\n", \ (x & RTE_CRYPTODEV_FF_CPU_NEON)?'y':'n', \
> > +(x & RTE_CRYPTODEV_FF_CPU_ARM_CE)?'y':'n'); \ printf("  - buffer
> > +offload\n"); \ printf("\t  -- IN_PLACE_SGL (%c)\n", \ (x &
> > +RTE_CRYPTODEV_FF_IN_PLACE_SGL)?'y':'n'); \ printf("\t  --
> > +OOP_SGL_IN_SGL_OUT (%c)\n", \ (x &
> > +RTE_CRYPTODEV_FF_OOP_SGL_IN_SGL_OUT)?'y':'n'); \ printf("\t  --
> > +OOP_SGL_IN_LB_OUT (%c)\n", \ (x &
> > +RTE_CRYPTODEV_FF_OOP_SGL_IN_LB_OUT)?'y':'n'); \ printf("\t  --
> > +OOP_LB_IN_SGL_OUT (%c)\n", \ (x &
> > +RTE_CRYPTODEV_FF_OOP_LB_IN_SGL_OUT)?'y':'n'); \ printf("\t  --
> > +OOP_LB_IN_LB_OUT (%c)\n", \ (x &
> > +RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT)?'y':'n'); \ } while (0)
> > 
> > This is a very big macro, better have static function for this instead of macro.
> >   
> 
> There are two thoughts in choosing MACRO over function.
> 1. The information need to display in certain format within the same context. 
> 2. As the API are modified, co locating all at same scope is easier to clean up and correct in future.
> 
> So I feel use of MACRO over function in this instance.


I don't agree with your arguments. Macros, are ugly and error prone. This is not performance
critical so it should be a function. The only reason to use macro's is if it is not possible to
write it as a function (as in a template for code generation).