netdev - Re: [EXT] Re: [PATCH v2 net-next 1/2] bnx2x: Utilize firmware 7.13.21.0

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: <a805fbcd-7246-1fe4-038d-2859ad072c72@molgen.mpg.de>
Date:   Fri, 11 Mar 2022 14:00:20 +0100
From:   Paul Menzel <pmenzel@...gen.mpg.de>
To:     Manish Chopra <manishc@...vell.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ariel Elior <aelior@...vell.com>,
        Alok Prasad <palok@...vell.com>,
        Prabhakar Kushwaha <pkushwaha@...vell.com>,
        "David S. Miller" <davem@...emloft.net>,
        Greg KH <gregkh@...uxfoundation.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "it+netdev@...gen.mpg.de" <it+netdev@...gen.mpg.de>,
        "regressions@...ts.linux.dev" <regressions@...ts.linux.dev>
Subject: Re: [EXT] Re: [PATCH v2 net-next 1/2] bnx2x: Utilize firmware
 7.13.21.0

Dear Manish,


As a side note, it’d be great if you could use an email client, better 
supporting quoting.


Am 11.03.22 um 13:11 schrieb Manish Chopra:
>> -----Original Message-----
>> From: Linus Torvalds <torvalds@...ux-foundation.org>
>> Sent: Thursday, March 10, 2022 3:48 AM

[…]

>> On Wed, Mar 9, 2022 at 11:46 AM Manish Chopra <manishc@...vell.com>
>> wrote:
>>>
>>> This has not changed anything functionally from driver/device perspective,
>>> FW is still being loaded only when device is opened.
>>> bnx2x_init_firmware() [I guess, perhaps the name is misleading] just
>>> request_firmware() to prepare the metadata to be used when device will be
>>> opened.
>>
>> So how do you explain the report by Paul Menzel that things used to work and
>> no longer work now?
> 
> The issue which Paul mentioned had to do with "/lib/firmware/bnx2x/*
> file not found" when driver probes, which was introduced by the patch
> in subject, And the commit e13ad1443684 ("bnx2x: fix driver load from
> initrd") fixes this issue. So things should work as it is with the
> mentioned fixed commit.
No, your statement is incorrect. I already corrected it in a previous 
reply. The commit you mentioned was backported to 5.10.103. As we used 
that version, your commit was present.

> The only discussion led by this problem now is why the
> request_firmware() was moved early on [from open() to probe()] by the
> patch in subject. I explained the intention to do this in my earlier
> emails and let me add more details below -
> 
> Note that we have just moved request_firmware() logic, *not*
> something significant which has to do with actual FW loading or
> device initialization from the FW file data which could cause
> significant functional change for this device/driver, FW load/init
> part still stays in open flow.
> 
> Before the patch in subject, driver used to only work with
> fixed/specific FW version file whose version was statically known to
> the driver function at probe() time to take some decision to fail the
> function probe early in the system if the function is supposed to run
> with a FW version which is not the same version loaded on the device
> by another PF (different ENV). Now when we sent this new FW patch (in
> subject) then we got feedback from community to maintain backward
> compatibility with older FW versions as well and we did it in same v2
> patch legitimately, just that now we can work with both older or
> newer FW file so we need this run time FW version information to
> cache (based on request_firmware() return success value for an old FW
> file or new FW file) which will be used in follow up probe() flows to
> decide the function probe failure early If there could be FW version
> mismatches against the loaded FW on the device by other PFs already
> 
> So we need to understand why we should not call request_firmware() in
> probe or at least what's really harmful in doing that in probe() if
> some of the follow up probe flows needs some of the metadata info
> (like the run time FW versions info in this case which we get based
> on request_firmware() return value), we could avoid this but we don't
> want to add some ugly/unsuitable file APIs checks to know which FW
> version file is available on the file system if there is already an
> API request_firmware() available for this to be used.
Your patches broke loading the driver, and as a result – as seen from 
the pastes I provided – the network devices were not functional.

> Please let us know. Thanks.
> 
>> You can't do request_firmware() early. When you actually then push the
>> firmware to the device is immaterial - but request_firmware() has to be done
>> after the system is up and running.


Kind regards,

Paul