On 3/19/2020 1:50 PM, Viacheslav Ovsiienko wrote:
> The execution time of rx/tx burst routine depends on the burst size.
> It would be meaningful to research this dependency, the patch
> provides an extra profiling data per rx/tx burst size.
>
> Signed-off-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
<...>
> @@ -1568,6 +1568,23 @@ struct extmem_param {
> }
> printf(" + %d%% of %d pkts + %d%% of others]\n",
> burst_percent[1], (int) pktnb_stats[1], burst_percent[2]);
> +#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> + if (!(fwdprof_flags & (nrx_tx ? RECORD_CORE_CYCLES_TX
> + : RECORD_CORE_CYCLES_RX)))
> + return;
> + for (nb_pkt = 0; nb_pkt < MAX_PKT_BURST; nb_pkt++) {
> + nb_burst = nrx_tx ? pbs->pkt_retry_spread[nb_pkt]
> + : pbs->pkt_burst_spread[nb_pkt];
> + if (nb_burst == 0)
> + continue;
Why "nb_burst == 0" excluded, wouldn't be interesting to see number of calls
with 0 Rx/Tx and cycles spent there?
> + printf(" CPU cycles/%u packet burst=%u (total cycles="
> + "%"PRIu64" / total %s bursts=%u)\n",
> + (unsigned int)nb_pkt,
> + (unsigned int)(pbs->pkt_ticks_spread[nb_pkt] / nb_burst),
> + pbs->pkt_ticks_spread[nb_pkt],
> + nrx_tx ? "TX" : "RX", nb_burst);
> + }
> +#endif
> }
> #endif /* RTE_TEST_PMD_RECORD_BURST_STATS */
>
> @@ -1601,8 +1618,8 @@ struct extmem_param {
> }
>
> #ifdef RTE_TEST_PMD_RECORD_BURST_STATS
> - pkt_burst_stats_display("RX", &fs->rx_burst_stats);
> - pkt_burst_stats_display("TX", &fs->tx_burst_stats);
> + pkt_burst_stats_display(false, &fs->rx_burst_stats);
> + pkt_burst_stats_display(true, &fs->tx_burst_stats);
Instead of this true/false, I would be OK to some duplication and have explicit
Rx/Tx, I believe it would be more clear that way, but no strong opinion on it.
<...>
> +++ b/app/test-pmd/testpmd.h
> @@ -89,6 +89,10 @@ enum {
> */
> struct pkt_burst_stats {
> unsigned int pkt_burst_spread[MAX_PKT_BURST];
> +#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> + unsigned int pkt_retry_spread[MAX_PKT_BURST];
Isn't this keep the value of all Tx burst count, why named as 'retry'?
