Re: [PATCH] net/mana: use RTE_LOG_DP for logs on datapath

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 3 Mar 2023 13:54:23 +0000
Ferruh Yigit <ferruh.yigit@amd.com> wrote:

> On 3/3/2023 2:16 AM, Long Li wrote:
> >> Subject: Re: [PATCH] net/mana: use RTE_LOG_DP for logs on datapath
> >>
> >> On Thu, 23 Feb 2023 10:09:17 -0800
> >> Stephen Hemminger <stephen@networkplumber.org> wrote:
> >>  
> >>> On Thu, 23 Feb 2023 14:07:25 +0000
> >>> Ferruh Yigit <ferruh.yigit@amd.com> wrote:
> >>>  
> >>>> Overall I am not sure if anyone is interested in driver datapath
> >>>> logs other than driver developers themselves.
> >>>>
> >>>> For datapath logging I think there are two concerns,
> >>>> 1) It should not eat *any* cycles unless explicitly enabled
> >>>> 2) Capability of enable/disable them because of massive amount of
> >>>> log it can generate
> >>>>
> >>>>
> >>>> Currently there are two existing approaches for driver datapath logging:
> >>>> i)  Controlled by 'RTE_ETHDEV_DEBUG_RX/TX' compile time flag,
> >>>>     when enabled 'rte_log()' is used with Rx/Tx specific log type.
> >>>> ii) 'RTE_LOG_DP' ', compile time control per logtype via
> >>>>     'RTE_LOG_DP_LEVEL',
> >>>>      when enabled 'rte_log()' is used with PMD logtype.
> >>>>
> >>>>
> >>>> In (ii), need to re-compile code when you need to increase the log
> >>>> verbosity, and it leaks to production code as mentioned above.
> >>>>
> >>>> For (i), developer compiles once enabling debug, later can fine
> >>>> grain log level dynamically. This is more DPDK developer focused approach.
> >>>>
> >>>>
> >>>> [1]
> >>>> According above, what do you think to retire 'RTE_LOG_DP', (at least
> >>>> within ethdev datapath), and chose (i) as preferred datapath logging?  
> >>>
> >>> I agree, the current tx/rx logging is a mess.
> >>> Each driver is different, each driver has to have something to enable
> >>> it; and it really isn't useful beyond the driver developer.
> >>>
> >>> Using tracing seems like a much better option. Could we agree on a
> >>> common set of trace points for drivers and fix all drivers to use the same  
> >> thing.  
> >>> Probably will cause some upset among driver developers:
> >>> "where did my nice printf's go, now I have to learn tracing"
> >>> but DPDK has a good facility here, lets use it.
> >>>
> >>> My proposal would be:
> >>> 	- agree on common set of trace points
> >>> 	- apply to all drivers
> >>> 	- remove RTE_LOG_DP()
> >>> 	- remove per driver RX/TX options
> >>> 	- side effect, more uses of RTE_LOGTYPE_PMD go away.  
> >>
> >> Here is an example of using tracepoints instead.
> >> Compile tested for example only.
> >>
> >> Note: using tracepoints it is possible to keep some of the tracepoints even if
> >> fastpath is not enabled.  Things like running out of Tx or Mbuf is not something
> >> that is perf critical; but would be good for application to see.  
> > 
> > Thank you for the example.
> > 
> > I sent another patch converting data path logs (mana) to trace points.
> >   
> 
> Hi Long,
> 
> Thanks for the effort, you were quick on this while discussion is going on.
> 
> Although tracepoint is a good feature, I am not sure if it can fully
> replace the logging.
> I think usage is slightly different, trace is missing custom human
> readable message, which can be very helpful for end user.


Agree in theory, but looking at existing drivers the datapath log messages
are all just printf's of existing variables. Plus the interpretation of
what those fields and offsets only has meaning and context for someone
familiar with the driver.

> 
> And overall, it is a high level decision to switch logging to trace, it
> is inconsistent to switch only single driver, perhaps techboard (cc'ed)
> can discuss this.

Code in existing datapath logging can be easily converted driver by driver.
The problem is the lack of testing.