Re: [dpdk-dev] [PATCH v1] examples/l3fwd-power: add telemetry mode support

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 21-May-19 3:53 PM, Pattan, Reshma wrote:
> 
> 
>> -----Original Message-----
>> From: Burakov, Anatoly
>> Sent: Monday, May 20, 2019 2:17 PM
>> To: Pattan, Reshma <reshma.pattan@intel.com>; dev@dpdk.org
>> Cc: Hunt, David <david.hunt@intel.com>; Ma, Liang J <liang.j.ma@intel.com>
>> Subject: Re: [PATCH v1] examples/l3fwd-power: add telemetry mode support
>>
> 
> <snip>
> 
>>> ---
>>
>> <snip>
>>
>>> +			poll_count = 0;
>>> +			prev_tel_tsc = cur_tsc;
>>> +			/* update stats for telemetry */
>>> +			rte_spinlock_lock(&stats[lcore_id].telemetry_lock);
>>> +			stats[lcore_id].ep_nep[0] = ep_nep[0];
>>> +			stats[lcore_id].ep_nep[1] = ep_nep[1];
>>> +			stats[lcore_id].fp_nfp[0] = fp_nfp[0];
>>> +			stats[lcore_id].fp_nfp[1] = fp_nfp[1];
>>> +			stats[lcore_id].br = br;
>>> +			rte_spinlock_unlock(&stats[lcore_id].telemetry_lock);
>>
>> Locking here seems relatively rare (per-lcore and once every N polls), but any
>> locking on a hotpath makes me nervous. What is the current performance
>> impact of this? Should we bother improving?
> 
> The performance impact is negligible, in thousands.

In thousands of packets? Out of?

> 
>>>
>>>    			if (!strncmp(lgopts[option_index].name,
>>> @@ -1869,6 +2068,52 @@ init_power_library(void)
>>>    	return ret;
>>>    }
>>>    static void
>>> +update_telemetry(__attribute__((unused)) struct rte_timer *tim,
>>> +		__attribute__((unused)) void *arg)
>>> +{
>>
>> I would question the need to put telemetry on a high precision 10ms timer. Is
>> there any reason why we cannot gather telemetry, say, once every 100ms, and
>> why we cannot do so from interrupt thread using alarm API? Using high-
>> precision timer API here seems like an overkill.
> 
> The l3-power uses the timers , so followed the same. But I am ok
> to use ALARM api.

Maybe just change the timer period then? 10ms to update telemetry looks 
way too often to me. Do we really expect telemetry to be gathered every 
10ms?

> 
> Thanks,
> Reshma
> 


-- 
Thanks,
Anatoly