netdev - Re: [PATCH v2 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200905150739.4853fd0c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Sat, 5 Sep 2020 15:07:39 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Shannon Nelson <snelson@...sando.io>
Cc:     netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH v2 net-next 2/2] ionic: add devlink firmware update

On Sat, 5 Sep 2020 14:47:58 -0700 Shannon Nelson wrote:
> On 9/5/20 12:52 PM, Jakub Kicinski wrote:
> > On Thu,  3 Sep 2020 17:05:34 -0700 Shannon Nelson wrote:  
> >> +		if (offset > next_interval) {
> >> +			devlink_flash_update_status_notify(dl, "Downloading",
> >> +							   NULL, offset, fw->size);
> >> +			next_interval = offset + (fw->size / IONIC_FW_INTERVAL_FRACTION);
> >> +		}
> >> +	}
> >> +	devlink_flash_update_status_notify(dl, "Downloading", NULL, 1, 1);  
> > This is quite awkward. You send a notification with a different size,
> > and potentially an unnecessary one if last iteration of the loop
> > triggered offset > next_interval.
> >
> > Please just add || offset == fw->size to the condition at the end of
> > the loop and it will always trigger, with the correct length.  
> 
> Or maybe make that last one look like
>      devlink_flash_update_status_notify(dl, "Downloading", NULL, 
> fw->size, fw->size);
> to be less awkward and to keep the style of using a final status_notify 
> at the end of the block, as done in the Install and Select blocks 
> further along?
 
That may still generate two notifications at the end tho, no?
Unless the loop one in the loop is && offset != fw->size.