[v3,02/16] bpf: stop using variadic argument pack extension
Checks
Commit Message
Use RTE_LOG_LINE_PREFIX instead of RTE_LOG_LINE in macro expansions
which allow a prefix and arguments to be inserted into the log line
without the need to use the ## args variadic argument pack extension.
Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
lib/bpf/bpf_impl.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
> Subject: [PATCH v3 02/16] bpf: stop using variadic argument pack extension
>
> Use RTE_LOG_LINE_PREFIX instead of RTE_LOG_LINE in macro expansions
> which allow a prefix and arguments to be inserted into the log line
> without the need to use the ## args variadic argument pack extension.
>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> lib/bpf/bpf_impl.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/bpf/bpf_impl.h b/lib/bpf/bpf_impl.h
> index 1a3d97d..680b1e5 100644
> --- a/lib/bpf/bpf_impl.h
> +++ b/lib/bpf/bpf_impl.h
> @@ -29,8 +29,8 @@ struct rte_bpf {
> extern int rte_bpf_logtype;
> #define RTE_LOGTYPE_BPF rte_bpf_logtype
>
> -#define RTE_BPF_LOG_LINE(lvl, fmt, args...) \
> - RTE_LOG_LINE(lvl, BPF, fmt, ##args)
> +#define RTE_BPF_LOG_LINE(level, ...) \
> + RTE_LOG_LINE_PREFIX(level, BPF, "%s(): ", __func__, __VA_ARGS__)
>
> static inline size_t
> bpf_size(uint32_t bpf_op_sz)
> --
Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> 1.8.3.1
On Mon, Feb 26, 2024 at 9:20 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
>
> Use RTE_LOG_LINE_PREFIX instead of RTE_LOG_LINE in macro expansions
> which allow a prefix and arguments to be inserted into the log line
> without the need to use the ## args variadic argument pack extension.
>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> lib/bpf/bpf_impl.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/bpf/bpf_impl.h b/lib/bpf/bpf_impl.h
> index 1a3d97d..680b1e5 100644
> --- a/lib/bpf/bpf_impl.h
> +++ b/lib/bpf/bpf_impl.h
> @@ -29,8 +29,8 @@ struct rte_bpf {
> extern int rte_bpf_logtype;
> #define RTE_LOGTYPE_BPF rte_bpf_logtype
>
> -#define RTE_BPF_LOG_LINE(lvl, fmt, args...) \
> - RTE_LOG_LINE(lvl, BPF, fmt, ##args)
> +#define RTE_BPF_LOG_LINE(level, ...) \
> + RTE_LOG_LINE_PREFIX(level, BPF, "%s(): ", __func__, __VA_ARGS__)
The patch $topic seems to be removal of variadic argument extension.
So, I would expect a simple:
+#define RTE_BPF_LOG_LINE(level, ...) \
+ RTE_LOG_LINE(level, BPF, __VA_ARGS__)
Konstantin, just to be sure, are you ok with this (debug from my pov)
prefix addition?
>
> On Mon, Feb 26, 2024 at 9:20 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> >
> > Use RTE_LOG_LINE_PREFIX instead of RTE_LOG_LINE in macro expansions
> > which allow a prefix and arguments to be inserted into the log line
> > without the need to use the ## args variadic argument pack extension.
> >
> > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> > lib/bpf/bpf_impl.h | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/bpf/bpf_impl.h b/lib/bpf/bpf_impl.h
> > index 1a3d97d..680b1e5 100644
> > --- a/lib/bpf/bpf_impl.h
> > +++ b/lib/bpf/bpf_impl.h
> > @@ -29,8 +29,8 @@ struct rte_bpf {
> > extern int rte_bpf_logtype;
> > #define RTE_LOGTYPE_BPF rte_bpf_logtype
> >
> > -#define RTE_BPF_LOG_LINE(lvl, fmt, args...) \
> > - RTE_LOG_LINE(lvl, BPF, fmt, ##args)
> > +#define RTE_BPF_LOG_LINE(level, ...) \
> > + RTE_LOG_LINE_PREFIX(level, BPF, "%s(): ", __func__, __VA_ARGS__)
>
> The patch $topic seems to be removal of variadic argument extension.
> So, I would expect a simple:
> +#define RTE_BPF_LOG_LINE(level, ...) \
> + RTE_LOG_LINE(level, BPF, __VA_ARGS__)
>
> Konstantin, just to be sure, are you ok with this (debug from my pov)
> prefix addition?
>
Thanks David for spotting it, yes somehow I missed that.
Actually yes, yours variant looks correct to me.
Konstantin.
On Wed, Feb 28, 2024 at 01:34:59PM +0000, Konstantin Ananyev wrote:
>
> >
> > On Mon, Feb 26, 2024 at 9:20 PM Tyler Retzlaff
> > <roretzla@linux.microsoft.com> wrote:
> > >
> > > Use RTE_LOG_LINE_PREFIX instead of RTE_LOG_LINE in macro expansions
> > > which allow a prefix and arguments to be inserted into the log line
> > > without the need to use the ## args variadic argument pack extension.
> > >
> > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > ---
> > > lib/bpf/bpf_impl.h | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/lib/bpf/bpf_impl.h b/lib/bpf/bpf_impl.h
> > > index 1a3d97d..680b1e5 100644
> > > --- a/lib/bpf/bpf_impl.h
> > > +++ b/lib/bpf/bpf_impl.h
> > > @@ -29,8 +29,8 @@ struct rte_bpf {
> > > extern int rte_bpf_logtype;
> > > #define RTE_LOGTYPE_BPF rte_bpf_logtype
> > >
> > > -#define RTE_BPF_LOG_LINE(lvl, fmt, args...) \
> > > - RTE_LOG_LINE(lvl, BPF, fmt, ##args)
> > > +#define RTE_BPF_LOG_LINE(level, ...) \
> > > + RTE_LOG_LINE_PREFIX(level, BPF, "%s(): ", __func__, __VA_ARGS__)
> >
> > The patch $topic seems to be removal of variadic argument extension.
> > So, I would expect a simple:
> > +#define RTE_BPF_LOG_LINE(level, ...) \
> > + RTE_LOG_LINE(level, BPF, __VA_ARGS__)
> >
> > Konstantin, just to be sure, are you ok with this (debug from my pov)
> > prefix addition?
> >
>
> Thanks David for spotting it, yes somehow I missed that.
> Actually yes, yours variant looks correct to me.
> Konstantin.
oh, sorry about this. i did not intend to add prefixes where there were
none. would you like me to restore this with David's suggestion or would
you like to keep the prefix with __func__?
let me know i'll make an adjustment to the series if necessary.
>
> On Wed, Feb 28, 2024 at 01:34:59PM +0000, Konstantin Ananyev wrote:
> >
> > >
> > > On Mon, Feb 26, 2024 at 9:20 PM Tyler Retzlaff
> > > <roretzla@linux.microsoft.com> wrote:
> > > >
> > > > Use RTE_LOG_LINE_PREFIX instead of RTE_LOG_LINE in macro expansions
> > > > which allow a prefix and arguments to be inserted into the log line
> > > > without the need to use the ## args variadic argument pack extension.
> > > >
> > > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > > ---
> > > > lib/bpf/bpf_impl.h | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/lib/bpf/bpf_impl.h b/lib/bpf/bpf_impl.h
> > > > index 1a3d97d..680b1e5 100644
> > > > --- a/lib/bpf/bpf_impl.h
> > > > +++ b/lib/bpf/bpf_impl.h
> > > > @@ -29,8 +29,8 @@ struct rte_bpf {
> > > > extern int rte_bpf_logtype;
> > > > #define RTE_LOGTYPE_BPF rte_bpf_logtype
> > > >
> > > > -#define RTE_BPF_LOG_LINE(lvl, fmt, args...) \
> > > > - RTE_LOG_LINE(lvl, BPF, fmt, ##args)
> > > > +#define RTE_BPF_LOG_LINE(level, ...) \
> > > > + RTE_LOG_LINE_PREFIX(level, BPF, "%s(): ", __func__, __VA_ARGS__)
> > >
> > > The patch $topic seems to be removal of variadic argument extension.
> > > So, I would expect a simple:
> > > +#define RTE_BPF_LOG_LINE(level, ...) \
> > > + RTE_LOG_LINE(level, BPF, __VA_ARGS__)
> > >
> > > Konstantin, just to be sure, are you ok with this (debug from my pov)
> > > prefix addition?
> > >
> >
> > Thanks David for spotting it, yes somehow I missed that.
> > Actually yes, yours variant looks correct to me.
> > Konstantin.
>
> oh, sorry about this. i did not intend to add prefixes where there were
> none. would you like me to restore this with David's suggestion or would
> you like to keep the prefix with __func__?
Please restore as David suggested.
No worries, after all I didn't spot it myself, all kudos goes to David :)
On Wed, Feb 28, 2024 at 05:27:54PM +0000, Konstantin Ananyev wrote:
>
>
> > On Wed, Feb 28, 2024 at 01:34:59PM +0000, Konstantin Ananyev wrote:
> > >
> > > >
> > > > On Mon, Feb 26, 2024 at 9:20 PM Tyler Retzlaff
> > > > <roretzla@linux.microsoft.com> wrote:
> > > > >
> > > > > Use RTE_LOG_LINE_PREFIX instead of RTE_LOG_LINE in macro expansions
> > > > > which allow a prefix and arguments to be inserted into the log line
> > > > > without the need to use the ## args variadic argument pack extension.
> > > > >
> > > > > Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > > > > ---
> > > > > lib/bpf/bpf_impl.h | 4 ++--
> > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/lib/bpf/bpf_impl.h b/lib/bpf/bpf_impl.h
> > > > > index 1a3d97d..680b1e5 100644
> > > > > --- a/lib/bpf/bpf_impl.h
> > > > > +++ b/lib/bpf/bpf_impl.h
> > > > > @@ -29,8 +29,8 @@ struct rte_bpf {
> > > > > extern int rte_bpf_logtype;
> > > > > #define RTE_LOGTYPE_BPF rte_bpf_logtype
> > > > >
> > > > > -#define RTE_BPF_LOG_LINE(lvl, fmt, args...) \
> > > > > - RTE_LOG_LINE(lvl, BPF, fmt, ##args)
> > > > > +#define RTE_BPF_LOG_LINE(level, ...) \
> > > > > + RTE_LOG_LINE_PREFIX(level, BPF, "%s(): ", __func__, __VA_ARGS__)
> > > >
> > > > The patch $topic seems to be removal of variadic argument extension.
> > > > So, I would expect a simple:
> > > > +#define RTE_BPF_LOG_LINE(level, ...) \
> > > > + RTE_LOG_LINE(level, BPF, __VA_ARGS__)
> > > >
> > > > Konstantin, just to be sure, are you ok with this (debug from my pov)
> > > > prefix addition?
> > > >
> > >
> > > Thanks David for spotting it, yes somehow I missed that.
> > > Actually yes, yours variant looks correct to me.
> > > Konstantin.
> >
> > oh, sorry about this. i did not intend to add prefixes where there were
> > none. would you like me to restore this with David's suggestion or would
> > you like to keep the prefix with __func__?
>
> Please restore as David suggested.
> No worries, after all I didn't spot it myself, all kudos goes to David :)
thanks, i'll submit a new rev!
>
@@ -29,8 +29,8 @@ struct rte_bpf {
extern int rte_bpf_logtype;
#define RTE_LOGTYPE_BPF rte_bpf_logtype
-#define RTE_BPF_LOG_LINE(lvl, fmt, args...) \
- RTE_LOG_LINE(lvl, BPF, fmt, ##args)
+#define RTE_BPF_LOG_LINE(level, ...) \
+ RTE_LOG_LINE_PREFIX(level, BPF, "%s(): ", __func__, __VA_ARGS__)
static inline size_t
bpf_size(uint32_t bpf_op_sz)