[dpdk-dev] crypto: normalize cryptodev pmd names with macros
Commit Message
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@tuxdriver.com>
CC: Bruce Richardson <bruce.richardson@intel.com>
CC: Thomas Monjalon <thomas.monjalon@6wind.com>
CC: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
CC: Stephen Hemminger <stephen@networkplumber.org>
CC: Panu Matilainen <pmatilai@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(-)
Comments
Hi Neil,
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Thursday, July 07, 2016 4:38 PM
> To: dev@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@tuxdriver.com>
> CC: Bruce Richardson <bruce.richardson@intel.com>
> CC: Thomas Monjalon <thomas.monjalon@6wind.com>
> CC: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
> CC: Stephen Hemminger <stephen@networkplumber.org>
> CC: Panu Matilainen <pmatilai@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.
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).
Thanks,
Pablo
2016-07-07 11:37, Neil Horman:
> -PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drvi, aesni_mb);
> -DRIVER_REGISTER_PARAM_STRING(aesni_gcm, "max_nb_queue_pairs=<int> "
> +PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drv, CRYPTODEV_NAME_AESNI_MB_PMD);
> +DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_MB_PMD, "max_nb_queue_pairs=<int> "
> "max_nb_sessions=<int> socket_id=<int>");
A coding style opinion:
The line DRIVER_REGISTER_PARAM_STRING is a bit long and I think it would be
nice to have only one param per line with an indent like this:
DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_MB_PMD,
"max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> "
"socket_id=<int>");
On Fri, Jul 08, 2016 at 09:09:10AM +0000, De Lara Guarch, Pablo wrote:
> Hi Neil,
>
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Thursday, July 07, 2016 4:38 PM
> > To: dev@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@tuxdriver.com>
> > CC: Bruce Richardson <bruce.richardson@intel.com>
> > CC: Thomas Monjalon <thomas.monjalon@6wind.com>
> > CC: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
> > CC: Stephen Hemminger <stephen@networkplumber.org>
> > CC: Panu Matilainen <pmatilai@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
>
2016-07-08 08:17, Neil Horman:
> 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).
Yes you're right.
IMHO, it is acceptable because it is free (just need a registration)
but it can be discussed.
I think there are some legal export issues because of crypto aim.
I suggest to avoid discussing this topic here and launch a dedicated thread
(with better visibility).
On Fri, Jul 08, 2016 at 02:40:01PM +0200, Thomas Monjalon wrote:
> 2016-07-08 08:17, Neil Horman:
> > 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).
>
> Yes you're right.
> IMHO, it is acceptable because it is free (just need a registration)
> but it can be discussed.
> I think there are some legal export issues because of crypto aim.
Yeah, crypto usually has munitions export restrictions. I'm fine with that, as
long as the license to the code is GPL. And I can't tell what the license is
because you have to register to get it before you can see it.
If you, or someone can confirm that the snow3g and kasumi ssl libraries are open
source compatible (or better still if the license can be announced prior to
registration on the web site), then I'm fine with it, and am happy to test with
it, I just don't want to register without knowing that beforehand.
> I suggest to avoid discussing this topic here and launch a dedicated thread
> (with better visibility).
>
Sounds good to me
Neil
>
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Friday, July 08, 2016 1:18 PM
> To: De Lara Guarch, Pablo
> Cc: dev@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@tuxdriver.com]
> > > Sent: Thursday, July 07, 2016 4:38 PM
> > > To: dev@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@tuxdriver.com>
> > > CC: Bruce Richardson <bruce.richardson@intel.com>
> > > CC: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > CC: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
> > > CC: Stephen Hemminger <stephen@networkplumber.org>
> > > CC: Panu Matilainen <pmatilai@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.
...
>
>
> > 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
> >
On Fri, Jul 08, 2016 at 02:00:20PM +0000, De Lara Guarch, Pablo wrote:
>
>
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Friday, July 08, 2016 1:18 PM
> > To: De Lara Guarch, Pablo
> > Cc: dev@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@tuxdriver.com]
> > > > Sent: Thursday, July 07, 2016 4:38 PM
> > > > To: dev@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@tuxdriver.com>
> > > > CC: Bruce Richardson <bruce.richardson@intel.com>
> > > > CC: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > > CC: "De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
> > > > CC: Stephen Hemminger <stephen@networkplumber.org>
> > > > CC: Panu Matilainen <pmatilai@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
> > >
>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> Sent: Friday, July 8, 2016 2:42 PM
> To: Thomas Monjalon <thomas.monjalon@6wind.com>
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>; Panu Matilainen <pmatilai@redhat.com>
> Subject: Re: [dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with
> macros
>
> On Fri, Jul 08, 2016 at 02:40:01PM +0200, Thomas Monjalon wrote:
> > 2016-07-08 08:17, Neil Horman:
> > > 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).
> >
> > Yes you're right.
> > IMHO, it is acceptable because it is free (just need a registration)
> > but it can be discussed.
> > I think there are some legal export issues because of crypto aim.
> Yeah, crypto usually has munitions export restrictions. I'm fine with
> that, as long as the license to the code is GPL. And I can't tell what
> the license is because you have to register to get it before you can see
> it.
>
> If you, or someone can confirm that the snow3g and kasumi ssl libraries
> are open source compatible (or better still if the license can be
> announced prior to registration on the web site), then I'm fine with it,
> and am happy to test with it, I just don't want to register without
> knowing that beforehand.
Hi Neil,
The license is "Intel Pre-Release Software License Agreement". We will send a
patch to make this this clear in the documentation. We will also see if it
can be made visible on the download page.
I can send you a copy of the license if you wish. Pablo has already tested
compilation with the latest version of the patch.
John
On Fri, Jul 08, 2016 at 07:03:10PM +0000, Mcnamara, John wrote:
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > Sent: Friday, July 8, 2016 2:42 PM
> > To: Thomas Monjalon <thomas.monjalon@6wind.com>
> > Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; dev@dpdk.org;
> > Richardson, Bruce <bruce.richardson@intel.com>; Stephen Hemminger
> > <stephen@networkplumber.org>; Panu Matilainen <pmatilai@redhat.com>
> > Subject: Re: [dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with
> > macros
> >
> > On Fri, Jul 08, 2016 at 02:40:01PM +0200, Thomas Monjalon wrote:
> > > 2016-07-08 08:17, Neil Horman:
> > > > 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).
> > >
> > > Yes you're right.
> > > IMHO, it is acceptable because it is free (just need a registration)
> > > but it can be discussed.
> > > I think there are some legal export issues because of crypto aim.
> > Yeah, crypto usually has munitions export restrictions. I'm fine with
> > that, as long as the license to the code is GPL. And I can't tell what
> > the license is because you have to register to get it before you can see
> > it.
> >
> > If you, or someone can confirm that the snow3g and kasumi ssl libraries
> > are open source compatible (or better still if the license can be
> > announced prior to registration on the web site), then I'm fine with it,
> > and am happy to test with it, I just don't want to register without
> > knowing that beforehand.
>
> Hi Neil,
>
> The license is "Intel Pre-Release Software License Agreement". We will send a
> patch to make this this clear in the documentation. We will also see if it
> can be made visible on the download page.
>
> I can send you a copy of the license if you wish. Pablo has already tested
> compilation with the latest version of the patch.
>
I understand and appreciate that, but I'd still like to have a copy of the
license. It seems a bit unreasonable to me to include open source drivers in an
open source project, if they can't be built without a non-open source library
included.
Neil
> John
>
>
> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Saturday, July 9, 2016 2:34 PM
> To: Mcnamara, John <john.mcnamara@intel.com>
> Cc: Thomas Monjalon <thomas.monjalon@6wind.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Richardson, Bruce
> <bruce.richardson@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>; Panu Matilainen <pmatilai@redhat.com>
> Subject: Re: [dpdk-dev] [PATCH] crypto: normalize cryptodev pmd names with
> macros
>
> On Fri, Jul 08, 2016 at 07:03:10PM +0000, Mcnamara, John wrote:
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman
> > > Sent: Friday, July 8, 2016 2:42 PM
> > > To: Thomas Monjalon <thomas.monjalon@6wind.com>
> > > Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> > > dev@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>;
> > > Stephen Hemminger <stephen@networkplumber.org>; Panu Matilainen
> > > <pmatilai@redhat.com>
> > > Subject: Re: [dpdk-dev] [PATCH] crypto: normalize cryptodev pmd
> > > names with macros
> > >
> > > On Fri, Jul 08, 2016 at 02:40:01PM +0200, Thomas Monjalon wrote:
> > > > 2016-07-08 08:17, Neil Horman:
> > > > > 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).
> > > >
> > > > Yes you're right.
> > > > IMHO, it is acceptable because it is free (just need a
> > > > registration) but it can be discussed.
> > > > I think there are some legal export issues because of crypto aim.
> > > Yeah, crypto usually has munitions export restrictions. I'm fine
> > > with that, as long as the license to the code is GPL. And I can't
> > > tell what the license is because you have to register to get it
> > > before you can see it.
> > >
> > > If you, or someone can confirm that the snow3g and kasumi ssl
> > > libraries are open source compatible (or better still if the license
> > > can be announced prior to registration on the web site), then I'm
> > > fine with it, and am happy to test with it, I just don't want to
> > > register without knowing that beforehand.
> >
> > Hi Neil,
> >
> > The license is "Intel Pre-Release Software License Agreement". We will
> > send a patch to make this this clear in the documentation. We will
> > also see if it can be made visible on the download page.
> >
> > I can send you a copy of the license if you wish. Pablo has already
> > tested compilation with the latest version of the patch.
> >
> I understand and appreciate that, but I'd still like to have a copy of the
> license. It seems a bit unreasonable to me to include open source drivers
> in an open source project, if they can't be built without a non-open
> source library included.
I'll send you a copy.
John
@@ -186,12 +186,12 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
ret = rte_eal_vdev_init(
- CRYPTODEV_NAME_AESNI_MB_PMD, NULL);
+ RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD), NULL);
TEST_ASSERT(ret == 0,
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_AESNI_MB_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD));
}
}
}
@@ -203,10 +203,10 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
- CRYPTODEV_NAME_AESNI_GCM_PMD, NULL),
+ RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD), NULL),
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_AESNI_GCM_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD));
}
}
}
@@ -217,10 +217,10 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
- CRYPTODEV_NAME_SNOW3G_PMD, NULL),
+ RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD), NULL),
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_SNOW3G_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD));
}
}
}
@@ -231,10 +231,10 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
TEST_ASSERT_SUCCESS(rte_eal_vdev_init(
- CRYPTODEV_NAME_KASUMI_PMD, NULL),
+ RTE_STR(CRYPTODEV_NAME_KASUMI_PMD), NULL),
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_KASUMI_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_KASUMI_PMD));
}
}
}
@@ -246,12 +246,12 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
int dev_id = rte_eal_vdev_init(
- CRYPTODEV_NAME_NULL_PMD, NULL);
+ RTE_STR(CRYPTODEV_NAME_NULL_PMD), NULL);
TEST_ASSERT(dev_id >= 0,
"Failed to create instance %u of"
" pmd : %s",
- i, CRYPTODEV_NAME_NULL_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_NULL_PMD));
}
}
}
@@ -120,15 +120,15 @@ static const char *chain_mode_name(enum chain_mode mode)
static const char *pmd_name(enum rte_cryptodev_type pmd)
{
switch (pmd) {
- case RTE_CRYPTODEV_NULL_PMD: return CRYPTODEV_NAME_NULL_PMD; break;
+ case RTE_CRYPTODEV_NULL_PMD: return RTE_STR(CRYPTODEV_NAME_NULL_PMD); break;
case RTE_CRYPTODEV_AESNI_GCM_PMD:
- return CRYPTODEV_NAME_AESNI_GCM_PMD;
+ return RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD);
case RTE_CRYPTODEV_AESNI_MB_PMD:
- return CRYPTODEV_NAME_AESNI_MB_PMD;
+ return RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD);
case RTE_CRYPTODEV_QAT_SYM_PMD:
- return CRYPTODEV_NAME_QAT_SYM_PMD;
+ return RTE_STR(CRYPTODEV_NAME_QAT_SYM_PMD);
case RTE_CRYPTODEV_SNOW3G_PMD:
- return CRYPTODEV_NAME_SNOW3G_PMD;
+ return RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD);
default:
return "";
}
@@ -249,11 +249,11 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
ret = rte_eal_vdev_init(
- CRYPTODEV_NAME_AESNI_MB_PMD, NULL);
+ RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD), NULL);
TEST_ASSERT(ret == 0,
"Failed to create instance %u of pmd : %s",
- i, CRYPTODEV_NAME_AESNI_MB_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD));
}
}
}
@@ -264,11 +264,11 @@ testsuite_setup(void)
if (nb_devs < 2) {
for (i = nb_devs; i < 2; i++) {
ret = rte_eal_vdev_init(
- CRYPTODEV_NAME_SNOW3G_PMD, NULL);
+ RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD), NULL);
TEST_ASSERT(ret == 0,
"Failed to create instance %u of pmd : %s",
- i, CRYPTODEV_NAME_SNOW3G_PMD);
+ i, RTE_STR(CRYPTODEV_NAME_SNOW3G_PMD));
}
}
}
@@ -57,7 +57,7 @@ create_unique_device_name(char *name, size_t size)
if (name == NULL)
return -EINVAL;
- ret = snprintf(name, size, "%s_%u", CRYPTODEV_NAME_AESNI_GCM_PMD,
+ ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD),
unique_name_id++);
if (ret < 0)
return ret;
@@ -515,12 +515,11 @@ aesni_gcm_uninit(const char *name)
}
static struct rte_driver aesni_gcm_pmd_drv = {
- .name = CRYPTODEV_NAME_AESNI_GCM_PMD,
.type = PMD_VDEV,
.init = aesni_gcm_init,
.uninit = aesni_gcm_uninit
};
-PMD_REGISTER_DRIVER(aesni_gcm_pmd_drv, aesni_gcm);
-DRIVER_REGISTER_PARAM_STRING(aesni_gcm, "max_nb_queue_pairs=<int> "
+PMD_REGISTER_DRIVER(aesni_gcm_pmd_drv, CRYPTODEV_NAME_AESNI_GCM_PMD);
+DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_GCM_PMD, "max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> socket_id=<int>");
@@ -37,18 +37,18 @@
#define GCM_LOG_ERR(fmt, args...) \
RTE_LOG(ERR, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- CRYPTODEV_NAME_AESNI_GCM_PMD, \
+ RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD), \
__func__, __LINE__, ## args)
#ifdef RTE_LIBRTE_AESNI_MB_DEBUG
#define GCM_LOG_INFO(fmt, args...) \
RTE_LOG(INFO, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- CRYPTODEV_NAME_AESNI_GCM_PMD, \
+ RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD), \
__func__, __LINE__, ## args)
#define GCM_LOG_DBG(fmt, args...) \
RTE_LOG(DEBUG, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- CRYPTODEV_NAME_AESNI_GCM_PMD, \
+ RTE_STR(CRYPTODEV_NAME_AESNI_GCM_PMD), \
__func__, __LINE__, ## args)
#else
#define GCM_LOG_INFO(fmt, args...)
@@ -54,7 +54,7 @@ create_unique_device_name(char *name, size_t size)
if (name == NULL)
return -EINVAL;
- ret = snprintf(name, size, "%s_%u", CRYPTODEV_NAME_AESNI_MB_PMD,
+ ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD),
unique_name_id++);
if (ret < 0)
return ret;
@@ -715,12 +715,11 @@ cryptodev_aesni_mb_uninit(const char *name)
}
static struct rte_driver cryptodev_aesni_mb_pmd_drv = {
- .name = CRYPTODEV_NAME_AESNI_MB_PMD,
.type = PMD_VDEV,
.init = cryptodev_aesni_mb_init,
.uninit = cryptodev_aesni_mb_uninit
};
-PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drvi, aesni_mb);
-DRIVER_REGISTER_PARAM_STRING(aesni_gcm, "max_nb_queue_pairs=<int> "
+PMD_REGISTER_DRIVER(cryptodev_aesni_mb_pmd_drv, CRYPTODEV_NAME_AESNI_MB_PMD);
+DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_AESNI_MB_PMD, "max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> socket_id=<int>");
@@ -37,7 +37,7 @@
#define MB_LOG_ERR(fmt, args...) \
RTE_LOG(ERR, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- CRYPTODEV_NAME_AESNI_MB_PMD, \
+ RTE_STR(CRYPTODEV_NAME_AESNI_MB_PMD), \
__func__, __LINE__, ## args)
#ifdef RTE_LIBRTE_AESNI_MB_DEBUG
@@ -648,12 +648,11 @@ cryptodev_kasumi_uninit(const char *name)
}
static struct rte_driver cryptodev_kasumi_pmd_drv = {
- .name = CRYPTODEV_NAME_KASUMI_PMD,
.type = PMD_VDEV,
.init = cryptodev_kasumi_init,
.uninit = cryptodev_kasumi_uninit
};
-PMD_REGISTER_DRIVER(cryptodev_kasumi_pmd_drv, kasumi);
-DRIVER_REGISTER_PARAM_STRING(kasumi, "max_nb_queue_pairs=<int> "
+PMD_REGISTER_DRIVER(cryptodev_kasumi_pmd_drv, CRYPTODEV_NAME_KASUMI_PMD);
+DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_KASUMI_PMD, "max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> socket_id=<int>");
@@ -51,7 +51,7 @@ create_unique_device_name(char *name, size_t size)
if (name == NULL)
return -EINVAL;
- ret = snprintf(name, size, "%s_%u", CRYPTODEV_NAME_NULL_PMD,
+ ret = snprintf(name, size, "%s_%u", RTE_STR(CRYPTODEV_NAME_NULL_PMD),
unique_name_id++);
if (ret < 0)
return ret;
@@ -269,12 +269,11 @@ cryptodev_null_uninit(const char *name)
}
static struct rte_driver cryptodev_null_pmd_drv = {
- .name = CRYPTODEV_NAME_NULL_PMD,
.type = PMD_VDEV,
.init = cryptodev_null_init,
.uninit = cryptodev_null_uninit
};
-PMD_REGISTER_DRIVER(cryptodev_null_pmd_drv, cryptodev_null_pmd);
-DRIVER_REGISTER_PARAM_STRING(aesni_gcm, "max_nb_queue_pairs=<int> "
+PMD_REGISTER_DRIVER(cryptodev_null_pmd_drv, CRYPTODEV_NAME_NULL_PMD);
+DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_NULL_PMD, "max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> socket_id=<int>");
@@ -37,18 +37,18 @@
#define NULL_CRYPTO_LOG_ERR(fmt, args...) \
RTE_LOG(ERR, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- CRYPTODEV_NAME_NULL_PMD, \
+ RTE_STR(CRYPTODEV_NAME_NULL_PMD), \
__func__, __LINE__, ## args)
#ifdef RTE_LIBRTE_NULL_CRYPTO_DEBUG
#define NULL_CRYPTO_LOG_INFO(fmt, args...) \
RTE_LOG(INFO, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- CRYPTODEV_NAME_NULL_PMD, \
+ RTE_STR(CRYPTODEV_NAME_NULL_PMD), \
__func__, __LINE__, ## args)
#define NULL_CRYPTO_LOG_DBG(fmt, args...) \
RTE_LOG(DEBUG, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- CRYPTODEV_NAME_NULL_PMD, \
+ RTE_STR(CRYPTODEV_NAME_NULL_PMD), \
__func__, __LINE__, ## args)
#else
#define NULL_CRYPTO_LOG_INFO(fmt, args...)
@@ -134,6 +134,6 @@ static struct rte_driver pmd_qat_drv = {
.init = rte_qat_pmd_init,
};
-PMD_REGISTER_DRIVER(pmd_qat_drv, qat);
-DRIVER_REGISTER_PCI_TABLE(qat, pci_id_qat_map);
+PMD_REGISTER_DRIVER(pmd_qat_drv, CRYPTODEV_NAME_QAT_SYM_PMD);
+DRIVER_REGISTER_PCI_TABLE(CRYPTODEV_NAME_QAT_SYM_PMD, pci_id_qat_map);
@@ -645,6 +645,6 @@ static struct rte_driver cryptodev_snow3g_pmd_drv = {
.uninit = cryptodev_snow3g_uninit
};
-PMD_REGISTER_DRIVER(cryptodev_snow3g_pmd_drv, snow3g);
-DRIVER_REGISTER_PARAM_STRING(aesni_gcm, "max_nb_queue_pairs=<int> "
+PMD_REGISTER_DRIVER(cryptodev_snow3g_pmd_drv, CRYPTODEV_NAME_SNOW3G_PMD);
+DRIVER_REGISTER_PARAM_STRING(CRYPTODEV_NAME_SNOW3G_PMD, "max_nb_queue_pairs=<int> "
"max_nb_sessions=<int> socket_id=<int>");
@@ -49,17 +49,17 @@ extern "C" {
#include "rte_crypto.h"
#include "rte_dev.h"
-#define CRYPTODEV_NAME_NULL_PMD ("cryptodev_null_pmd")
+#define CRYPTODEV_NAME_NULL_PMD cryptodev_null_pmd
/**< Null crypto PMD device name */
-#define CRYPTODEV_NAME_AESNI_MB_PMD ("cryptodev_aesni_mb_pmd")
+#define CRYPTODEV_NAME_AESNI_MB_PMD cryptodev_aesni_mb_pmd
/**< AES-NI Multi buffer PMD device name */
-#define CRYPTODEV_NAME_AESNI_GCM_PMD ("cryptodev_aesni_gcm_pmd")
+#define CRYPTODEV_NAME_AESNI_GCM_PMD cryptodev_aesni_gcm_pmd
/**< AES-NI GCM PMD device name */
-#define CRYPTODEV_NAME_QAT_SYM_PMD ("cryptodev_qat_sym_pmd")
+#define CRYPTODEV_NAME_QAT_SYM_PMD cryptodev_qat_sym_pmd
/**< Intel QAT Symmetric Crypto PMD device name */
-#define CRYPTODEV_NAME_SNOW3G_PMD ("cryptodev_snow3g_pmd")
+#define CRYPTODEV_NAME_SNOW3G_PMD cryptodev_snow3g_pmd
/**< SNOW 3G PMD device name */
-#define CRYPTODEV_NAME_KASUMI_PMD ("cryptodev_kasumi_pmd")
+#define CRYPTODEV_NAME_KASUMI_PMD cryptodev_kasumi_pmd
/**< KASUMI PMD device name */
/** Crypto device type */