[dpdk-dev,v3,2/4] ethdev: move error checking macros to header
Commit Message
Move the function ptr and port id checking macros to the header file, so
that they can be used in the static inline functions there. In doxygen
comments, mark them as for internal use only.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
lib/librte_ether/rte_ethdev.c | 38 ------------------------------
lib/librte_ether/rte_ethdev.h | 54 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 38 deletions(-)
Comments
2015-11-03 12:00, Bruce Richardson:
> Move the function ptr and port id checking macros to the header file, so
> that they can be used in the static inline functions there. In doxygen
> comments, mark them as for internal use only.
[...]
> +/**
> + * @internal
> + * Macro to print a message if in debugging mode
> + */
> +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> +#define RTE_PMD_DEBUG_TRACE(fmt, args...) \
> + RTE_LOG(ERR, PMD, "%s: " fmt, __func__, ## args)
> +#else
> +#define RTE_PMD_DEBUG_TRACE(fmt, args...)
> +#endif
It does not compile because Mellanox drivers are pedantic:
In file included from /home/thomas/projects/dpdk/dpdk/drivers/net/mlx4/mlx4.c:78:0:
/home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h: At top level:
/home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h:933:38: error: ISO C does not permit named variadic macros [-Werror=variadic-macros]
#define RTE_PMD_DEBUG_TRACE(fmt, args...) \
On Wed, Nov 04, 2015 at 02:19:36AM +0100, Thomas Monjalon wrote:
> 2015-11-03 12:00, Bruce Richardson:
> > Move the function ptr and port id checking macros to the header file, so
> > that they can be used in the static inline functions there. In doxygen
> > comments, mark them as for internal use only.
> [...]
> > +/**
> > + * @internal
> > + * Macro to print a message if in debugging mode
> > + */
> > +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > +#define RTE_PMD_DEBUG_TRACE(fmt, args...) \
> > + RTE_LOG(ERR, PMD, "%s: " fmt, __func__, ## args)
> > +#else
> > +#define RTE_PMD_DEBUG_TRACE(fmt, args...)
> > +#endif
>
> It does not compile because Mellanox drivers are pedantic:
>
> In file included from /home/thomas/projects/dpdk/dpdk/drivers/net/mlx4/mlx4.c:78:0:
> /home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h: At top level:
> /home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h:933:38: error: ISO C does not permit named variadic macros [-Werror=variadic-macros]
> #define RTE_PMD_DEBUG_TRACE(fmt, args...) \
I suggest something like the following definitions as a pedantic-proof and
standard compliant method (one drawback being that it cannot be done with a
single macro), see PMD_DRV_LOG() in drivers/net/mlx5/mlx5_utils.h which also
automatically appends a line feed:
#ifdef RTE_LIBRTE_ETHDEV_DEBUG
#define STRIP(a, b) a
#define OPAREN (
#define CPAREN )
#define COMMA ,
#define RTE_PMD_DEBUG_TRACE(...) \
RTE_PMD_DEBUG_TRACE_(__VA_ARGS__ STRIP OPAREN, CPAREN)
#define RTE_PMD_DEBUG_TRACE_(fmt, ...) \
RTE_PMD_DEBUG_TRACE__(fmt COMMA __func__, __VA_ARGS__)
#define RTE_PMD_DEBUG_TRACE__(...) \
RTE_LOG(ERR, PMD, "%s: " __VA_ARGS__)
#else /* RTE_LIBRTE_ETHDEV_DEBUG */
#define RTE_PMD_DEBUG_TRACE(...)
#endif /* RTE_LIBRTE_ETHDEV_DEBUG */
STRIP() and other helper macros are used to manage the dangling comma issue
when __VA_ARGS__ is empty as in the first call below:
RTE_PMD_DEBUG_TRACE("foo\n");
RTE_PMD_DEBUG_TRACE("foo %u\n", 42);
On Wed, Nov 04, 2015 at 11:24:18AM +0100, Adrien Mazarguil wrote:
> On Wed, Nov 04, 2015 at 02:19:36AM +0100, Thomas Monjalon wrote:
> > 2015-11-03 12:00, Bruce Richardson:
> > > Move the function ptr and port id checking macros to the header file, so
> > > that they can be used in the static inline functions there. In doxygen
> > > comments, mark them as for internal use only.
> > [...]
> > > +/**
> > > + * @internal
> > > + * Macro to print a message if in debugging mode
> > > + */
> > > +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > > +#define RTE_PMD_DEBUG_TRACE(fmt, args...) \
> > > + RTE_LOG(ERR, PMD, "%s: " fmt, __func__, ## args)
> > > +#else
> > > +#define RTE_PMD_DEBUG_TRACE(fmt, args...)
> > > +#endif
> >
> > It does not compile because Mellanox drivers are pedantic:
> >
> > In file included from /home/thomas/projects/dpdk/dpdk/drivers/net/mlx4/mlx4.c:78:0:
> > /home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h: At top level:
> > /home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h:933:38: error: ISO C does not permit named variadic macros [-Werror=variadic-macros]
> > #define RTE_PMD_DEBUG_TRACE(fmt, args...) \
>
> I suggest something like the following definitions as a pedantic-proof and
> standard compliant method (one drawback being that it cannot be done with a
> single macro), see PMD_DRV_LOG() in drivers/net/mlx5/mlx5_utils.h which also
> automatically appends a line feed:
>
> #ifdef RTE_LIBRTE_ETHDEV_DEBUG
>
> #define STRIP(a, b) a
> #define OPAREN (
> #define CPAREN )
> #define COMMA ,
>
> #define RTE_PMD_DEBUG_TRACE(...) \
> RTE_PMD_DEBUG_TRACE_(__VA_ARGS__ STRIP OPAREN, CPAREN)
>
> #define RTE_PMD_DEBUG_TRACE_(fmt, ...) \
> RTE_PMD_DEBUG_TRACE__(fmt COMMA __func__, __VA_ARGS__)
>
> #define RTE_PMD_DEBUG_TRACE__(...) \
> RTE_LOG(ERR, PMD, "%s: " __VA_ARGS__)
>
> #else /* RTE_LIBRTE_ETHDEV_DEBUG */
>
> #define RTE_PMD_DEBUG_TRACE(...)
>
> #endif /* RTE_LIBRTE_ETHDEV_DEBUG */
>
> STRIP() and other helper macros are used to manage the dangling comma issue
> when __VA_ARGS__ is empty as in the first call below:
>
> RTE_PMD_DEBUG_TRACE("foo\n");
> RTE_PMD_DEBUG_TRACE("foo %u\n", 42);
>
> --
> Adrien Mazarguil
> 6WIND
Is this really the best way around this? It looks like a lot more complicated
than the original solution. Do we really need to support the -pedantic flag
in our header files?
/Bruce
On Wed, Nov 04, 2015 at 02:10:50PM +0000, Bruce Richardson wrote:
> On Wed, Nov 04, 2015 at 11:24:18AM +0100, Adrien Mazarguil wrote:
> > On Wed, Nov 04, 2015 at 02:19:36AM +0100, Thomas Monjalon wrote:
> > > 2015-11-03 12:00, Bruce Richardson:
> > > > Move the function ptr and port id checking macros to the header file, so
> > > > that they can be used in the static inline functions there. In doxygen
> > > > comments, mark them as for internal use only.
> > > [...]
> > > > +/**
> > > > + * @internal
> > > > + * Macro to print a message if in debugging mode
> > > > + */
> > > > +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> > > > +#define RTE_PMD_DEBUG_TRACE(fmt, args...) \
> > > > + RTE_LOG(ERR, PMD, "%s: " fmt, __func__, ## args)
> > > > +#else
> > > > +#define RTE_PMD_DEBUG_TRACE(fmt, args...)
> > > > +#endif
> > >
> > > It does not compile because Mellanox drivers are pedantic:
> > >
> > > In file included from /home/thomas/projects/dpdk/dpdk/drivers/net/mlx4/mlx4.c:78:0:
> > > /home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h: At top level:
> > > /home/thomas/projects/dpdk/dpdk/x86_64-native-linuxapp-gcc-shared-next/include/rte_ethdev.h:933:38: error: ISO C does not permit named variadic macros [-Werror=variadic-macros]
> > > #define RTE_PMD_DEBUG_TRACE(fmt, args...) \
> >
> > I suggest something like the following definitions as a pedantic-proof and
> > standard compliant method (one drawback being that it cannot be done with a
> > single macro), see PMD_DRV_LOG() in drivers/net/mlx5/mlx5_utils.h which also
> > automatically appends a line feed:
> >
> > #ifdef RTE_LIBRTE_ETHDEV_DEBUG
> >
> > #define STRIP(a, b) a
> > #define OPAREN (
> > #define CPAREN )
> > #define COMMA ,
> >
> > #define RTE_PMD_DEBUG_TRACE(...) \
> > RTE_PMD_DEBUG_TRACE_(__VA_ARGS__ STRIP OPAREN, CPAREN)
> >
> > #define RTE_PMD_DEBUG_TRACE_(fmt, ...) \
> > RTE_PMD_DEBUG_TRACE__(fmt COMMA __func__, __VA_ARGS__)
> >
> > #define RTE_PMD_DEBUG_TRACE__(...) \
> > RTE_LOG(ERR, PMD, "%s: " __VA_ARGS__)
> >
> > #else /* RTE_LIBRTE_ETHDEV_DEBUG */
> >
> > #define RTE_PMD_DEBUG_TRACE(...)
> >
> > #endif /* RTE_LIBRTE_ETHDEV_DEBUG */
> >
> > STRIP() and other helper macros are used to manage the dangling comma issue
> > when __VA_ARGS__ is empty as in the first call below:
> >
> > RTE_PMD_DEBUG_TRACE("foo\n");
> > RTE_PMD_DEBUG_TRACE("foo %u\n", 42);
> >
> > --
> > Adrien Mazarguil
> > 6WIND
>
> Is this really the best way around this? It looks like a lot more complicated
> than the original solution. Do we really need to support the -pedantic flag
> in our header files?
I know you didn't ask me directly but I think we should, at least for
exposed/installed headers. What we do internally does not matter, but we
cannot prevent user applications from adding -pedantic if they want to.
The above solution is one I suggest, perhaps it can be done in a different
manner if you think it's too complicated, although it's difficult using
macros only.
@@ -69,14 +69,6 @@
#include "rte_ether.h"
#include "rte_ethdev.h"
-#ifdef RTE_LIBRTE_ETHDEV_DEBUG
-#define RTE_PMD_DEBUG_TRACE(fmt, args...) do { \
- RTE_LOG(ERR, PMD, "%s: " fmt, __func__, ## args); \
- } while (0)
-#else
-#define RTE_PMD_DEBUG_TRACE(fmt, args...)
-#endif
-
/* Macros for checking for restricting functions to primary instance only */
#define PROC_PRIMARY_OR_ERR_RET(retval) do { \
if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
@@ -92,36 +84,6 @@
} \
} while (0)
-/* Macros to check for invalid function pointers in dev_ops structure */
-#define RTE_ETH_FPTR_OR_ERR_RET(func, retval) do { \
- if ((func) == NULL) { \
- RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
- return (retval); \
- } \
-} while (0)
-
-#define RTE_ETH_FPTR_OR_RET(func) do { \
- if ((func) == NULL) { \
- RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
- return; \
- } \
-} while (0)
-
-/* Macros to check for valid port */
-#define RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, retval) do { \
- if (!rte_eth_dev_is_valid_port(port_id)) { \
- RTE_PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); \
- return retval; \
- } \
-} while (0)
-
-#define RTE_ETH_VALID_PORTID_OR_RET(port_id) do { \
- if (!rte_eth_dev_is_valid_port(port_id)) { \
- RTE_PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); \
- return; \
- } \
-} while (0)
-
static const char *MZ_RTE_ETH_DEV_DATA = "rte_eth_dev_data";
struct rte_eth_dev rte_eth_devices[RTE_MAX_ETHPORTS];
static struct rte_eth_dev_data *rte_eth_dev_data;
@@ -925,6 +925,60 @@ struct rte_eth_dev_callback;
/** @internal Structure to keep track of registered callbacks */
TAILQ_HEAD(rte_eth_dev_cb_list, rte_eth_dev_callback);
+/**
+ * @internal
+ * Macro to print a message if in debugging mode
+ */
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+#define RTE_PMD_DEBUG_TRACE(fmt, args...) \
+ RTE_LOG(ERR, PMD, "%s: " fmt, __func__, ## args)
+#else
+#define RTE_PMD_DEBUG_TRACE(fmt, args...)
+#endif
+
+/**
+ * @internal
+ * Macro to check for invalid function pointer in dev_ops structure
+ */
+#define RTE_ETH_FPTR_OR_ERR_RET(func, retval) do { \
+ if ((func) == NULL) { \
+ RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
+ return retval; \
+ } \
+} while (0)
+/**
+ * @internal
+ * Macro to check for invalid function pointer in dev_ops structure
+ */
+#define RTE_ETH_FPTR_OR_RET(func) do { \
+ if ((func) == NULL) { \
+ RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
+ return; \
+ } \
+} while (0)
+
+/**
+ * @internal
+ * Macro to check for valid port id
+ */
+#define RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, retval) do { \
+ if (!rte_eth_dev_is_valid_port(port_id)) { \
+ RTE_PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); \
+ return retval; \
+ } \
+} while (0)
+
+/**
+ * @internal
+ * Macro to check for valid port id
+ */
+#define RTE_ETH_VALID_PORTID_OR_RET(port_id) do { \
+ if (!rte_eth_dev_is_valid_port(port_id)) { \
+ RTE_PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id); \
+ return; \
+ } \
+} while (0)
+
/*
* Definitions of all functions exported by an Ethernet driver through the
* the generic structure of type *eth_dev_ops* supplied in the *rte_eth_dev*