[dpdk-dev] [PATCH v4 02/18] telemetry: move code to metrics for later reuse

[Power, Ciara]

Hi Stephen,
 
Some of your comments are addressed in the next patch, which reduces this code copied from the existing telemetry library. 
Some others I will fix for v5.


>From: Stephen Hemminger <stephen at networkplumber.org>
>Sent: Friday 24 April 2020 16:30
>To: Power, Ciara <ciara.power at intel.com>
>Cc: dev at dpdk.org; Laatz, Kevin <kevin.laatz at intel.com>; Pattan, Reshma
><reshma.pattan at intel.com>; jerinjacobk at gmail.com;
>david.marchand at redhat.com; Wiles, Keith <keith.wiles at intel.com>;
>mb at smartsharesystems.com; thomas at monjalon.net
>Subject: Re: [dpdk-dev] [PATCH v4 02/18] telemetry: move code to metrics for
>later reuse
>
>On Fri, 24 Apr 2020 13:41:43 +0100
>Ciara Power <ciara.power at intel.com> wrote:
>
>> This commit moves some of the telemetry library code to a new file in
>> the metrics library. No modifications are made to the moved code,
>> except what is needed to allow it to compile and run. The additional
>> code in metrics is built only when the Jansson library is  present.
>> Telemetry functions as normal, using the functions from the
>> metrics_telemetry file. This move will enable code be reused by the
>> new version of telemetry in a later commit, to support backward
>> compatibility with the existing telemetry usage.
>>
>> Signed-off-by: Ciara Power <ciara.power at intel.com>
>
>
>Minor comments, none of these are show stoppers.

<snip>

>> +static int32_t
>> +rte_metrics_tel_update_metrics_ethdev(uint16_t port_id, int
>> +reg_start_index) {
>> +	int ret, num_xstats, i;
>> +	struct rte_eth_xstat *eth_xstats;
>> +
>> +	if (!rte_eth_dev_is_valid_port(port_id)) {
>> +		METRICS_LOG_ERR("port_id: %d is invalid", port_id);
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = rte_metrics_tel_is_port_active(port_id);
>> +	if (ret < 1)
>> +		return -EINVAL;
>> +
>> +	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
>> +	if (num_xstats < 0) {
>> +		METRICS_LOG_ERR("rte_eth_xstats_get(%u) failed: %d",
>port_id,
>> +				num_xstats);
>> +		return -EPERM;
>> +	}
>
>The number of metrics on a port should not change (as long as it has not been
>hot plugged). So you could optimize by knowing number of stats from last
>query.
>

Requerying is safer, this code is for backward compatibility and the existing telemetry 
requeries the number of stats each time.
 
<snip>


Thanks,
Ciara