Re: [PATCH v8 05/12] net/nfp: add flower PF setup logic

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

On 9/9/2022 3:36 AM, Chaoyong He wrote:
>> On 9/8/2022 9:44 AM, Chaoyong He wrote:
>>> Adds the vNIC initialization logic for the flower PF vNIC. The flower
>>> firmware exposes this vNIC for the purposes of fallback traffic in the
>>> switchdev use-case.
>>>
>>> Adds minimal dev_ops for this PF device. Because the device is being
>>> exposed externally to DPDK it should also be configured using DPDK
>>> helpers like rte_eth_configure(). For these helpers to work the flower
>>> logic needs to implements a minimal set of dev_ops.
>>>
>>> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>> ---
>>>    drivers/net/nfp/flower/nfp_flower.c            | 398
>> ++++++++++++++++++++++++-
>>>    drivers/net/nfp/flower/nfp_flower.h            |   6 +
>>>    drivers/net/nfp/flower/nfp_flower_ovs_compat.h |  37 +++
>>
>> Can you please detail why OVS specific header is required? Having application
>> specific code in PMD can be sign of some design issue, that is why can you
>> please explain more what it does?
>>
> 
> Basically, there exist two layers polling mode to move a pkt from firmware to OVS.
> 
> When our card using flower application firmware receive pkt and find the pkt can't be offloaded,
> it will record the input port in a place of the pkt, we call it metadata.
> 
> There exist a rte_ring for each representor port.
> 
> We use the pf device as a multiplexer, which keeps polling pkts from the firmware.
> Based on the metadata, it will enqueue the pkt into the rte_ring of the corresponding representor port.
> 
> On the OVS side, it will keeps try to dequeue the pkt from the rte_ring of the representor port.
> Once it gets the pkt, the OVS will go its logic and treat the pkt as `struct dp_packet`.
> 
> So we copy the definition of `struct dp_packet` from OVS to prevent the coredump caused by memory read/write out of range.
> 
> Another possible way is defining a big enough mbuf_priv_len using macro to prevent this structure definition from OVS.
> Is this the right way?
> 

Thanks for clarification.

PMD is creating a mempool to hold 'struct dp_packet', that is why it is 
duplicating OvS 'struct dp_packet' struct definition to DPDK.

But many PMDs are used with OvS, why only nfp need to duplicate the 
struct or create OvS specific mempool?
Having application specific code in driver looks wrong anyway.


@Ian, @David,

Can you please help on OvS related change in the driver? What is the 
proper way to support it?


Thanks,
ferruh