netdev - Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.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]

Date:   Wed, 27 Oct 2021 14:18:36 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Ariel Elior <aelior@...vell.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Manish Chopra <manishc@...vell.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Sudarsana Reddy Kalluru <skalluru@...vell.com>,
        "malin1024@...il.com" <malin1024@...il.com>,
        Shai Malin <smalin@...vell.com>,
        Omkar Kulkarni <okulkarni@...vell.com>,
        Nilesh Javali <njavali@...vell.com>,
        "GR-everest-linux-l2@...vell.com" <GR-everest-linux-l2@...vell.com>
Subject: Re: [EXT] Re: [PATCH net-next 1/2] bnx2x: Utilize firmware 7.13.20.0

> > ----------------------------------------------------------------------
> > On Tue, 26 Oct 2021 12:37:16 -0700 Manish Chopra wrote:
> > > Commit 0050dcf3e848 ("bnx2x: Add FW 7.13.20.0") added a new .bin
> > > firmware file to linux-firmware.git tree. This new firmware addresses
> > > few important issues and enhancements as mentioned below -
> > >
> > > - Support direct invalidation of FP HSI Ver per function ID, required for
> > >   invalidating FP HSI Ver prior to each VF start, as there is no VF
> > > start
> > > - BRB hardware block parity error detection support for the driver
> > > - Fix the FCOE underrun flow
> > > - Fix PSOD during FCoE BFS over the NIC ports after preboot driver
> > >
> > > This patch incorporates this new firmware 7.13.20.0 in bnx2x driver.
> > 
> > How is this expected to work? Your drivers seems to select a very specific FW
> > version:
> > 
> > 	/* Check FW version */
> > 	offset = be32_to_cpu(fw_hdr->fw_version.offset);
> > 	fw_ver = firmware->data + offset;
> > 	if ((fw_ver[0] != BCM_5710_FW_MAJOR_VERSION) ||
> > 	    (fw_ver[1] != BCM_5710_FW_MINOR_VERSION) ||
> > 	    (fw_ver[2] != BCM_5710_FW_REVISION_VERSION) ||
> > 	    (fw_ver[3] != BCM_5710_FW_ENGINEERING_VERSION)) {
> > 		BNX2X_ERR("Bad FW version:%d.%d.%d.%d. Should be
> > %d.%d.%d.%d\n",
> > 		       fw_ver[0], fw_ver[1], fw_ver[2], fw_ver[3],
> > 		       BCM_5710_FW_MAJOR_VERSION,
> > 		       BCM_5710_FW_MINOR_VERSION,
> > 		       BCM_5710_FW_REVISION_VERSION,
> > 		       BCM_5710_FW_ENGINEERING_VERSION);
> > 		return -EINVAL;
> > 	}
> > 
> > so this change has a dependency on user updating their /lib/firmware.
> > 
> > Is it really okay to break the systems for people who do not have that FW
> > version with a stable backport?
> > 
> > Greg, do you have general guidance for this or is it subsystem by subsystem?

I have been pushing back on a similar change for the Marvell Prestera
driver, which also loads the firmware from /lib/firmware and they are
proposing to break the ABI to the firmware, and not support older
version.

I don't like this. As Jakub points out, you are going to break systems
which don't update the firmware and the kernel at the same time. I
really would prefer you support two versions of the firmware, and
detect what features it supports to runtime.

	Andrew