On 9.1.2024. 3:04, Jonathan Matthew wrote: > On Wed, Jan 03, 2024 at 10:14:12AM +0100, Hrvoje Popovski wrote: >> On 3.1.2024. 7:51, Jonathan Matthew wrote: >>> On Wed, Jan 03, 2024 at 01:50:06AM +0100, Alexander Bluhm wrote: >>>> On Wed, Jan 03, 2024 at 12:26:26AM +0100, Hrvoje Popovski wrote: >>>>> While testing kettenis@ ipl diff from tech@ and doing iperf3 to bnxt >>>>> interface and ifconfig bnxt0 down/up at the same time I can trigger >>>>> panic. Panic can be triggered without kettenis@ diff... >>>> It is easy to reproduce. ifconfig bnxt1 down/up a few times while >>>> receiving TCP traffic with iperf3. Machine still has kettenis@ diff. >>>> My panic looks different. >>> It looks like I wasn't trying very hard when I wrote bnxt_down(). >>> I think there's also a problem with bnxt_up() unwinding after failure >>> in various places, but that's a different issue. >>> >>> This makes it a more resilient for me, though it still logs >>> 'bnxt0: unexpected completion type 3' a lot if I take the interface >>> down while it's in use. I'll look at that separately. >> >> Hi, >> >> with this diff I can still panic box with ifconfig up/down but not as >> fast as without it > > Right, this is the other problem where bnxt_up() wasn't cleaning up properly > after failing part way through. This diff should fix that, but I don't think > it will fix the 'HWRM_RING_ALLOC command returned RESOURCE_ALLOC_ERROR error' > problem, so the interface will still stop working at that point. >
With this diff bnxt behaves exactly as you said. After a lot of ifconfig down/up at some point I get smc24# ifconfig bnxt0 down smc24# ifconfig bnxt0 up bnxt0: attempt to re-allocate ring 0010 bnxt0: failed to allocate completion queue 0 and bnxt stop working .. > > Index: if_bnxt.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/if_bnxt.c,v > retrieving revision 1.39 > diff -u -p -r1.39 if_bnxt.c > --- if_bnxt.c 10 Nov 2023 15:51:20 -0000 1.39 > +++ if_bnxt.c 9 Jan 2024 01:59:38 -0000 > @@ -1073,7 +1081,7 @@ bnxt_up(struct bnxt_softc *sc) > if (bnxt_hwrm_vnic_ctx_alloc(sc, &sc->sc_vnic.rss_id) != 0) { > printf("%s: failed to allocate vnic rss context\n", > DEVNAME(sc)); > - goto down_queues; > + goto down_all_queues; > } > > sc->sc_vnic.id = (uint16_t)HWRM_NA_SIGNATURE; > @@ -1139,8 +1147,11 @@ dealloc_vnic: > bnxt_hwrm_vnic_free(sc, &sc->sc_vnic); > dealloc_vnic_ctx: > bnxt_hwrm_vnic_ctx_free(sc, &sc->sc_vnic.rss_id); > + > +down_all_queues: > + i = sc->sc_nqueues; > down_queues: > - for (i = 0; i < sc->sc_nqueues; i++) > + while (i-- > 0) > bnxt_queue_down(sc, &sc->sc_queues[i]); > > bnxt_dmamem_free(sc, sc->sc_rx_cfg); >