git: f7b50d986cb5 - stable/13 - cxgbe(4): Do not configure traffic classes automatically on attach.

From: Navdeep Parhar <np_at_FreeBSD.org>
Date: Wed, 20 Oct 2021 17:39:37 UTC
The branch stable/13 has been updated by np:

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

commit f7b50d986cb5a34757b1f636d90b543b6556ed95
Author:     Navdeep Parhar <np@FreeBSD.org>
AuthorDate: 2021-06-24 20:05:57 +0000
Commit:     Navdeep Parhar <np@FreeBSD.org>
CommitDate: 2021-10-20 17:35:14 +0000

    cxgbe(4): Do not configure traffic classes automatically on attach.
    
    The driver used to configure all available classes with some default
    parameters on attach and the rest of t4_sched.c was written with the
    assumption that all traffic classes are always valid in the hardware.
    But this resulted in a lot of informational messages being logged in the
    firmware's circular log, crowding out other more useful messages.
    
    This change leaves the tx scheduler alone during attach to reduce the
    spam in the devlog.  The state of every class is now tracked separately
    from its flags and there is support for an 'uninitialized' state.
    
    Sponsored by:   Chelsio Communications
    
    (cherry picked from commit ec8004dd419d8c8acfc9025dd050f141c949d53a)
---
 sys/dev/cxgbe/adapter.h    |  16 ++++--
 sys/dev/cxgbe/t4_main.c    |   4 +-
 sys/dev/cxgbe/t4_sched.c   | 135 +++++++++++++++++++++++++++------------------
 sys/dev/cxgbe/tom/t4_tom.c |   9 ++-
 4 files changed, 101 insertions(+), 63 deletions(-)

diff --git a/sys/dev/cxgbe/adapter.h b/sys/dev/cxgbe/adapter.h
index 630e9c4ac1b9..a394e8f11017 100644
--- a/sys/dev/cxgbe/adapter.h
+++ b/sys/dev/cxgbe/adapter.h
@@ -258,14 +258,22 @@ struct tx_ch_rl_params {
 	uint32_t maxrate;
 };
 
