Re: [dpdk-dev] [PATCH 09/22] net/bnxt: use dedicated cpr for async events

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 7/22/2019 6:57 PM, Lance Richardson wrote:
> On Mon, Jul 22, 2019 at 11:06 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 22/07/2019 16:57, Ferruh Yigit:
>>> On 7/18/2019 4:36 AM, Ajit Khaparde wrote:
>>>> From: Lance Richardson <lance.richardson@broadcom.com>
>>>> --- a/config/common_base
>>>> +++ b/config/common_base
>>>> @@ -212,6 +212,7 @@ CONFIG_RTE_LIBRTE_BNX2X_DEBUG_PERIODIC=n
>>>>  # Compile burst-oriented Broadcom BNXT PMD driver
>>>>  #
>>>>  CONFIG_RTE_LIBRTE_BNXT_PMD=y
>>>> +CONFIG_RTE_LIBRTE_BNXT_SHARED_ASYNC_RING=n
>>>
>>> Normally we don't want to introduce new config time options as much as possible.
>>>
>>> This is what happens when patches slip in the last minute, please Ajit try to
>>> send patches before proposal next time.
>>>
>>> And back to the config option, is it something have to be a compile time flag
>>> (if so why?), can it be replaced by a devarg?
>>
>> I confirm it is nack.
>> I don't see any reason not to replace it with a runtime devargs.
>>
>>
> 
> This build-time configuration option was introduced because the
> "shared async completion
> ring" configuration is needed for one specific platform,
> arm64-stingray-linux-gcc. This
> configuration has a number of drawbacks and should be avoided
> otherwise. Making it
> a run-time option will add complexity and introduce the possibility
> that existing stingray
> users will not know that they need to specify this option as of 19.08,
> so it would be good
> if some other way could be found to handle this.

I see this provides a configuration transparent to user, but won't this kill the
binary portability? If a distro packages an 'armv8a' target and distributes it,
this won't be usable in your 'stingray' platform for the driver.

But if this is added as a devargs, it can be usable even with distributed
binaries, by providing proper devargs to binary for a specific platform. And
documenting this devargs in NIC guide can help users to figure it out.

> 
> Other than perhaps using RTE_ARCH_ARM64 to select the shared completion ring
> configuration (which would obviously affect all ARM64 users), are
> there any other options
> that might be acceptable?
> 
> It's maybe worthwhile to note that the last several DPDK releases use
> the "shared completion
> ring" configuration.
> 

I dropped this patch from next-net since discussion is going on, I did my best
to resolve the conflicts but please confirm the final result in next-net.