On 7/1/2019 9:15 AM, Bruce Richardson wrote: > On Mon, Jul 01, 2019 at 04:57:30AM +0000, Slava Ovsiienko wrote: >> I think we should compromise: keep existing RTE_TEST_PMD_RECORD_CORE_CYCLES >> and extend with runtime switch under this build-time option: >> >> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES >> if (record_tx) >> .. gather tx related stats... >> if (record_rx) >> .. gather rx related stats... >> #endif >> >> This is very specific feature, it is needed while debugging and testing >> datapath >> routines, and It seems this feature with appropriate overhead should not be >> always enabled. >> existing build-time configuration options looks OK as for me.
+1, if we will enable this I am for having compile time config options. Only a concern about the implementation, 'RTE_TEST_PMD_RECORD_CORE_CYCLES' and 'RTE_TEST_PMD_RECORD_CORE_RX_CYCLES' are both using same variable, like 'start_rx_tsc', is there an assumption that both won't be enabled at same time? I think better to able to enable CORE_CYCLES, RX_CYCLES and TX_CYCLES separately / independently. Another think to consider, for long term - not for this patch, to move introduce RX/TX ifdefs to ethdev Rx/Tx functions so that all applications can use them, not just testpmd. >> >> Bruce, if proposed runtime extension is acceptable - I will update the patch. >> > Ok for me.> > Thanks, > /Bruce >
