git: 68386f6b551e - main - igc: Fix the flow control sysctl

From: R. Christian McDonald <rcm_at_FreeBSD.org>
Date: Mon, 05 Oct 2026 14:06:38 UTC
The branch main has been updated by rcm:

URL: https://cgit.FreeBSD.org/src/commit/?id=68386f6b551e64c956c9623b9973be9e462ce1de

commit 68386f6b551e64c956c9623b9973be9e462ce1de
Author:     R. Christian McDonald <rcm@FreeBSD.org>
AuthorDate: 2026-10-05 14:01:54 +0000
Commit:     R. Christian McDonald <rcm@FreeBSD.org>
CommitDate: 2026-10-05 14:01:54 +0000

    igc: Fix the flow control sysctl
    
    dev.igc.N.fc read and wrote a function-static variable shared by every
    igc 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.
    - igc_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.
    - 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: a port set to 0 came back
    honoring pause frames, for example.
    
    Start the softc value as full, report it, use it as it is in
    igc_reset(), reject invalid values, and apply a change by
    reinitializing the interface, as the dmac and eee_control sysctls do.
    
    Two things behave differently now.  Changing the mode of a running
    interface takes the link down to renegotiate.  And with the default
    mode and more than one receive queue DROP_EN is no longer set: a full
    receive ring makes the MAC send pause frames, and holds up the other
    queues, where it used to drop that queue's packets.  Setting fc to 0 or
    1 selects per-queue drop.
    
    Tested on I226-V ports with four receive queues: for each mode, the
    flow control bits in CTRL, the pause thresholds and DROP_EN were read
    back, and the mode the link partner resolved, also after a down/up.
    
    Reviewed by:            gallatin
    Fixes:                  517904de5cca ("igc(4): Introduce new driver for the Intel I225 Ethernet controller.")
    MFC after:              2 weeks
    Sponsored by:           Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D60363
---
 sys/dev/igc/if_igc.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/sys/dev/igc/if_igc.c b/sys/dev/igc/if_igc.c
index bc24849693e1..7bfbf1c9d1a7 100644
--- a/sys/dev/igc/if_igc.c
+++ b/sys/dev/igc/if_igc.c
@@ -181,6 +181,7 @@ static int	igc_if_msix_intr_assign(if_ctx_t, int);
 static int	igc_msix_link(void *);
 static void	igc_handle_link(void *context);
 
+static void	igc_sysctl_request_reinit(struct igc_softc *);
 static int	igc_set_flowcntl(SYSCTL_HANDLER_ARGS);
 static int	igc_sysctl_dmac(SYSCTL_HANDLER_ARGS);
 static int	igc_sysctl_eee(SYSCTL_HANDLER_ARGS);
@@ -533,6 +534,7 @@ igc_if_attach_pre(if_ctx_t ctx)
 	    OID_AUTO, "debug", CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_NEEDGIANT,
 	    sc, 0, igc_sysctl_debug_info, "I", "Debug Information");
 
+	sc->fc = igc_fc_full;
 	SYSCTL_ADD_PROC(device_get_sysctl_ctx(dev),
 	    SYSCTL_CHILDREN(device_get_sysctl_tree(dev)),
 	    OID_AUTO, "fc", CTLTYPE_INT | CTLFLAG_RW | CTLFLAG_NEEDGIANT,
@@ -2190,10 +2192,7 @@ igc_reset(if_ctx_t ctx)
 	/* 16-byte granularity */
 	hw->fc.low_water = hw->fc.high_water - 16;
 
-	if (sc->fc) /* locally set flow control value? */
-		hw->fc.requested_mode = sc->fc;
-	else
-		hw->fc.requested_mode = igc_fc_full;
+	hw->fc.requested_mode = sc->fc;
 
 	hw->fc.pause_time = IGC_FC_PAUSE_TIME;
 
@@ -3736,34 +3735,35 @@ igc_sysctl_tso_tcp_flags_mask(SYSCTL_HANDLER_ARGS)
 static int
 igc_set_flowcntl(SYSCTL_HANDLER_ARGS)
 {
-	int error;
-	static int input = 3; /* default is full */
+	int error, input;
 	struct igc_softc *sc = (struct igc_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 igc_fc_rx_pause:
 	case igc_fc_tx_pause:
 	case igc_fc_full:
 	case igc_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;
-	igc_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;
+	igc_sysctl_request_reinit(sc);
+	return (0);
 }
 
 static void