From nobody Wed Oct 20 17:39:37 2021 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 54F6518122E3; Wed, 20 Oct 2021 17:39:39 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HZHtL40nMz3Lqb; Wed, 20 Oct 2021 17:39:38 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id E40903F0E; Wed, 20 Oct 2021 17:39:37 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 19KHdbEg094498; Wed, 20 Oct 2021 17:39:37 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 19KHdbt9094497; Wed, 20 Oct 2021 17:39:37 GMT (envelope-from git) Date: Wed, 20 Oct 2021 17:39:37 GMT Message-Id: <202110201739.19KHdbt9094497@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Navdeep Parhar Subject: git: f7b50d986cb5 - stable/13 - cxgbe(4): Do not configure traffic classes automatically on attach. List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: np X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: f7b50d986cb5a34757b1f636d90b543b6556ed95 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by np: URL: https://cgit.FreeBSD.org/src/commit/?id=f7b50d986cb5a34757b1f636d90b543b6556ed95 commit f7b50d986cb5a34757b1f636d90b543b6556ed95 Author: Navdeep Parhar AuthorDate: 2021-06-24 20:05:57 +0000 Commit: Navdeep Parhar 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++;