Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with NFDk

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

On 2/16/2023 10:41 AM, Chaoyong He wrote:
> 
> 
>> -----Original Message-----
>> From: Niklas Soderlund <niklas.soderlund@corigine.com>
>> Sent: Thursday, February 16, 2023 6:37 PM
>> To: Kevin Traynor <ktraynor@redhat.com>
>> Cc: Ferruh Yigit <ferruh.yigit@amd.com>; Xueming(Steven) Li
>> <xuemingl@nvidia.com>; Chaoyong He <chaoyong.he@corigine.com>;
>> dev@dpdk.org; Luca Boccassi <bluca@debian.org>; oss-drivers <oss-
>> drivers@corigine.com>; Nole Zhang <peng.zhang@corigine.com>; Kevin Liu
>> <jin.liu@corigine.com>; stable@dpdk.org
>> Subject: Re: [PATCH] net/nfp: support 48-bit DMA address for firmware with
>> NFDk
>>
>> Hi Kevin,
>>
>> Thanks for your input.
>>
>> On 2023-02-16 10:28:34 +0000, Kevin Traynor wrote:
>>> On 15/02/2023 18:28, Ferruh Yigit wrote:
>>>> On 2/15/2023 5:47 PM, Niklas Söderlund wrote:
>>>>> Hi Ferruh,
>>>>>
>>>>> Thanks for your continues effort in dealing with NFP patches.
>>>>>
>>>>> On 2023-02-15 13:42:01 +0000, Ferruh Yigit wrote:
>>>>>> On 2/8/2023 9:15 AM, Chaoyong He wrote:
>>>>>>> From: Peng Zhang <peng.zhang@corigine.com>
>>>>>>>
>>>>>>> 48-bit DMA address is supported in the firmware with NFDk, so
>>>>>>> enable this feature in PMD now. But the firmware with NFD3
>>>>>>> still just support 40-bit DMA address.
>>>>>>>
>>>>>>> RX free list descriptor, used by both NFD3 and NFDk, is also
>>>>>>> modified to support 48-bit DMA address. That's OK because the
>>>>>>> top bits is always set to 0 when assigned with 40-bit DMA address.
>>>>>>>
>>>>>>> Fixes: c73dced48c8c ("net/nfp: add NFDk Tx")
>>>>>>> Cc: jin.liu@corigine.com
>>>>>>> Cc: stable@dpdk.org
>>>>>>>
>>>>>>
>>>>>> Why a backport is requested? As far as I understand this is not
>>>>>> fixing anything but extending device capability. Is this a fix?
>>>>>
>>>>> I agree this is a bit of a grey zone. We reasoned this was a fix
>>>>> as we should have done this from the start in the commit that
>>>>> added support for NFDk. Are you OK moving forward with this as a
>>>>> fix or would you prefer we resubmit without the request to backport?
>>>>>
>>>>
>>>> I am not sure, is this change have any potential to change behavior
>>>> for existing users?
>>>> Like if one of your user is using 22.11.1 release, and if this patch
>>>> backported to next LTS version, 22.11.2, will user notice any difference?
>>>>
>>>>
>>>> @Luca, @Kevin, what is your comment as LTS maintainers?
>>>>
>>>
>>> A bit difficult to know. If NFDk is not practicably usable without it,
>>> then it could be considered a fix. If it's just extending to add
>>> nice-to-have functionality then probably it is not a fix.
>>
>> I think we can treat this as a nice-to-have and not something that makes
>> NFDk unusable. As stated above, we marked this as a Fix as we *really*
>> should have done this in the commit which added NFDk support.
>>
>> @Ferruh, would you prefer we send a v2 or will you drop the Fixes and CC
>> tags when/if applying?
>>
> 
> Actually, the DPDK app using the nfp card with a firmware of NFDk will coredump without this patch.
> And that's the directly reason we consider backport this patch.
> 

It has been long since NFDk FW support added, how a crash missed until
this point, is it crashing in a edge case or something?

>>>
>>> It would need to ensure that it is tested on 22.11 branch and there
>>> are no regressions. It is only relevant to DPDK 22.11 LTS so Cc
>>> Xueming who will ultimately decide.
>>>
>>> A guide below on some things to consider for this type of backport is here:
>>> http://doc.dpdk.org/guides/contributing/stable.html#what-changes-shoul
>>> d-be-backported
>>>
>>>>>>
>>>>>>> Signed-off-by: Peng Zhang <peng.zhang@corigine.com>
>>>>>>> Reviewed-by: Chaoyong He <chaoyong.he@corigine.com>
>>>>>>> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>>>>>>
>>>>>
>>>>
>>>
>>
>> --
>> Kind Regards,
>> Niklas Söderlund