bnx2x driver causes 100% CPU load

Bug #1832082 reported by Przemyslaw Hausman
20
This bug affects 3 people
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
Fix Released
High
Guilherme G. Piccoli
Xenial
Fix Released
High
Guilherme G. Piccoli
Bionic
Fix Released
High
Guilherme G. Piccoli
Cosmic
Won't Fix
High
Guilherme G. Piccoli
Disco
Fix Released
High
Guilherme G. Piccoli
Eoan
Fix Released
High
Guilherme G. Piccoli
Focal
Fix Released
High
Guilherme G. Piccoli

Bug Description

[Impact]

* The PTP feature in bnx2x driver is implemented in a way that if the NIC firmware takes some time to perform the timestamping - which is observed as a bad register read in bnx2x_ptp_task() - then the ptp worker function will reschedule itself indefinitely until the value read from the register is meaningful. With that behavior, if an userspace tool request a bad configured RX filter to bnx2x (or if NIC firmware has any other issue in timestamping), the function bnx2x_ptp_task() will be rescheduled forever and cause a unbound resource consumption. This manifests as a kworker thread consuming 100% of CPU.

* The dmesg log will show the following message regarding other packets being skipped on timestamp routine due to a packet getting stuck in the timestamping "pipeline":

"bnx2x: [bnx2x_start_xmit:3862(eno4)]The device supports only a single
outstanding packet to timestamp, this packet will not be timestamped"

Also, by using ftrace user can notice that function bnx2x_ptp_task() is being called a lot, and by enabling bnx2x PTP debugging log (ethtool -s <iface> msglvl 16777216) it's possible to observe the following message flooding the kernel log:

"bnx2x: [bnx2x_ptp_task:15242(eno4)]There is no valid Tx timestamp yet"

* The patch proposed in this SRU request is accepted upstream and is available currently (2019-07-03) in David Miller's linux-net tree:
git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=3c91f25c2f72
Besides fixing the issue, it also adds an ethtool statistics for accounting the ptp errors and reduces message flooding in case of errors.

[Test case]

Reproducing the problem is not difficult; we've used chrony in Bionic to trigger the problem. The steps are:

a) Install chrony on Bionic in a system with working NIC managed by bnx2x;

b) Edit chrony configuration and add: "hwtimestamp *" to the top of its conf file;

c) Restart chrony service

Check dmesg for the "[...]single outstanding packet" message and the overall CPU workload using a tool like "top" to observe a kthread consuming 100% of CPU.

[Regression potential]

The patch scope is restricted to bnx2x ptp handler, and was validated by the driver maintainer. If there's any possibility of regressions, we believe the worst would be an issue affecting the packet timestamping, not messing with the regular xmit path for the driver.

information type: Public → Private
information type: Private → Public
Revision history for this message
Ubuntu Kernel Bot (ubuntu-kernel-bot) wrote : Missing required logs.

This bug is missing log files that will aid in diagnosing the problem. While running an Ubuntu kernel (not a mainline or third-party kernel) please enter the following command in a terminal window:

apport-collect 1832082

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

Changed in linux (Ubuntu):
status: New → Incomplete
tags: added: bionic
Revision history for this message
Guilherme G. Piccoli (gpiccoli) wrote :

Thanks Przemyslaw, good explanation on bug's description! I'm dealing with this one, will update status here with news.

Cheers,

Guilherme

Changed in linux (Ubuntu Eoan):
status: Incomplete → Confirmed
Changed in linux (Ubuntu Disco):
status: New → Confirmed
Changed in linux (Ubuntu Cosmic):
status: New → Confirmed
Changed in linux (Ubuntu Bionic):
status: New → Confirmed
Changed in linux (Ubuntu Xenial):
status: New → Confirmed
importance: Undecided → High
Changed in linux (Ubuntu Bionic):
importance: Undecided → High
Changed in linux (Ubuntu Cosmic):
importance: Undecided → High
Changed in linux (Ubuntu Disco):
importance: Undecided → High
Changed in linux (Ubuntu Eoan):
importance: Undecided → High
Changed in linux (Ubuntu Xenial):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
Changed in linux (Ubuntu Bionic):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
Changed in linux (Ubuntu Cosmic):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
Changed in linux (Ubuntu Eoan):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
Changed in linux (Ubuntu Disco):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
tags: added: bnx2x sts
removed: bionic
description: updated
Revision history for this message
Guilherme G. Piccoli (gpiccoli) wrote :
description: updated
Revision history for this message
Guilherme G. Piccoli (gpiccoli) wrote :

SRU submitted to kernel mailing-list: https://lists.ubuntu.com/archives/kernel-team/2019-July/101925.html

