diff mbox

[dpdk-dev,v5,02/10] ethdev: make error checking macros public

Message ID 1447101259-18972-3-git-send-email-declan.doherty@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Doherty, Declan Nov. 9, 2015, 8:34 p.m. UTC 
Move the function pointer and port id checking macros to rte_ethdev and
rte_dev header files, so that they can be used in the static inline
functions there. Also replace the RTE_LOG call within
RTE_PMD_DEBUG_TRACE so this macro can be built with the -pedantic flag

Signed-off-by: Declan Doherty <declan.doherty@intel.com>
---
 lib/librte_eal/common/include/rte_dev.h | 52 +++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.c           | 54 ---------------------------------
 lib/librte_ether/rte_ethdev.h           | 26 ++++++++++++++++
 3 files changed, 78 insertions(+), 54 deletions(-)

Comments

Bruce Richardson Nov. 10, 2015, 10:32 a.m. UTC | #1 
On Mon, Nov 09, 2015 at 08:34:11PM +0000, Declan Doherty wrote:
> Move the function pointer and port id checking macros to rte_ethdev and
> rte_dev header files, so that they can be used in the static inline
> functions there. Also replace the RTE_LOG call within
> RTE_PMD_DEBUG_TRACE so this macro can be built with the -pedantic flag
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>

