Skip to contentSkip to conversation

eth: fbnic: ring the doorbell if a burst ends in a drop

PATCH Applied Jakub Kicinski 3 participants 3 messages Sep 15 – Sep 17

Subsystems (3)
lore
Share & tools
lore

Jakub Kicinski

author maintainer
#0 lore raw

PATCH 1/1 eth: fbnic: ring the doorbell if a burst ends in a drop

fbnic_tx_map() skips the doorbell write, and the completion request, for every packet handed to it with xmit_more set, counting on the packet which ends the burst to publish them all. When that packet is dropped instead - skb_put_padto(), skb_cow_head() or a DMA mapping failure - nothing rings. The descriptors of the preceding packets stay invisible to the HW until the next transmit on that queue, which for a burst-then-idle workload may never come.

Remember the meta descriptor of the last packet left without a doorbell and flush it from the error paths. The completion request has to be set on that descriptor rather than simply writing the tail, otherwise the HW would transmit the packets but never report a head, and the ring would fill up and stall for good.

This is very similar to Joe's recent series of fixes for bnxt. Not seen in real life, reproduced under QEMU with failure injection.

Fixes 9a57bacd574b ("eth: fbnic: Add basic Tx handling")

---
Reproduced under the QEMU fbnic model by failing the DMA mapping of the last segment of a software-GSO burst: 21 descriptors (7 packets) stayed in the ring with head stuck at 24 and tail at 45, 10094 bytes stuck in BQL, and QEMU saw none of the 7 frames.

---
 drivers/net/ethernet/meta/fbnic/fbnic_txrx.h |  9 ++++-
 drivers/net/ethernet/meta/fbnic/fbnic_txrx.c | 42 +++++++++++++++-----
 2 files changed, 40 insertions(+), 11 deletions(-)
drivers/net/ethernet/meta/fbnic/fbnic_txrx.h +7 -2
index e03c9d2c38dc..f5899446dcc5 100644
@@ -128,9 +128,14 @@ struct fbnic_ring {
 		/* Rx BDQs only */
 		struct page_pool *page_pool;
 
-		/* Deferred_head is used to cache the head for TWQ1 if
+		/* TWQ0 only, index of the meta descriptor of the last packet
+		 * placed in the ring without ringing the doorbell, -1 if the
+		 * doorbell is in sync with the tail.
+		 */
+		s32 deferred_meta;
+
+		/* TCQ only, used to cache the head for TWQ1 if
 		 * an attempt is made to clean TWQ1 with zero napi_budget.
-		 * We do not use it for any other ring.
 		 */
 		s32 deferred_head;
 	};
drivers/net/ethernet/meta/fbnic/fbnic_txrx.c +33 -9

73 lines not on this page · raw message

Signature · 1 line
2.55.0
reviewer Reviewed-by on v1 1/1
#1 lore raw
Quoted 17 lines · Jakub Kicinski

fbnic_tx_map() skips the doorbell write, and the completion request, for every packet handed to it with xmit_more set, counting on the packet which ends the burst to publish them all. When that packet is dropped instead - skb_put_padto(), skb_cow_head() or a DMA mapping failure - nothing rings. The descriptors of the preceding packets stay invisible to the HW until the next transmit on that queue, which for a burst-then-idle workload may never come.

Remember the meta descriptor of the last packet left without a doorbell and flush it from the error paths. The completion request has to be set on that descriptor rather than simply writing the tail, otherwise the HW would transmit the packets but never report a head, and the ring would fill up and stall for good.

This is very similar to Joe's recent series of fixes for bnxt. Not seen in real life, reproduced under QEMU with failure injection.

Quoted 8 lines
Fixes 9a57bacd574b ("eth: fbnic: Add basic Tx handling")

---
Reproduced under the QEMU fbnic model by failing the DMA mapping of the last segment of a software-GSO burst: 21 descriptors (7 packets) stayed in the ring with head stuck at 24 and tail at 45, 10094 bytes stuck in BQL, and QEMU saw none of the 7 frames.

#2 lore raw
In reply to Jakub Kicinski
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 9 lines · On Mon, 14 Sep 2026 19:23:27 -0700 you wrote:

fbnic_tx_map() skips the doorbell write, and the completion request, for every packet handed to it with xmit_more set, counting on the packet which ends the burst to publish them all. When that packet is dropped instead - skb_put_padto(), skb_cow_head() or a DMA mapping failure - nothing rings. The descriptors of the preceding packets stay invisible to the HW until the next transmit on that queue, which for a burst-then-idle workload may never come.

[...]

Here is the summary with links:
- [net] eth: fbnic: ring the doorbell if a burst ends in a drop

    https://git.kernel.org/netdev/net/c/490599ab2313

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html