I've marked Xenial->Disco as "In Progress", because we need acceptance from kernel team.
On the other hand, Devel series (Eoan/Ff) will get the fix via regular rebase with
Linus tree, hence I've put "Fix Committed".

Thanks,

Guilherme

Changed in linux (Ubuntu Eoan):
status: Confirmed → Fix Committed
Changed in linux (Ubuntu Disco):
status: Confirmed → Fix Committed
Changed in linux (Ubuntu Cosmic):
status: Confirmed → In Progress
Changed in linux (Ubuntu Eoan):
status: Fix Committed → In Progress
status: In Progress → Fix Committed
Changed in linux (Ubuntu Disco):
status: Fix Committed → In Progress
Changed in linux (Ubuntu Bionic):
status: Confirmed → In Progress
Changed in linux (Ubuntu Xenial):
status: Confirmed → In Progress
Stefan Bader (smb)
Changed in linux (Ubuntu Cosmic):
status: In Progress → Won't Fix
Changed in linux (Ubuntu Bionic):
status: In Progress → Fix Committed
Changed in linux (Ubuntu Disco):
status: In Progress → Fix Committed
Changed in linux (Ubuntu Xenial):
status: In Progress → Fix Committed
Brad Figg (brad-figg)
tags: added: cscc
Revision history for this message
Ubuntu Kernel Bot (ubuntu-kernel-bot) wrote :

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-disco' to 'verification-done-disco'. If the problem still exists, change the tag 'verification-needed-disco' to 'verification-failed-disco'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-disco
Revision history for this message
Ubuntu Kernel Bot (ubuntu-kernel-bot) wrote :

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-bionic' to 'verification-done-bionic'. If the problem still exists, change the tag 'verification-needed-bionic' to 'verification-failed-bionic'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-bionic
Revision history for this message
Ubuntu Kernel Bot (ubuntu-kernel-bot) wrote :

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-xenial' to 'verification-done-xenial'. If the problem still exists, change the tag 'verification-needed-xenial' to 'verification-failed-xenial'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-xenial
Revision history for this message
Guilherme G. Piccoli (gpiccoli) wrote :

I've validated the -proposed kernels for Xenial (4.4.0-158), Bionic (4.15.0-56) and Disco (5.0.0-22), using the test case mentioned in the description. All working fine, the issue is gone.
Also, the patch was released upstream in the 5.3.x series, so I'll mark ff-series as Released.

Cheers,

Guilherme

tags: added: verification-done-bionic verification-done-disco verification-done-xenial
removed: verification-needed-bionic verification-needed-disco verification-needed-xenial
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (37.9 KiB)

This bug was fixed in the package linux - 5.2.0-10.11

