Re: [PATCH 0/8] refactor logic to support secondary process

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

On 5/21/2024 12:24 PM, Chaoyong He wrote:
>> On 5/21/2024 3:17 AM, Chaoyong He wrote:
>>>> On 4/19/2024 6:23 AM, Chaoyong He wrote:
>>>>> Refactor data structure and related logic to make the secondary
>>>>> process can work as expect.
>>>>>
>>>>
>>>> Hi Chaoyong,
>>>>
>>>> Patchset looks good, but I have a question related to the motivation
>>>> of moving so many structs to process private data?
>>>>
>>>> Normally ethdev is process private, and ethdev->data is shared.
>>>> Primary configures the device and secondary learns shared data and
>>>> uses it for datapath.
>>>> There are cases, like file descriptors for same file can be different
>>>> for different process and process private structure is used.
>>>>
>>>> In below patches, device private data level structs seems moved to
>>>> the process private structure, is the intention both primary process
>>>> and secondary process do the control path and configuration?
>>>> If so, just a reminder that this is not intended usage of the
>>>> multi-process support and many control APIs are not designed as thread-
>> safe.
>>>>
>>>> Would you mind describing a little more about your use case that
>>>> requires below process private data changes?
>>>
>>> The main motivation of these changes is to solve the problems when
>>> customers using the secondary process (they use primary process for
>> monitor and secondary process for rx/tx/configuration ...).
>>>
>>
>> Got it, if you want to do 'configuration' on the secondary, it requires more
>> information, and as this is not shared you need to move them to process
>> private.
>>
>> This approach requires synchronizing secondaries for this control path.
>> So more work for the application layer.
>>
>> I understand you are trying to enable your customer, and this is a solution but
>> I am not sure this is the best approach. And this solution won't be portable,
>> because many PMDs won't support configuring from secondary.
>>
>> Can you guide your customer that configuring on primary and limit
>> secondaries for the datapath?
> 
> Sorry, but it is almost certain that it is impossible to make customer modify their architecture.
> Because they have complex software stack, and they themselves are only user of the software stack.
> They are just responsible for the basic NIC adaptation work and the upper software architecture are 
> design and maintain by some other department.
> 
> For this generation NFP hardware and firmware architecture, seems this is the only best solution
> we can achieve.
> 
> But for the next generation NIC and firmware (we already in development, and will publish in a near future),
> we will have one PCI BDF address for one physical port and use a unified firmware.
> Then the problems we meet for now will not exist anymore, and we can make the logic just as what you said.
> 

Ack, it looks like you are making an informed decision, although it is
not ideal, I will proceed with the set.


>> This way only some limited information is required to shared with secondaries
>> (lets say like firmware application version) and these can be passed via shared
>> data or even MP communication.
>>
>> Although this is not a hard requirement, I believe this can make both your and
>> your customer's life easier in long run.
>>
>>
>>> The NFP card support different firmware applications, this means we
>>> need read message from firmware to decide to run which logic.
>>>
>>> And with single-pf firmware (this means one PCI BDF address for multi
>>> physical ports), we also need read message (how many ports totally) from
>> firmware before attach to the primary process.
>>>
>>> All this relates with CPP and symbol table, and they are different for primary
>> process and secondary process.
>>> If still put them in the process shared section (ethdev->data), the
>>> assignment logic in secondary process will overwrite it and cause the primary
>> process crash.
>>>
>>> So we move them into the process private section (ethdev-
>>> process_private).
>>>
>>
>>
>