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

[Nole Zhang <peng.zhang@corigine.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Thursday, February 16, 2023 7:00 PM
> To: Chaoyong He <chaoyong.he@corigine.com>; Niklas Soderlund
> <niklas.soderlund@corigine.com>; Kevin Traynor <ktraynor@redhat.com>
> Cc: Xueming(Steven) Li <xuemingl@nvidia.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
> 
> 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?
> 
Yes, this occur in the server with CPU FT-2000/64, it has 2 PCIE1 x8 and 1 PCIE0 x16,
Pcie x8 can only support 48 bit, but the pcie16 can support 40bit.
> >>>
> >>> 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-sho
> >>> ul
> >>> 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