Re: [PATCH v2 net-next 2/2] ionic: add devlink firmware update

From: Jakub Kicinski <kuba@kernel.org>
To: Shannon Nelson <snelson@pensando.io>
Cc: netdev@vger.kernel.org, davem@davemloft.net
Subject: Re: [PATCH v2 net-next 2/2] ionic: add devlink firmware update
Date: Sat, 5 Sep 2020 15:19:14 -0700	[thread overview]
Message-ID: <20200905151914.339b00e0@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <328d7cc0-9bf9-3051-52d5-9f7ac2fd4075@pensando.io>

On Sat, 5 Sep 2020 15:06:07 -0700 Shannon Nelson wrote:
> On 9/5/20 1:04 PM, Jakub Kicinski wrote:
> > On Thu,  3 Sep 2020 17:05:34 -0700 Shannon Nelson wrote:  
> >> +/* The worst case wait for the install activity is about 25 minutes when
> >> + * installing a new CPLD, which is very seldom.  Normal is about 30-35
> >> + * seconds.  Since the driver can't tell if a CPLD update will happen we
> >> + * set the timeout for the ugly case.  
> > 25 minutes is really quite scary. And user will see no notification
> > other than "Installing 50%" for 25 minutes? And will most likely not
> > be able to do anything that'd involve talking to the FW, like setting
> > port info/speed?  
> 
> Yeah, it's pretty annoying, and the READMEs with the FW will need to 
> warn that the install time will be much longer than usual.
> 
> > Is it possible for the FW to inform that it's updating the CPLD?  
> 
> We don't have any useful feedback mechanism for this kind of thing, but 
> I'll think about how it might work and see if I can get something from 
> the FW folks.  Another option would be for the driver to learn how to 
> read the FW blob, but I'd really rather not go down that road.

Yes, parsing the firmware blobs in the drivers in not advisable.

> > Can you release the dev_cmd_lock periodically to allow other commands
> > to be executed while waiting?  
> 
> I think this could be done.  I suspect I'll need to give the dev_cmd the 
> regular timeout and have this routine manage the longer potential 
> timeout.  I'll likely have to mess with the low-level dev_cmd_wait to 
> not complain about a timeout when it is a FW status command.
> 
> The status_notify messages could then be updated in order to show some 
> progress, but would we base the 100% on the remote possibility that it 
> might take 25 minutes?  Or use some scaled update time, taking longer 
> between updates as time goes on? Hmmm...

I wonder if we can steal a page from systemd's book and display
"time until timeout", or whatchamacallit, like systemd does when it's
waiting for processes to quit. All drivers have some timeout set on the
operation. If users knew the driver sets timeout to n minutes and they
see the timer ticking up they'd be less likely to think the command has
hanged..