Thanks, Declan. I'll rebase my other patchset that was very similar to this on
top of this patch and your previous one.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/common/include/rte_dev.h | 52 +++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ethdev.c           | 54 ---------------------------------
>  lib/librte_ether/rte_ethdev.h           | 26 ++++++++++++++++
>  3 files changed, 78 insertions(+), 54 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index f601d21..fd09b3d 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -46,8 +46,60 @@
>  extern "C" {
>  #endif
>  
> +#include <stdio.h>
>  #include <sys/queue.h>
>  
> +#include <rte_log.h>
> +
> +__attribute__((format(printf, 2, 0)))
> +static inline void
> +rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
> +{
> +	va_list ap;
> +
> +	va_start(ap, fmt);
> +	char buffer[vsnprintf(NULL, 0, fmt, ap)];
> +
> +	va_end(ap);
> +
> +	va_start(ap, fmt);
> +	vsnprintf(buffer, sizeof(buffer), fmt, ap);
> +	va_end(ap);
> +
> +	rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s", func_name, buffer);
> +}
> +
> +/* Macros for checking for restricting functions to primary instance only */
> +#define RTE_PROC_PRIMARY_OR_ERR_RET(retval) do { \
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> +		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> +		return retval; \
> +	} \
> +} while (0)
> +
> +#define RTE_PROC_PRIMARY_OR_RET() do { \
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> +		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> +		return; \
> +	} \
> +} while (0)
> +
> +/* Macros to check for invalid function pointers */
> +#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
> +	if ((func) == NULL) { \
> +		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
> +		return retval; \
> +	} \
> +} while (0)
> +
> +#define RTE_FUNC_PTR_OR_RET(func) do { \
> +	if ((func) == NULL) { \
> +		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
> +		return; \
> +	} \
> +} while (0)
> +
> +
>  /** Double linked list of device drivers. */
>  TAILQ_HEAD(rte_driver_list, rte_driver);
>  
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 7387f65..d3c8aba 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -69,60 +69,6 @@
>  #include "rte_ether.h"
>  #include "rte_ethdev.h"
>  
> -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> -#define RTE_PMD_DEBUG_TRACE(fmt, args...) do {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 RTE_PROC_PRIMARY_OR_ERR_RET(retval) do { \
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> -		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> -		return (retval); \
> -	} \
> -} while (0)
> -
> -#define RTE_PROC_PRIMARY_OR_RET() do { \
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> -		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> -		return; \
> -	} \
> -} while (0)
> -
> -/* Macros to check for invalid function pointers in dev_ops structure */
> -#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
> -	if ((func) == NULL) { \
> -		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
> -		return (retval); \
> -	} \
> -} while (0)
> -
> -#define RTE_FUNC_PTR_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;
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 48a540d..9b07a0b 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -172,6 +172,8 @@ extern "C" {
>  
>  #include <stdint.h>
>  
> +#include <rte_dev.h>
> +
>  /* Use this macro to check if LRO API is supported */
>  #define RTE_ETHDEV_HAS_LRO_SUPPORT
>  
> @@ -931,6 +933,30 @@ struct rte_eth_dev_callback;
>  /** @internal Structure to keep track of registered callbacks */
>  TAILQ_HEAD(rte_eth_dev_cb_list, rte_eth_dev_callback);
>  
> +
> +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> +#define RTE_PMD_DEBUG_TRACE(...) \
> +	rte_pmd_debug_trace(__func__, __VA_ARGS__)
> +#else
> +#define RTE_PMD_DEBUG_TRACE(fmt, args...)
> +#endif
> +
> +
> +/* 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)
> +
>  /*
>   * 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*
> -- 
> 2.4.3
>
Adrien Mazarguil Nov. 10, 2015, 3:50 p.m. UTC | #2 
On Mon, Nov 09, 2015 at 08:34:11PM +0000, Declan Doherty wrote:
> Move the function pointer and port id checking macros to rte_ethdev and
> rte_dev header files, so that they can be used in the static inline
> functions there. Also replace the RTE_LOG call within
> RTE_PMD_DEBUG_TRACE so this macro can be built with the -pedantic flag
> 
> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
> ---
>  lib/librte_eal/common/include/rte_dev.h | 52 +++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ethdev.c           | 54 ---------------------------------
>  lib/librte_ether/rte_ethdev.h           | 26 ++++++++++++++++
>  3 files changed, 78 insertions(+), 54 deletions(-)
> 
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index f601d21..fd09b3d 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -46,8 +46,60 @@
>  extern "C" {
>  #endif
>  
> +#include <stdio.h>
>  #include <sys/queue.h>
>  
> +#include <rte_log.h>
> +
> +__attribute__((format(printf, 2, 0)))
> +static inline void
> +rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
> +{
> +	va_list ap;
> +
> +	va_start(ap, fmt);

I suggest adding an empty line here since we're mixing code and
declarations.

> +	char buffer[vsnprintf(NULL, 0, fmt, ap)];

I forgot an extra byte for trailing '\0' in my original comment, the above
line should read:

 char buffer[vsnprintf(NULL, 0, fmt, ap) + 1];

Otherwise the last character will be missing. Did you test that function?

> +
> +	va_end(ap);
> +
> +	va_start(ap, fmt);
> +	vsnprintf(buffer, sizeof(buffer), fmt, ap);
> +	va_end(ap);
> +
> +	rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s", func_name, buffer);
> +}
> +
> +/* Macros for checking for restricting functions to primary instance only */
> +#define RTE_PROC_PRIMARY_OR_ERR_RET(retval) do { \
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> +		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> +		return retval; \
> +	} \
> +} while (0)
> +
> +#define RTE_PROC_PRIMARY_OR_RET() do { \
> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> +		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> +		return; \
> +	} \
> +} while (0)
> +
> +/* Macros to check for invalid function pointers */
> +#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
> +	if ((func) == NULL) { \
> +		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
> +		return retval; \
> +	} \
> +} while (0)
> +
> +#define RTE_FUNC_PTR_OR_RET(func) do { \
> +	if ((func) == NULL) { \
> +		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
> +		return; \
> +	} \
> +} while (0)
> +
> +
>  /** Double linked list of device drivers. */
>  TAILQ_HEAD(rte_driver_list, rte_driver);
>  
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 7387f65..d3c8aba 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -69,60 +69,6 @@
>  #include "rte_ether.h"
>  #include "rte_ethdev.h"
>  
> -#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> -#define RTE_PMD_DEBUG_TRACE(fmt, args...) do {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 RTE_PROC_PRIMARY_OR_ERR_RET(retval) do { \
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> -		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> -		return (retval); \
> -	} \
> -} while (0)
> -
> -#define RTE_PROC_PRIMARY_OR_RET() do { \
> -	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
> -		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
> -		return; \
> -	} \
> -} while (0)
> -
> -/* Macros to check for invalid function pointers in dev_ops structure */
> -#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
> -	if ((func) == NULL) { \
> -		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
> -		return (retval); \
> -	} \
> -} while (0)
> -
> -#define RTE_FUNC_PTR_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;
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 48a540d..9b07a0b 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -172,6 +172,8 @@ extern "C" {
>  
>  #include <stdint.h>
>  
> +#include <rte_dev.h>
> +
>  /* Use this macro to check if LRO API is supported */
>  #define RTE_ETHDEV_HAS_LRO_SUPPORT
>  
> @@ -931,6 +933,30 @@ struct rte_eth_dev_callback;
>  /** @internal Structure to keep track of registered callbacks */
>  TAILQ_HEAD(rte_eth_dev_cb_list, rte_eth_dev_callback);
>  
> +
> +#ifdef RTE_LIBRTE_ETHDEV_DEBUG
> +#define RTE_PMD_DEBUG_TRACE(...) \
> +	rte_pmd_debug_trace(__func__, __VA_ARGS__)
> +#else
> +#define RTE_PMD_DEBUG_TRACE(fmt, args...)
> +#endif
> +
> +
> +/* 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)
> +
>  /*
>   * 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*
> -- 
> 2.4.3
>
Doherty, Declan Nov. 10, 2015, 5 p.m. UTC | #3 
On 10/11/15 15:50, Adrien Mazarguil wrote:
> On Mon, Nov 09, 2015 at 08:34:11PM +0000, Declan Doherty wrote:
>> Move the function pointer and port id checking macros to rte_ethdev and
>> rte_dev header files, so that they can be used in the static inline
>> functions there. Also replace the RTE_LOG call within
>> RTE_PMD_DEBUG_TRACE so this macro can be built with the -pedantic flag
>>
>> Signed-off-by: Declan Doherty <declan.doherty@intel.com>
>> ---
>>   lib/librte_eal/common/include/rte_dev.h | 52 +++++++++++++++++++++++++++++++
>>   lib/librte_ether/rte_ethdev.c           | 54 ---------------------------------
>>   lib/librte_ether/rte_ethdev.h           | 26 ++++++++++++++++
>>   3 files changed, 78 insertions(+), 54 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
>> index f601d21..fd09b3d 100644
>> --- a/lib/librte_eal/common/include/rte_dev.h
>> +++ b/lib/librte_eal/common/include/rte_dev.h
>> @@ -46,8 +46,60 @@
>>   extern "C" {
>>   #endif
>>
>> +#include <stdio.h>
>>   #include <sys/queue.h>
>>
>> +#include <rte_log.h>
>> +
>> +__attribute__((format(printf, 2, 0)))
>> +static inline void
>> +rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
>> +{
>> +	va_list ap;
>> +
>> +	va_start(ap, fmt);
>
> I suggest adding an empty line here since we're mixing code and
> declarations.
>

sure no problem.

>> +	char buffer[vsnprintf(NULL, 0, fmt, ap)];
>
> I forgot an extra byte for trailing '\0' in my original comment, the above
> line should read:
>
>   char buffer[vsnprintf(NULL, 0, fmt, ap) + 1];
>
> Otherwise the last character will be missing. Did you test that function?
>

I didn't notice the truncation in the log message, I was just missing 
and "!" in the log message and didn't miss it. I'll push a new version 
with this fix.

>> +
>> +	va_end(ap);
>> +
>> +	va_start(ap, fmt);
>> +	vsnprintf(buffer, sizeof(buffer), fmt, ap);
>> +	va_end(ap);
>> +
>> +	rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s", func_name, buffer);
>> +}
>> +
<snip>
>> --
>> 2.4.3
>>
>
diff mbox

Patch

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index f601d21..fd09b3d 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -46,8 +46,60 @@ 
 extern "C" {
 #endif
 
+#include <stdio.h>
 #include <sys/queue.h>
 
+#include <rte_log.h>
+
+__attribute__((format(printf, 2, 0)))
+static inline void
+rte_pmd_debug_trace(const char *func_name, const char *fmt, ...)
+{
+	va_list ap;
+
+	va_start(ap, fmt);
+	char buffer[vsnprintf(NULL, 0, fmt, ap)];
+
+	va_end(ap);
+
+	va_start(ap, fmt);
+	vsnprintf(buffer, sizeof(buffer), fmt, ap);
+	va_end(ap);
+
+	rte_log(RTE_LOG_ERR, RTE_LOGTYPE_PMD, "%s: %s", func_name, buffer);
+}
+
+/* Macros for checking for restricting functions to primary instance only */
+#define RTE_PROC_PRIMARY_OR_ERR_RET(retval) do { \
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+		return retval; \
+	} \
+} while (0)
+
+#define RTE_PROC_PRIMARY_OR_RET() do { \
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
+		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
+		return; \
+	} \
+} while (0)
+
+/* Macros to check for invalid function pointers */
+#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
+	if ((func) == NULL) { \
+		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
+		return retval; \
+	} \
+} while (0)
+
+#define RTE_FUNC_PTR_OR_RET(func) do { \
+	if ((func) == NULL) { \
+		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
+		return; \
+	} \
+} while (0)
+
+
 /** Double linked list of device drivers. */
 TAILQ_HEAD(rte_driver_list, rte_driver);
 
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 7387f65..d3c8aba 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -69,60 +69,6 @@ 
 #include "rte_ether.h"
 #include "rte_ethdev.h"
 
