Re: panic: igc_rxrefill: msix=0 id=351

[Thomas Klausner <wiz%netbsd.org@localhost>]

On Fri, Apr 03, 2026 at 12:04:38AM +0100, Thomas Klausner wrote:
> Today I had a reboot on 11.99.5/20260320 because of
> 
> panic: igc_rxrefill: msix=0 id=351

Since I hit this again, I looked at the code and at if_mcx and came up
with the attached patch. Since it's my first diff to a network driver,
I'd like some feedback. I have compile-tested it and I think it should
be correct but have done no further tests yet.

The idea is to register a callout that is triggered when the code
would panic before, i.e. when during providing mbufs for rx there is a
memory problem.

The callout will re-try allocating the missing buffers.  The driver
already had pointers that I could use in the callout callback:
last_desc_filled (+1) as the low end - the last/furthest-away mbuf
provided to the hardware - and next_to_check as the high end - the
next buffer containing an mbuf that will be used by the hardware.

I changed igc_rxrefill to remove the 'end' argument since it can be
computed from next_to_check and to return a bool, so the callers know
when they have to schedule the callout.

Please review the change.

Thanks,
 Thomas

Index: if_igc.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/igc/if_igc.c,v
retrieving revision 1.24
diff -u -r1.24 if_igc.c
--- if_igc.c	3 Jan 2026 20:17:16 -0000	1.24
+++ if_igc.c	23 May 2026 20:35:17 -0000
@@ -196,7 +196,8 @@
 static int	igc_rxrinfo(struct igc_softc *, struct if_rxrinfo *);
 #endif
 static void	igc_rxfill(struct rx_ring *);
-static void	igc_rxrefill(struct rx_ring *, int);
+static bool	igc_rxrefill(struct rx_ring *);
+static void	igc_rxrefill_callout(void *);
 static bool	igc_rxeof(struct rx_ring *, u_int);
 static int	igc_rx_checksum(struct igc_queue *, uint64_t, uint32_t,
 		    uint32_t);
@@ -692,6 +693,9 @@
 #ifdef OPENBSD
 		timeout_set(&rxr->rx_refill, igc_rxrefill, rxr);
 #endif
+		callout_init(&rxr->rx_refill, CALLOUT_MPSAFE);
+		callout_setfunc(&rxr->rx_refill, igc_rxrefill_callout, rxr);
+
 		if (igc_dma_malloc(sc, rsize, &rxr->rxdma)) {
 			aprint_error_dev(dev,
 			    "unable to allocate RX descriptor\n");
@@ -1919,6 +1923,7 @@
 	for (int iq = 0; iq < sc->sc_nqueues; iq++) {
 		struct rx_ring *rxr = &sc->rx_rings[iq];
 
+		callout_halt(&rxr->rx_refill, &rxr->rxr_lock);
 		igc_clear_receive_status(rxr);
 	}
 
@@ -2044,25 +2049,39 @@
 	rxr->next_to_check = 0;
 }
 
-static void
-igc_rxrefill(struct rx_ring *rxr, int end)
+static bool
+igc_rxrefill(struct rx_ring *rxr)
 {
 	struct igc_softc *sc = rxr->sc;
 	int id;
+	const int start = igc_rxdesc_incr(sc, rxr->last_desc_filled);
 
-	for (id = rxr->next_to_check; id != end; id = igc_rxdesc_incr(sc, id)) {
-		if (igc_get_buf(rxr, id, true)) {
-			/* XXXRO */
-			panic("%s: msix=%d id=%d\n", __func__, rxr->me, id);
-		}
+	for (id = start; id != rxr->next_to_check; id = igc_rxdesc_incr(sc, id)) {
+		if (igc_get_buf(rxr, id, true))
+			break;
 	}
 
-	id = igc_rxdesc_decr(sc, id);
-	DPRINTF(RX, "%s RDT %d id %d\n",
-	    rxr->last_desc_filled == id ? "same" : "diff",
-	    rxr->last_desc_filled, id);
-	rxr->last_desc_filled = id;
-	IGC_WRITE_REG(&sc->hw, IGC_RDT(rxr->me), id);
+        if (id != start) {
+		int last = igc_rxdesc_decr(sc, id);
+		DPRINTF(RX, "%s RDT %d last%d\n",
+			rxr->last_desc_filled == last ? "same" : "diff",
+			rxr->last_desc_filled, last);
+		rxr->last_desc_filled = last;
+		IGC_WRITE_REG(&sc->hw, IGC_RDT(rxr->me), last);
+	}
+
+	return id != rxr->next_to_check;
+}
+
+static void
+igc_rxrefill_callout(void *callout_arg)
+{
+	struct rx_ring *rxr = callout_arg;
+
+	mutex_enter(&rxr->rxr_lock);
+	if (igc_rxrefill(rxr))
+		callout_schedule(&rxr->rx_refill, 1);
+	mutex_exit(&rxr->rxr_lock);
 }
 
 /*********************************************************************
@@ -2217,14 +2236,15 @@
 		id = igc_rxdesc_incr(sc, id);
 	}
 
-	DPRINTF(RX, "fill queue[%d]\n", rxr->me);
-	igc_rxrefill(rxr, id);
-
 	DPRINTF(RX, "%s n2c %d id %d\n",
 	    rxr->next_to_check == id ? "same" : "diff",
 	    rxr->next_to_check, id);
 	rxr->next_to_check = id;
 
+	DPRINTF(RX, "fill queue[%d]\n", rxr->me);
+	if (igc_rxrefill(rxr))
+		callout_schedule(&rxr->rx_refill, 1);
+
 #ifdef OPENBSD
 	if (!(staterr & IGC_RXD_STAT_DD))
 		return 0;
@@ -3555,6 +3575,9 @@
 {
 	struct igc_softc *sc = rxr->sc;
 
+	callout_halt(&rxr->rx_refill, &rxr->rxr_lock);
+	callout_destroy(&rxr->rx_refill);
+
 	if (rxr->rx_buffers != NULL) {
 		for (int id = 0; id < sc->num_rx_desc; id++) {
 			struct igc_rx_buf *rxbuf = &rxr->rx_buffers[id];
Index: if_igc.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/igc/if_igc.h,v
retrieving revision 1.3
diff -u -r1.3 if_igc.h
--- if_igc.h	27 Jun 2024 07:31:41 -0000	1.3
+++ if_igc.h	23 May 2026 20:35:17 -0000
@@ -40,6 +40,7 @@
 
 #include <sys/types.h>
 #include <sys/atomic.h>
+#include <sys/callout.h>
 #include <sys/pcq.h>
 #include <sys/workqueue.h>
 
@@ -315,6 +316,7 @@
 	struct if_rxring	rx_ring;
 #endif
 
+	callout_t		rx_refill;
 	kmutex_t		rxr_lock;
 
 	struct igc_queue	*rxr_igcq;