<div dir="auto"><div>These are very useful insights Ferruh. I think RTE_LOG_DP() is something that would have been more suitable here.</div><div dir="auto">Also, the DEBUG statement combined with a statistic will be more useful than ERR from developer perspective if they see a potential memory leak in their program. <div class="gmail_quote" dir="auto"><div dir="ltr" class="gmail_attr">On Mon, Feb 19, 2024, 3:03 PM Ferruh Yigit <<a href="mailto:ferruh.yigit@amd.com" target="_blank" rel="noreferrer">ferruh.yigit@amd.com</a>> wrote: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 2/19/2024 2:44 AM, Rushil Gupta wrote: > This was causing failure for testpmd runs (for queues >=15) > presumably due to flooding of logs due to descriptor ring being > overwritten. > > Fixes: a01854 ("net/gve: fix dqo bug for chained descriptors") > Cc: <a href="mailto:stable@dpdk.org" rel="noreferrer noreferrer" target="_blank">stable@dpdk.org</a> > > Signed-off-by: Rushil Gupta <<a href="mailto:rushilg@google.com" rel="noreferrer noreferrer" target="_blank">rushilg@google.com</a>> > Reviewed-by: Joshua Washington <<a href="mailto:joshwash@google.com" rel="noreferrer noreferrer" target="_blank">joshwash@google.com</a>> > --- > drivers/net/gve/gve_tx_dqo.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/gve/gve_tx_dqo.c b/drivers/net/gve/gve_tx_dqo.c > index 1a8eb96ea9..30a1455b20 100644 > --- a/drivers/net/gve/gve_tx_dqo.c > +++ b/drivers/net/gve/gve_tx_dqo.c > @@ -116,7 +116,7 @@ gve_tx_burst_dqo(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts) > first_sw_id = sw_id; > do { > if (sw_ring[sw_id] != NULL) > - PMD_DRV_LOG(ERR, "Overwriting an entry in sw_ring"); > + PMD_DRV_LOG(DEBUG, "Overwriting an entry in sw_ring"); > > txd = &txr[tx_id]; > sw_ring[sw_id] = tx_pkt; Hi Rushil, I will continue with this patch, BUT logging in the datapath has potential problems like this, also it may have performance impact even log is not printed, because of additional check in the log function. For datapath, you can prefer: - Add log macros controlled by RTE_ETHDEV_DEBUG_RX & RTE_ETHDEV_DEBUG_TX flags - Or use RTE_LOG_DP() macro which is compiled out if default log level is less than the log uses Also another perspective for the logs, when end-user observes this bloat of logs, what she can do? Can she change some driver arguments or environment variables to fix the issue, if not what is the point of the log? If this is a condition that can occur dynamically based on received traffic and user doesn't have control on it, maybe driver should handle the error without log? And if this is a log for driver developer, perhaps it should be assert or similar that is disabled in the release build? </blockquote></div></div></div>