[dpdk-dev] [PATCH 0/6] app/testpmd: add runtime config

[Ferruh Yigit]

On 8/26/2020 6:07 PM, Dharmik Thakkar wrote:
> 
> 
>> On Aug 26, 2020, at 11:41 AM, Bruce Richardson <bruce.richardson at intel.com> wrote:
>>
>> On Wed, Aug 26, 2020 at 05:33:20PM +0100, Ferruh Yigit wrote:
>>> On 7/14/2020 10:51 PM, Dharmik Thakkar wrote:
>>>> Meson build system lacks support for
>>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options.
>>>>
>>>> One solution is to add these options within meson_options.txt
>>>>
>>>> Since adding these as runtime config causes no performance impact,
>>>
>>> Hi Dharmik,
>>>
>>> These are on the datapath, and even disable there will be additional
>>> checks, isn't it expected to have some impact?  Did you do any
>>> measurements for it?
> 
> Hi Ferruh,
> 
> In my measurements, I saw a maximum performance degradation of 0.3% for rx and tx throughput (pps)
> with runtime option (both disabled and enabled cases) when compared to compile time option..
> There was no difference in cycles per packet measurement.
> I did these measurements with Mellanox-ConnectX-5 card on N1SDP server with ’set fwd mac retry’ option.
>

Thanks Dharmik,

0.3% does not look a lot, I also don't get any recognizable drop when the stats
are disabled (there is a drop (up to %15) when stats are enabled but that is OK).
And there is a benefit to have the stats runtime configurable.

This minor drop for runtime configuration looks OK to me, but lets wait a little
more if there is any strong objection to it, we can proceed afterwards.

> 
>>>
>> Branches that always predict the same way can be very cheap, and unless
>> proven to be a problem, I'd see no issue with having a few on the datapath
>> - especially if it just one or two per burst. If we start seeing a
>> significant number, or ones that occur for every packet, then we perhaps
>> need to be more cautious.
>>
>> I also think that using lots of different CFLAGS for turning things on and
>> off is as bad - or even worse - than using the old build-time config
>> options, just this time we don't have a single list of them somewhere!
>> Therefore, I think we really need to start converting these to runtime
>> options where we can.
>>
>> Regards,
>> /Bruce
> 
> 
>