Re: [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Dec 26, 2022 at 12:53:57PM +0800, fengchengwen wrote:
> On 2022/12/19 17:33, Bruce Richardson wrote:
> > On Mon, Dec 19, 2022 at 09:07:20AM +0000, Chengwen Feng wrote:
> >> When telemetry callback didn't set dict and return a non-negative
> >> number, the telemetry will repeat to display the last result.
> >>
> >> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >> ---
> > 
> > Hi Chengwen,
> > 
> > I'm a little curious about this bug. Can you describe some steps to
> > reproduce it as I'm curious as to exactly what is happening. The fix seems
> > a little strange to me so I'd like to investigate a little more to see if
> > other approaches might work.
> 
> Hi Bruce,
> 
> Sorry for late reply.
> 
> The steps:
>   1. applay "[PATCH v5 1/5] dmadev: support stats reset telemetry command"
>   2. compile
>   3. start dpdk-dma: dpdk-dma -a DMA.BDF -a NIC.BDF -- -c hw
>   4. start telemetry, and execute /dmadev/stats,0, and then /dmadev/stats_reset,0
>      the output of /dmadev/stats_reset,0 will be the same of previous cmd "/dmadev/stats,0"
>      e.g. my environment:
> --> /dmadev/stats,0
> {
>   "/dmadev/stats": {
>     "submitted": 23,
>     "completed": 23,
>     "errors": 0
>   }
> }
> --> /dmadev/stats_reset,0
> {
>   "/dmadev/stats_reset": {
>     "submitted": 23,
>     "completed": 23,
>     "errors": 0
>   }
> }
> 
> The rootcause is that the /dmadev/stats_reset don't set the outer parameter "struct rte_tel_data *info"
> and return zero.
>
Thanks for the fuller explanation, I'll hopefully test it out myself.

However, in the meantime, looking at the telemetry library code, would the
following change work rather than explicitly always setting the telemetry
data to a dictionary by default? Zeroing the data by default sets it to a
null return which is what you probably want as default rather than an empty
dictionary. (And it's also a smaller diff)

/Bruce

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 8fbb4f3060..7b905355cd 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
 static void
 perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
 {
-       struct rte_tel_data data;
+       struct rte_tel_data data = {0};
 
        int ret = fn(cmd, param, &data);
        if (ret < 0) {