From: Visa Hankala Subject: Re: sys/cnmac: fix command buffer leak on transmit failure To: "Kirill A. Korinsky" Cc: tech@openbsd.org Date: Thu, 9 Jul 2026 14:22:51 +0000 On Thu, Jul 09, 2026 at 12:28:47AM +0200, Kirill A. Korinsky wrote: > visa@, > > my ER-4 whcih I used as router had reached interesting state: it floods > errors like this: > > Jun 30 11:11:39 gw /bsd: cnmac2: cannot allocate command buffer from free pool allocator > Jun 30 11:11:39 gw /bsd: cnmac2: failed to transmit packet > Jun 30 11:11:40 gw /bsd: cnmac2: cannot allocate command buffer from free pool allocator > Jun 30 11:11:40 gw /bsd: cnmac2: failed to transmit packet > > tons of them, I have no idea what had happened before I discovered it in > that state, but device was ok via serial, but network was dead. > > After reading code near that error I think I had spotted a leak. It seems to me that there is no leak in the code. The local cmdptr pointer is derived using cmdptr_idx and discarded when the function returns. cnmac_send_cmd() updates cmdptr_idx only if it was able to add the command words to the queue. > Index: sys/arch/octeon/dev/if_cnmac.c > =================================================================== > RCS file: /home/cvs/src/sys/arch/octeon/dev/if_cnmac.c,v > diff -u -p -r1.91 if_cnmac.c > --- sys/arch/octeon/dev/if_cnmac.c 19 Jun 2026 15:12:10 -0000 1.91 > +++ sys/arch/octeon/dev/if_cnmac.c 8 Jul 2026 22:20:34 -0000 > @@ -918,9 +918,6 @@ cnmac_send_cmd(struct cnmac_softc *sc, u > > OCTEON_ETH_KASSERT(cmdptr != NULL); > > - *cmdptr++ = pko_cmd_w0; > - *cmdptr++ = pko_cmd_w1; > - > OCTEON_ETH_KASSERT(sc->sc_cmdptr.cmdptr_idx + 2 <= FPA_COMMAND_BUFFER_POOL_NWORDS - 1); > > if (sc->sc_cmdptr.cmdptr_idx + 2 == FPA_COMMAND_BUFFER_POOL_NWORDS - 1) { > @@ -934,10 +931,14 @@ cnmac_send_cmd(struct cnmac_softc *sc, u > result = 1; > goto done; > } > + *cmdptr++ = pko_cmd_w0; > + *cmdptr++ = pko_cmd_w1; > *cmdptr++ = buf; > sc->sc_cmdptr.cmdptr = (uint64_t)buf; > sc->sc_cmdptr.cmdptr_idx = 0; > } else { > + *cmdptr++ = pko_cmd_w0; > + *cmdptr++ = pko_cmd_w1; > sc->sc_cmdptr.cmdptr_idx += 2; > } > > > > -- > wbr, Kirill >