net/nfp: support 48-bit DMA address for firmware with NFDk
Checks
Commit Message
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
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>
---
drivers/net/nfp/flower/nfp_flower.c | 12 ++++--------
drivers/net/nfp/flower/nfp_flower_ctrl.c | 2 +-
drivers/net/nfp/nfp_common.c | 18 ++++++++++++++++++
drivers/net/nfp/nfp_common.h | 1 +
drivers/net/nfp/nfp_ethdev.c | 11 +++--------
drivers/net/nfp/nfp_ethdev_vf.c | 11 +++--------
drivers/net/nfp/nfp_rxtx.c | 4 ++--
drivers/net/nfp/nfp_rxtx.h | 4 ++--
8 files changed, 34 insertions(+), 29 deletions(-)
Comments
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?
> 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>
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?
>
> > 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>
>
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?
>>
>>> 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>
>>
>
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.
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-should-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>
>>>
>>
>
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?
>
> 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-should-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>
> > > >
> > >
> >
>
> -----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 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
Hello again,
On 2023-02-16 11:41:13 +0100, 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.
Wops, you are right I was thinking of a different patch when replying.
Sorry about the confusion.
Yes this is a bug fix that should be back ported. Thanks Chaoyong for
correcting me.
>
> > >
> > > 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
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
> -----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
On 2/16/2023 11:11 AM, Nole Zhang wrote:
>
>
>> -----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.
OK, can you please send a new version of the patch updating patch title
and commit log to highlight that patch is fixing a crash.
It can be helpful to detail the reason of crash in commit log.
>>>>>
>>>>> 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
>
@@ -452,7 +452,7 @@ nfp_flower_pf_recv_pkts(void *rx_queue,
rxds->vals[1] = 0;
dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
rxds->fld.dd = 0;
- rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+ rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
nb_hold++;
@@ -632,13 +632,6 @@ nfp_flower_init_vnic_common(struct nfp_net_hw *hw, const char *vnic_type)
pf_dev = hw->pf_dev;
pci_dev = hw->pf_dev->pci_dev;
- /* NFP can not handle DMA addresses requiring more than 40 bits */
- if (rte_mem_check_dma_mask(40)) {
- PMD_INIT_LOG(ERR, "Device %s can not be used: restricted dma mask to 40 bits!\n",
- pci_dev->device.name);
- return -ENODEV;
- };
-
hw->device_id = pci_dev->id.device_id;
hw->vendor_id = pci_dev->id.vendor_id;
hw->subsystem_device_id = pci_dev->id.subsystem_device_id;
@@ -667,6 +660,9 @@ nfp_flower_init_vnic_common(struct nfp_net_hw *hw, const char *vnic_type)
hw->mtu = hw->max_mtu;
hw->flbufsz = DEFAULT_FLBUF_SIZE;
+ if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+ return -ENODEV;
+
/* read the Rx offset configured from firmware */
if (NFD_CFG_MAJOR_VERSION_of(hw->ver) < 2)
hw->rx_offset = NFP_NET_RX_OFFSET;
@@ -122,7 +122,7 @@ nfp_flower_ctrl_vnic_recv(void *rx_queue,
rxds->vals[1] = 0;
dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
rxds->fld.dd = 0;
- rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+ rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
nb_hold++;
@@ -1563,6 +1563,24 @@ nfp_net_set_vxlan_port(struct nfp_net_hw *hw,
return ret;
}
+/*
+ * The firmware with NFD3 can not handle DMA address requiring more
+ * than 40 bits
+ */
+int
+nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name)
+{
+ if (NFD_CFG_CLASS_VER_of(hw->ver) == NFP_NET_CFG_VERSION_DP_NFD3 &&
+ rte_mem_check_dma_mask(40) != 0) {
+ PMD_DRV_LOG(ERR,
+ "The device %s can't be used: restricted dma mask to 40 bits!",
+ name);
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
RTE_LOG_REGISTER_SUFFIX(nfp_logtype_init, init, NOTICE);
RTE_LOG_REGISTER_SUFFIX(nfp_logtype_driver, driver, NOTICE);
RTE_LOG_REGISTER_SUFFIX(nfp_logtype_cpp, cpp, NOTICE);
@@ -454,6 +454,7 @@ int nfp_net_rx_desc_limits(struct nfp_net_hw *hw,
int nfp_net_tx_desc_limits(struct nfp_net_hw *hw,
uint16_t *min_tx_desc,
uint16_t *max_tx_desc);
+int nfp_net_check_dma_mask(struct nfp_net_hw *hw, char *name);
#define NFP_NET_DEV_PRIVATE_TO_HW(adapter)\
(&((struct nfp_net_adapter *)adapter)->hw)
@@ -517,14 +517,6 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
/* Use backpointer to the CoreNIC app struct */
app_fw_nic = NFP_PRIV_TO_APP_FW_NIC(pf_dev->app_fw_priv);
- /* NFP can not handle DMA addresses requiring more than 40 bits */
- if (rte_mem_check_dma_mask(40)) {
- RTE_LOG(ERR, PMD,
- "device %s can not be used: restricted dma mask to 40 bits!\n",
- pci_dev->device.name);
- return -ENODEV;
- }
-
port = ((struct nfp_net_hw *)eth_dev->data->dev_private)->idx;
if (port < 0 || port > 7) {
PMD_DRV_LOG(ERR, "Port value is wrong");
@@ -572,6 +564,9 @@ nfp_net_init(struct rte_eth_dev *eth_dev)
hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
+ if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+ return -ENODEV;
+
if (nfp_net_ethdev_ops_mount(hw, eth_dev))
return -EINVAL;
@@ -291,14 +291,6 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
pci_dev = RTE_ETH_DEV_TO_PCI(eth_dev);
- /* NFP can not handle DMA addresses requiring more than 40 bits */
- if (rte_mem_check_dma_mask(40)) {
- RTE_LOG(ERR, PMD,
- "device %s can not be used: restricted dma mask to 40 bits!\n",
- pci_dev->device.name);
- return -ENODEV;
- }
-
hw = NFP_NET_DEV_PRIVATE_TO_HW(eth_dev->data->dev_private);
hw->ctrl_bar = (uint8_t *)pci_dev->mem_resource[0].addr;
@@ -312,6 +304,9 @@ nfp_netvf_init(struct rte_eth_dev *eth_dev)
hw->ver = nn_cfg_readl(hw, NFP_NET_CFG_VERSION);
+ if (nfp_net_check_dma_mask(hw, pci_dev->name) != 0)
+ return -ENODEV;
+
if (nfp_netvf_ethdev_ops_mount(hw, eth_dev))
return -EINVAL;
@@ -48,7 +48,7 @@ nfp_net_rx_fill_freelist(struct nfp_net_rxq *rxq)
rxd = &rxq->rxds[i];
rxd->fld.dd = 0;
- rxd->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+ rxd->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
rxd->fld.dma_addr_lo = dma_addr & 0xffffffff;
rxe[i].mbuf = mbuf;
PMD_RX_LOG(DEBUG, "[%d]: %" PRIx64, i, dma_addr);
@@ -454,7 +454,7 @@ nfp_net_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
rxds->vals[1] = 0;
dma_addr = rte_cpu_to_le_64(RTE_MBUF_DMA_ADDR_DEFAULT(new_mb));
rxds->fld.dd = 0;
- rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xff;
+ rxds->fld.dma_addr_hi = (dma_addr >> 32) & 0xffff;
rxds->fld.dma_addr_lo = dma_addr & 0xffffffff;
nb_hold++;
@@ -287,8 +287,8 @@ struct nfp_net_rx_desc {
union {
/* Freelist descriptor */
struct {
- uint8_t dma_addr_hi;
- __le16 spare;
+ __le16 dma_addr_hi;
+ uint8_t spare;
uint8_t dd;
__le32 dma_addr_lo;