[0/8] optimize the firmware loading process

  This patch series aims to speedup the DPDK application start by optimize
the firmware loading process in sereval places.
We also simplify the port name in multiple PF firmware case to make the
customer happy.

Peng Zhang (8):
  net/nfp: add the interface for getting the firmware name
  net/nfp: speed up the firmware loading process
  net/nfp: optimize loading the firmware process
  net/nfp: enlarge the range of skipping loading the firmware
  net/nfp: add the step of clearing the beat time
  net/nfp: add the elf module
  net/nfp: reload the firmware only when firmware changed
  net/nfp: simplify the port name for multiple PFs

 drivers/net/nfp/meson.build       |    1 +
 drivers/net/nfp/nfp_ethdev.c      |  264 +++++--
 drivers/net/nfp/nfp_net_common.c  |   17 +
 drivers/net/nfp/nfp_net_common.h  |    2 +
 drivers/net/nfp/nfpcore/nfp_elf.c | 1078 +++++++++++++++++++++++++++++
 drivers/net/nfp/nfpcore/nfp_elf.h |   13 +
 drivers/net/nfp/nfpcore/nfp_mip.c |   30 +-
 drivers/net/nfp/nfpcore/nfp_mip.h |   70 +-
 8 files changed, 1388 insertions(+), 87 deletions(-)
 create mode 100644 drivers/net/nfp/nfpcore/nfp_elf.c
 create mode 100644 drivers/net/nfp/nfpcore/nfp_elf.h
  

On 1/15/2024 2:54 AM, Chaoyong He wrote:
> This patch series aims to speedup the DPDK application start by optimize
> the firmware loading process in sereval places.
> We also simplify the port name in multiple PF firmware case to make the
> customer happy.
> 

<...>

>   net/nfp: add the elf module
>   net/nfp: reload the firmware only when firmware changed

Above commit adds elf parser capability and second one loads firmware
when build time is different.

I can see this is an optimization effort, to understand FW status before
loading FW, but relying on build time seems fragile. Does it help to add
a new section to store version information and evaluate based on this
information?
  

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, January 22, 2024 11:09 PM
> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
> Cc: oss-drivers <oss-drivers@corigine.com>
> Subject: Re: [PATCH 0/8] optimize the firmware loading process
> 
> On 1/15/2024 2:54 AM, Chaoyong He wrote:
> > This patch series aims to speedup the DPDK application start by
> > optimize the firmware loading process in sereval places.
> > We also simplify the port name in multiple PF firmware case to make
> > the customer happy.
> >
> 
> <...>
> 
> >   net/nfp: add the elf module
> >   net/nfp: reload the firmware only when firmware changed
> 
> Above commit adds elf parser capability and second one loads firmware when
> build time is different.
> 
> I can see this is an optimization effort, to understand FW status before loading
> FW, but relying on build time seems fragile. Does it help to add a new section
> to store version information and evaluate based on this information?
> 

We have a branch of firmware (several app type combined with NFD3/NFDk) with the same version information(monthly publish),
so the version information can't help us, because we can't distinguish among them.

But the build time is different for every firmware, and that's the reason we choose it.
  

On 1/23/2024 1:46 AM, Chaoyong He wrote:
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Monday, January 22, 2024 11:09 PM
>> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
>> Cc: oss-drivers <oss-drivers@corigine.com>
>> Subject: Re: [PATCH 0/8] optimize the firmware loading process
>>
>> On 1/15/2024 2:54 AM, Chaoyong He wrote:
>>> This patch series aims to speedup the DPDK application start by
>>> optimize the firmware loading process in sereval places.
>>> We also simplify the port name in multiple PF firmware case to make
>>> the customer happy.
>>>
>>
>> <...>
>>
>>>   net/nfp: add the elf module
>>>   net/nfp: reload the firmware only when firmware changed
>>
>> Above commit adds elf parser capability and second one loads firmware when
>> build time is different.
>>
>> I can see this is an optimization effort, to understand FW status before loading
>> FW, but relying on build time seems fragile. Does it help to add a new section
>> to store version information and evaluate based on this information?
>>
> 
> We have a branch of firmware (several app type combined with NFD3/NFDk) with the same version information(monthly publish),
> so the version information can't help us, because we can't distinguish among them.
> 
> But the build time is different for every firmware, and that's the reason we choose it.
> 

If version is same although FW itself is different, isn't this problem
on its own? Perhaps an additional field is required in version syntax.
  

> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> Sent: Monday, January 22, 2024 11:09 PM
> >> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
> >> Cc: oss-drivers <oss-drivers@corigine.com>
> >> Subject: Re: [PATCH 0/8] optimize the firmware loading process
> >>
> >> On 1/15/2024 2:54 AM, Chaoyong He wrote:
> >>> This patch series aims to speedup the DPDK application start by
> >>> optimize the firmware loading process in sereval places.
> >>> We also simplify the port name in multiple PF firmware case to make
> >>> the customer happy.
> >>>
> >>
> >> <...>
> >>
> >>>   net/nfp: add the elf module
> >>>   net/nfp: reload the firmware only when firmware changed
> >>
> >> Above commit adds elf parser capability and second one loads firmware
> >> when build time is different.
> >>
> >> I can see this is an optimization effort, to understand FW status
> >> before loading FW, but relying on build time seems fragile. Does it
> >> help to add a new section to store version information and evaluate based
> on this information?
> >>
> >
> > We have a branch of firmware (several app type combined with
> > NFD3/NFDk) with the same version information(monthly publish), so the
> version information can't help us, because we can't distinguish among them.
> >
> > But the build time is different for every firmware, and that's the reason we
> choose it.
> >
> 
> If version is same although FW itself is different, isn't this problem on its own?
> Perhaps an additional field is required in version syntax.

Actually, it's just the role the build time which embed in the firmware plays, which is unique for every firmware, and which can't be modified once the firmware was published.

What you said also has its mean, but in practice we can't accept(at least can't do it immediately), which needs to discuss with firmware team.

If you insist that we should change the design, maybe we can just kick off two commits, and upstream the other commits?
- net/nfp: add the elf module
- net/nfp: reload the firmware only when firmware changed
  

On 1/23/2024 11:27 AM, Chaoyong He wrote:
>>>> -----Original Message-----
>>>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>>>> Sent: Monday, January 22, 2024 11:09 PM
>>>> To: Chaoyong He <chaoyong.he@corigine.com>; dev@dpdk.org
>>>> Cc: oss-drivers <oss-drivers@corigine.com>
>>>> Subject: Re: [PATCH 0/8] optimize the firmware loading process
>>>>
>>>> On 1/15/2024 2:54 AM, Chaoyong He wrote:
>>>>> This patch series aims to speedup the DPDK application start by
>>>>> optimize the firmware loading process in sereval places.
>>>>> We also simplify the port name in multiple PF firmware case to make
>>>>> the customer happy.
>>>>>
>>>>
>>>> <...>
>>>>
>>>>>   net/nfp: add the elf module
>>>>>   net/nfp: reload the firmware only when firmware changed
>>>>
>>>> Above commit adds elf parser capability and second one loads firmware
>>>> when build time is different.
>>>>
>>>> I can see this is an optimization effort, to understand FW status
>>>> before loading FW, but relying on build time seems fragile. Does it
>>>> help to add a new section to store version information and evaluate based
>> on this information?
>>>>
>>>
>>> We have a branch of firmware (several app type combined with
>>> NFD3/NFDk) with the same version information(monthly publish), so the
>> version information can't help us, because we can't distinguish among them.
>>>
>>> But the build time is different for every firmware, and that's the reason we
>> choose it.
>>>
>>
>> If version is same although FW itself is different, isn't this problem on its own?
>> Perhaps an additional field is required in version syntax.
> 
> Actually, it's just the role the build time which embed in the firmware plays, which is unique for every firmware, and which can't be modified once the firmware was published.
> 

I see it is already in the elf binary but relying on build time of a FW
to decide to load it or not looks a weak method to me, and fragile as
stated before.

> What you said also has its mean, but in practice we can't accept(at least can't do it immediately), which needs to discuss with firmware team.
> 

As it is an optimization I assume it is not urgent, so would you mind
discussing the issue with the FW team, perhaps it can lead to a better
solution, we can proceed after that.

Meanwhile I will continue with remaining patches, excluding these two
patches.

> If you insist that we should change the design, maybe we can just kick off two commits, and upstream the other commits?
> - net/nfp: add the elf module
> - net/nfp: reload the firmware only when firmware changed
  

On 1/15/2024 2:54 AM, Chaoyong He wrote:
> This patch series aims to speedup the DPDK application start by optimize
> the firmware loading process in sereval places.
> We also simplify the port name in multiple PF firmware case to make the
> customer happy.
> 
> Peng Zhang (8):
>   net/nfp: add the interface for getting the firmware name
>   net/nfp: speed up the firmware loading process
>   net/nfp: optimize loading the firmware process
>   net/nfp: enlarge the range of skipping loading the firmware
>   net/nfp: add the step of clearing the beat time
>   net/nfp: add the elf module
>   net/nfp: reload the firmware only when firmware changed
>   net/nfp: simplify the port name for multiple PFs
> 

