Re: [PATCH net v2 1/6] net: bcm4908_enet: fix the TX ring wrap

[Florian Fainelli <florian.fainelli@broadcom.com>]

On 11/12/26 11:50, Kordian Chmielewski wrote:
> start_xmit() wraps write_idx at length - 1 and sets WRAP on the
> descriptor at length - 2, while poll_tx() wraps read_idx at length,
> and the free descriptor count is taken from length. With read_idx at 0
> and write_idx at length - 2 the count comes out as 2, the frame is
> queued and write_idx wraps to 0 == read_idx: a full ring looks empty.
> The following frames overwrite slots whose skbs were never unmapped
> or freed, their bytes stay in BQL, and the queue stops for good with
> nothing left to wake it.
> 
> On a TP-Link Archer AX6000 this stopped everything the CPU sent to the
> switch under sustained TX load (about 1.3 Gbit/s through the switch's
> CPU port): tx_packets froze, nothing was logged, and BQL showed about
> 185 KB in flight against a limit of about 10 KB.
> 
> The same mismatch also lets poll_tx() step onto slot length - 1, which
> start_xmit() never fills: once per lap it completes that slot,
> unmapping address 0 with length 0 and counting a packet that was never
> sent. That one is from reading the code.
> 
> Use every descriptor, with WRAP on the last one, wrap both indexes at
> length and keep one descriptor free, so that read_idx == write_idx can
> only mean an empty ring. Stop and wake the queue with the
> netdev_queues.h helpers: start_xmit() stops it when the ring is full,
> also when it finds the ring full on entry, and poll_tx() wakes it once
> there is room again.
> 
> Fixes: 4feffeadbcb2 ("net: broadcom: bcm4908enet: add BCM4908 controller driver")
> Signed-off-by: Kordian Chmielewski <kudzo51@gmail.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian