RE: [PATCH v2 3/3] baseband/acc: add internal logging

["Chautru, Nicolas" <nicolas.chautru@intel.com>]

Hi Stephen, 

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Friday, January 24, 2025 10:01 AM
> To: Chautru, Nicolas <nicolas.chautru@intel.com>
> Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
> hemant.agrawal@nxp.com; Vargas, Hernan <hernan.vargas@intel.com>
> Subject: Re: [PATCH v2 3/3] baseband/acc: add internal logging
> 
> On Fri, 24 Jan 2025 17:52:43 +0000
> "Chautru, Nicolas" <nicolas.chautru@intel.com> wrote:
> 
> > Hi Stephen,
> >
> > The commit message may be misleading, the logging interface doesn't
> change here. Note also that I reuse existing trace point framework for some
> of the error logging when relevant (see previous commit).
> > Here the scope for that buffer is more limited, not a new logging method
> really (the commit message is misleading).
> > The queue_ops_dump() already provides api to dump device specific
> information related to queue into file (logging in real time is not an option)
> based on information already in PMD memory.
> > This new buffer is purely there to add storage for the string out of
> rte_bbdev_ops_param_string() for failed operation on that queue, so that
> extend that capture as this info is not stored by PMD.
> > The name of the buffer could be renamed probably, or I could store copy
> of the actual operation instead of the string in case that makes a difference
> for you.
> >
> > I guess it would possible to move this to trace point but I thought it would
> be quite convoluted. That information would fits nicely in the queue dump
> capture, and this would require adding trace point for each operation type (I
> don't believe it can manage arbitrary string) and would be a bit of an
> unconventional use of trace point.
> >
> > Any thought?
> >
> > Thanks
> > Nic
> >
> >
> > > -----Original Message-----
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > Sent: Thursday, January 23, 2025 3:24 PM
> > > To: Chautru, Nicolas <nicolas.chautru@intel.com>
> > > Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
> > > hemant.agrawal@nxp.com; Vargas, Hernan <hernan.vargas@intel.com>
> > > Subject: Re: [PATCH v2 3/3] baseband/acc: add internal logging
> > >
> > > On Thu, 23 Jan 2025 14:55:19 -0800
> > > Nicolas Chautru <nicolas.chautru@intel.com> wrote:
> > >
> > > > Adds internal buffer for more flexible logging.
> > > >
> > > > Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
> > >
> > > Inventing another device specific error log seems like a short
> > > sighted concept.
> > > Why doesn't existing DPDK logging work well enough?
> >
> 
> My feedback is that why can't you just use DEBUG logging for this.

In practice this logging cannot be enabled as this impact real time workload. This is disabled by default for deployment as too intrusive, ie. logging some warning causing application to miss the tight real time constraints. 
Hence meaningful logging is being done in practice after the fact using trace point (ie. rte_trace_save()) and/or using the bbdev queue_ops_dump() which are both called outside of real time constraints when we can write to file system.