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

["Pattan, Reshma" <reshma.pattan@intel.com>]

> -----Original Message-----
> From: Burakov, Anatoly
> Sent: Wednesday, May 22, 2019 10:46 AM
> 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
> 
> On 21-May-19 3:53 PM, Pattan, Reshma wrote:
> >>
> >>> +			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?


@  input rate  ~120mpps, with 1 core , 2 ports, 2 queues per port.
I have tested below 2 cases

1)l3fwd-power telemetry  mode w/o stats and locks,
2)l3fwd-power telemetry with stats and locks

The case 2 has 0.32% of packet loss compared to case1. 

> 
> >
> >>>
> >>>    			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?
> 

Will increase the timer value to 500ms.

Thanks,
Reshma