[dpdk-stable] [PATCH v2 1/5] app/testpmd: clock gettime call in throughput calculation

[Ferruh Yigit]

On 6/26/2020 11:09 PM, Honnappa Nagarahalli wrote:
> The throughput calculation requires a counter that measures
> passing of time. However, the kernel saves and restores the PMU
> state when a thread is unscheduled and scheduled. This ensures
> that the PMU cycles are not counted towards a thread that is
> not scheduled. Hence, when RTE_ARM_EAL_RDTSC_USE_PMU is enabled,
> the PMU cycles do not represent the passing of time.

Does this mean 'rte_rdtsc()' is broken for Arm?
Wouldn't it cause more serious issue than testpmd throughput stats?

> This results in incorrect calculation of throughput numbers.
> Use clock_gettime system call to calculate the time passed since
> last call.
> 
> Bugzilla ID: 450
> Fixes: 0e106980301d ("app/testpmd: show throughput in port stats")
> Cc: zhihong.wang at intel.com
> Cc: stable at dpdk.org
> 
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
> Reviewed-by: Phil Yang <phil.yang at arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang at arm.com>
> Tested-by: Phil Yang <phil.yang at arm.com>
> Tested-by: Ali Alnubani <alialnu at mellanox.com>

<...>

> @@ -195,10 +204,17 @@ nic_stats_display(portid_t port_id)
>  		}
>  	}
>  
> -	diff_cycles = prev_cycles[port_id];
> -	prev_cycles[port_id] = rte_rdtsc();
> -	if (diff_cycles > 0)
> -		diff_cycles = prev_cycles[port_id] - diff_cycles;
> +	diff_ns = 0;
> +	if (clock_gettime(CLOCK_TYPE_ID, &cur_time) == 0) {

I guess 'rte_rdtsc()' is faster, but since this is not in the data path, I think
it is OK.

<...>

> @@ -217,10 +233,10 @@ nic_stats_display(portid_t port_id)
>  		(stats.obytes - prev_bytes_tx[port_id]) : 0;
>  	prev_bytes_rx[port_id] = stats.ibytes;
>  	prev_bytes_tx[port_id] = stats.obytes;
> -	mbps_rx = diff_cycles > 0 ?
> -		diff_bytes_rx * rte_get_tsc_hz() / diff_cycles : 0;
> -	mbps_tx = diff_cycles > 0 ?
> -		diff_bytes_tx * rte_get_tsc_hz() / diff_cycles : 0;
> +	mbps_rx = diff_ns > 0 ?
> +		(double)diff_bytes_rx / diff_ns * NS_PER_SEC : 0;
> +	mbps_tx = diff_ns > 0 ?
> +		(double)diff_bytes_tx / diff_ns * NS_PER_SEC : 0;

The calculation also fixes other issue in the stats.
With previous method, if the sampling between two stats is a little long,
"diff_pkts_rx * rte_get_tsc_hz()" can overflow and produce wrong 'bps'.


Reviewed-by: Ferruh Yigit <ferruh.yigit at intel.com>