Re: [PATCH net v3 3/7] net: bcm4908_enet: release the TX ring on stop

[Kordian Chmielewski]

Sashiko's review of this patch [1] attributes one Medium point to it:
when the TX channel does not stop in time, the skbs left in the ring
leak, and on unbind bcm4908_enet_dma_free() frees the slots and the
descriptor ring while the channel may still be running. It suggests
forcing the channel idle with ENET_DMA_CTRL_CHANNEL_RESET or by clearing
MASTER_EN, and then cleaning the ring.

The leak after a timeout is on purpose, and the changelog says so. The
review of v2 suggested either forcing a reset or leaking the buffers.
I don't know what a channel reset does to a transfer in flight, so I
chose the leak: it costs memory, but the DMA never reads a freed skb.
The timeout never fired in my tests; every stop, all made while the
router was sending at line rate, finished in time. The unbind path is
older than this patch: before it, stop() did not wait for the TX
channel at all, and bcm4908_enet_dma_free() kfree()d the slot array
with the skbs still in it.

The review also points out that RX has the same problem the other way
round: bcm4908_enet_dma_rx_ring_disable() only warns on a timeout, and
stop() frees the RX buffers anyway, while the DMA may still write into
them. That one is older too, and I'll add it to the follow-ups, next to
the ignored dma_init() error in open().

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261113235847.517308-1-kudzo51%40gmail.com