Re: [PATCH 07/11] net/txgbe: move wrapper to base driver

[David Marchand <david.marchand@redhat.com>]

On Mon, Sep 9, 2024 at 8:18 AM Jiawen Wu <jiawenwu@trustnetic.com> wrote:
>
> On  Sat, Sep 7, 2024 10:54 PM, David Marchand wrote:
> > BP_LOG() is only used in the base driver.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> >  drivers/net/txgbe/base/txgbe_osdep.h | 8 ++++++++
> >  drivers/net/txgbe/txgbe_logs.h       | 7 -------
> >  2 files changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/txgbe/base/txgbe_osdep.h b/drivers/net/txgbe/base/txgbe_osdep.h
> > index 62d16a6abb..91c8abf12e 100644
> > --- a/drivers/net/txgbe/base/txgbe_osdep.h
> > +++ b/drivers/net/txgbe/base/txgbe_osdep.h
> > @@ -28,6 +28,14 @@
> >  #define TMZ_VADDR(mz)  ((mz)->addr)
> >  #define TDEV_NAME(eth_dev)  ((eth_dev)->device->name)
> >
> > +extern int txgbe_logtype_bp;
> > +#define RTE_LOGTYPE_TXGBE_BP txgbe_logtype_bp
> > +#define BP_LOG(fmt, ...) \
> > +     RTE_LOG(DEBUG, TXGBE_BP, \
> > +             "[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
> > +             usec_stamp() / 1000000, usec_stamp() % 1000000, \
> > +             __func__, __LINE__, ## __VA_ARGS__)
> > +
> >  #define ASSERT(x) do {                       \
> >       if (!(x))                       \
> >               PMD_DRV_LOG(ERR, "TXGBE: %d", x);       \
> > diff --git a/drivers/net/txgbe/txgbe_logs.h b/drivers/net/txgbe/txgbe_logs.h
> > index 74f49ab9ef..b5a5a9233f 100644
> > --- a/drivers/net/txgbe/txgbe_logs.h
> > +++ b/drivers/net/txgbe/txgbe_logs.h
> > @@ -51,11 +51,4 @@ extern int txgbe_logtype_tx_free;
> >  #define DEBUGOUT(fmt, args...)    PMD_DRV_LOG(DEBUG, fmt, ##args)
> >  #define PMD_INIT_FUNC_TRACE()     PMD_DRV_LOG(DEBUG, ">>")
> >
> > -extern int txgbe_logtype_bp;
> > -#define BP_LOG(fmt, args...) \
> > -     rte_log(RTE_LOG_DEBUG, txgbe_logtype_bp, \
> > -             "[%"PRIu64".%"PRIu64"]%s(%d): " fmt, \
> > -             usec_stamp() / 1000000, usec_stamp() % 1000000, \
> > -             __func__, __LINE__, ##args)
> > -
> >  #endif /* _TXGBE_LOGS_H_ */
> > --
> > 2.46.0
> >
>
> Hi,
>
> Does this have to change? It looks a little weird.

Can you be more specific about the part that you don't like?

There is a change in behavior, I agree: messages would now be prefixed
with the logtype, here TXGBE_BP.
I can revert this part if you prefer.

As for moving the macro, in all other drivers in DPDK, the osdep.h
header provides helpers for the base driver code, so it is the right
location.


-- 
David Marchand