Re: [PATCH v2 25/25] net/nfp: support new tunnel solution

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

On 10/25/2022 2:44 AM, Chaoyong He wrote:
>> On 10/22/2022 9:24 AM, Chaoyong He wrote:
>>> The new version of flower firmware application add the support of a
>>> new tunnel solution.
>>>
>>> It changes the structure of tunnel neighbor, and use a feature flag to
>>> indicate which tunnel solution is used.
>>>
>>> Add the logic of read extra features from firmware, and store it in
>>> the app private structure.
>>>
>>> Adjust the data structure and related logic to make the PMD support
>>> both version of tunnel solutions.
>>>
>>> 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      |  14 ++++
>>>    drivers/net/nfp/flower/nfp_flower.h      |  24 +++++++
>>>    drivers/net/nfp/flower/nfp_flower_cmsg.c |   4 ++
>>>    drivers/net/nfp/flower/nfp_flower_cmsg.h |  17 +++++
>>>    drivers/net/nfp/nfp_flow.c               | 118 +++++++++++++++++++++++++-
>> -----
>>>    5 files changed, 157 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/net/nfp/flower/nfp_flower.c
>>> b/drivers/net/nfp/flower/nfp_flower.c
>>> index 41b0fe2..aa8199d 100644
>>> --- a/drivers/net/nfp/flower/nfp_flower.c
>>> +++ b/drivers/net/nfp/flower/nfp_flower.c
>>> @@ -1074,6 +1074,8 @@
>>>    nfp_init_app_fw_flower(struct nfp_pf_dev *pf_dev)
>>>    {
>>>    	int ret;
>>> +	int err;
>>> +	uint64_t ext_features;
>>>    	unsigned int numa_node;
>>>    	struct nfp_net_hw *pf_hw;
>>>    	struct nfp_net_hw *ctrl_hw;
>>> @@ -1115,6 +1117,18 @@
>>>    		goto vnic_cleanup;
>>>    	}
>>>
>>> +	/* Read the extra features */
>>> +	ext_features = nfp_rtsym_read_le(pf_dev->sym_tbl,
>> "_abi_flower_extra_features",
>>> +			&err);
>>> +	if (err != 0) {
>>> +		PMD_INIT_LOG(ERR, "Couldn't read extra features from fw");
>>> +		ret = -EIO;
>>> +		goto pf_cpp_area_cleanup;
>>> +	}
>>
>> Hi Chaoyong,
>>
>> It looks like there are two flavor of the flower firmware application, one with
>> 'extra_features' other without it.
>> Does this worth documenting in the driver documentation and the release
>> notes?
> 
> Actually, it's just two different methods to process the tunnel decap action in the flower
> firmware application.
> 
> The old version flower firmware application needs 'tunnel neighbor' and 'pre-tunnel' table
> to get needed information to decap the tunnel packet.
> While the new version flower firmware application extends the 'tunnel neighbor' table and
> does not need 'pre-tunnel' table anymore when decap the tunnel packet.
> 
> The app which use the rte_flow know nothing about this difference.
> So, should we still explain this in the documentation and the release notes? I'm not quite sure
> about how details should we expose in these documents.

Thanks for clarification, if this is transparent to user/app may not 
need to document.