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 <step...@networkplumber.org>
>Sent: Friday 24 April 2020 16:30
>To: Power, Ciara <ciara.po...@intel.com>
>Cc: dev@dpdk.org; Laatz, Kevin <kevin.la...@intel.com>; Pattan, Reshma
><reshma.pat...@intel.com>; jerinjac...@gmail.com;
>david.march...@redhat.com; Wiles, Keith <keith.wi...@intel.com>;
>m...@smartsharesystems.com; tho...@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.po...@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.po...@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
