[pull request][Trusty] (upstream) bnx2x: Utilize FW 7.10.51
Dan Streetman
dan.streetman at canonical.com
Wed Mar 2 14:03:26 UTC 2016
On Fri, Feb 26, 2016 at 3:47 PM, Brad Figg <brad.figg at canonical.com> wrote:
> 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.
I've tested it for basic functionality - boots, can connect to the
interface, ran iperf and several flent tests between it and a second
system - but as far as testing to fix the specific bug, I can't
reproduce it and so can't test for that, and the original
reporter/customer of the problem is now refusing to update their
kernel at all, even just to test and verify the driver update fixes
their problem. So...unless they change their mind, or someone else
with this problem tests, this pull request can die.
>
> --
> Brad Figg brad.figg at canonical.com http://www.canonical.com
More information about the kernel-team
mailing list