On 3/3/2023 7:04 PM, Long Li wrote: >> Subject: Re: [PATCH] net/mana: use RTE_LOG_DP for logs on datapath >> >> 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 <step...@networkplumber.org> wrote: >>>> >>>>> On Thu, 23 Feb 2023 14:07:25 +0000 >>>>> Ferruh Yigit <ferruh.yi...@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. >> >> 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. >> >> Until such consensus reached, I think driver should continue with logging. >> >> >> >> And for the logging, I suggest option (i) above, I was hoping more comments >> but since it is missing I hope this can be discussed in techboard for a >> conclusion. > > Hi Ferruh, > > Are you suggesting that MANA should use 'RTE_ETHDEV_DEBUG_RX/TX'? > > I'm happy to implement the logging in this way. >
Yes, that looks to me better balance for compile time / runtime config for drive developers. But it prevents product code / end user to get data path logs, although I believe this is OK I am not sure how useful datapath logs in production code, that is why I am looking for more comment for a decision. Let's wait for next techboard meeting, in case this is discussed there, before making new implementation.
