* [dpdk-dev] [PATCH] ethdev: add function name to log message
@ 2018-10-11 17:59 Stephen Hemminger
2018-10-12 10:42 ` Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2018-10-11 17:59 UTC (permalink / raw)
To: thomas, ferruh.yigit; +Cc: dev, Stephen Hemminger
While debugging some virtio driver issues, saw:
Invalid port_id=3
but it was not clear which function was logging this because the
same log is being done in multiple places in ethdev.
Trivial fix is to prefix with function name as is done in other
drivers.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
drivers/net/failsafe/failsafe.c | 1 -
lib/librte_ethdev/rte_ethdev.h | 5 +++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c
index c3999f0265b2..3830ed33dba2 100644
--- a/drivers/net/failsafe/failsafe.c
+++ b/drivers/net/failsafe/failsafe.c
@@ -2,7 +2,6 @@
* Copyright 2017 6WIND S.A.
* Copyright 2017 Mellanox Technologies, Ltd
*/
-
#include <rte_alarm.h>
#include <rte_malloc.h>
#include <rte_ethdev_driver.h>
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 012577b0ab22..9632c67b929b 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -161,8 +161,9 @@ extern "C" {
extern int rte_eth_dev_logtype;
-#define RTE_ETHDEV_LOG(level, ...) \
- rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
+#define RTE_ETHDEV_LOG(level, fmt, ...) \
+ rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
+ "%s():" fmt, __func__, ## __VA_ARGS__)
struct rte_mbuf;
--
2.19.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add function name to log message
2018-10-11 17:59 [dpdk-dev] [PATCH] ethdev: add function name to log message Stephen Hemminger
@ 2018-10-12 10:42 ` Ferruh Yigit
2018-10-12 10:45 ` Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2018-10-12 10:42 UTC (permalink / raw)
To: Stephen Hemminger, thomas; +Cc: dev
On 10/11/2018 6:59 PM, Stephen Hemminger wrote:
> @@ -161,8 +161,9 @@ extern "C" {
>
> extern int rte_eth_dev_logtype;
>
> -#define RTE_ETHDEV_LOG(level, ...) \
> - rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
> +#define RTE_ETHDEV_LOG(level, fmt, ...) \
> + rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
> + "%s():" fmt, __func__, ## __VA_ARGS__)
+1 to adding function name, but
failsafe is giving build error [1] with clang because of ## usage [2], that is
why I add this as ` "" __VA_ARGS__` at first place but you can't do this trick
if __VA_ARGS__ used after fmt.
I am not aware of a solution for this, __VA_OPT__(,) also didn't worked with clang.
[1]
.../build/include/rte_ethdev.h:166:26: error: token pasting of ',' and
__VA_ARGS__ is a GNU extension [-Werror,-Wgnu-zero-variadic-macro-arguments]
"%s():" fmt, __func__, ## __VA_ARGS__)
^
[2]
This seems because of "-pedantic" argument driver uses, and other PMDs using
"-pedantic", like mlx, will have same error although they are disable by
default and error not observed in default build.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add function name to log message
2018-10-12 10:42 ` Ferruh Yigit
@ 2018-10-12 10:45 ` Ferruh Yigit
2018-10-12 12:43 ` Adrien Mazarguil
0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2018-10-12 10:45 UTC (permalink / raw)
To: Stephen Hemminger, thomas; +Cc: dev, Adrien Mazarguil, Gaetan Rivet
On 10/12/2018 11:42 AM, Ferruh Yigit wrote:
> On 10/11/2018 6:59 PM, Stephen Hemminger wrote:
>> @@ -161,8 +161,9 @@ extern "C" {
>>
>> extern int rte_eth_dev_logtype;
>>
>> -#define RTE_ETHDEV_LOG(level, ...) \
>> - rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
>> +#define RTE_ETHDEV_LOG(level, fmt, ...) \
>> + rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
>> + "%s():" fmt, __func__, ## __VA_ARGS__)
>
> +1 to adding function name, but
>
> failsafe is giving build error [1] with clang because of ## usage [2], that is
> why I add this as ` "" __VA_ARGS__` at first place but you can't do this trick
> if __VA_ARGS__ used after fmt.
>
> I am not aware of a solution for this, __VA_OPT__(,) also didn't worked with clang.
+cc Adrien & Gaetan,
I saw Adrien put some "workaround" to this for mlx5
>
>
> [1]
> .../build/include/rte_ethdev.h:166:26: error: token pasting of ',' and
> __VA_ARGS__ is a GNU extension [-Werror,-Wgnu-zero-variadic-macro-arguments]
> "%s():" fmt, __func__, ## __VA_ARGS__)
> ^
>
> [2]
> This seems because of "-pedantic" argument driver uses, and other PMDs using
> "-pedantic", like mlx, will have same error although they are disable by
> default and error not observed in default build.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add function name to log message
2018-10-12 10:45 ` Ferruh Yigit
@ 2018-10-12 12:43 ` Adrien Mazarguil
2018-10-12 14:54 ` Stephen Hemminger
0 siblings, 1 reply; 8+ messages in thread
From: Adrien Mazarguil @ 2018-10-12 12:43 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Stephen Hemminger, thomas, dev, Gaetan Rivet
On Fri, Oct 12, 2018 at 11:45:01AM +0100, Ferruh Yigit wrote:
> On 10/12/2018 11:42 AM, Ferruh Yigit wrote:
> > On 10/11/2018 6:59 PM, Stephen Hemminger wrote:
> >> @@ -161,8 +161,9 @@ extern "C" {
> >>
> >> extern int rte_eth_dev_logtype;
> >>
> >> -#define RTE_ETHDEV_LOG(level, ...) \
> >> - rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
> >> +#define RTE_ETHDEV_LOG(level, fmt, ...) \
> >> + rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
> >> + "%s():" fmt, __func__, ## __VA_ARGS__)
> >
> > +1 to adding function name, but
> >
> > failsafe is giving build error [1] with clang because of ## usage [2], that is
> > why I add this as ` "" __VA_ARGS__` at first place but you can't do this trick
> > if __VA_ARGS__ used after fmt.
> >
> > I am not aware of a solution for this, __VA_OPT__(,) also didn't worked with clang.
>
> +cc Adrien & Gaetan,
>
> I saw Adrien put some "workaround" to this for mlx5
Yes, through RTE_FMT() (rte_common.h). Something like this:
#define RTE_ETHDEV_LOG(level, fmt, ...) \
rte_log(RTE_LOG_ ## level, \
rte_eth_dev_logtype, \
"%s():" fmt, \
__func__, \
## __VA_ARGS__)
Can be rewritten like that:
#define RTE_ETHDEV_LOG(level, ...) \
rte_log(RTE_LOG_ ## level, \
rte_eth_dev_logtype, \
RTE_FMT("%s():" RTE_FMT_HEAD(__VA_ARGS__,), \
__func__, \
RTE_FMT_TAIL(__VA_ARGS__,)))
Although not too pretty and convenient, it does the job. In short:
- Remove "fmt" argument from prototype.
- Enclose format string and its arguments in RTE_FMT().
- Replace "fmt" with RTE_FMT_HEAD(__VA_ARGS__,).
- Replace "## __VA_ARGS__" with RTE_FMT_TAIL(__VA_ARGS__,).
- Yes, trailing commas are mandatory in RTE_FMT_(HEAD|TAIL)().
- Note it quietly appends a dummy "%.0s" argument to the format string.
> > [1]
> > .../build/include/rte_ethdev.h:166:26: error: token pasting of ',' and
> > __VA_ARGS__ is a GNU extension [-Werror,-Wgnu-zero-variadic-macro-arguments]
> > "%s():" fmt, __func__, ## __VA_ARGS__)
> > ^
> >
> > [2]
> > This seems because of "-pedantic" argument driver uses, and other PMDs using
> > "-pedantic", like mlx, will have same error although they are disable by
> > default and error not observed in default build.
> >
>
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add function name to log message
2018-10-12 12:43 ` Adrien Mazarguil
@ 2018-10-12 14:54 ` Stephen Hemminger
2018-10-16 15:55 ` Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2018-10-12 14:54 UTC (permalink / raw)
To: Adrien Mazarguil; +Cc: Ferruh Yigit, thomas, dev, Gaetan Rivet
On Fri, 12 Oct 2018 14:43:57 +0200
Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote:
> On Fri, Oct 12, 2018 at 11:45:01AM +0100, Ferruh Yigit wrote:
> > On 10/12/2018 11:42 AM, Ferruh Yigit wrote:
> > > On 10/11/2018 6:59 PM, Stephen Hemminger wrote:
> > >> @@ -161,8 +161,9 @@ extern "C" {
> > >>
> > >> extern int rte_eth_dev_logtype;
> > >>
> > >> -#define RTE_ETHDEV_LOG(level, ...) \
> > >> - rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
> > >> +#define RTE_ETHDEV_LOG(level, fmt, ...) \
> > >> + rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
> > >> + "%s():" fmt, __func__, ## __VA_ARGS__)
> > >
> > > +1 to adding function name, but
> > >
> > > failsafe is giving build error [1] with clang because of ## usage [2], that is
> > > why I add this as ` "" __VA_ARGS__` at first place but you can't do this trick
> > > if __VA_ARGS__ used after fmt.
> > >
> > > I am not aware of a solution for this, __VA_OPT__(,) also didn't worked with clang.
> >
> > +cc Adrien & Gaetan,
> >
> > I saw Adrien put some "workaround" to this for mlx5
>
> Yes, through RTE_FMT() (rte_common.h). Something like this:
>
> #define RTE_ETHDEV_LOG(level, fmt, ...) \
> rte_log(RTE_LOG_ ## level, \
> rte_eth_dev_logtype, \
> "%s():" fmt, \
> __func__, \
> ## __VA_ARGS__)
>
> Can be rewritten like that:
>
> #define RTE_ETHDEV_LOG(level, ...) \
> rte_log(RTE_LOG_ ## level, \
> rte_eth_dev_logtype, \
> RTE_FMT("%s():" RTE_FMT_HEAD(__VA_ARGS__,), \
> __func__, \
> RTE_FMT_TAIL(__VA_ARGS__,)))
>
> Although not too pretty and convenient, it does the job. In short:
>
> - Remove "fmt" argument from prototype.
> - Enclose format string and its arguments in RTE_FMT().
> - Replace "fmt" with RTE_FMT_HEAD(__VA_ARGS__,).
> - Replace "## __VA_ARGS__" with RTE_FMT_TAIL(__VA_ARGS__,).
> - Yes, trailing commas are mandatory in RTE_FMT_(HEAD|TAIL)().
> - Note it quietly appends a dummy "%.0s" argument to the format string.
>
> > > [1]
> > > .../build/include/rte_ethdev.h:166:26: error: token pasting of ',' and
> > > __VA_ARGS__ is a GNU extension [-Werror,-Wgnu-zero-variadic-macro-arguments]
> > > "%s():" fmt, __func__, ## __VA_ARGS__)
> > > ^
> > >
> > > [2]
> > > This seems because of "-pedantic" argument driver uses, and other PMDs using
> > > "-pedantic", like mlx, will have same error although they are disable by
> > > default and error not observed in default build.
> > >
> >
>
Since zero varadic macros is a GNU extension, maybe just adding GNU source?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add function name to log message
2018-10-12 14:54 ` Stephen Hemminger
@ 2018-10-16 15:55 ` Ferruh Yigit
2018-10-17 9:26 ` Gaëtan Rivet
0 siblings, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2018-10-16 15:55 UTC (permalink / raw)
To: Stephen Hemminger, Adrien Mazarguil, Gaetan Rivet; +Cc: thomas, dev
On 10/12/2018 3:54 PM, Stephen Hemminger wrote:
> On Fri, 12 Oct 2018 14:43:57 +0200
> Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote:
>
>> On Fri, Oct 12, 2018 at 11:45:01AM +0100, Ferruh Yigit wrote:
>>> On 10/12/2018 11:42 AM, Ferruh Yigit wrote:
>>>> On 10/11/2018 6:59 PM, Stephen Hemminger wrote:
>>>>> @@ -161,8 +161,9 @@ extern "C" {
>>>>>
>>>>> extern int rte_eth_dev_logtype;
>>>>>
>>>>> -#define RTE_ETHDEV_LOG(level, ...) \
>>>>> - rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
>>>>> +#define RTE_ETHDEV_LOG(level, fmt, ...) \
>>>>> + rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
>>>>> + "%s():" fmt, __func__, ## __VA_ARGS__)
>>>>
>>>> +1 to adding function name, but
>>>>
>>>> failsafe is giving build error [1] with clang because of ## usage [2], that is
>>>> why I add this as ` "" __VA_ARGS__` at first place but you can't do this trick
>>>> if __VA_ARGS__ used after fmt.
>>>>
>>>> I am not aware of a solution for this, __VA_OPT__(,) also didn't worked with clang.
>>>
>>> +cc Adrien & Gaetan,
>>>
>>> I saw Adrien put some "workaround" to this for mlx5
>>
>> Yes, through RTE_FMT() (rte_common.h). Something like this:
>>
>> #define RTE_ETHDEV_LOG(level, fmt, ...) \
>> rte_log(RTE_LOG_ ## level, \
>> rte_eth_dev_logtype, \
>> "%s():" fmt, \
>> __func__, \
>> ## __VA_ARGS__)
>>
>> Can be rewritten like that:
>>
>> #define RTE_ETHDEV_LOG(level, ...) \
>> rte_log(RTE_LOG_ ## level, \
>> rte_eth_dev_logtype, \
>> RTE_FMT("%s():" RTE_FMT_HEAD(__VA_ARGS__,), \
>> __func__, \
>> RTE_FMT_TAIL(__VA_ARGS__,)))
>>
>> Although not too pretty and convenient, it does the job. In short:
>>
>> - Remove "fmt" argument from prototype.
>> - Enclose format string and its arguments in RTE_FMT().
>> - Replace "fmt" with RTE_FMT_HEAD(__VA_ARGS__,).
>> - Replace "## __VA_ARGS__" with RTE_FMT_TAIL(__VA_ARGS__,).
>> - Yes, trailing commas are mandatory in RTE_FMT_(HEAD|TAIL)().
>> - Note it quietly appends a dummy "%.0s" argument to the format string.
>>
>>>> [1]
>>>> .../build/include/rte_ethdev.h:166:26: error: token pasting of ',' and
>>>> __VA_ARGS__ is a GNU extension [-Werror,-Wgnu-zero-variadic-macro-arguments]
>>>> "%s():" fmt, __func__, ## __VA_ARGS__)
>>>> ^
>>>>
>>>> [2]
>>>> This seems because of "-pedantic" argument driver uses, and other PMDs using
>>>> "-pedantic", like mlx, will have same error although they are disable by
>>>> default and error not observed in default build.
>>>>
>>>
>>
>
> Since zero varadic macros is a GNU extension, maybe just adding GNU source?
I think `-pedantic` is preventing using extension whether we have `_GNU_SOURCE`
or not.
Gaetan,
Why we have `-pedantic` option enabled for failsafe?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add function name to log message
2018-10-16 15:55 ` Ferruh Yigit
@ 2018-10-17 9:26 ` Gaëtan Rivet
2018-10-17 17:08 ` Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Gaëtan Rivet @ 2018-10-17 9:26 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Stephen Hemminger, Adrien Mazarguil, thomas, dev
Hi Ferruh,
On Tue, Oct 16, 2018 at 04:55:43PM +0100, Ferruh Yigit wrote:
> On 10/12/2018 3:54 PM, Stephen Hemminger wrote:
> > On Fri, 12 Oct 2018 14:43:57 +0200
> > Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote:
> >
> >> On Fri, Oct 12, 2018 at 11:45:01AM +0100, Ferruh Yigit wrote:
> >>> On 10/12/2018 11:42 AM, Ferruh Yigit wrote:
> >>>> On 10/11/2018 6:59 PM, Stephen Hemminger wrote:
> >>>>> @@ -161,8 +161,9 @@ extern "C" {
> >>>>>
> >>>>> extern int rte_eth_dev_logtype;
> >>>>>
> >>>>> -#define RTE_ETHDEV_LOG(level, ...) \
> >>>>> - rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
> >>>>> +#define RTE_ETHDEV_LOG(level, fmt, ...) \
> >>>>> + rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
> >>>>> + "%s():" fmt, __func__, ## __VA_ARGS__)
> >>>>
> >>>> +1 to adding function name, but
> >>>>
> >>>> failsafe is giving build error [1] with clang because of ## usage [2], that is
> >>>> why I add this as ` "" __VA_ARGS__` at first place but you can't do this trick
> >>>> if __VA_ARGS__ used after fmt.
> >>>>
> >>>> I am not aware of a solution for this, __VA_OPT__(,) also didn't worked with clang.
> >>>
> >>> +cc Adrien & Gaetan,
> >>>
> >>> I saw Adrien put some "workaround" to this for mlx5
> >>
> >> Yes, through RTE_FMT() (rte_common.h). Something like this:
> >>
> >> #define RTE_ETHDEV_LOG(level, fmt, ...) \
> >> rte_log(RTE_LOG_ ## level, \
> >> rte_eth_dev_logtype, \
> >> "%s():" fmt, \
> >> __func__, \
> >> ## __VA_ARGS__)
> >>
> >> Can be rewritten like that:
> >>
> >> #define RTE_ETHDEV_LOG(level, ...) \
> >> rte_log(RTE_LOG_ ## level, \
> >> rte_eth_dev_logtype, \
> >> RTE_FMT("%s():" RTE_FMT_HEAD(__VA_ARGS__,), \
> >> __func__, \
> >> RTE_FMT_TAIL(__VA_ARGS__,)))
> >>
> >> Although not too pretty and convenient, it does the job. In short:
> >>
> >> - Remove "fmt" argument from prototype.
> >> - Enclose format string and its arguments in RTE_FMT().
> >> - Replace "fmt" with RTE_FMT_HEAD(__VA_ARGS__,).
> >> - Replace "## __VA_ARGS__" with RTE_FMT_TAIL(__VA_ARGS__,).
> >> - Yes, trailing commas are mandatory in RTE_FMT_(HEAD|TAIL)().
> >> - Note it quietly appends a dummy "%.0s" argument to the format string.
> >>
> >>>> [1]
> >>>> .../build/include/rte_ethdev.h:166:26: error: token pasting of ',' and
> >>>> __VA_ARGS__ is a GNU extension [-Werror,-Wgnu-zero-variadic-macro-arguments]
> >>>> "%s():" fmt, __func__, ## __VA_ARGS__)
> >>>> ^
> >>>>
> >>>> [2]
> >>>> This seems because of "-pedantic" argument driver uses, and other PMDs using
> >>>> "-pedantic", like mlx, will have same error although they are disable by
> >>>> default and error not observed in default build.
> >>>>
> >>>
> >>
> >
> > Since zero varadic macros is a GNU extension, maybe just adding GNU source?
>
> I think `-pedantic` is preventing using extension whether we have `_GNU_SOURCE`
> or not.
>
> Gaetan,
>
> Why we have `-pedantic` option enabled for failsafe?
Because I wanted to enforce it in failsafe, and core DPDK headers were
compatible so it wasn't too much of a hassle.
If rte_ethdev.h is meant to lose this compatibility, then pedantic will
be disabled upon including it. But I guess if it was kept compatible
until now, then it was a deliberate effort?
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [PATCH] ethdev: add function name to log message
2018-10-17 9:26 ` Gaëtan Rivet
@ 2018-10-17 17:08 ` Ferruh Yigit
0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2018-10-17 17:08 UTC (permalink / raw)
To: Gaëtan Rivet; +Cc: Stephen Hemminger, Adrien Mazarguil, thomas, dev
On 10/17/2018 10:26 AM, Gaëtan Rivet wrote:
> Hi Ferruh,
>
> On Tue, Oct 16, 2018 at 04:55:43PM +0100, Ferruh Yigit wrote:
>> On 10/12/2018 3:54 PM, Stephen Hemminger wrote:
>>> On Fri, 12 Oct 2018 14:43:57 +0200
>>> Adrien Mazarguil <adrien.mazarguil@6wind.com> wrote:
>>>
>>>> On Fri, Oct 12, 2018 at 11:45:01AM +0100, Ferruh Yigit wrote:
>>>>> On 10/12/2018 11:42 AM, Ferruh Yigit wrote:
>>>>>> On 10/11/2018 6:59 PM, Stephen Hemminger wrote:
>>>>>>> @@ -161,8 +161,9 @@ extern "C" {
>>>>>>>
>>>>>>> extern int rte_eth_dev_logtype;
>>>>>>>
>>>>>>> -#define RTE_ETHDEV_LOG(level, ...) \
>>>>>>> - rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, "" __VA_ARGS__)
>>>>>>> +#define RTE_ETHDEV_LOG(level, fmt, ...) \
>>>>>>> + rte_log(RTE_LOG_ ## level, rte_eth_dev_logtype, \
>>>>>>> + "%s():" fmt, __func__, ## __VA_ARGS__)
>>>>>>
>>>>>> +1 to adding function name, but
>>>>>>
>>>>>> failsafe is giving build error [1] with clang because of ## usage [2], that is
>>>>>> why I add this as ` "" __VA_ARGS__` at first place but you can't do this trick
>>>>>> if __VA_ARGS__ used after fmt.
>>>>>>
>>>>>> I am not aware of a solution for this, __VA_OPT__(,) also didn't worked with clang.
>>>>>
>>>>> +cc Adrien & Gaetan,
>>>>>
>>>>> I saw Adrien put some "workaround" to this for mlx5
>>>>
>>>> Yes, through RTE_FMT() (rte_common.h). Something like this:
>>>>
>>>> #define RTE_ETHDEV_LOG(level, fmt, ...) \
>>>> rte_log(RTE_LOG_ ## level, \
>>>> rte_eth_dev_logtype, \
>>>> "%s():" fmt, \
>>>> __func__, \
>>>> ## __VA_ARGS__)
>>>>
>>>> Can be rewritten like that:
>>>>
>>>> #define RTE_ETHDEV_LOG(level, ...) \
>>>> rte_log(RTE_LOG_ ## level, \
>>>> rte_eth_dev_logtype, \
>>>> RTE_FMT("%s():" RTE_FMT_HEAD(__VA_ARGS__,), \
>>>> __func__, \
>>>> RTE_FMT_TAIL(__VA_ARGS__,)))
>>>>
>>>> Although not too pretty and convenient, it does the job. In short:
>>>>
>>>> - Remove "fmt" argument from prototype.
>>>> - Enclose format string and its arguments in RTE_FMT().
>>>> - Replace "fmt" with RTE_FMT_HEAD(__VA_ARGS__,).
>>>> - Replace "## __VA_ARGS__" with RTE_FMT_TAIL(__VA_ARGS__,).
>>>> - Yes, trailing commas are mandatory in RTE_FMT_(HEAD|TAIL)().
>>>> - Note it quietly appends a dummy "%.0s" argument to the format string.
>>>>
>>>>>> [1]
>>>>>> .../build/include/rte_ethdev.h:166:26: error: token pasting of ',' and
>>>>>> __VA_ARGS__ is a GNU extension [-Werror,-Wgnu-zero-variadic-macro-arguments]
>>>>>> "%s():" fmt, __func__, ## __VA_ARGS__)
>>>>>> ^
>>>>>>
>>>>>> [2]
>>>>>> This seems because of "-pedantic" argument driver uses, and other PMDs using
>>>>>> "-pedantic", like mlx, will have same error although they are disable by
>>>>>> default and error not observed in default build.
>>>>>>
>>>>>
>>>>
>>>
>>> Since zero varadic macros is a GNU extension, maybe just adding GNU source?
>>
>> I think `-pedantic` is preventing using extension whether we have `_GNU_SOURCE`
>> or not.
>>
>> Gaetan,
>>
>> Why we have `-pedantic` option enabled for failsafe?
>
> Because I wanted to enforce it in failsafe, and core DPDK headers were
> compatible so it wasn't too much of a hassle.
>
> If rte_ethdev.h is meant to lose this compatibility, then pedantic will
> be disabled upon including it. But I guess if it was kept compatible
> until now, then it was a deliberate effort?
I think it is deliberate effort, I remember Adrien put some patches in the past
to make DPDK headers pedantic compatible by adding __extension__ keywords.
I tried to use __extension__ for ## but not able to make it work, is there a way
to do this?
And I see the reasoning of making DPDK headers pedantic compatible, I think we
should keep them compatible.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-10-17 17:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-11 17:59 [dpdk-dev] [PATCH] ethdev: add function name to log message Stephen Hemminger
2018-10-12 10:42 ` Ferruh Yigit
2018-10-12 10:45 ` Ferruh Yigit
2018-10-12 12:43 ` Adrien Mazarguil
2018-10-12 14:54 ` Stephen Hemminger
2018-10-16 15:55 ` Ferruh Yigit
2018-10-17 9:26 ` Gaëtan Rivet
2018-10-17 17:08 ` Ferruh Yigit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).