-#ifdef RTE_LIBRTE_ETHDEV_DEBUG
-#define RTE_PMD_DEBUG_TRACE(fmt, args...) do {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 RTE_PROC_PRIMARY_OR_ERR_RET(retval) do { \
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
-		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
-		return (retval); \
-	} \
-} while (0)
-
-#define RTE_PROC_PRIMARY_OR_RET() do { \
-	if (rte_eal_process_type() != RTE_PROC_PRIMARY) { \
-		RTE_PMD_DEBUG_TRACE("Cannot run in secondary processes\n"); \
-		return; \
-	} \
-} while (0)
-
-/* Macros to check for invalid function pointers in dev_ops structure */
-#define RTE_FUNC_PTR_OR_ERR_RET(func, retval) do { \
-	if ((func) == NULL) { \
-		RTE_PMD_DEBUG_TRACE("Function not supported\n"); \
-		return (retval); \
-	} \
-} while (0)
-
-#define RTE_FUNC_PTR_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;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 48a540d..9b07a0b 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -172,6 +172,8 @@  extern "C" {
 
 #include <stdint.h>
 
+#include <rte_dev.h>
+
 /* Use this macro to check if LRO API is supported */
 #define RTE_ETHDEV_HAS_LRO_SUPPORT
 
@@ -931,6 +933,30 @@  struct rte_eth_dev_callback;
 /** @internal Structure to keep track of registered callbacks */
 TAILQ_HEAD(rte_eth_dev_cb_list, rte_eth_dev_callback);
 
+
+#ifdef RTE_LIBRTE_ETHDEV_DEBUG
+#define RTE_PMD_DEBUG_TRACE(...) \
+	rte_pmd_debug_trace(__func__, __VA_ARGS__)
+#else
+#define RTE_PMD_DEBUG_TRACE(fmt, args...)
+#endif
+
+
+/* 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)
+
 /*
  * 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*