bnx2x: Latest firmware requirement breaks no regression policy

7 messages, 4 authors, 2020-02-24 · open the first message on its own page

bnx2x: Latest firmware requirement breaks no regression policy

From: Paul Menzel <hidden>
Date: 2020-02-17 13:18:15

Dear Linux folks,


Updating a server from 4.19.x to 5.4.x, the network device

    Ethernet controller [0200]: Broadcom Inc. and subsidiaries NetXtreme II BCM57711 10-Gigabit PCIe [14e4:164f]

failed to initialize due to missing firmware.

    bnx2x 0000:41:00.0: Direct firmware load for bnx2x/bnx2x-e1h-7.13.11.0.fw failed with error -2
    bnx2x: [bnx2x_init_firmware:13557(net02)]Can't load firmware file bnx2x/bnx2x-e1h-7.13.11.0.fw
    bnx2x: [bnx2x_func_hw_init:6002(net02)]Error loading firmware
    bnx2x: [bnx2x_nic_load:2730(net02)]HW init failed, aborting

That is undesired, because without network access one has to have
direct system access to find out what is wrong.

Could you please change the policy to only print a big warning,
if the latest firmware is not available, and an update suggestion?


Kind regards,

Paul

RE: bnx2x: Latest firmware requirement breaks no regression policy

From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Date: 2020-02-19 08:50:07

Hi Paul,
    The firmware file referred below (i.e., storm FW) should be present on the host (i.e., /lib/firmware/bnx2x/ path), not the device. Driver must require this version of the FW to initialize the device, and hence provide the network functionality. Also, the driver is not backward compatible with older FW versions.
So it's not possible to handle the below error scenario in the driver,
	>     bnx2x 0000:41:00.0: Direct firmware load for bnx2x/bnx2x-e1h-7.13.11.0.fw failed with error -2
	>     bnx2x: [bnx2x_init_firmware:13557(net02)]Can't load firmware file bnx2x/bnx2x-e1h-7.13.11.0.fw
At the most, we can validate the existence of FW file on the host during the kernel build or installation.
FW image name from driver sources:
	drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:
	#define FW_FILE_NAME_E1         "bnx2x/bnx2x-e1-" FW_FILE_VERSION ".fw"
	#define FW_FILE_NAME_E1H        "bnx2x/bnx2x-e1h-" FW_FILE_VERSION ".fw"
	#define FW_FILE_NAME_E2         "bnx2x/bnx2x-e2-" FW_FILE_VERSION ".fw"
FW image path on the host:
	/lib/firmware/bnx2x/bnx2x-e1h-7.13.11.0.fw

Thanks,
Sudarsana
-----Original Message-----
From: netdev-owner@vger.kernel.org <redacted> On
Behalf Of Paul Menzel
Sent: Monday, February 17, 2020 6:48 PM
To: Ariel Elior <redacted>; Sudarsana Reddy Kalluru
[off-list ref]; GR-everest-linux-l2 <GR-everest-linux-
l2@marvell.com>
Cc: netdev@vger.kernel.org; LKML <redacted>; it+linux-
netdev@molgen.mpg.de
Subject: bnx2x: Latest firmware requirement breaks no regression policy

Dear Linux folks,


Updating a server from 4.19.x to 5.4.x, the network device

    Ethernet controller [0200]: Broadcom Inc. and subsidiaries NetXtreme II
BCM57711 10-Gigabit PCIe [14e4:164f]

failed to initialize due to missing firmware.

    bnx2x 0000:41:00.0: Direct firmware load for bnx2x/bnx2x-e1h-7.13.11.0.fw
failed with error -2
    bnx2x: [bnx2x_init_firmware:13557(net02)]Can't load firmware file
bnx2x/bnx2x-e1h-7.13.11.0.fw
    bnx2x: [bnx2x_func_hw_init:6002(net02)]Error loading firmware
    bnx2x: [bnx2x_nic_load:2730(net02)]HW init failed, aborting

