[pull request][Trusty] (upstream) bnx2x: Utilize FW 7.10.51

Brad Figg brad.figg at canonical.com
Fri Feb 26 20:47:26 UTC 2016


On Fri, Feb 26, 2016 at 02:26:20PM -0500, Dan Streetman wrote:
> BugLink: http://bugs.launchpad.net/bugs/1454286
> 
> Please pull git://git.launchpad.net/~ddstreet/+git/linux lp1454286
> 
> First, the problem; under some conditions, the chip hangs in firmware.
> We have had reports of this happening.  From the 7.10.51 fw changelog:
> "Chip may stall in very rare cases under heavy traffic with FW GRO
> enabled".  So to fix the problem, the driver must be updated to use
> the new firmware level.  The commit in lts-vivid to update to the new
> fw is e42780b, and:
> 
> $ git log --no-merges --oneline ubuntu/trusty/master..e42780b
> drivers/net/ethernet/broadcom/bnx2x | wc -l
> 73
> 
> however, some of those don't need to be included, specifically:
> 
> $ ( git log --no-merges --oneline ubuntu/trusty/master..e42780b
> drivers/net/ethernet/broadcom/bnx2x ; git log --no-merges --oneline
> ubuntu/trusty/master..lp1454286 drivers/net/ethernet/broadcom/bnx2x )
> | sort -k 2 | uniq -f 1 -u
> 4e857c5 arch: Mass conversion of smp_mb__*()
> 0c0e634 bnx2x: Adapter not recovery from EEH error injection
> fe26566d bnx2x: fix crash during TSO tunneling
> 9aaae04 bnx2x: Fix kernel crash and data miscompare after EEH recovery
> dad91ee bnx2x: Fix link for KR with swapped polarity lane
> 07b0f00 bnx2x: fix possible panic under memory stress
> fe62d00 ethtool: Replace ethtool_ops::{get,set}_rxfh_indir() with
> {get,set}_rxfh()
> 99932d4 netdevice: add queue selection fallback handler for ndo_select_queue
> 7ad24ea net: get rid of SET_ETHTOOL_OPS
> ed61668 net-next:v4: Add support to configure SR-IOV VF minimum and
> maximum Tx rate through ip tool.
> 9baa3c3 PCI: Remove DEFINE_PCI_DEVICE_TABLE macro use
> c265947 Update setapp/getapp prototypes in dcbnl_rtnl_ops to return
> int instead of u8
> 
> of those, all the bnx2x: ones are already included in trusty/master.
> The others are all cleanup/renaming commits, or commits that change
> structures or functions, but those changes aren't used by bnx2x and
> aren't required by any of the commits in the pull.
> 
> So what does the pull change?  It *only* changes the bnx2x driver
> (except changing the bnx2x owner in MAINTAINERS, and changing a bnx2
> mod param mode from 0 to 444).
> 
> $ git diff --stat ubuntu/trusty/master..lp1454286 MAINTAINERS
>                                    |    2 +-
>  drivers/net/ethernet/broadcom/bnx2.c                    |    2 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x.h             |   64 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c         |  386 +++---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.h         |  180 +--
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.c         |   10 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_dcb.h         |    2 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c     |  142 ++-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_defs.h     |  223 ++--
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_fw_file_hdr.h |    4 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_hsi.h         |  203 +++-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_init.h        |    4 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_init_ops.h    |    4 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c        |   77 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.h        |   10 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c        |  676 ++++++++---
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_reg.h         |    1 +
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.c          |  498 +++-----
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_sp.h          |   73 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c       | 2263
> +++++++++++++----------------------
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.h       |  428 ++-----
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.c       |    2 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_stats.h       |    2 +-
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.c        |  746 ++++++------
>  drivers/net/ethernet/broadcom/bnx2x/bnx2x_vfpf.h        |   49 +-
>  25 files changed, 2820 insertions(+), 3231 deletions(-)
> 
> 
> Why not just cherry-pick the single commit that updates the driver to
> use the new fw?
> 
> I tried this, and on boot the driver fell into a loop failing to talk
> correctly to the hardware/fw.  So at least 1 additional commit is
> required for it to properly work with the new firmware.
> 
> 
> Why do we need to pull *all* the commits?  Why not cherry-pick the fw
> update commit, and then work backwards pulling only the commit(s)
> required for the driver to start working with the updated fw?
> 
> In short, testing.
> I can spend the time to find the specific commits required to get the
> driver to boot with the new fw, and I can perform basic tests, but I
> don't have the hardware or the time to fully test the driver's
> functionality in all configurations.  However the chip vendor
> (broadcom/qlogic) *has* done testing with the driver when the
> fw-update commit was added upstream, but that testing was done with
> all the previous commits.  So leaving out commits is likely to be less
> tested, and more problematic, than just including all commits leading
> up to fw update commit.
> 
> If this bug was in the kernel, I'd definitely agree that only the
> specific commit(s) that fix the bug should be backported.  However,
> since the bug is in the firmware, and we must update the driver to use
> the new firmware to fix the bug, I feel the driver should be fully
> updated to the level that was vendor-tested with the new fw level.
> 
> Ok, let the fun begin...
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

There is no indication of positive testing in the bug. Without that I will
not accept this pull request.

-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com




More information about the kernel-team mailing list