On Fri, Sep 20, 2024 at 05:05:14PM +0300, Wei Fang wrote:
zero init is good but shouldn't you be draining these buffers when removing XDP resources at least? what happens with DMA mappings that are related to these cached buffers?
All the buffers will be freed and DMA will be unmapped when XDP program is installed.
There is still a problem with the patch you proposed here, which is that enetc_reconfigure() has one more call site, from enetc_hwtstamp_set(). If enetc_free_rxtx_rings() is the one that gets rid of the stale buffers, it should also be the one that resets xdp_tx_in_flight, otherwise you will still leave the problem unsolved where XDP_TX can be interrupted by a change in hwtstamping state, and the software "in flight" counter gets out of sync with the ring state.
Also, I suspect that the blamed commit is wrong. Also the normal netdev close path should be susceptible to this issue, not just enetc_reconfigure(). Maybe something like ff58fda09096 ("net: enetc: prioritize ability to go down over packet processing"). That's when we started rushing the NAPI poll routing to finish. I don't think it was possible, before that, to close the netdev while there were XDP_TX frames pending to be recycled.
I am thinking that another solution may be better, which is mentioned in another thread replying to Vladimir, so that xdp_tx_in_flight will naturally drop to 0, and the TX-related statistics will be more accurate.
Please give me some more time to analyze the flow after just your patch 2/3. I have a draft reply, but I would still like to test some things.