That is undesired, because without network access one has to have
direct system access to find out what is wrong.

Could you please change the policy to only print a big warning,
if the latest firmware is not available, and an update suggestion?


Kind regards,

Paul

Re: bnx2x: Latest firmware requirement breaks no regression policy

From: Paul Menzel <hidden>
Date: 2020-02-19 12:43:55

Dear Sudarsana,


Thank you for your reply.


On 2020-02-19 09:49, Sudarsana Reddy Kalluru wrote:
The firmware file referred below (i.e., storm FW) should be present
on the host (i.e., /lib/firmware/bnx2x/ path), not the device. Driver
must require this version of the FW to initialize the device, and
hence provide the network functionality. Also, the driver is not
backward compatible with older FW versions.

So it's not possible to handle the below error scenario in the driver,

	>     bnx2x 0000:41:00.0: Direct firmware load for bnx2x/bnx2x-e1h-7.13.11.0.fw failed with error -2
	>     bnx2x: [bnx2x_init_firmware:13557(net02)]Can't load firmware file bnx2x/bnx2x-e1h-7.13.11.0.fw

At the most, we can validate the existence of FW file on the host
during the kernel build or installation.
That is what I thought about the current state. But why was this
design decision made? It’s not user-friendly, and as written breaks
the no regression policy. Users can update the Linux kernel without
any regressions, and everything working as before. Dave, what is
your opinion?

Where are the driver requirements/implementation short-comings
documented?

If an older Linux kernel works with a certain firmware version, why
shouldn’t a newer Linux kernel work with that firmware version.
Maybe some features are missing, but at least I should get the same
state as with the older version.

Do you have plans to switch the driver to a model, where the
features/requirements of the firmware are queried by the driver, so
older versions can be supported?
FW image name from driver sources:
	drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:
	#define FW_FILE_NAME_E1         "bnx2x/bnx2x-e1-" FW_FILE_VERSION ".fw"
	#define FW_FILE_NAME_E1H        "bnx2x/bnx2x-e1h-" FW_FILE_VERSION ".fw"
	#define FW_FILE_NAME_E2         "bnx2x/bnx2x-e2-" FW_FILE_VERSION ".fw"
FW image path on the host:
	/lib/firmware/bnx2x/bnx2x-e1h-7.13.11.0.fw
Yes, that is what I found in my original research, and that is how
we fixed it, but with the non-working interface it was more work
than necessary.


Kind regards,

Paul

RE: [EXT] Re: bnx2x: Latest firmware requirement breaks no regression policy

From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Date: 2020-02-20 09:17:33

Hi Paul,
    Bnx2x driver and the storm FW are tightly coupled, and the info is exchanged between them via shmem (i.e., common structures which might change between the releases). Also, FW provides some offset addresses to the driver which could change between the FW releases, following is one such commit,
	https://www.spinics.net/lists/netdev/msg609889.html
Hence it's not very straight forward to provide the backward compatibility i.e., newer (updated) kernel driver with the older FW.
Currently we don’t have plans to implement the new model mentioned below.

Thanks,
Sudarsana
-----Original Message-----
From: Paul Menzel <redacted>
Sent: Wednesday, February 19, 2020 6:14 PM
To: Sudarsana Reddy Kalluru <skalluru@marvell.com>; Ariel Elior
[off-list ref]; GR-everest-linux-l2 <GR-everest-linux-
l2@marvell.com>
Cc: netdev@vger.kernel.org; LKML <redacted>; it+linux-
netdev@molgen.mpg.de; David S. Miller [off-list ref]
Subject: [EXT] Re: bnx2x: Latest firmware requirement breaks no regression
policy

External Email

----------------------------------------------------------------------
Dear Sudarsana,


Thank you for your reply.


On 2020-02-19 09:49, Sudarsana Reddy Kalluru wrote:
quoted
The firmware file referred below (i.e., storm FW) should be present
on the host (i.e., /lib/firmware/bnx2x/ path), not the device. Driver
must require this version of the FW to initialize the device, and
hence provide the network functionality. Also, the driver is not
backward compatible with older FW versions.

So it's not possible to handle the below error scenario in the driver,

	>     bnx2x 0000:41:00.0: Direct firmware load for bnx2x/bnx2x-e1h-
7.13.11.0.fw failed with error -2
quoted
	>     bnx2x: [bnx2x_init_firmware:13557(net02)]Can't load firmware file
bnx2x/bnx2x-e1h-7.13.11.0.fw
quoted
At the most, we can validate the existence of FW file on the host
during the kernel build or installation.
That is what I thought about the current state. But why was this
design decision made? It’s not user-friendly, and as written breaks
the no regression policy. Users can update the Linux kernel without
any regressions, and everything working as before. Dave, what is
your opinion?

Where are the driver requirements/implementation short-comings
documented?

If an older Linux kernel works with a certain firmware version, why
shouldn’t a newer Linux kernel work with that firmware version.
Maybe some features are missing, but at least I should get the same
state as with the older version.

Do you have plans to switch the driver to a model, where the
features/requirements of the firmware are queried by the driver, so
older versions can be supported?
quoted
FW image name from driver sources:
	drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c:
	#define FW_FILE_NAME_E1         "bnx2x/bnx2x-e1-" FW_FILE_VERSION
".fw"
quoted
	#define FW_FILE_NAME_E1H        "bnx2x/bnx2x-e1h-"
FW_FILE_VERSION ".fw"
quoted
	#define FW_FILE_NAME_E2         "bnx2x/bnx2x-e2-" FW_FILE_VERSION
".fw"
quoted
FW image path on the host:
	/lib/firmware/bnx2x/bnx2x-e1h-7.13.11.0.fw
Yes, that is what I found in my original research, and that is how
we fixed it, but with the non-working interface it was more work
than necessary.


Kind regards,

Paul

RE: [EXT] Re: bnx2x: Latest firmware requirement breaks no regression policy

From: Ariel Elior <hidden>
Date: 2020-02-20 15:40:57

-----Original Message-----
From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Sent: Thursday, February 20, 2020 11:17 AM
To: Paul Menzel <redacted>; Ariel Elior
[off-list ref]; GR-everest-linux-l2 <GR-everest-linux-
l2@marvell.com>
Cc: netdev@vger.kernel.org; LKML <redacted>; it+linux-
netdev@molgen.mpg.de; David S. Miller [off-list ref]
Subject: RE: [EXT] Re: bnx2x: Latest firmware requirement breaks no regression
policy

Hi Paul,
    Bnx2x driver and the storm FW are tightly coupled, and the info is exchanged
between them via shmem (i.e., common structures which might change
between the releases). Also, FW provides some offset addresses to the driver
which could change between the FW releases, following is one such commit,
	https://www.spinics.net/lists/netdev/msg609889.html
Hence it's not very straight forward to provide the backward compatibility i.e.,
newer (updated) kernel driver with the older FW.
Currently we don’t have plans to implement the new model mentioned below.

Thanks,
Sudarsana
Hi,
There are additional reasons why backwards/forwards compatibility considerations
are not applicable here. This Fw is not nvram based, and does not reside in the
device. It is programed to the device on every driver load. The driver will
never face a device "already initialized" with a version of FW it is not
familiar with.
The device also has traditional management FW in nvram in the device with which
we have a backwards and forwards compatibility mechanism, which works just
fine.
But the FW under discussion is fastpath Fw, used to craft every packet going out
of the device and analyze and place every packet coming into the device.
Supporting multiple versions of FW would be tantamount to implementing dozens of
versions of start_xmit and napi_poll in the driver (not to mention multiple
fastpath handles of all the offloads the device supports, roce, iscsi, fcoe and
iwarp, as all of these are offloaded by the FW).
The entire device initialization sequence also changes significantly from one FW
version to the Next. All of these differences are abstracted away in the FW
file, which includes the init sequence and the compiled FW. The amount of
changes required in driver are very significant when moving from one version to
the next. Trying to keep all those versions alive concurrently would cause this
already very large driver to be 20x larger.
We don't have a method of keeping the device operational if the kernel was
upgraded but firmware tree was not updated. The best that can be done is report
the problem, which is what we do.
Thanks,
Ariel

