On 9/14/20 5:53 PM, Keller, Jacob E wrote:
-----Original Message-----
From: Shannon Nelson <snel...@pensando.io>
Sent: Monday, September 14, 2020 4:47 PM
To: Jakub Kicinski <k...@kernel.org>; Keller, Jacob E <jacob.e.kel...@intel.com>
Cc: netdev@vger.kernel.org; da...@davemloft.net
Subject: Re: [PATCH v3 net-next 2/2] ionic: add devlink firmware update
On 9/14/20 4:36 PM, Jakub Kicinski wrote:
On Mon, 14 Sep 2020 16:15:28 -0700 Jacob Keller wrote:
On 9/10/2020 10:56 AM, Jakub Kicinski wrote:
IOW drop the component parameter from the normal helper, cause almost
nobody uses that. The add a more full featured __ version, which would
take the arg struct, the struct would include the timeout value.
I would point out that the ice driver does use it to help indicate which
section of the flash is currently being updated.
i.e.
$ devlink dev flash pci/0000:af:00.0 file firmware.bin
Preparing to flash
[fw.mgmt] Erasing
[fw.mgmt] Erasing done
[fw.mgmt] Flashing 100%
[fw.mgmt] Flashing done 100%
[fw.undi] Erasing
[fw.undi] Erasing done
[fw.undi] Flashing 100%
[fw.undi] Flashing done 100%
[fw.netlist] Erasing
[fw.netlist] Erasing done
[fw.netlist] Flashing 100%
[fw.netlist] Flashing done 100%
I'd like to keep that, as it helps tell which component is currently
being updated. If we drop this, then either I have to manually build
strings which include the component name, or we lose this information on
display.
Thanks for pointing that out. My recollection was that ice and netdevsim
were the only two users, so I thought those could use the full __*
helper and pass an arg struct. But no strong feelings.
Thanks, both.
I'd been going back and forth all morning about whether a simple single
timeout or a timeout for each "chunk" would be appropriate. I'll try to
be back in another day or three with an RFC.
sln
For ice, a timeout for each message/chunk makes the most sense, but I could see
a different reasoning when you have multiple steps all bounded by the same
timeout
Thanks,
Jake
So now we're beginning to dance around timeout boundaries - how can we
define the beginning and end of a timeout boundary, and how do they
relate to the component and label? Currently, if either the component
or status_msg changes, the devlink user program does a newline to start
a new status line. The done and total values are used from each notify
message to create a % value displayed, but are not dependent on any
previous done or total values, so the total doesn't need to be the same
value from status message to status message, even if the component and
label remain the same, devlink will just print whatever % gets
calculated that time.
I'm thinking that the behavior of the timeout value should remain
separate from the component and status_msg values, such that once given,
then the userland countdown continues on that timeout. Each subsequent
notify, regardless of component or label changes, should continue
reporting that same timeout value for as long as it applies to the
action. If a new timeout value is reported, the countdown starts over.
This continues until either the countdown finishes or the driver reports
the flash as completed. I think this allows is the flexibility for
multiple steps that Jake alludes to above. Does this make sense?
What should the userland program do when the timeout expires? Start
counting backwards? Stop waiting? Do we care to define this at the moment?
sln
