On 2/16/2023 11:53 AM, fengchengwen wrote:
> On 2023/2/15 11:19, Dongdong Liu wrote:
>> Hi Chengwen
>>
>> On 2023/2/9 10:32, Chengwen Feng wrote:
>>> The xstats reset is useful for debugging, so add it to the ethdev
>>> telemetry command lists.
>>>
>>> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com>
>> This patch looks good, so
>> Reviewed-by: Dongdong Liu <liudongdo...@huawei.com>
>>
>> A minior question
>> Do we need to support stats reset ?
>
> Stats is contained by xstats, and future direction I think is xstats.
> So I think we don't need support stats reset.
>
I have similar question with Dongdong, readonly values are safe for
telemetry, but modifying data can be more tricky since we don't have
locking in ethdev APIs, this can cause concurrency issues.
Overall do we want telemetry go that way and become something that
alters ethdev data/config?
> Thanks.
>
>>
>> Thanks,
>> Dongdong
>>> ---
>>> lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++++++
>>> 1 file changed, 31 insertions(+)
>>>
>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>> index d25db35f7f..e85c98f307 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -5915,6 +5915,35 @@ eth_dev_handle_port_xstats(const char *cmd
>>> __rte_unused,
>>> return 0;
>>> }
>>>
>>> +static int
>>> +eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
>>> + const char *params,
>>> + struct rte_tel_data *d)
>>> +{
>>> + int port_id, ret;
>>> + char *end_param;
>>> +
>>> + if (params == NULL || strlen(params) == 0 || !isdigit(*params))
>>> + return -1;
>>> +
>>> + port_id = strtoul(params, &end_param, 0);
>>> + if (*end_param != '\0')
>>> + RTE_ETHDEV_LOG(NOTICE,
>>> + "Extra parameters passed to ethdev telemetry command,
>>> ignoring\n");
>>> + if (!rte_eth_dev_is_valid_port(port_id))
>>> + return -1;
>>> +
>>> + ret = rte_eth_xstats_reset(port_id);
>>> + if (ret == 0) {
>>> + rte_tel_data_string(d, "success");
>>> + RTE_ETHDEV_LOG(NOTICE, "Port %d reset xstats success\n", port_id);
>>> + } else {
>>> + RTE_ETHDEV_LOG(ERR, "Port %d reset xstats failed! ret: %d\n",
>>> port_id, ret);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> #ifndef RTE_EXEC_ENV_WINDOWS
>>> static int
>>> eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
>>> @@ -6329,6 +6358,8 @@ RTE_INIT(ethdev_init_telemetry)
>>> "Returns the common stats for a port. Parameters: int
>>> port_id");
>>> rte_telemetry_register_cmd("/ethdev/xstats",
>>> eth_dev_handle_port_xstats,
>>> "Returns the extended stats for a port. Parameters: int
>>> port_id");
>>> + rte_telemetry_register_cmd("/ethdev/xstats_reset",
>>> eth_dev_handle_port_xstats_reset,
>>> + "Reset the extended stats for a port. Parameters: int
>>> port_id");
>>> #ifndef RTE_EXEC_ENV_WINDOWS
>>> rte_telemetry_register_cmd("/ethdev/dump_priv",
>>> eth_dev_handle_port_dump_priv,
>>> "Returns dump private information for a port. Parameters: int
>>> port_id");
>>>
>> .