Re: [EXT] Re: bnx2x: Latest firmware requirement breaks no regression policy

From: Jakub Kicinski <kuba@kernel.org>
Date: 2020-02-21 00:37:50

On Thu, 20 Feb 2020 15:40:37 +0000 Ariel Elior wrote:
quoted
-----Original Message-----
From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Sent: Thursday, February 20, 2020 11:17 AM
To: Paul Menzel <redacted>; Ariel Elior
[off-list ref]; GR-everest-linux-l2 <GR-everest-linux-  
l2@marvell.com>  
Cc: netdev@vger.kernel.org; LKML <redacted>; it+linux-
netdev@molgen.mpg.de; David S. Miller [off-list ref]
Subject: RE: [EXT] Re: bnx2x: Latest firmware requirement breaks no regression
policy

Hi Paul,
    Bnx2x driver and the storm FW are tightly coupled, and the info is exchanged
between them via shmem (i.e., common structures which might change
between the releases). Also, FW provides some offset addresses to the driver
which could change between the FW releases, following is one such commit,
	https://www.spinics.net/lists/netdev/msg609889.html
Hence it's not very straight forward to provide the backward compatibility i.e.,
newer (updated) kernel driver with the older FW.
Currently we don’t have plans to implement the new model mentioned below.
Hi,
There are additional reasons why backwards/forwards compatibility considerations
are not applicable here. This Fw is not nvram based, and does not reside in the
device. It is programed to the device on every driver load. The driver will
never face a device "already initialized" with a version of FW it is not
familiar with.
How do you deal with VFs?
The device also has traditional management FW in nvram in the device with which
we have a backwards and forwards compatibility mechanism, which works just
fine.
But the FW under discussion is fastpath Fw, used to craft every packet going out
of the device and analyze and place every packet coming into the device.
Supporting multiple versions of FW would be tantamount to implementing dozens of
versions of start_xmit and napi_poll in the driver (not to mention multiple
fastpath handles of all the offloads the device supports, roce, iscsi, fcoe and
iwarp, as all of these are offloaded by the FW).
The entire device initialization sequence also changes significantly from one FW
version to the Next. All of these differences are abstracted away in the FW
file, which includes the init sequence and the compiled FW. The amount of
changes required in driver are very significant when moving from one version to
the next. Trying to keep all those versions alive concurrently would cause this
already very large driver to be 20x larger.
All your points are disproved by all the devices which have drivers
upstream and don't break backward compatibility on every release.
We don't have a method of keeping the device operational if the kernel was
upgraded but firmware tree was not updated. The best that can be done is report
the problem, which is what we do.

RE: [EXT] Re: bnx2x: Latest firmware requirement breaks no regression policy

From: Ariel Elior <hidden>
Date: 2020-02-24 17:34:29

-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: Friday, February 21, 2020 2:38 AM
To: Ariel Elior <redacted>
Cc: Sudarsana Reddy Kalluru <skalluru@marvell.com>; Paul Menzel
[off-list ref]; GR-everest-linux-l2 <GR-everest-linux-
l2@marvell.com>; netdev@vger.kernel.org; LKML <linux-
kernel@vger.kernel.org>; it+linux-netdev@molgen.mpg.de; David S. Miller
[off-list ref]
Subject: Re: [EXT] Re: bnx2x: Latest firmware requirement breaks no regression
policy

