[dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with macros
Neil Horman
nhorman at tuxdriver.com
Fri Jul 8 16:14:37 CEST 2016
On Fri, Jul 08, 2016 at 02:00:20PM +0000, De Lara Guarch, Pablo wrote:
>
>
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman at tuxdriver.com]
> > Sent: Friday, July 08, 2016 1:18 PM
> > To: De Lara Guarch, Pablo
> > Cc: dev at dpdk.org; Richardson, Bruce; Thomas Monjalon; Stephen
> > Hemminger; Panu Matilainen
> > Subject: Re: [PATCH] crypto: normalize cryptodev pmd names with macros
> >
> > 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?
>
> drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function 'snow3g_pmd_qp_create_processed_ops_ring':
> drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: error: 'cryptodev_snow3g_pmd' undeclared (first use in this function)
> SNOW3G_LOG_ERR("Unable to reuse existing ring %s"
> ^
> dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:208:152: note: each undeclared identifier is reported only once for each function it appears in
> dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c: In function 'snow3g_pmd_session_configure':
> dpdk-16.04/drivers/crypto/snow3g/rte_snow3g_pmd_ops.c:296:117: error: 'cryptodev_snow3g_pmd' undeclared (first use in this function)
> SNOW3G_LOG_ERR("invalid session struct");
>
> ...
>
> The solution is adding RTE_STR in SNOW3G_LOG_ERR.
>
Ah,thanks. Its odd, I used cscope to do a find and replace for all the other
instances of the RTE_STR conversion, I wonder how I missed that.
Neil
> ...
> >
> >
> > > 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.
>
> Will do that in a separate patch.
>
> >
> > 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.
>
> Some driver names have changed (like eth_pcap to pcap).
> I can revert that to the original name and we can rename them in the next release,
> after a deprecation notice, since this is breaking the API.
>
> >
> > Neil
> >
> > > Thanks,
> > > Pablo
> > >
>
More information about the dev
mailing list