---------------
linux (5.2.0-10.11) eoan; urgency=medium

  * eoan/linux: 5.2.0-10.11 -proposed tracker (LP: #1838113)

  * Packaging resync (LP: #1786013)
    - [Packaging] resync git-ubuntu-log

  * Eoan update: v5.2.4 upstream stable release (LP: #1838428)
    - bnx2x: Prevent load reordering in tx completion processing
    - caif-hsi: fix possible deadlock in cfhsi_exit_module()
    - hv_netvsc: Fix extra rcu_read_unlock in netvsc_recv_callback()
    - igmp: fix memory leak in igmpv3_del_delrec()
    - ipv4: don't set IPv6 only flags to IPv4 addresses
    - ipv6: rt6_check should return NULL if 'from' is NULL
    - ipv6: Unlink sibling route in case of failure
    - net: bcmgenet: use promisc for unsupported filters
    - net: dsa: mv88e6xxx: wait after reset deactivation
    - net: make skb_dst_force return true when dst is refcounted
    - net: neigh: fix multiple neigh timer scheduling
    - net: openvswitch: fix csum updates for MPLS actions
    - net: phy: sfp: hwmon: Fix scaling of RX power
    - net_sched: unset TCQ_F_CAN_BYPASS when adding filters
    - net: stmmac: Re-work the queue selection for TSO packets
    - net/tls: make sure offload also gets the keys wiped
    - nfc: fix potential illegal memory access
    - r8169: fix issue with confused RX unit after PHY power-down on RTL8411b
    - rxrpc: Fix send on a connected, but unbound socket
    - sctp: fix error handling on stream scheduler initialization
    - sctp: not bind the socket in sctp_connect
    - sky2: Disable MSI on ASUS P6T
    - tcp: be more careful in tcp_fragment()
    - tcp: fix tcp_set_congestion_control() use from bpf hook
    - tcp: Reset bytes_acked and bytes_received when disconnecting
    - vrf: make sure skb->data contains ip header to make routing
    - net/mlx5e: IPoIB, Add error path in mlx5_rdma_setup_rn
    - net: bridge: mcast: fix stale nsrcs pointer in igmp3/mld2 report handling
    - net: bridge: mcast: fix stale ipv6 hdr pointer when handling v6 query
    - net: bridge: don't cache ether dest pointer on input
    - net: bridge: stp: don't cache eth dest pointer before skb pull
    - macsec: fix use-after-free of skb during RX
    - macsec: fix checksumming after decryption
    - netrom: fix a memory leak in nr_rx_frame()
    - netrom: hold sock when setting skb->destructor
    - selftests: txring_overwrite: fix incorrect test of mmap() return value
    - net/tls: fix poll ignoring partially copied records
    - net/tls: reject offload of TLS 1.3
    - net/mlx5e: Fix port tunnel GRE entropy control
    - net/mlx5e: Rx, Fix checksum calculation for new hardware
    - net/mlx5e: Fix return value from timeout recover function
    - net/mlx5e: Fix error flow in tx reporter diagnose
    - bnxt_en: Fix VNIC accounting when enabling aRFS on 57500 chips.
    - mlxsw: spectrum_dcb: Configure DSCP map as the last rule is removed
    - net/mlx5: E-Switch, Fix default encap mode
    - mlxsw: spectrum: Do not process learned records with a dummy FID
    - dma-buf: balance refcount inbalance
    - dma-buf: Discard old fence_excl on retrying get_fences_rcu for realloc
    - Revert "gpio/spi: Fix spi-gpio...

Changed in linux (Ubuntu Eoan):
status: Fix Committed → Fix Released
Revision history for this message
Pedro Guimarães (pguimaraes) wrote :

Hi, does the fix for Bionic has been backported to stable?

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux - 5.0.0-25.26

---------------
linux (5.0.0-25.26) disco; urgency=medium

  * CVE-2019-1125
    - x86/cpufeatures: Carve out CQM features retrieval
    - x86/cpufeatures: Combine word 11 and 12 into a new scattered features word
    - x86/speculation: Prepare entry code for Spectre v1 swapgs mitigations
    - x86/speculation: Enable Spectre v1 swapgs mitigations
    - x86/entry/64: Use JMP instead of JMPQ
    - x86/speculation/swapgs: Exclude ATOMs from speculation through SWAPGS

 -- Kleber Sacilotto de Souza <email address hidden> Thu, 01 Aug 2019 12:04:35 +0200

Changed in linux (Ubuntu Disco):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (171.3 KiB)

This bug was fixed in the package linux - 4.15.0-58.64

---------------
linux (4.15.0-58.64) bionic; urgency=medium

  * unable to handle kernel NULL pointer dereference at 000000000000002c (IP:
    iget5_locked+0x9e/0x1f0) (LP: #1838982)
    - Revert "ovl: set I_CREATING on inode being created"
    - Revert "new primitive: discard_new_inode()"

linux (4.15.0-57.63) bionic; urgency=medium

  * CVE-2019-1125
    - x86/cpufeatures: Carve out CQM features retrieval
    - x86/cpufeatures: Combine word 11 and 12 into a new scattered features word
    - x86/speculation: Prepare entry code for Spectre v1 swapgs mitigations
    - x86/speculation: Enable Spectre v1 swapgs mitigations
    - x86/entry/64: Use JMP instead of JMPQ
    - x86/speculation/swapgs: Exclude ATOMs from speculation through SWAPGS

  * Packaging resync (LP: #1786013)
    - update dkms package versions

linux (4.15.0-56.62) bionic; urgency=medium

  * bionic/linux: 4.15.0-56.62 -proposed tracker (LP: #1837626)

  * Packaging resync (LP: #1786013)
    - [Packaging] resync git-ubuntu-log
    - [Packaging] update helper scripts

  * CVE-2019-2101
    - media: uvcvideo: Fix 'type' check leading to overflow

  * hibmc-drm Causes Unreadable Display for Huawei amd64 Servers (LP: #1762940)
    - [Config] Set CONFIG_DRM_HISI_HIBMC to arm64 only
    - SAUCE: Make CONFIG_DRM_HISI_HIBMC depend on ARM64

  * Bionic: support for Solarflare X2542 network adapter (sfc driver)
    (LP: #1836635)
    - sfc: make mem_bar a function rather than a constant
    - sfc: support VI strides other than 8k
    - sfc: add Medford2 (SFC9250) PCI Device IDs
    - sfc: improve PTP error reporting
    - sfc: update EF10 register definitions
    - sfc: populate the timer reload field
    - sfc: update MCDI protocol headers
    - sfc: support variable number of MAC stats
    - sfc: expose FEC stats on Medford2
    - sfc: expose CTPIO stats on NICs that support them
    - sfc: basic MCDI mapping of 25/50/100G link speeds
    - sfc: support the ethtool ksettings API properly so that 25/50/100G works
    - sfc: add bits for 25/50/100G supported/advertised speeds
    - sfc: remove tx and MCDI handling from NAPI budget consideration
    - sfc: handle TX timestamps in the normal data path
    - sfc: add function to determine which TX timestamping method to use
    - sfc: use main datapath for HW timestamps if available
    - sfc: only enable TX timestamping if the adapter is licensed for it
    - sfc: MAC TX timestamp handling on the 8000 series
    - sfc: on 8000 series use TX queues for TX timestamps
    - sfc: only advertise TX timestamping if we have the license for it
    - sfc: simplify RX datapath timestamping
    - sfc: support separate PTP and general timestamping
    - sfc: support second + quarter ns time format for receive datapath
    - sfc: support Medford2 frequency adjustment format
    - sfc: add suffix to large constant in ptp
    - sfc: mark some unexported symbols as static
    - sfc: update MCDI protocol headers
    - sfc: support FEC configuration through ethtool
    - sfc: remove ctpio_dmabuf_start from stats
    - sfc: stop the TX queue before pushing new buffers

  * [18.04 FEAT] zKVM: Add hardwar...

Changed in linux (Ubuntu Bionic):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (12.0 KiB)

This bug was fixed in the package linux - 4.4.0-159.187

---------------
linux (4.4.0-159.187) xenial; urgency=medium

  * CVE-2019-1125
    - x86/cpufeatures: Carve out CQM features retrieval
    - x86/cpufeatures: Combine word 11 and 12 into a new scattered features word
    - x86/speculation: Prepare entry code for Spectre v1 swapgs mitigations
    - x86/speculation: Enable Spectre v1 swapgs mitigations
    - x86/entry/64: Use JMP instead of JMPQ
    - x86/speculation/swapgs: Exclude ATOMs from speculation through SWAPGS

linux (4.4.0-158.186) xenial; urgency=medium

  * xenial/linux: 4.4.0-158.186 -proposed tracker (LP: #1837609)

  * Packaging resync (LP: #1786013)
    - [Packaging] resync git-ubuntu-log
    - [Packaging] update helper scripts

  * ixgbe{vf} - Physical Function gets IRQ when VF checks link state
    (LP: #1836760)
    - ixgbevf: Use cached link state instead of re-reading the value for ethtool

  * CVE-2018-5383
    - crypto: kpp - Key-agreement Protocol Primitives API (KPP)
    - crypto: dh - Add DH software implementation
    - crypto: ecdh - Add ECDH software support
    - crypto: ecdh - make ecdh_shared_secret unique
    - crypto: doc - add KPP documentation
    - crypto: kpp, (ec)dh - fix typos
    - crypto: ecc - remove unused function arguments
    - crypto: ecc - remove unnecessary casts
    - crypto: ecc - rename ecdh_make_pub_key()
    - crypto: ecdh - add privkey generation support
    - crypto: ecc - Fix NULL pointer deref. on no default_rng
    - [Config] CRYPTO_ECDH=m
    - Bluetooth: convert smp and selftest to crypto kpp API
    - crypto: ecdh - add public key verification test

  * Xenial update: 4.4.185 upstream stable release (LP: #1836668)
    - fs/binfmt_flat.c: make load_flat_shared_library() work
    - scsi: vmw_pscsi: Fix use-after-free in pvscsi_queue_lck()
    - tracing: Silence GCC 9 array bounds warning
    - gcc-9: silence 'address-of-packed-member' warning
    - usb: chipidea: udc: workaround for endpoint conflict issue
    - Input: uinput - add compat ioctl number translation for UI_*_FF_UPLOAD
    - apparmor: enforce nullbyte at end of tag string
    - parport: Fix mem leak in parport_register_dev_model
    - parisc: Fix compiler warnings in float emulation code
    - IB/hfi1: Insure freeze_work work_struct is canceled on shutdown
    - MIPS: uprobes: remove set but not used variable 'epc'
    - net: hns: Fix loopback test failed at copper ports
    - sparc: perf: fix updated event period in response to PERF_EVENT_IOC_PERIOD
    - scripts/checkstack.pl: Fix arm64 wrong or unknown architecture
    - scsi: ufs: Check that space was properly alloced in copy_query_response
    - s390/qeth: fix VLAN attribute in bridge_hostnotify udev event
    - hwmon: (pmbus/core) Treat parameters as paged if on multiple pages
    - Btrfs: fix race between readahead and device replace/removal
    - btrfs: start readahead also in seed devices
    - can: flexcan: fix timeout when set small bitrate
    - can: purge socket error queue on sock destruct
    - ARM: imx: cpuidle-imx6sx: Restrict the SW2ISO increase to i.MX6SX
    - Bluetooth: Align minimum encryption key size for LE and BR/EDR connections
    - Bluet...

Changed in linux (Ubuntu Xenial):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.