| 1b97a269 | 14-Sep-2026 |
Alexander Duyck <alexanderduyck@fb.com> |
eth: fbnic: Handle FW mailbox completions flagged with an error
The firmware can complete a mailbox descriptor while also setting FW_ERR to indicate it could not process the request, for example on
eth: fbnic: Handle FW mailbox completions flagged with an error
The firmware can complete a mailbox descriptor while also setting FW_ERR to indicate it could not process the request, for example on a mailbox DMA error. The completion carries no valid data.
The driver did not check FW_ERR. On the Rx mailbox it would sync and parse the stale page as a normal message, and on the Tx mailbox it silently freed the request. If the initial capabilities exchange in fbnic_mbx_poll_tx_ready() hit FW_ERR -- on the Tx request or on the Rx response descriptor -- no response was parsed and the poll spun until it timed out even though the ring was healthy.
Check FW_ERR on both mailboxes. Count it per-mailbox in fbnic_fw_mbx.resp_error, which is also shown in debugfs, warn (rate limited, since the bit is firmware controlled), and drop the Rx page instead of parsing it.
In fbnic_mbx_poll_tx_ready() re-issue the capabilities request when either the Tx or the Rx resp_error counter advances, so a FW_ERR on the request or on its response triggers a retry rather than a timeout. A valid capabilities response is honored before the retry check, so a response parsed in the same poll as an unrelated FW_ERR is not discarded. The counters are mailbox-wide rather than keyed to the capabilities request; that is sufficient here because the exchange runs during bring-up before any other mailbox traffic, and any spurious retry is bounded by the existing 10s timeout.
Fixes: da3cde08209e ("eth: fbnic: Add FW communication mechanism") Signed-off-by: Alexander Duyck <alexanderduyck@fb.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/178942023343.7700.9423398932961964439.stgit@ahduyck-xeon-server.home.arpa Signed-off-by: Jakub Kicinski <kuba@kernel.org>
show more ...
|
| 8947f13e | 14-Sep-2026 |
Alexander Duyck <alexanderduyck@fb.com> |
eth: fbnic: Set AW_FLUSH_MODE alongside AW_FLUSH when flushing the mailbox
When tearing down the FW mailbox Rx ring, fbnic_mbx_reset_desc_ring() writes AW_CFG with FLUSH set and everything else, BME
eth: fbnic: Set AW_FLUSH_MODE alongside AW_FLUSH when flushing the mailbox
When tearing down the FW mailbox Rx ring, fbnic_mbx_reset_desc_ring() writes AW_CFG with FLUSH set and everything else, BME included, cleared. Clearing BME halts the device's writes to the host but leaves the staged requests parked in the PUL write pipeline rather than draining them, so on the write path FLUSH alone never terminates the outstanding requests and the flush the firmware waits on never completes.
Add the FLUSH_MODE definition and set both bits so the staged writes drain out of the pipeline on their own. BME stays cleared, so nothing lands on the host; it is restored later in fbnic_mbx_init_desc_ring() when the ring is rebuilt, once the outstanding writes are gone.
The read path is unaffected. AR_CFG has no equivalent mode bit and AR_FLUSH terminates the outstanding reads by itself, so it is left as is.
Both writes remain plain stores rather than read-modify-writes. That is deliberate: the matching write in fbnic_mbx_init_desc_ring() restores BME and the TLP attributes, and clears both flush bits as a side effect.
Fixes: 3b12f00ddd08 ("fbnic: Gate AXI read/write enabling on FW mailbox") Signed-off-by: Alexander Duyck <alexanderduyck@fb.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/178942022583.7700.11050671998277309744.stgit@ahduyck-xeon-server.home.arpa Signed-off-by: Jakub Kicinski <kuba@kernel.org>
show more ...
|
| 4bcc4a92 | 14-Sep-2026 |
Alexander Duyck <alexanderduyck@fb.com> |
eth: fbnic: reset num_napi when the napi vectors are freed
fbn->num_napi is the count of live napi vectors, each of which owns an IRQ. The PM path had freed them without clearing the count. fbnic_p
eth: fbnic: reset num_napi when the napi vectors are freed
fbn->num_napi is the count of live napi vectors, each of which owns an IRQ. The PM path had freed them without clearing the count. fbnic_pm_suspend() tears the datapath down via ndo_stop() and frees the IRQs, but leaves netif_running() true so resume knows to re-open. Resume rebuilds the datapath in __fbnic_pm_resume() and fbnic_reset_queues() sets num_napi and __fbnic_open() re-allocates the vectors.
When the datapath is torn down but never rebuilt, num_napi is left pointing at freed vectors under 2 different scenarios: - a PCIe error recovery that fails (fbnic_err_slot_reset() -> __fbnic_pm_resume() returns an error -> PCI_ERS_RESULT_DISCONNECT), so .resume never runs; or - an __fbnic_open() that fails partway on resume and unwinds, freeing the vectors after fbnic_reset_queues() has already set num_napi.
The netdev is then running with num_napi > 0 but napi[] freed, and the eventual remove/unbind close re-enters fbnic_down() -> fbnic_dbg_down() and dereferences the freed vectors: BUG: kernel NULL pointer dereference, address: 0000000000000210 RIP: fbnic_dbg_down+0x28
Clear num_napi when the vectors are freed: in the suspend teardown (a good resume re-establishes it before __fbnic_open()) and on the resume open failure. A redundant ndo_stop() then walks an empty napi[]. The normal ndo_stop() down/up cycle is untouched and keeps num_napi for the next ndo_open().
Fixes: bc6107771bb4 ("eth: fbnic: Allocate a netdevice and napi vectors with queues") Signed-off-by: Alexander Duyck <alexanderduyck@fb.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/178942021809.7700.10804028989308077839.stgit@ahduyck-xeon-server.home.arpa Signed-off-by: Jakub Kicinski <kuba@kernel.org>
show more ...
|
| b5d9e9d4 | 14-Sep-2026 |
Alexander Duyck <alexanderduyck@fb.com> |
eth: fbnic: use the Rx queue napi pointer to find the napi vector
The queue management ndos pick the napi vector for an Rx queue with: nv = fbn->napi[idx % fbn->num_napi];
The issue is this is onl
eth: fbnic: use the Rx queue napi pointer to find the napi vector
The queue management ndos pick the napi vector for an Rx queue with: nv = fbn->napi[idx % fbn->num_napi];
The issue is this is only correct in the cases where there are no standalone Tx vectors. In those cases we were allocating the Tx vectors first and then the Rx so the queues would be pointing to Tx NAPI vectors instead of the Rx ones.
The mapping the ndos want is already recorded. fbnic_set_netif_napi() publishes it with netif_queue_set_napi(), which stores the napi pointer in netdev_rx_queue.napi, and fbnic_reset_netif_napi() clears it again. Both run under the netdev instance lock that the queue management ndos also hold, so the pointer can be read directly.
Use it and drop the divide. The pointer is NULL exactly while the datapath is down, so fbnic_queue_mem_alloc() can reject that case rather than reaching into freed state: netdev_rx_queue_restart() calls it before it tests netif_running(), and fbnic_pm_suspend() leaves netif_running() true across a PCIe recovery that never completes, so a queue restart can arrive after fbnic_stop() has freed the rings and the vectors. fbnic_stop() clears the association in fbnic_reset_netif_queues() before fbnic_free_napi_vectors(), so the NULL is always published first. fbnic_queue_start() and fbnic_queue_stop() need no check of their own, as netdev_rx_queue_reconfig() only reaches them once fbnic_queue_mem_alloc() has succeeded under the same instance lock.
Fixes: da43127a8edc ("eth: fbnic: support queue ops / zero-copy Rx") Signed-off-by: Alexander Duyck <alexanderduyck@fb.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/178942021136.7700.4391219358260544104.stgit@ahduyck-xeon-server.home.arpa Signed-off-by: Jakub Kicinski <kuba@kernel.org>
show more ...
|
| 1f4c7306 | 14-Sep-2026 |
Björn Töpel <bjorn@kernel.org> |
eth: fbnic: Handle maximum standalone channels
Standalone channels use one NAPI vector for each Tx and Rx queue. fbnic's allocation path excludes FBNIC_MAX_TXQS from that layout. A 64-Tx/64-Rx confi
eth: fbnic: Handle maximum standalone channels
Standalone channels use one NAPI vector for each Tx and Rx queue. fbnic's allocation path excludes FBNIC_MAX_TXQS from that layout. A 64-Tx/64-Rx configuration therefore records 128 vectors but allocates only 64, leaving NULL entries that resource setup dereferences.
Include the maximum vector count in standalone allocation.
Fixes: bc6107771bb4 ("eth: fbnic: Allocate a netdevice and napi vectors with queues") Signed-off-by: Björn Töpel <bjorn@kernel.org> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/178942020457.7700.13129750616387075931.stgit@ahduyck-xeon-server.home.arpa Signed-off-by: Jakub Kicinski <kuba@kernel.org>
show more ...
|
| 1b82958f | 14-Sep-2026 |
Alexander Duyck <alexanderduyck@fb.com> |
net: ethtool: keep rtnl_lock for the ioctl self test
An offline self test that brings the interface down and back up with netif_close() / netif_open() requires rtnl_lock for both. Since the ethtool
net: ethtool: keep rtnl_lock for the ioctl self test
An offline self test that brings the interface down and back up with netif_close() / netif_open() requires rtnl_lock for both. Since the ethtool IOCTL path became rtnl-optional for ops-locked drivers, the ETHTOOL_TEST ioctl runs holding only the netdev instance lock, so on an ops-locked driver the self test now tears the device down without rtnl_lock.
With lockdep this reproduces deterministically on every offline self test on such a driver; note the sole lock held is the instance lock, not rtnl:
WARNING: suspicious RCU usage net/core/netpoll.c:207 suspicious rcu_dereference_protected() usage! 1 lock held by ethtool/107: #0: (&dev->lock){+.+.}, at: dev_ethtool Call Trace: netpoll_poll_disable __dev_close_many netif_close_many netif_close fbnic_self_test dev_ethtool_locked dev_ethtool dev_ioctl sock_ioctl __x64_sys_ioctl
Without lockdep the same condition trips ASSERT_RTNL() in __dev_close_many() / __dev_open(); that check only samples the global rtnl state, so it can be masked by a concurrent rtnl holder, but the device is still being reconfigured without the lock it requires.
The ethtool self_test is a legacy ioctl-only command, so an ETHTOOL_TEST case is only needed on the ioctl path. Add an opt-in bit for drivers whose self test needs rtnl_lock and set it on the ops-locked drivers whose offline self test tears the interface down and up:
- fbnic (ops-locked via queue_mgmt_ops): fbnic_self_test() offline path uses netif_close() / netif_open(). - bnxt (ops-locked via queue_mgmt_ops): bnxt_self_test() offline path goes through bnxt_close_nic() / bnxt_half_open_nic() / bnxt_half_close_nic() / bnxt_open_nic(), which close and reopen the device.
Fixes: f994752b1127 ("net: ethtool: optionally skip rtnl_lock on IOCTL path") Signed-off-by: Alexander Duyck <alexanderduyck@fb.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/178942019771.7700.338431553546884773.stgit@ahduyck-xeon-server.home.arpa Signed-off-by: Jakub Kicinski <kuba@kernel.org>
show more ...
|
| f9a3e051 | 05-Jun-2026 |
Jakub Kicinski <kuba@kernel.org> |
net: ethtool: optionally skip rtnl_lock on Netlink path for SET ops
Make ethtool not take rtnl_lock for SET commands when operation is performed on an ops-locked driver. cfg/cfg_pending are now ops-
net: ethtool: optionally skip rtnl_lock on Netlink path for SET ops
Make ethtool not take rtnl_lock for SET commands when operation is performed on an ops-locked driver. cfg/cfg_pending are now ops-locked, since only ethtool modifies them.
Some SET driver callbacks will still need rtnl_lock, most notably those which may end up calling netdev_update_features() or the qdisc layer (via netif_set_real_num_tx_queues()). Let drivers selectively opt back into the rtnl_lock with a new bitfield in ops.
We need two helpers since Netlink and ioctl cmds have different values. Keep the helpers side by side in common.h to make sure they get updated together, even tho they will only get called from ioctl.c and netlink.c.
SET commands which don't use ethnl_default_set_doit() are converted by subsequent commits.
Reviewed-by: Eric Dumazet <edumazet@google.com> Acked-by: Stanislav Fomichev <sdf@fomichev.me> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Link: https://patch.msgid.link/20260605002912.3456868-6-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
show more ...
|
| 3877097c | 30-Apr-2026 |
Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com> |
net: eth: fbnic: Add pma read and write access
Document the MDIO interface topology with an ASCII diagram showing the MAC, PCS (MMD 3), FEC, Separated PMA (MMD 8), and PMD (MMD 1) blocks and their i
net: eth: fbnic: Add pma read and write access
Document the MDIO interface topology with an ASCII diagram showing the MAC, PCS (MMD 3), FEC, Separated PMA (MMD 8), and PMD (MMD 1) blocks and their interconnects. The diagram illustrates how 4 lanes connect the MAC through PCS, FEC, and PMA, then narrow to 2 lanes at the PMD.
The c45 read and write routines are enhanced to support read and write of the separated PMA for the fbnic.
Co-developed-by: Alexander Duyck <alexanderduyck@fb.com> Signed-off-by: Alexander Duyck <alexanderduyck@fb.com> Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com> Link: https://patch.msgid.link/20260430150802.3521-4-mike.marciniszyn@gmail.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
show more ...
|