crypto/armv8: fix typos and compilation
Checks
Commit Message
Typo in debug log switch macro caused debug log cannot be enabled.
Fixed the typo to match option defined in config file.
Typo in crypto dev name macro caused unexpected device name in log.
Fixed the typo to log with correct device name.
Solved compilation error when debug log is enabled:
rte_armv8_pmd.c: In function ‘process_armv8_chained_op’:
rte_armv8_pmd.c:633:22: error: expected ‘)’ before ‘crypto_func’
ARMV8_CRYPTO_ASSERT(crypto_func != NULL);
^
Fixes: 169ca3db550c ("crypto/armv8: add PMD optimized for ARMv8 processors")
Cc: stable@dpdk.org
Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
https://mails.dpdk.org/archives/dev/2020-July/175241.html
drivers/crypto/armv8/armv8_pmd_private.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Comments
Hello Ruifeng,
On Mon, Jul 27, 2020 at 10:58 AM Ruifeng Wang <ruifeng.wang@arm.com> wrote:
>
> Typo in debug log switch macro caused debug log cannot be enabled.
> Fixed the typo to match option defined in config file.
>
> Typo in crypto dev name macro caused unexpected device name in log.
> Fixed the typo to log with correct device name.
>
> Solved compilation error when debug log is enabled:
> rte_armv8_pmd.c: In function ‘process_armv8_chained_op’:
> rte_armv8_pmd.c:633:22: error: expected ‘)’ before ‘crypto_func’
> ARMV8_CRYPTO_ASSERT(crypto_func != NULL);
> ^
>
> Fixes: 169ca3db550c ("crypto/armv8: add PMD optimized for ARMv8 processors")
> Cc: stable@dpdk.org
>
Reported-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
> https://mails.dpdk.org/archives/dev/2020-July/175241.html
>
> drivers/crypto/armv8/armv8_pmd_private.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/crypto/armv8/armv8_pmd_private.h b/drivers/crypto/armv8/armv8_pmd_private.h
> index e08d0df78..b183c739b 100644
> --- a/drivers/crypto/armv8/armv8_pmd_private.h
> +++ b/drivers/crypto/armv8/armv8_pmd_private.h
> @@ -12,25 +12,25 @@
>
> #define ARMV8_CRYPTO_LOG_ERR(fmt, args...) \
> RTE_LOG(ERR, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
> - RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
> + RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
> __func__, __LINE__, ## args)
>
- Those macros can use a dedicated logtype for this driver rather than
pollute the CRYPTODEV general logtype.
- Looking at their uses:
drivers/crypto/armv8/armv8_pmd_private.h:#define
ARMV8_CRYPTO_LOG_ERR(fmt, args...) \
drivers/crypto/armv8/armv8_pmd_private.h:#define
ARMV8_CRYPTO_LOG_INFO(fmt, args...) \
drivers/crypto/armv8/armv8_pmd_private.h:#define
ARMV8_CRYPTO_LOG_DBG(fmt, args...) \
drivers/crypto/armv8/armv8_pmd_private.h:#define
ARMV8_CRYPTO_LOG_INFO(fmt, args...)
drivers/crypto/armv8/armv8_pmd_private.h:#define
ARMV8_CRYPTO_LOG_DBG(fmt, args...)
drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
drivers/crypto/armv8/rte_armv8_pmd.c:
ARMV8_CRYPTO_LOG_ERR("Invalid/unsupported operation");
drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
drivers/crypto/armv8/rte_armv8_pmd.c:
ARMV8_CRYPTO_LOG_ERR("failed to create cryptodev vdev");
drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
drivers/crypto/armv8/rte_armv8_pmd_ops.c:
ARMV8_CRYPTO_LOG_INFO(
drivers/crypto/armv8/rte_armv8_pmd_ops.c: ARMV8_CRYPTO_LOG_ERR(
drivers/crypto/armv8/rte_armv8_pmd_ops.c:
ARMV8_CRYPTO_LOG_ERR("invalid session struct");
drivers/crypto/armv8/rte_armv8_pmd_ops.c:
ARMV8_CRYPTO_LOG_ERR("failed configure session parameters");
There is nothing for debug, and the rest of these messages are in setup steps.
I would rather have them always enabled and remove the debug option entirely.
WDYT?
> -#ifdef RTE_LIBRTE_ARMV8_CRYPTO_DEBUG
> +#ifdef RTE_LIBRTE_PMD_ARMV8_CRYPTO_DEBUG
> #define ARMV8_CRYPTO_LOG_INFO(fmt, args...) \
> RTE_LOG(INFO, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
> - RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
> + RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
> __func__, __LINE__, ## args)
>
> #define ARMV8_CRYPTO_LOG_DBG(fmt, args...) \
> RTE_LOG(DEBUG, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
> - RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
> + RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
> __func__, __LINE__, ## args)
>
> #define ARMV8_CRYPTO_ASSERT(con) \
> do { \
> if (!(con)) { \
> - rte_panic("%s(): " \
> - con "condition failed, line %u", __func__); \
> + rte_panic("condition failed, line %u", \
> + __LINE__); \
> } \
> } while (0)
RTE_VERIFY does the same.
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, July 27, 2020 5:39 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: Akhil.goyal@nxp.com; dev <dev@dpdk.org>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>; dpdk stable
> <stable@dpdk.org>
> Subject: Re: [dpdk-dev] [PATCH] crypto/armv8: fix typos and compilation
>
> Hello Ruifeng,
>
> On Mon, Jul 27, 2020 at 10:58 AM Ruifeng Wang <ruifeng.wang@arm.com>
> wrote:
> >
> > Typo in debug log switch macro caused debug log cannot be enabled.
> > Fixed the typo to match option defined in config file.
> >
> > Typo in crypto dev name macro caused unexpected device name in log.
> > Fixed the typo to log with correct device name.
> >
> > Solved compilation error when debug log is enabled:
> > rte_armv8_pmd.c: In function ‘process_armv8_chained_op’:
> > rte_armv8_pmd.c:633:22: error: expected ‘)’ before ‘crypto_func’
> > ARMV8_CRYPTO_ASSERT(crypto_func != NULL);
> > ^
> >
> > Fixes: 169ca3db550c ("crypto/armv8: add PMD optimized for ARMv8
> > processors")
> > Cc: stable@dpdk.org
> >
>
> Reported-by: David Marchand <david.marchand@redhat.com>
>
> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> > https://mails.dpdk.org/archives/dev/2020-July/175241.html
> >
> > drivers/crypto/armv8/armv8_pmd_private.h | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/crypto/armv8/armv8_pmd_private.h
> > b/drivers/crypto/armv8/armv8_pmd_private.h
> > index e08d0df78..b183c739b 100644
> > --- a/drivers/crypto/armv8/armv8_pmd_private.h
> > +++ b/drivers/crypto/armv8/armv8_pmd_private.h
> > @@ -12,25 +12,25 @@
> >
> > #define ARMV8_CRYPTO_LOG_ERR(fmt, args...) \
> > RTE_LOG(ERR, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
> > - RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
> > + RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
> > __func__, __LINE__, ## args)
> >
>
>
> - Those macros can use a dedicated logtype for this driver rather than pollute
> the CRYPTODEV general logtype.
>
Agreed. Will add a dedicated logtype for this driver.
> - Looking at their uses:
> drivers/crypto/armv8/armv8_pmd_private.h:#define
> ARMV8_CRYPTO_LOG_ERR(fmt, args...) \
> drivers/crypto/armv8/armv8_pmd_private.h:#define
> ARMV8_CRYPTO_LOG_INFO(fmt, args...) \
> drivers/crypto/armv8/armv8_pmd_private.h:#define
> ARMV8_CRYPTO_LOG_DBG(fmt, args...) \
> drivers/crypto/armv8/armv8_pmd_private.h:#define
> ARMV8_CRYPTO_LOG_INFO(fmt, args...)
> drivers/crypto/armv8/armv8_pmd_private.h:#define
> ARMV8_CRYPTO_LOG_DBG(fmt, args...)
> drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
> drivers/crypto/armv8/rte_armv8_pmd.c:
> ARMV8_CRYPTO_LOG_ERR(
> drivers/crypto/armv8/rte_armv8_pmd.c:
> ARMV8_CRYPTO_LOG_ERR("Invalid/unsupported operation");
> drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
> drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
> drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
> drivers/crypto/armv8/rte_armv8_pmd.c:
> ARMV8_CRYPTO_LOG_ERR("failed to create cryptodev vdev");
> drivers/crypto/armv8/rte_armv8_pmd.c: ARMV8_CRYPTO_LOG_ERR(
> drivers/crypto/armv8/rte_armv8_pmd_ops.c:
> ARMV8_CRYPTO_LOG_INFO(
> drivers/crypto/armv8/rte_armv8_pmd_ops.c:
> ARMV8_CRYPTO_LOG_ERR(
> drivers/crypto/armv8/rte_armv8_pmd_ops.c:
> ARMV8_CRYPTO_LOG_ERR("invalid session struct");
> drivers/crypto/armv8/rte_armv8_pmd_ops.c:
> ARMV8_CRYPTO_LOG_ERR("failed configure session parameters");
>
> There is nothing for debug, and the rest of these messages are in setup steps.
> I would rather have them always enabled and remove the debug option
> entirely.
>
> WDYT?
>
Agreed. The logs are not in data path. They can be always enabled.
Will change in next version.
>
> > -#ifdef RTE_LIBRTE_ARMV8_CRYPTO_DEBUG
> > +#ifdef RTE_LIBRTE_PMD_ARMV8_CRYPTO_DEBUG
> > #define ARMV8_CRYPTO_LOG_INFO(fmt, args...) \
> > RTE_LOG(INFO, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
> > - RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
> > + RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
> > __func__, __LINE__, ## args)
> >
> > #define ARMV8_CRYPTO_LOG_DBG(fmt, args...) \
> > RTE_LOG(DEBUG, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
> > - RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
> > + RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
> > __func__, __LINE__, ## args)
> >
> > #define ARMV8_CRYPTO_ASSERT(con) \
> > do { \
> > if (!(con)) { \
> > - rte_panic("%s(): " \
> > - con "condition failed, line %u", __func__); \
> > + rte_panic("condition failed, line %u", \
> > + __LINE__); \
> > } \
> > } while (0)
>
>
> RTE_VERIFY does the same.
>
Yes. Will switch to RTE_VERIFY in next version.
>
> --
> David Marchand
@@ -12,25 +12,25 @@
#define ARMV8_CRYPTO_LOG_ERR(fmt, args...) \
RTE_LOG(ERR, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
+ RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
__func__, __LINE__, ## args)
-#ifdef RTE_LIBRTE_ARMV8_CRYPTO_DEBUG
+#ifdef RTE_LIBRTE_PMD_ARMV8_CRYPTO_DEBUG
#define ARMV8_CRYPTO_LOG_INFO(fmt, args...) \
RTE_LOG(INFO, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
+ RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
__func__, __LINE__, ## args)
#define ARMV8_CRYPTO_LOG_DBG(fmt, args...) \
RTE_LOG(DEBUG, CRYPTODEV, "[%s] %s() line %u: " fmt "\n", \
- RTE_STR(CRYPTODEV_NAME_ARMV8_CRYPTO_PMD), \
+ RTE_STR(CRYPTODEV_NAME_ARMV8_PMD), \
__func__, __LINE__, ## args)
#define ARMV8_CRYPTO_ASSERT(con) \
do { \
if (!(con)) { \
- rte_panic("%s(): " \
- con "condition failed, line %u", __func__); \
+ rte_panic("condition failed, line %u", \
+ __LINE__); \
} \
} while (0)