On Thu, 20 Feb 2020 15:40:37 +0000 Ariel Elior wrote:
quoted
quoted
-----Original Message-----
From: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Sent: Thursday, February 20, 2020 11:17 AM
To: Paul Menzel <redacted>; Ariel Elior
[off-list ref]; GR-everest-linux-l2 <GR-everest-linux-
l2@marvell.com>
Cc: netdev@vger.kernel.org; LKML <redacted>;
it+linux- netdev@molgen.mpg.de; David S. Miller
[off-list ref]
Subject: RE: [EXT] Re: bnx2x: Latest firmware requirement breaks no
regression policy

Hi Paul,
    Bnx2x driver and the storm FW are tightly coupled, and the info
is exchanged between them via shmem (i.e., common structures which
might change between the releases). Also, FW provides some offset
addresses to the driver which could change between the FW releases,
following is one such commit,
quoted
quoted
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net
_lists_netdev_msg609889.html&d=DwIFaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=c
WB
quoted
quoted
gNIFUifZRx2xhypdcaYrfIsMGt93NxP1r8GQtC0s&m=W4pEZSYmEGyvbakqtsYcE7
OaK
quoted
quoted
vjaEvlWjSFfUUOc65A&s=A7UihKukjHlFL16Z1MFf9zY0BItRgbDSAzwq48PkmvY&e
=
quoted
quoted
Hence it's not very straight forward to provide the backward
compatibility i.e., newer (updated) kernel driver with the older FW.
Currently we don’t have plans to implement the new model mentioned
below.
quoted
quoted
Hi,
There are additional reasons why backwards/forwards compatibility
considerations are not applicable here. This Fw is not nvram based,
and does not reside in the device. It is programed to the device on
every driver load. The driver will never face a device "already
initialized" with a version of FW it is not familiar with.
How do you deal with VFs?
quoted
The device also has traditional management FW in nvram in the device
with which we have a backwards and forwards compatibility mechanism,
which works just fine.
But the FW under discussion is fastpath Fw, used to craft every packet
going out of the device and analyze and place every packet coming into the
device.
quoted
Supporting multiple versions of FW would be tantamount to implementing
dozens of versions of start_xmit and napi_poll in the driver (not to
mention multiple fastpath handles of all the offloads the device
supports, roce, iscsi, fcoe and iwarp, as all of these are offloaded by the FW).
The entire device initialization sequence also changes significantly
from one FW version to the Next. All of these differences are
abstracted away in the FW file, which includes the init sequence and
the compiled FW. The amount of changes required in driver are very
significant when moving from one version to the next. Trying to keep
all those versions alive concurrently would cause this already very large
driver to be 20x larger.

All your points are disproved by all the devices which have drivers upstream
and don't break backward compatibility on every release.
Note that we are talking about compatibility with a file on the disk, not with
the FW in the device. I don't think any of the other drivers have fastpath
FW which needs to be programmed to the device on every driver load. As
noted, this is not FW that resides in flash. It has to be reprogrammed on
driver load. Note that we do always make sure to have the new FW file
accepted in the Linux FW tree before we send to Dave the series for making
use of it. And the driver does detect that the FW file is not available and
fails the load flow gracefully.

I think this boils down to a simple question: Is the use case of upgrading
the kernel without pulling the FW tree a legitimate one? If so, I think the
solution we can offer is to create an .h file with the required init sequence
(FW an all) and have the driver compile with it, so it can always initialize
the device. This is how this device is initialized in OSes which don't support
dynamic FW load. This is not difficult to do and would solve the problem
you are facing (FW file not available on disk) but would also mean that the
compiled driver size would be much larger. Is that desirable?

As an aside, please note that this model is implemented qede for a few
years and in bnx2x for many years, and I did not get this complaint yet.
quoted
We don't have a method of keeping the device operational if the kernel
was upgraded but firmware tree was not updated. The best that can be
done is report the problem, which is what we do.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help