git: 481447ad9be2 - main - e1000: Fix the flow control sysctl

The branch main has been updated by rcm:

URL: https://cgit.FreeBSD.org/src/commit/?id=481447ad9be2e60e0bb3744ab199ce121baadf84

commit 481447ad9be2e60e0bb3744ab199ce121baadf84
Author:     R. Christian McDonald <rcm@FreeBSD.org>
AuthorDate: 2026-10-05 20:21:47 +0000
Commit:     R. Christian McDonald <rcm@FreeBSD.org>
CommitDate: 2026-10-05 20:22:44 +0000

    e1000: Fix the flow control sysctl
    
    dev.em.N.fc and dev.igb.N.fc read and wrote a function-static variable
    shared by every em(4) and igb(4) device, so a read returned the last
    value written to any of them (3 until the first write), not the state of
    the device.  The softc value started as 0, which is also the value of
    "no flow control", while the hardware was set up for full flow control.
    As a result:
    
    - Writing 0 was taken for no change and did nothing, unless another
      value had been written to that device before.
    - em_reset() took a softc value of 0 for "not set", so a device set to 0
      went back to full flow control on the next init.
    - On igb(4) with more than one receive queue the driver enabled
      per-queue drop (SRRCTL.DROP_EN), which is meant for a MAC that does
      not send pause frames, although the MAC was told to send them.
    - Values out of range were accepted and ignored.
    
    A write only forced the MAC's flow control bits.  The pause bits
    advertised to the link partner, the pause thresholds and DROP_EN stayed
    as the last init had set them, and the next link event resolved flow
    control from the stale advertisement again.  On 82577 and 82578 (pchlan)
    a write could also turn on transmit flow control, which em_reset() keeps
    off there as a workaround.
    
    Start the softc value as full, report it, use it as it is in em_reset(),
    reject invalid values, and apply a change by reinitializing the
    interface, as the dmac and eee_control sysctls do.  This is the same fix
    as 68386f6b551e ("igc: Fix the flow control sysctl"); igc(4) inherited
    the handler from this driver.  On pchlan, remove transmit pause from the
    requested mode instead of replacing the mode with receive pause, so that
    the workaround stays and 0 still turns flow control off.
    
    Reviewed by:            kbowling
    MFC after:              2 weeks
    Sponsored by:           Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D60376
---
 sys/dev/e1000/if_em.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c
index eac1798c9d25..5a44956f2a88 100644
--- a/sys/dev/e1000/if_em.c
+++ b/sys/dev/e1000/if_em.c
@@ -544,6 +544,7 @@ static void	em_handle_link(void *);
 
 static void	em_enable_vectors_82574(if_ctx_t);
 
+static void	em_sysctl_request_reinit(struct e1000_softc *);
 static int	em_set_flowcntl(SYSCTL_HANDLER_ARGS);
 static int	em_sysctl_eee(SYSCTL_HANDLER_ARGS);
 static int	igb_sysctl_dmac(SYSCTL_HANDLER_ARGS);
@@ -1223,6 +1224,7 @@ em_add_device_sysctls(struct e1000_softc *sc)
 	    CTLTYPE_STRING | CTLFLAG_RD, sc, 0,
 	    em_sysctl_print_fw_version, "A",
 	    "Prints FW/NVM Versions");
+	sc->fc = e1000_fc_full;
 	SYSCTL_ADD_PROC(ctx_list, child, OID_AUTO, "fc",
 	    CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_NEEDGIANT, sc, 0,
 	    em_set_flowcntl, "I", "Flow Control");
@@ -5095,10 +5097,7 @@ em_reset(if_ctx_t ctx)
 	    roundup2(hw->mac.max_frame_size, 1024);
 	hw->fc.low_water = hw->fc.high_water - 1500;
 
-	if (sc->fc) /* locally set flow control value? */
-		hw->fc.requested_mode = sc->fc;
-	else
-		hw->fc.requested_mode = e1000_fc_full;
+	hw->fc.requested_mode = sc->fc;
 
 	if (hw->mac.type == e1000_80003es2lan)
 		hw->fc.pause_time = 0xFFFF;
@@ -5111,7 +5110,10 @@ em_reset(if_ctx_t ctx)
 	switch (hw->mac.type) {
 	case e1000_pchlan:
 		/* Workaround: no TX flow ctrl for PCH */
-		hw->fc.requested_mode = e1000_fc_rx_pause;
+		if (hw->fc.requested_mode == e1000_fc_full)
+			hw->fc.requested_mode = e1000_fc_rx_pause;
+		else if (hw->fc.requested_mode == e1000_fc_tx_pause)
+			hw->fc.requested_mode = e1000_fc_none;
 		hw->fc.pause_time = 0xFFFF; /* override */
 		if (if_getmtu(ifp) > ETHERMTU) {
 			hw->fc.high_water = 0x3500;
@@ -8533,34 +8535,35 @@ em_add_int_delay_sysctl(struct e1000_softc *sc, const char *name,
 static int
 em_set_flowcntl(SYSCTL_HANDLER_ARGS)
 {
-	int error;
-	static int input = 3; /* default is full */
+	int error, input;
 	struct e1000_softc *sc = (struct e1000_softc *) arg1;
 
+	input = sc->fc;
 	error = sysctl_handle_int(oidp, &input, 0, req);
 
 	if ((error) || (req->newptr == NULL))
 		return (error);
 
-	if (input == sc->fc) /* no change? */
-		return (error);
-
 	switch (input) {
 	case e1000_fc_rx_pause:
 	case e1000_fc_tx_pause:
 	case e1000_fc_full:
 	case e1000_fc_none:
-		sc->hw.fc.requested_mode = input;
-		sc->fc = input;
 		break;
 	default:
-		/* Do nothing */
-		return (error);
+		return (EINVAL);
 	}
 
-	sc->hw.fc.current_mode = sc->hw.fc.requested_mode;
-	e1000_force_mac_fc(&sc->hw);
-	return (error);
+	if (input == sc->fc) /* no change? */
+		return (0);
+
+	/*
+	 * The mode is advertised to the link partner and also decides the
+	 * pause thresholds and per-queue drop, so it takes a reinit to apply.
+	 */
+	sc->fc = input;
+	em_sysctl_request_reinit(sc);
+	return (0);
 }
 
 static void