On Fri, Jul 10, 2026 at 10:45 AM Thomas Monjalon <[email protected]> wrote: > > 09/07/2026 06:07, Joshua Washington: > > The GQ TX datapath was set up to write the mbuf head into the sw_ring > > before writing the descriptors. This poses a problem because it's > > possible for the packet to be dropped due to lacking the FIFO space to > > do a proper TX. In such a case, the packet won't be sent, and will lead > > to leaked mbufs in the subsequent segments. > > > > There is also no real reason that the head mbuf must be set in the > > sw_ring separately from the others; the mbuf chain is not actually > > walked as part of GQ TX. > > > > Fixes: a46583cf43c8 ("net/gve: support Rx/Tx") > > Cc: [email protected] > > Signed-off-by: Joshua Washington <[email protected]> > > Reviewed-by: Jasper Tran O'Leary <[email protected]> > > AI found this issue: > > **Error:** Resource leak on early return from `gve_tx_burst_qpl()`. > > The patch modifies the loop to store mbufs in `sw_ring` only after checking > FIFO availability, but introduces a leak when `goto end_of_tx` is taken after > the first segment has been processed. At line 306-307: > > ```c > if (!is_fifo_avail(txq, hlen)) { > gve_tx_clean(txq); > if (!is_fifo_avail(txq, hlen)) > goto end_of_tx; > } > ``` > > If the second `is_fifo_avail()` check fails and we `goto end_of_tx`, the > current `tx_pkt` (which is the mbuf chain head) has not been added to > `sw_ring` yet (that happens later in the modified loop at lines 346-351). The > function will return without freeing `tx_pkt`, and the mbuf chain is lost. > > **Suggested fix:** Free the current packet before `goto end_of_tx`: > > ```c > if (!is_fifo_avail(txq, hlen)) { > gve_tx_clean(txq); > if (!is_fifo_avail(txq, hlen)) { > rte_pktmbuf_free(tx_pkt); > goto end_of_tx; > } > } > ``` > > Similarly, for the TSO path at lines 314-316, if the FIFO space check fails, > `tx_pkt` should be freed before the goto.
I believe this is a hallucination. The driver does not free the TX packet in the case of a failed send because it does not count that packet in the packets sent to the application. It ultimately did not take ownership of the mbuf for the failing packet from the application, so it should not free that mbuf. The tx_id pointer is not incremented until after it is sure that the packet data can be copied to the DMA area (FIFO buffer), so there is no unintended descriptor write either, as far as I can tell. -- Josh