Except 6/8 & 7/8,
Series applied to dpdk-next-net/main, thanks.
  

> On 1/25/2024 2:06 AM, Chaoyong He wrote:
> >>>>>> On 1/15/2024 2:54 AM, Chaoyong He wrote:
> >>>>>>> This patch series aims to speedup the DPDK application start by
> >>>>>>> optimize the firmware loading process in sereval places.
> >>>>>>> We also simplify the port name in multiple PF firmware case to
> >>>>>>> make the customer happy.
> >>>>>>>
> >>>>>>
> >>>>>> <...>
> >>>>>>
> >>>>>>>   net/nfp: add the elf module
> >>>>>>>   net/nfp: reload the firmware only when firmware changed
> >>>>>>
> >>>>>> Above commit adds elf parser capability and second one loads
> >>>>>> firmware when build time is different.
> >>>>>>
> >>>>>> I can see this is an optimization effort, to understand FW status
> >>>>>> before loading FW, but relying on build time seems fragile. Does
> >>>>>> it help to add a new section to store version information and
> >>>>>> evaluate based
> >>>> on this information?
> >>>>>>
> >>>>>
> >>>>> We have a branch of firmware (several app type combined with
> >>>>> NFD3/NFDk) with the same version information(monthly publish), so
> >>>>> the
> >>>> version information can't help us, because we can't distinguish
> >>>> among
> >> them.
> >>>>>
> >>>>> But the build time is different for every firmware, and that's the
> >>>>> reason we
> >>>> choose it.
> >>>>>
> >>>>
> >>>> If version is same although FW itself is different, isn't this
> >>>> problem on its
> >> own?
> >>>> Perhaps an additional field is required in version syntax.
> >>>
> >>> Actually, it's just the role the build time which embed in the
> >>> firmware plays,
> >> which is unique for every firmware, and which can't be modified once
> >> the firmware was published.
> >>>
> >>
> >> I see it is already in the elf binary but relying on build time of a
> >> FW to decide to load it or not looks a weak method to me, and fragile as
> stated before.
> >>
> >>> What you said also has its mean, but in practice we can't accept(at
> >>> least can't
> >> do it immediately), which needs to discuss with firmware team.
> >>>
> >>
> >> As it is an optimization I assume it is not urgent, so would you mind
> >> discussing the issue with the FW team, perhaps it can lead to a
> >> better solution, we can proceed after that.
> >
> > After discussing with the FW team, seems we still can't just use version
> because our firmware are published monthly.
> >
> > Between two public version, we also have internal versions like 'rc0, rc1'
> scheme just as DPDK.
> > But the problem is, we may compile and share many firmware in daily
> development.
> > They are maybe only valid for one time and will never publish, so we won't
> assign a version for them.
> >
> 
> So is my understanding correct that this effort is not for customers and not for
> published FWs, but mostly for developers with development FWs.
> 
> If so perhaps it can be simplified even more, there can be a devarg that force
> loads the FW, for development scenario.
> By default, driver checks the FW version and loads according, but if flag is set it
> loads the FW even with same version. When a new development FW is out,
> developer can run DPDK app with this parameter and it loads the FW to test,
> does this make sense.

Yeah, it's a perfect method, thanks.

> 
> I am not trying to be difficult, just the proposed approach didn't satisfy me I
> am trying to understand it and help if I can. And if you please continue
> discussion in the mailing list it enables others to comment and perhaps
> provide a better option.
> 
> 
> > So the build time is the only information we can distinguish them, if just
> using version, PMD will not reload the firmware which needs to test and
> debug.
> >
> > How about we first using version to check, and then using build time to
> check for the same version?
> >
> 
> Nope, I though elf parsing and relying on build time can be removed, if you
> have them anyway, no need to add the version check, can use just the build
> time to check.
> 
> 
> And another related question, if developers are using daily FWs with exact
> same version, when a developer had an unexpected behavior, how she will
> know exactly which version of the FW (in the device), this maybe required to
> track down the problem. Shouldn't each FW have a unique identifier, except
> the "build time" info  which is stored in the elf binary not FW itself?

We will use the devarg and version as your advice, and remove the build time logics.

> 
> >>
> >> Meanwhile I will continue with remaining patches, excluding these two
> >> patches.
> >>
> >>> If you insist that we should change the design, maybe we can just
> >>> kick off
> >> two commits, and upstream the other commits?
> >>> - net/nfp: add the elf module
> >>> - net/nfp: reload the firmware only when firmware changed
> >