[dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros

[Neil Horman]

On Fri, Jul 08, 2016 at 09:09:10AM +0000, De Lara Guarch, Pablo wrote:
> Hi Neil,
> 
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > Sent: Thursday, July 07, 2016 4:38 PM
> > To: dev at dpdk.org
> > Cc: Neil Horman; Richardson, Bruce; Thomas Monjalon; De Lara Guarch,
> > Pablo; Stephen Hemminger; Panu Matilainen
> > Subject: [PATCH] crypto: normalize cryptodev pmd names with macros
> > 
> > Recently reported, the introduction of pmd information exports led to a
> > breakage of cryptodev unit tests because the test infrastructure relies on the
> > cryptodev names being available in macros.  This patch fixes the pmd naming
> > to
> > use the macro names.  Note that the macro names were already pre-
> > stringified,
> > which won't work as the PMD_REGISTER_DRIVER macro requires the name in
> > both a
> > processing token and stringified form.  As such the names are defined now as
> > tokens, and converted where needed to stringified form on demand using
> > RTE_STR.
> > 
> > Tested using the null and aesni_mb crypto pmds, as I don't have hardware for
> > any
> > other device.  Also not build tested on snow3g or kasumi pmd because
> > building
> > those requires external libraries that appear to not be open source licensed.
> > 
> > Signed-off-by: Neil Horman <nhorman at tuxdriver.com>
> > CC: Bruce Richardson <bruce.richardson at intel.com>
> > CC: Thomas Monjalon <thomas.monjalon at 6wind.com>
> > CC: "De Lara Guarch, Pablo" <pablo.de.lara.guarch at intel.com>
> > CC: Stephen Hemminger <stephen at networkplumber.org>
> > CC: Panu Matilainen <pmatilai at redhat.com>
> > ---
> >  app/test/test_cryptodev.c                          | 20 ++++++++++----------
> >  app/test/test_cryptodev_perf.c                     | 18 +++++++++---------
> >  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c           |  7 +++----
> >  drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h   |  6 +++---
> >  drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c         |  7 +++----
> >  drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h |  2 +-
> >  drivers/crypto/kasumi/rte_kasumi_pmd.c             |  5 ++---
> >  drivers/crypto/null/null_crypto_pmd.c              |  7 +++----
> >  drivers/crypto/null/null_crypto_pmd_private.h      |  6 +++---
> >  drivers/crypto/qat/rte_qat_cryptodev.c             |  4 ++--
> >  drivers/crypto/snow3g/rte_snow3g_pmd.c             |  4 ++--
> >  lib/librte_cryptodev/rte_cryptodev.h               | 12 ++++++------
> >  12 files changed, 47 insertions(+), 51 deletions(-)
> 
> Thanks for this patch. I tested snow3g and kasumi, and they don't compile.
> I have a fix for that, so I can send a v2 of this patch if it is OK for you.
> 

I suppose thats fine, sure, though I'm really not comfortable with an open
source project requiring what appears to be non-open source components (though I
can't really tell what the snow3g or kasumi license is).  Regardless, whats the
compilation error?


> Also, we should make these changes in the other PMDs as well, right?
> I mean, avoid setting the name of the driver directly in the structure and go back to the original name.
> I can do that as well, if you want (maybe a separate patch, as this one is only related to crypto).
> 
I think thats kind of two questions:

1) Should we remove the static setting of the name in the pmd_driver structure
in favor of doing it in the registration macro?

2) Should we be consistent in the name conversion (from the setting in the
structure instance definition to the string in the macro parameter)?

The answer to (1) is yes, though having it in both places is harmless, since the
former will just get overridden.  We should definately remove the static
setting, to avoid confusion, but theres not any functional rush to do so.

The answer to (2) is yes, but I think thats already done.  I don't think we
deviated in too many places (if any), as the strings for all the net devices
were directly set (i.e. not through macros), and I just transferred them.

Neil

> Thanks,
> Pablo
>