From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: bnxt_queue_up: free completion ring on failure
To: tech@openbsd.org, jmatthew@openbsd.org
Date: Wed, 8 Oct 2025 21:02:17 +0200

On Wed, Oct 08, 2025 at 04:53:38PM +0200, Stefan Sperling wrote:
> On Tue, Oct 07, 2025 at 06:51:39PM +0200, Alexander Bluhm wrote:
> > I have tested it on my usual network interface throughput setup.
> > 
> > OK bluhm@
> 
> Thanks!
> 
> > Looks like bnxt_attach() is also missing bnxt_hwrm_ring_free() 
> > in the error path.
> 
> Right. Proposed fix:

OK bluhm@

> M  sys/dev/pci/if_bnxt.c  |  5+  2-
> 
> 1 file changed, 5 insertions(+), 2 deletions(-)
> 
> commit - 9b0af9465c11f4f47dc48ded70e83e5604864630
> commit + 8efd895c35ae0a377613dfecb311d5197c54736a
> blob - 2d1a03d469dcd29e12986562bbde42922e5f1b21
> blob + d5530564bf533bf7f80347011f3e08cb553ff216
> --- sys/dev/pci/if_bnxt.c
> +++ sys/dev/pci/if_bnxt.c
> @@ -625,14 +625,14 @@ bnxt_attach(struct device *parent, struct device *self
>  	if (bnxt_cfg_async_cr(sc, cpr) != 0) {
>  		printf("%s: failed to set async completion ring\n",
>  		    DEVNAME(sc));
> -		goto free_cp_mem;
> +		goto free_cp_ring;
>  	}
>  	bnxt_write_cp_doorbell(sc, &cpr->ring, 1);
>  
>  	if (bnxt_set_cp_ring_aggint(sc, cpr) != 0) {
>  		printf("%s: failed to set interrupt aggregation\n",
>  		    DEVNAME(sc));
> -		goto free_cp_mem;
> +		goto free_cp_ring;
>  	}
>  
>  	strlcpy(ifp->if_xname, DEVNAME(sc), IFNAMSIZ);
> @@ -720,6 +720,9 @@ intrdisestablish:
>  		pci_intr_disestablish(sc->sc_pc, bq->q_ihc);
>  		bq->q_ihc = NULL;
>  	}
> +free_cp_ring:
> +	bnxt_hwrm_ring_free(sc,
> +	    HWRM_RING_ALLOC_INPUT_RING_TYPE_L2_CMPL, &cpr->ring);
>  free_cp_mem:
>  	bnxt_dmamem_free(sc, cpr->ring_mem);
>  deintr: