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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20200915152758.16a61a90@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 15 Sep 2020 15:27:58 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Shannon Nelson <snelson@...sando.io>
Cc:     Jacob Keller <jacob.e.keller@...el.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH v3 net-next 2/2] ionic: add devlink firmware update

On Tue, 15 Sep 2020 15:11:06 -0700 Shannon Nelson wrote:
> On 9/15/20 12:00 PM, Jakub Kicinski wrote:
> > On Tue, 15 Sep 2020 11:44:07 -0700 Jacob Keller wrote:  
> >> Exactly how I saw it.
> >>
> >> Basically, the timeout should take effect as long as the (component,
> >> msg) pair stays the same.
> >>
> >> So if you send percentage reports with the same message and component,
> >> then the timeout stays in effect. Once you start a new message, then the
> >> timeout would be reset.  
> > I don't think I agree with that. As I said that'd make the timeout not
> > match the reality of what happens in the driver.  
> 
> I have an asynchronous FW interaction where the driver sends one FW 
> command to start the fw-install and sends several more FW commands to 
> check on the status until either it gets a done or error status or too 
> many seconds have elapsed.  How would you suggest this gets modeled?

It's still one command. The fact that the driver periodically checks if
its finished is an implementation detail. Drivers which periodically
check if "done bit" in some register got cleared or not also don't send
a notification every time they've done so.

> In the model you are suggesting, the driver can only do a single 
> status_notify with a timeout before the initial async FW command, then 
> no other status_notify messages until the driver gets the done/error 
> status, or the time has elapsed, regardless of how long that might 
> take.  The user will only see the timeout ticking, but no activity from 
> the driver.  This allows the user to see what the deadline is, but 
> doesn't reassure them that the process is still moving.

The timer should be a sufficient indication to the user not to worry,
yet. The worrying starts once the timer expires, then something is up.

> I'm suggesting that the driver might send some intermediate 
> status_notify messages in order to assure the user that things are not 
> stalled out.  Driving a spinner would be nice, but we don't have that 
> concept in this interface, so poking the done/total values could be used 
> for that.  In order to not reset the timeout on each of these 
> intermediate updates, we pass the same timeout value.

What useful status_notify messages can a driver send mid-command?
Timeout tells the user "for this much time you may not see any real
status updates".

> At this point I'm going to try a patchset that implements the basics 
> that we already have agreed upon, and this detail can get worked out 
> later, as I believe it doesn't change the internal implementation.