[dpdk-stable] [PATCH] telemetry: fix struct reset after value assignment

[dpdk-stable] [PATCH] telemetry: fix struct reset after value assignment

[Ciara Power]

The ep struct is used to track what type of stats are required by the
client. For PORT_STATS type, it contains the lists of port and metric
ids to query, and the number of ids in each list.

The ep struct has values set (num of port and metric ids) when a request
for port stats values by name is received. However, after this value
assignment, the struct is reset to all 0 values, meaning the number of
port and metric ids required now both show as 0, and the client will not
receive the requested data in response. To fix this issue, the memset
call is now moved above the ep struct value assignment.

Fixes: 4080e46c8078 ("telemetry: support global metrics")
Cc: reshma.pattan@intel.com
Cc: stable@dpdk.org

Signed-off-by: Ciara Power <ciara.power@intel.com>
---
 lib/librte_telemetry/rte_telemetry_parser.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_telemetry/rte_telemetry_parser.c b/lib/librte_telemetry/rte_telemetry_parser.c
index 960132397..e8c269e85 100644
--- a/lib/librte_telemetry/rte_telemetry_parser.c
+++ b/lib/librte_telemetry/rte_telemetry_parser.c
@@ -456,9 +456,9 @@ rte_telemetry_command_ports_stats_values_by_name(struct telemetry_impl
 	size_t index;
 	json_t *value;
 
+	memset(&ep, 0, sizeof(ep));
 	ep.pp.num_port_ids = json_array_size(port_ids_json);
 	ep.pp.num_metric_ids = num_stat_names;
-	memset(&ep, 0, sizeof(ep));
 	if (telemetry == NULL) {
 		TELEMETRY_LOG_ERR("Invalid telemetry argument");
 		return -1;
-- 
2.17.1

Re: [dpdk-stable] [PATCH] telemetry: fix struct reset after value assignment

[Laatz, Kevin]

> The ep struct is used to track what type of stats are required by the client.
> For PORT_STATS type, it contains the lists of port and metric ids to query, and
> the number of ids in each list.
> 
> The ep struct has values set (num of port and metric ids) when a request for
> port stats values by name is received. However, after this value assignment,
> the struct is reset to all 0 values, meaning the number of port and metric ids
> required now both show as 0, and the client will not receive the requested
> data in response. To fix this issue, the memset call is now moved above the
> ep struct value assignment.
> 
> Fixes: 4080e46c8078 ("telemetry: support global metrics")
> Cc: reshma.pattan@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ciara Power <ciara.power@intel.com>

Reviewed-by: Kevin Laatz <kevin.laatz@intel.com>

Re: [dpdk-stable] [dpdk-dev] [PATCH] telemetry: fix struct reset after value assignment

[David Marchand]

Hello,

On Thu, Feb 27, 2020 at 6:34 PM Laatz, Kevin <kevin.laatz@intel.com> wrote:
>
> > The ep struct is used to track what type of stats are required by the client.
> > For PORT_STATS type, it contains the lists of port and metric ids to query, and
> > the number of ids in each list.
> >
> > The ep struct has values set (num of port and metric ids) when a request for
> > port stats values by name is received. However, after this value assignment,
> > the struct is reset to all 0 values, meaning the number of port and metric ids
> > required now both show as 0, and the client will not receive the requested
> > data in response. To fix this issue, the memset call is now moved above the
> > ep struct value assignment.
> >
> > Fixes: 4080e46c8078 ("telemetry: support global metrics")
> > Cc: reshma.pattan@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> Reviewed-by: Kevin Laatz <kevin.laatz@intel.com>
>

For the title, how about:
telemetry: fix port stats retrieval

I can fix while applying.


-- 
David Marchand

Re: [dpdk-stable] [dpdk-dev] [PATCH] telemetry: fix struct reset after value assignment

[Power, Ciara]

Hi David, 


> Hello,
> 
> On Thu, Feb 27, 2020 at 6:34 PM Laatz, Kevin <kevin.laatz@intel.com>
> wrote:
> >
> > > The ep struct is used to track what type of stats are required by the
> client.
> > > For PORT_STATS type, it contains the lists of port and metric ids to
> > > query, and the number of ids in each list.
> > >
> > > The ep struct has values set (num of port and metric ids) when a
> > > request for port stats values by name is received. However, after
> > > this value assignment, the struct is reset to all 0 values, meaning
> > > the number of port and metric ids required now both show as 0, and
> > > the client will not receive the requested data in response. To fix
> > > this issue, the memset call is now moved above the ep struct value
> assignment.
> > >
> > > Fixes: 4080e46c8078 ("telemetry: support global metrics")
> > > Cc: reshma.pattan@intel.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> >
> > Reviewed-by: Kevin Laatz <kevin.laatz@intel.com>
> >
> 
> For the title, how about:
> telemetry: fix port stats retrieval
> 
> I can fix while applying.
> 
> 
> --
> David Marchand


That title change is okay with me, thanks.

- Ciara

Re: [dpdk-stable] [dpdk-dev] [PATCH] telemetry: fix struct reset after value assignment

[David Marchand]

On Thu, Feb 27, 2020 at 6:34 PM Laatz, Kevin <kevin.laatz@intel.com> wrote:
>
> > The ep struct is used to track what type of stats are required by the client.
> > For PORT_STATS type, it contains the lists of port and metric ids to query, and
> > the number of ids in each list.
> >
> > The ep struct has values set (num of port and metric ids) when a request for
> > port stats values by name is received. However, after this value assignment,
> > the struct is reset to all 0 values, meaning the number of port and metric ids
> > required now both show as 0, and the client will not receive the requested
> > data in response. To fix this issue, the memset call is now moved above the
> > ep struct value assignment.
> >
> > Fixes: 4080e46c8078 ("telemetry: support global metrics")
> > Cc: reshma.pattan@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> Reviewed-by: Kevin Laatz <kevin.laatz@intel.com>

Applied, thanks.


-- 
David Marchand