From: Stefan Sperling Subject: bnxt_queue_up: free completion ring on failure To: tech@openbsd.org Cc: jmatthew@openbsd.org Date: Tue, 7 Oct 2025 15:34:56 +0200 If bnxt firmware fails to allocate a Tx ring in bnxt_queue_up() then we jump into an error path which leaks a completion ring. Found while running ping traffic across bnxt0 in parallel with: while true; do ifconfig bnxt0 down up; done The patch below avoids leaking a completion ring but it does not resolve the problem where the above down/up loop eventually fails: ifconfig: bnxt0: SIOCSIFFLAGS: Cannot allocate memory bnxt0: HWRM_RING_ALLOC command returned RESOURCE_ALLOC_ERROR error. bnxt0: failed to set up tx ring There might be another resource leak caused by the driver, or perhaps firmware is leaking memory and will eventually always hit this problem. bnxt0 at pci14 dev 0 function 0 "Broadcom BCM57414" rev 0x01: fw ver 234.0.150, msix, 8 queues, address xx:xx:xx:xx:xx:xx ok? free completion ring on failure in bnxt_queue_up() M sys/dev/pci/if_bnxt.c | 13+ 3- 1 file changed, 13 insertions(+), 3 deletions(-) commit - dfd988efd85f876f66c82e5f98ff3ff8ac9ed907 commit + bdb26caee92dfd55bf4a07741f3e3b15cf02cfc7 blob - 49b4a418a8801b88632d1a78fe29d589c5188f92 blob + 87a8f220796fc433431e49a7484e81e4b8ebd50e --- sys/dev/pci/if_bnxt.c +++ sys/dev/pci/if_bnxt.c @@ -821,13 +821,13 @@ bnxt_queue_up(struct bnxt_softc *sc, struct bnxt_queue HWRM_NA_SIGNATURE, 1) != 0) { printf("%s: failed to allocate completion queue %d\n", DEVNAME(sc), bq->q_index); - goto free_rx; + goto free_cp_ring_mem; } if (bnxt_set_cp_ring_aggint(sc, cp) != 0) { printf("%s: failed to set interrupt %d aggregation\n", DEVNAME(sc), bq->q_index); - goto free_rx; + goto free_cp_ring; } bnxt_write_cp_doorbell(sc, &cp->ring, 1); } @@ -836,7 +836,7 @@ bnxt_queue_up(struct bnxt_softc *sc, struct bnxt_queue BNXT_DMA_DVA(sc->sc_stats_ctx_mem) + (bq->q_index * sizeof(struct ctx_hw_stats))) != 0) { printf("%s: failed to set up stats context\n", DEVNAME(sc)); - goto free_rx; + goto free_cp_ring; } tx->tx_ring.phys_id = (uint16_t)HWRM_NA_SIGNATURE; @@ -998,6 +998,16 @@ dealloc_rx: &rx->rx_ring); dealloc_stats: bnxt_hwrm_stat_ctx_free(sc, cp); +free_cp_ring: + if (sc->sc_intrmap != NULL) { + bnxt_hwrm_ring_free(sc, + HWRM_RING_ALLOC_INPUT_RING_TYPE_L2_CMPL, &cp->ring); + } +free_cp_ring_mem: + if (sc->sc_intrmap != NULL) { + bnxt_dmamem_free(sc, cp->ring_mem); + cp->ring_mem = NULL; + } free_rx: bnxt_dmamem_free(sc, rx->rx_ring_mem); rx->rx_ring_mem = NULL;