Testers wanted - MPSAFE watchdog for rge(4)

Paul Goyette <[email protected]>
Newsgroups gmane.os.netbsd.current
Message-ID <[email protected]>
For those of you who have any of the various rge(4) hardware (for
RealTek 8125/8126/8127) it would be great if you could test with
the attached changes.  This provides an MPSAFE watchdog mechanism
rather trhan using BIG-LOCK or spl() tactics.  It should improve
overall system performance.

A big "Thanks!" goes out to skrll@ for doing all the heavy lifting
on this.  (I'm just doing the "easy" part of coordinating testing
and the actual commit.)

Thanks in advance for all your efforts.


+---------------------+--------------------------+----------------------+
| Paul Goyette (.sig) | PGP Key fingerprint:     | E-mail addresses:    |
| (Retired)           | 1B11 1849 721C 56C8 F63A | [email protected]    |
| Software Developer  | 6E2E 05FD 15CE 9F2D 5102 | [email protected]  |
| & Network Engineer  |                          | [email protected] |
+---------------------+--------------------------+----------------------+
if_rge.diff (text/plain, 6.1 KB)
Index: if_rge.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_rge.c,v
retrieving revision 1.47
diff -u -p -r1.47 if_rge.c
--- if_rge.c	28 Jan 2026 06:15:37 -0000	1.47
+++ if_rge.c	1 Feb 2026 01:04:32 -0000
@@ -94,6 +94,11 @@ MCLGETL(struct rge_softc *sc __unused, i
 #endif
 #endif
 
+#ifndef RGE_WATCHDOG_TIMEOUT
+#define RGE_WATCHDOG_TIMEOUT 5
+#endif
+static int rge_watchdog_timeout = RGE_WATCHDOG_TIMEOUT;
+
 #ifdef RGE_DEBUG
 #define DPRINTF(x)	do { if (rge_debug > 0) printf x; } while (0)
 int rge_debug = 0;
@@ -109,7 +114,6 @@ static int	rge_encap(struct rge_softc *,
     struct mbuf *, int);
 static int	rge_ioctl(struct ifnet *, u_long, void *);
 static void	rge_start(struct ifnet *);
-static void	rge_watchdog(struct ifnet *);
 static int	rge_init(struct ifnet *);
 static void	rge_stop(struct ifnet *, int);
 static int	rge_ifmedia_upd(struct ifnet *);
@@ -120,6 +124,7 @@ static int	rge_newbuf(struct rge_queues 
 static int	rge_rx_list_init(struct rge_queues *);
 static void	rge_rx_list_fini(struct rge_queues *);
 static void	rge_tx_list_init(struct rge_queues *);
+/* static void	rge_tx_list_fini(struct rge_queues *); */
 static int	rge_rxeof(struct rge_softc *);
 static int	rge_txeof(struct rge_softc *);
 static int	rge_reset(struct rge_softc *);
@@ -171,9 +176,11 @@ static uint16_t	rge_read_phy(struct rge_
 static void	rge_write_phy_ocp(struct rge_softc *, uint16_t, uint16_t);
 static uint16_t	rge_read_phy_ocp(struct rge_softc *, uint16_t);
 static int	rge_get_link_status(struct rge_softc *);
-static void	rge_txstart(void *);
+static void	rge_txstart(struct rge_softc *);
 static void	rge_tick(void *);
 static void	rge_link_state(struct rge_softc *);
+static bool	rge_watchdog_tick(struct ifnet *);
+static void	rge_handle_reset_work(struct work *, void *);
 
 static const struct {
 	uint16_t reg;
@@ -367,6 +374,16 @@ rge_attach(device_t parent, device_t sel
 	if (rge_allocmem(sc))
 		return;
 
+	char wqname[MAXCOMLEN];
+	snprintf(wqname, sizeof(wqname), "%sReset", device_xname(sc->sc_dev));
+	int error = workqueue_create(&sc->sc_reset_wq, wqname,
+	    rge_handle_reset_work, sc, PRI_NONE, IPL_SOFTCLOCK,WQ_MPSAFE);
+	if (error) {
+		aprint_error_dev(sc->sc_dev,
+		    "unable to create reset workqueue\n");
+		return;
+	}
+
 	ifp = &sc->sc_ec.ec_if;
 	ifp->if_softc = sc;
 	strlcpy(ifp->if_xname, device_xname(sc->sc_dev), IFNAMSIZ);
@@ -378,7 +395,7 @@ rge_attach(device_t parent, device_t sel
 	ifp->if_stop = rge_stop;
 	ifp->if_start = rge_start;
 	ifp->if_init = rge_init;
-	ifp->if_watchdog = rge_watchdog;
+	ifp->if_watchdog = NULL;
 	IFQ_SET_MAXLEN(&ifp->if_snd, RGE_TX_LIST_CNT - 1);
 
 	ifp->if_capabilities = IFCAP_CSUM_IPv4_Rx |
@@ -388,7 +405,7 @@ rge_attach(device_t parent, device_t sel
 	sc->sc_ec.ec_capabilities |= ETHERCAP_VLAN_MTU;
 	sc->sc_ec.ec_capabilities |= ETHERCAP_VLAN_HWTAGGING;
 
-	callout_init(&sc->sc_timeout, CALLOUT_FLAGS);
+	callout_init(&sc->sc_timeout, CALLOUT_MPSAFE);
 	callout_setfunc(&sc->sc_timeout, rge_tick, sc);
 
 	command = pci_conf_read(pa->pa_pc, pa->pa_tag, PCI_COMMAND_STATUS_REG);
@@ -735,15 +752,67 @@ rge_start(struct ifnet *ifp)
 	rge_txstart(sc);
 }
 
-static void
-rge_watchdog(struct ifnet *ifp)
+static bool
+rge_watchdog_check(struct rge_softc * const sc)
 {
-	struct rge_softc *sc = ifp->if_softc;
 
-	device_printf(sc->sc_dev, "watchdog timeout\n");
+	KASSERT(mutex_owned(sc->sc_core_lock));
+
+	if (!sc->sc_tx_sending)
+		return true;
+
+	if (time_uptime - sc->sc_tx_lastsent <= rge_watchdog_timeout)
+		return true;
+
+	return false;
+}
+
+static bool
+rge_watchdog_tick(struct ifnet *ifp)
+{
+	struct rge_softc * const sc = ifp->if_softc;
+
+	KASSERT(mutex_owned(sc->sc_core_lock));
+
+	if (!sc->sc_trigger_reset && rge_watchdog_check(sc))
+		return true;
+
 	if_statinc(ifp, if_oerrors);
 
+	if (atomic_swap_uint(&sc->sc_reset_pending, 1) == 0)
+		workqueue_enqueue(sc->sc_reset_wq, &sc->sc_reset_work, NULL);
+
+	return false;
+}
+
+/*
+ * Perform an interface watchdog reset.
+ */
+static void
+rge_handle_reset_work(struct work *work, void *arg)
+{
+	struct rge_softc * const sc = arg;
+	struct ifnet * const ifp = &sc->sc_ec.ec_if;
+
+	printf("%s: watchdog timeout -- resetting\n", ifp->if_xname);
+
+	/* Don't want ioctl operations to happen */
+	IFNET_LOCK(ifp);
+
+	/* reset the interface. */
 	rge_init(ifp);
+
+	IFNET_UNLOCK(ifp);
+	/*
+	 * There are still some upper layer processing which call
+	 * ifp->if_start(). e.g. ALTQ or one CPU system
+	 */
+
+	/* Try to get more packets going. */
+	ifp->if_start(ifp);
+
+	atomic_store_relaxed(&sc->sc_reset_pending, 0);
+
 }
 
 static int
@@ -3804,9 +3873,8 @@ rge_get_link_status(struct rge_softc *sc
 }
 
 static void
-rge_txstart(void *arg)
+rge_txstart(struct rge_softc *sc)
 {
-	struct rge_softc *sc = arg;
 
 	RGE_WRITE_2(sc, RGE_TXSTART, RGE_TXSTART_START);
 }
@@ -3815,13 +3883,21 @@ static void
 rge_tick(void *arg)
 {
 	struct rge_softc *sc = arg;
-	int s;
 
-	s = splnet();
+	mutex_enter(sc->sc_core_lock);
+	if (sc->sc_stopping) {
+		mutex_exit(sc->sc_core_lock);
+		return;
+	}
+
 	rge_link_state(sc);
-	splx(s);
 
-	callout_schedule(&sc->sc_timeout, hz);
+	struct ifnet * const ifp = &sc->sc_ec.ec_if;
+	const bool ok = rge_watchdog_tick(ifp);
+	if (ok)
+		callout_schedule(&sc->sc_timeout, hz);
+
+	mutex_exit(sc->sc_core_lock);
 }
 
 static void
Index: if_rgereg.h
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/if_rgereg.h,v
retrieving revision 1.18
diff -u -p -r1.18 if_rgereg.h
--- if_rgereg.h	28 Jan 2026 06:15:37 -0000	1.18
+++ if_rgereg.h	1 Feb 2026 01:04:34 -0000
@@ -424,6 +424,18 @@ struct rge_softc {
 	int			rge_timerintr;
 #define RGE_IMTYPE_NONE		0
 #define RGE_IMTYPE_SIM		1
+
+	kmutex_t		*sc_core_lock;
+	time_t			sc_tx_lastsent;
+	bool			sc_tx_sending;
+
+	struct workqueue 	*sc_reset_wq;
+	struct work		sc_reset_work;
+	volatile unsigned	sc_reset_pending;
+	bool			sc_stopping;
+
+	bool			sc_trigger_reset;
+
 };
 
 /*
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.