+/* CLRL state */
+enum clrl_state {
+	CS_UNINITIALIZED = 0,
+	CS_PARAMS_SET,			/* sw parameters have been set. */
+	CS_HW_UPDATE_REQUESTED,		/* async HW update requested. */
+	CS_HW_UPDATE_IN_PROGRESS,	/* sync hw update in progress. */
+	CS_HW_CONFIGURED		/* configured in the hardware. */
+};
+
+/* CLRL flags */
 enum {
-	CLRL_USER	= (1 << 0),	/* allocated manually. */
-	CLRL_SYNC	= (1 << 1),	/* sync hw update in progress. */
-	CLRL_ASYNC	= (1 << 2),	/* async hw update requested. */
-	CLRL_ERR	= (1 << 3),	/* last hw setup ended in error. */
+	CF_USER		= (1 << 0),	/* was configured by driver ioctl. */
 };
 
 struct tx_cl_rl_params {
+	enum clrl_state state;
 	int refcount;
 	uint8_t flags;
 	enum fw_sched_params_rate ratemode;	/* %port REL or ABS value */
diff --git a/sys/dev/cxgbe/t4_main.c b/sys/dev/cxgbe/t4_main.c
index 09b4534b1b73..2fa54909aa5a 100644
--- a/sys/dev/cxgbe/t4_main.c
+++ b/sys/dev/cxgbe/t4_main.c
@@ -7784,7 +7784,7 @@ cxgbe_sysctls(struct port_info *pi)
 	struct adapter *sc = pi->adapter;
 	int i;
 	char name[16];
-	static char *tc_flags = {"\20\1USER\2SYNC\3ASYNC\4ERR"};
+	static char *tc_flags = {"\20\1USER"};
 
 	ctx = device_get_sysctl_ctx(pi->dev);
 
@@ -7861,6 +7861,8 @@ cxgbe_sysctls(struct port_info *pi)
 		children2 = SYSCTL_CHILDREN(SYSCTL_ADD_NODE(ctx,
 		    SYSCTL_CHILDREN(oid), OID_AUTO, name,
 		    CTLFLAG_RD | CTLFLAG_MPSAFE, NULL, "traffic class"));
+		SYSCTL_ADD_UINT(ctx, children2, OID_AUTO, "state",
+		    CTLFLAG_RD, &tc->state, 0, "current state");
 		SYSCTL_ADD_PROC(ctx, children2, OID_AUTO, "flags",
 		    CTLTYPE_STRING | CTLFLAG_RD | CTLFLAG_MPSAFE, tc_flags,
 		    (uintptr_t)&tc->flags, sysctl_bitfield_8b, "A", "flags");
diff --git a/sys/dev/cxgbe/t4_sched.c b/sys/dev/cxgbe/t4_sched.c
index 827add3c27ec..b19e62474bbb 100644
--- a/sys/dev/cxgbe/t4_sched.c
+++ b/sys/dev/cxgbe/t4_sched.c
@@ -185,17 +185,19 @@ set_sched_class_params(struct adapter *sc, struct t4_sched_class_params *p,
 	if (p->level == SCHED_CLASS_LEVEL_CL_RL) {
 		tc = &pi->sched_params->cl_rl[p->cl];
 		mtx_lock(&sc->tc_lock);
-		if (tc->refcount > 0 || tc->flags & (CLRL_SYNC | CLRL_ASYNC))
+		if (tc->refcount > 0 || tc->state == CS_HW_UPDATE_IN_PROGRESS)
 			rc = EBUSY;
 		else {
-			tc->flags |= CLRL_SYNC | CLRL_USER;
+			old = *tc;
+
+			tc->flags |= CF_USER;
+			tc->state = CS_HW_UPDATE_IN_PROGRESS;
 			tc->ratemode = fw_ratemode;
 			tc->rateunit = fw_rateunit;
 			tc->mode = fw_mode;
 			tc->maxrate = p->maxrate;
 			tc->pktsize = p->pktsize;
 			rc = 0;
-			old= *tc;
 		}
 		mtx_unlock(&sc->tc_lock);
 		if (rc != 0)
@@ -207,6 +209,9 @@ set_sched_class_params(struct adapter *sc, struct t4_sched_class_params *p,
 	if (rc != 0) {
 		if (p->level == SCHED_CLASS_LEVEL_CL_RL) {
 			mtx_lock(&sc->tc_lock);
+			MPASS(tc->refcount == 0);
+			MPASS(tc->flags & CF_USER);
+			MPASS(tc->state == CS_HW_UPDATE_IN_PROGRESS);
 			*tc = old;
 			mtx_unlock(&sc->tc_lock);
 		}
@@ -221,15 +226,23 @@ set_sched_class_params(struct adapter *sc, struct t4_sched_class_params *p,
 
 	if (p->level == SCHED_CLASS_LEVEL_CL_RL) {
 		mtx_lock(&sc->tc_lock);
-		MPASS(tc->flags & CLRL_SYNC);
-		MPASS(tc->flags & CLRL_USER);
 		MPASS(tc->refcount == 0);
+		MPASS(tc->flags & CF_USER);
+		MPASS(tc->state == CS_HW_UPDATE_IN_PROGRESS);
 
-		tc->flags &= ~CLRL_SYNC;
 		if (rc == 0)
-			tc->flags &= ~CLRL_ERR;
-		else
-			tc->flags |= CLRL_ERR;
+			tc->state = CS_HW_CONFIGURED;
+		else {
+			/* parameters failed so we don't park at params_set */
+			tc->state = CS_UNINITIALIZED;
+			tc->flags &= ~CF_USER;
+			CH_ERR(pi, "failed to configure traffic class %d: %d.  "
+			    "params: mode %d, rateunit %d, ratemode %d, "
+			    "channel %d, minrate %d, maxrate %d, pktsize %d, "
+			    "burstsize %d\n", p->cl, rc, fw_mode, fw_rateunit,
+			    fw_ratemode, p->channel, p->minrate, p->maxrate,
+			    p->pktsize, 0);
+		}
 		mtx_unlock(&sc->tc_lock);
 	}
 
@@ -251,7 +264,7 @@ update_tx_sched(void *context, int pending)
 		tc = &pi->sched_params->cl_rl[0];
 		for (j = 0; j < n; j++, tc++) {
 			MPASS(mtx_owned(&sc->tc_lock));
-			if ((tc->flags & CLRL_ASYNC) == 0)
+			if (tc->state != CS_HW_UPDATE_REQUESTED)
 				continue;
 			mtx_unlock(&sc->tc_lock);
 
@@ -267,12 +280,22 @@ update_tx_sched(void *context, int pending)
 			end_synchronized_op(sc, 0);
 
 			mtx_lock(&sc->tc_lock);
-			MPASS(tc->flags & CLRL_ASYNC);
-			tc->flags &= ~CLRL_ASYNC;
-			if (rc == 0)
-				tc->flags &= ~CLRL_ERR;
+			MPASS(tc->state == CS_HW_UPDATE_REQUESTED);
+			if (rc == 0) {
+				tc->state = CS_HW_CONFIGURED;
+				continue;
+			}
+			/* parameters failed so we try to avoid params_set */
+			if (tc->refcount > 0)
+				tc->state = CS_PARAMS_SET;
 			else
-				tc->flags |= CLRL_ERR;
+				tc->state = CS_UNINITIALIZED;
+			CH_ERR(pi, "failed to configure traffic class %d: %d.  "
+			    "params: mode %d, rateunit %d, ratemode %d, "
+			    "channel %d, minrate %d, maxrate %d, pktsize %d, "
+			    "burstsize %d\n", j, rc, tc->mode, tc->rateunit,
+			    tc->ratemode, pi->tx_chan, 0, tc->maxrate,
+			    tc->pktsize, tc->burstsize);
 		}
 	}
 	mtx_unlock(&sc->tc_lock);
@@ -320,7 +343,7 @@ bind_txq_to_traffic_class(struct adapter *sc, struct sge_txq *txq, int idx)
 		 * Bind to a different class at index idx.
 		 */
 		tc = &tc0[idx];
-		if (tc->flags & CLRL_ERR) {
+		if (tc->state != CS_HW_CONFIGURED) {
 			rc = ENXIO;
 			goto done;
 		} else {
@@ -338,14 +361,15 @@ bind_txq_to_traffic_class(struct adapter *sc, struct sge_txq *txq, int idx)
 	mtx_unlock(&sc->tc_lock);
 
 	rc = begin_synchronized_op(sc, NULL, SLEEP_OK | INTR_OK, "t4btxq");
-	if (rc != 0)
-		return (rc);
-	fw_mnem = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DMAQ) |
-	    V_FW_PARAMS_PARAM_X(FW_PARAMS_PARAM_DMAQ_EQ_SCHEDCLASS_ETH) |
-	    V_FW_PARAMS_PARAM_YZ(txq->eq.cntxt_id));
-	fw_class = idx < 0 ? 0xffffffff : idx;
-	rc = -t4_set_params(sc, sc->mbox, sc->pf, 0, 1, &fw_mnem, &fw_class);
-	end_synchronized_op(sc, 0);
+	if (rc == 0) {
+		fw_mnem = (V_FW_PARAMS_MNEM(FW_PARAMS_MNEM_DMAQ) |
+		    V_FW_PARAMS_PARAM_X(FW_PARAMS_PARAM_DMAQ_EQ_SCHEDCLASS_ETH) |
+		    V_FW_PARAMS_PARAM_YZ(txq->eq.cntxt_id));
+		fw_class = idx < 0 ? 0xffffffff : idx;
+		rc = -t4_set_params(sc, sc->mbox, sc->pf, 0, 1, &fw_mnem,
+		    &fw_class);
+		end_synchronized_op(sc, 0);
+	}
 
 	mtx_lock(&sc->tc_lock);
 	MPASS(txq->tc_idx == -2);
@@ -430,29 +454,16 @@ t4_set_sched_queue(struct adapter *sc, struct t4_sched_queue *p)
 int
 t4_init_tx_sched(struct adapter *sc)
 {
-	int i, j;
+	int i;
 	const int n = sc->params.nsched_cls;
 	struct port_info *pi;
-	struct tx_cl_rl_params *tc;
 
 	mtx_init(&sc->tc_lock, "tx_sched lock", NULL, MTX_DEF);
 	TASK_INIT(&sc->tc_task, 0, update_tx_sched, sc);
 	for_each_port(sc, i) {
 		pi = sc->port[i];
 		pi->sched_params = malloc(sizeof(*pi->sched_params) +
-		    n * sizeof(*tc), M_CXGBE, M_ZERO | M_WAITOK);
-		tc = &pi->sched_params->cl_rl[0];
-		for (j = 0; j < n; j++, tc++) {
-			tc->refcount = 0;
-			tc->ratemode = FW_SCHED_PARAMS_RATE_ABS;
-			tc->rateunit = FW_SCHED_PARAMS_UNIT_BITRATE;
-			tc->mode = FW_SCHED_PARAMS_MODE_CLASS;
-			tc->maxrate = 1000 * 1000;	/* 1 Gbps.  Arbitrary */
-
-			if (t4_sched_params_cl_rl_kbps(sc, pi->tx_chan, j,
-			    tc->mode, tc->maxrate, tc->pktsize, 1) != 0)
-				tc->flags = CLRL_ERR;
-		}
+		    n * sizeof(struct tx_cl_rl_params), M_CXGBE, M_ZERO | M_WAITOK);
 	}
 
 	return (0);
@@ -487,7 +498,7 @@ int
 t4_reserve_cl_rl_kbps(struct adapter *sc, int port_id, u_int maxrate,
     int *tc_idx)
 {
-	int rc = 0, fa = -1, i, pktsize, burstsize;
+	int rc = 0, fa, fa2, i, pktsize, burstsize;
 	bool update;
 	struct tx_cl_rl_params *tc;
 	struct port_info *pi;
@@ -506,30 +517,47 @@ t4_reserve_cl_rl_kbps(struct adapter *sc, int port_id, u_int maxrate,
 	tc = &pi->sched_params->cl_rl[0];
 
 	update = false;
+	fa = fa2 = -1;
 	mtx_lock(&sc->tc_lock);
 	for (i = 0; i < sc->params.nsched_cls; i++, tc++) {
-		if (fa < 0 && tc->refcount == 0 && !(tc->flags & CLRL_USER))
-			fa = i;		/* first available */
-
-		if (tc->ratemode == FW_SCHED_PARAMS_RATE_ABS &&
+		if (tc->state >= CS_PARAMS_SET &&
+		    tc->ratemode == FW_SCHED_PARAMS_RATE_ABS &&
 		    tc->rateunit == FW_SCHED_PARAMS_UNIT_BITRATE &&
 		    tc->mode == FW_SCHED_PARAMS_MODE_FLOW &&
 		    tc->maxrate == maxrate && tc->pktsize == pktsize &&
 		    tc->burstsize == burstsize) {
 			tc->refcount++;
 			*tc_idx = i;
-			if ((tc->flags & (CLRL_ERR | CLRL_ASYNC | CLRL_SYNC)) ==
-			    CLRL_ERR) {
+			if (tc->state == CS_PARAMS_SET) {
+				tc->state = CS_HW_UPDATE_REQUESTED;
 				update = true;
 			}
 			goto done;
 		}
+
+		if (fa < 0 && tc->state == CS_UNINITIALIZED) {
+			MPASS(tc->refcount == 0);
+			fa = i;		/* first available, never used. */
+		}
+		if (fa2 < 0 && tc->refcount == 0 && !(tc->flags & CF_USER)) {
+			fa2 = i;	/* first available, used previously.  */
+		}
 	}
 	/* Not found */
 	MPASS(i == sc->params.nsched_cls);
-	if (fa != -1) {
+	if (fa == -1)
+		fa = fa2;
+	if (fa == -1) {
+		*tc_idx = -1;
+		rc = ENOSPC;
+	} else {
+		MPASS(fa >= 0 && fa < sc->params.nsched_cls);
 		tc = &pi->sched_params->cl_rl[fa];
+		MPASS(!(tc->flags & CF_USER));
+		MPASS(tc->refcount == 0);
+
 		tc->refcount = 1;
+		tc->state = CS_HW_UPDATE_REQUESTED;
 		tc->ratemode = FW_SCHED_PARAMS_RATE_ABS;
 		tc->rateunit = FW_SCHED_PARAMS_UNIT_BITRATE;
 		tc->mode = FW_SCHED_PARAMS_MODE_FLOW;
@@ -538,16 +566,11 @@ t4_reserve_cl_rl_kbps(struct adapter *sc, int port_id, u_int maxrate,
 		tc->burstsize = burstsize;
 		*tc_idx = fa;
 		update = true;
-	} else {
-		*tc_idx = -1;
-		rc = ENOSPC;
 	}
 done:
 	mtx_unlock(&sc->tc_lock);
-	if (update) {
-		tc->flags |= CLRL_ASYNC;
+	if (update)
 		t4_update_tx_sched(sc);
-	}
 	return (rc);
 }
 
@@ -616,6 +639,11 @@ sysctl_tc_params(SYSCTL_HANDLER_ARGS)
 	tc = sc->port[port_id]->sched_params->cl_rl[i];
 	mtx_unlock(&sc->tc_lock);
 
+	if (tc.state < CS_PARAMS_SET) {
+		sbuf_printf(sb, "uninitialized");
+		goto done;
+	}
+
 	switch (tc.rateunit) {
 	case SCHED_CLASS_RATEUNIT_BITS:
 		switch (tc.ratemode) {
@@ -649,6 +677,7 @@ sysctl_tc_params(SYSCTL_HANDLER_ARGS)
 
 	switch (tc.mode) {
 	case SCHED_CLASS_MODE_CLASS:
+		/* Note that pktsize and burstsize are not used in this mode. */
 		sbuf_printf(sb, " aggregate");
 		break;
 	case SCHED_CLASS_MODE_FLOW:
diff --git a/sys/dev/cxgbe/tom/t4_tom.c b/sys/dev/cxgbe/tom/t4_tom.c
index d4a995a5559a..fe0ebfbd832b 100644
--- a/sys/dev/cxgbe/tom/t4_tom.c
+++ b/sys/dev/cxgbe/tom/t4_tom.c
@@ -170,11 +170,10 @@ init_toepcb(struct vi_info *vi, struct toepcb *toep)
 	if (cp->tc_idx >= 0 && cp->tc_idx < sc->params.nsched_cls) {
 		tc = &pi->sched_params->cl_rl[cp->tc_idx];
 		mtx_lock(&sc->tc_lock);
-		if (tc->flags & CLRL_ERR) {
-			log(LOG_ERR,
-			    "%s: failed to associate traffic class %u with tid %u\n",
-			    device_get_nameunit(vi->dev), cp->tc_idx,
-			    toep->tid);
+		if (tc->state != CS_HW_CONFIGURED) {
+			CH_ERR(vi, "tid %d cannot be bound to traffic class %d "
+			    "because it is not configured (its state is %d)\n",
+			    toep->tid, cp->tc_idx, tc->state);
 			cp->tc_idx = -1;
 		} else {
 			tc->refcount++;