The branch main has been updated by adrian: URL: https://cgit.FreeBSD.org/src/commit/?id=d99eb8230eb717ab0b2eba948614d0f2f2b5dd2b
commit d99eb8230eb717ab0b2eba948614d0f2f2b5dd2b Author: Adrian Chadd <[email protected]> AuthorDate: 2024-11-12 04:48:12 +0000 Commit: Adrian Chadd <[email protected]> CommitDate: 2024-11-21 01:56:56 +0000 rtwn: change the USB TX transfers to only do one pending transfer per endpoint I found I was getting constant device timeouts when doing anything more complicated than a single SSH on laptop with RTL8811AU. After digging into it, i found a variety of fun situations, including traffic stalls that would recover w/ a shorter (1 second) USB transfer timeout. However, the big one is a straight up hang of any TX endpoint until the NIC was reset. The RX side kept going just fine; only the TX endpoints would hang. Reproducing it was easy - just start up a couple of traffic streams on different WME AC's - eg a best effort + bulk transfer, like browsing the web and doing an ssh clone - throw in a ping -i 0.1 to your gateway, and it would very quickly hit device timeouts every couple of seconds. I put everything into a single TX EP and the hangs went away. Well, mostly. So after some MORE digging, I found that this driver isn't checking if the transfers are going into the correct EPs for the packet WME access category / 802.11 TID; and would frequently be able to schedule multiple transfers into the same endpoint. Then there's a second problem - there's an array of endpoints used for setting up the USB device, with .endpoint = UE_ADDR_ANY, however they're also being setup with the same endpoint configured in multiple transfer configs. Eg, a NIC with 3 or 4 bulk TX endpoints will configure the BK and BE endpoints with the same physical endpoint ID. This also leads to timed out transfers. My /guess/ was that the firmware isn't happy with one or both of the above, and so I solved both. * drop the USB transfer timeout to 1 second, not 5 seconds - that way we'll either get a 1 second traffic pause and USB transfer failure, or a 5 second device timeout. Having both the TX timeout and the USB transfer timeout made recovery from a USB transfer timeout (without a NIC reset) almost impossible. * enforce one transfer per endpoint; * separate pending/active buffer tracking per endpoint; * each endpoint now has its own TX callback to make sure the queue / end point ID is known; * and only frames from a given endpoint pending queue is going into the active queue and into that endpoint. * Finally, create a local wme2qid array and populate it with the endpoint mapping that ensures unique physical endpoint use. Locally tested: * rtl8812AU, 11n STA mode * rtl8192EU, 11n STA mode (with diffs to fix the channel config / power timeouts.) Differential Revision: https://reviews.freebsd.org/D47522 --- sys/dev/rtwn/if_rtwnvar.h | 2 +- sys/dev/rtwn/usb/rtwn_usb_attach.c | 26 ++++++---- sys/dev/rtwn/usb/rtwn_usb_ep.c | 23 ++++++--- sys/dev/rtwn/usb/rtwn_usb_tx.c | 100 ++++++++++++++++++++++++++++++------- sys/dev/rtwn/usb/rtwn_usb_tx.h | 5 +- sys/dev/rtwn/usb/rtwn_usb_var.h | 12 +++-- 6 files changed, 129 insertions(+), 39 deletions(-) diff --git a/sys/dev/rtwn/if_rtwnvar.h b/sys/dev/rtwn/if_rtwnvar.h index 6a44b7b73902..f4c6d7ee64b4 100644 --- a/sys/dev/rtwn/if_rtwnvar.h +++ b/sys/dev/rtwn/if_rtwnvar.h @@ -32,7 +32,7 @@ #define RTWN_MACID_VALID 0x8000 #define RTWN_MACID_LIMIT 128 -#define RTWN_TX_TIMEOUT 5000 /* ms */ +#define RTWN_TX_TIMEOUT 1000 /* ms */ #define RTWN_MAX_EPOUT 4 #define RTWN_PORT_COUNT 2 diff --git a/sys/dev/rtwn/usb/rtwn_usb_attach.c b/sys/dev/rtwn/usb/rtwn_usb_attach.c index 71798ffc14f9..4958939a768a 100644 --- a/sys/dev/rtwn/usb/rtwn_usb_attach.c +++ b/sys/dev/rtwn/usb/rtwn_usb_attach.c @@ -156,10 +156,12 @@ rtwn_usb_alloc_tx_list(struct rtwn_softc *sc) if (error != 0) return (error); - STAILQ_INIT(&uc->uc_tx_active); - STAILQ_INIT(&uc->uc_tx_inactive); - STAILQ_INIT(&uc->uc_tx_pending); + for (i = RTWN_BULK_TX_FIRST; i < RTWN_BULK_EP_COUNT; i++) { + STAILQ_INIT(&uc->uc_tx_active[i]); + STAILQ_INIT(&uc->uc_tx_pending[i]); + } + STAILQ_INIT(&uc->uc_tx_inactive); for (i = 0; i < RTWN_USB_TX_LIST_COUNT; i++) STAILQ_INSERT_HEAD(&uc->uc_tx_inactive, &uc->uc_tx[i], next); @@ -207,23 +209,29 @@ static void rtwn_usb_free_tx_list(struct rtwn_softc *sc) { struct rtwn_usb_softc *uc = RTWN_USB_SOFTC(sc); + int i; rtwn_usb_free_list(sc, uc->uc_tx, RTWN_USB_TX_LIST_COUNT); - STAILQ_INIT(&uc->uc_tx_active); + for (i = RTWN_BULK_TX_FIRST; i < RTWN_BULK_EP_COUNT; i++) { + STAILQ_INIT(&uc->uc_tx_active[i]); + STAILQ_INIT(&uc->uc_tx_pending[i]); + } STAILQ_INIT(&uc->uc_tx_inactive); - STAILQ_INIT(&uc->uc_tx_pending); } static void rtwn_usb_reset_lists(struct rtwn_softc *sc, struct ieee80211vap *vap) { struct rtwn_usb_softc *uc = RTWN_USB_SOFTC(sc); + int i; RTWN_ASSERT_LOCKED(sc); - rtwn_usb_reset_tx_list(uc, &uc->uc_tx_active, vap); - rtwn_usb_reset_tx_list(uc, &uc->uc_tx_pending, vap); + for (i = RTWN_BULK_TX_FIRST; i < RTWN_BULK_EP_COUNT; i++) { + rtwn_usb_reset_tx_list(uc, &uc->uc_tx_active[i], vap); + rtwn_usb_reset_tx_list(uc, &uc->uc_tx_pending[i], vap); + } if (vap == NULL) { rtwn_usb_reset_rx_list(uc); sc->qfullmsk = 0; @@ -295,7 +303,7 @@ rtwn_usb_abort_xfers(struct rtwn_softc *sc) /* abort any pending transfers */ RTWN_UNLOCK(sc); - for (i = 0; i < RTWN_N_TRANSFER; i++) + for (i = 0; i < RTWN_BULK_EP_COUNT; i++) usbd_transfer_drain(uc->uc_xfer[i]); RTWN_LOCK(sc); } @@ -432,7 +440,7 @@ rtwn_usb_detach(device_t self) rtwn_usb_free_rx_list(sc); /* Detach all USB transfers. */ - usbd_transfer_unsetup(uc->uc_xfer, RTWN_N_TRANSFER); + usbd_transfer_unsetup(uc->uc_xfer, RTWN_BULK_EP_COUNT); rtwn_detach_private(sc); mtx_destroy(&sc->sc_mtx); diff --git a/sys/dev/rtwn/usb/rtwn_usb_ep.c b/sys/dev/rtwn/usb/rtwn_usb_ep.c index 0848a45a9f86..f9b0672324fe 100644 --- a/sys/dev/rtwn/usb/rtwn_usb_ep.c +++ b/sys/dev/rtwn/usb/rtwn_usb_ep.c @@ -55,7 +55,7 @@ #include <dev/rtwn/rtl8192c/usb/r92cu_reg.h> -static const struct usb_config rtwn_config_common[RTWN_N_TRANSFER] = { +static const struct usb_config rtwn_config_common[RTWN_BULK_EP_COUNT] = { [RTWN_BULK_RX] = { .type = UE_BULK, .endpoint = UE_ADDR_ANY, @@ -76,7 +76,7 @@ static const struct usb_config rtwn_config_common[RTWN_N_TRANSFER] = { .pipe_bof = 1, .force_short_xfer = 1, }, - .callback = rtwn_bulk_tx_callback, + .callback = rtwn_bulk_tx_callback_be, .timeout = RTWN_TX_TIMEOUT, /* ms */ }, [RTWN_BULK_TX_BK] = { @@ -89,7 +89,7 @@ static const struct usb_config rtwn_config_common[RTWN_N_TRANSFER] = { .pipe_bof = 1, .force_short_xfer = 1, }, - .callback = rtwn_bulk_tx_callback, + .callback = rtwn_bulk_tx_callback_bk, .timeout = RTWN_TX_TIMEOUT, /* ms */ }, [RTWN_BULK_TX_VI] = { @@ -102,7 +102,7 @@ static const struct usb_config rtwn_config_common[RTWN_N_TRANSFER] = { .pipe_bof = 1, .force_short_xfer = 1 }, - .callback = rtwn_bulk_tx_callback, + .callback = rtwn_bulk_tx_callback_vi, .timeout = RTWN_TX_TIMEOUT, /* ms */ }, [RTWN_BULK_TX_VO] = { @@ -115,7 +115,7 @@ static const struct usb_config rtwn_config_common[RTWN_N_TRANSFER] = { .pipe_bof = 1, .force_short_xfer = 1 }, - .callback = rtwn_bulk_tx_callback, + .callback = rtwn_bulk_tx_callback_vo, .timeout = RTWN_TX_TIMEOUT, /* ms */ }, }; @@ -200,22 +200,33 @@ rtwn_usb_setup_endpoints(struct rtwn_usb_softc *uc) /* NB: keep in sync with rtwn_dma_init(). */ rtwn_config[RTWN_BULK_TX_VO].endpoint = addr[0]; + uc->wme2qid[WME_AC_VO] = RTWN_BULK_TX_VO; switch (uc->ntx) { case 4: case 3: rtwn_config[RTWN_BULK_TX_BE].endpoint = addr[2]; rtwn_config[RTWN_BULK_TX_BK].endpoint = addr[2]; rtwn_config[RTWN_BULK_TX_VI].endpoint = addr[1]; + uc->wme2qid[WME_AC_BE] = RTWN_BULK_TX_BE; + uc->wme2qid[WME_AC_BK] = RTWN_BULK_TX_BE; + uc->wme2qid[WME_AC_VI] = RTWN_BULK_TX_VI; break; case 2: rtwn_config[RTWN_BULK_TX_BE].endpoint = addr[1]; rtwn_config[RTWN_BULK_TX_BK].endpoint = addr[1]; rtwn_config[RTWN_BULK_TX_VI].endpoint = addr[0]; + uc->wme2qid[WME_AC_BE] = RTWN_BULK_TX_VI; + uc->wme2qid[WME_AC_BK] = RTWN_BULK_TX_VI; + uc->wme2qid[WME_AC_VI] = RTWN_BULK_TX_VO; break; case 1: rtwn_config[RTWN_BULK_TX_BE].endpoint = addr[0]; rtwn_config[RTWN_BULK_TX_BK].endpoint = addr[0]; rtwn_config[RTWN_BULK_TX_VI].endpoint = addr[0]; + + uc->wme2qid[WME_AC_BE] = RTWN_BULK_TX_VO; + uc->wme2qid[WME_AC_BK] = RTWN_BULK_TX_VO; + uc->wme2qid[WME_AC_VI] = RTWN_BULK_TX_VO; break; default: KASSERT(0, ("unhandled number of endpoints %d\n", uc->ntx)); @@ -225,7 +236,7 @@ rtwn_usb_setup_endpoints(struct rtwn_usb_softc *uc) rtwn_config[RTWN_BULK_RX].bufsize = uc->uc_rx_buf_size * RTWN_USB_RXBUFSZ_UNIT; error = usbd_transfer_setup(uc->uc_udev, &iface_index, - uc->uc_xfer, rtwn_config, RTWN_N_TRANSFER, uc, &sc->sc_mtx); + uc->uc_xfer, rtwn_config, RTWN_BULK_EP_COUNT, uc, &sc->sc_mtx); free(rtwn_config, M_TEMP); if (error) { diff --git a/sys/dev/rtwn/usb/rtwn_usb_tx.c b/sys/dev/rtwn/usb/rtwn_usb_tx.c index 0fb8632d9a16..86d41ed10d91 100644 --- a/sys/dev/rtwn/usb/rtwn_usb_tx.c +++ b/sys/dev/rtwn/usb/rtwn_usb_tx.c @@ -65,10 +65,6 @@ static struct rtwn_data * rtwn_usb_getbuf(struct rtwn_usb_softc *); static void rtwn_usb_txeof(struct rtwn_usb_softc *, struct rtwn_data *, int); -static const uint8_t wme2qid[] = - { RTWN_BULK_TX_BE, RTWN_BULK_TX_BK, - RTWN_BULK_TX_VI, RTWN_BULK_TX_VO }; - static struct rtwn_data * _rtwn_usb_getbuf(struct rtwn_usb_softc *uc) { @@ -105,6 +101,7 @@ static void rtwn_usb_txeof(struct rtwn_usb_softc *uc, struct rtwn_data *data, int status) { struct rtwn_softc *sc = &uc->uc_sc; + bool is_empty = true; RTWN_ASSERT_LOCKED(sc); @@ -120,42 +117,54 @@ rtwn_usb_txeof(struct rtwn_usb_softc *uc, struct rtwn_data *data, int status) STAILQ_INSERT_TAIL(&uc->uc_tx_inactive, data, next); sc->qfullmsk = 0; + #ifndef D4054 - if (STAILQ_EMPTY(&uc->uc_tx_active) && STAILQ_EMPTY(&uc->uc_tx_pending)) + for (int i = RTWN_BULK_TX_FIRST; i < RTWN_BULK_EP_COUNT; i++) { + if (!STAILQ_EMPTY(&uc->uc_tx_active[i]) || + !STAILQ_EMPTY(&uc->uc_tx_pending[i])) + is_empty = false; + } + + if (is_empty) sc->sc_tx_timer = 0; else sc->sc_tx_timer = 5; #endif } -void -rtwn_bulk_tx_callback(struct usb_xfer *xfer, usb_error_t error) +static void +rtwn_bulk_tx_callback_qid(struct usb_xfer *xfer, usb_error_t error, int qid) { struct rtwn_usb_softc *uc = usbd_xfer_softc(xfer); struct rtwn_softc *sc = &uc->uc_sc; struct rtwn_data *data; + bool do_is_empty_check = false; + int i; + + RTWN_DPRINTF(sc, RTWN_DEBUG_XMIT, + "%s: called, qid=%d\n", __func__, qid); RTWN_ASSERT_LOCKED(sc); switch (USB_GET_STATE(xfer)){ case USB_ST_TRANSFERRED: - data = STAILQ_FIRST(&uc->uc_tx_active); + data = STAILQ_FIRST(&uc->uc_tx_active[qid]); if (data == NULL) goto tr_setup; - STAILQ_REMOVE_HEAD(&uc->uc_tx_active, next); + STAILQ_REMOVE_HEAD(&uc->uc_tx_active[qid], next); rtwn_usb_txeof(uc, data, 0); /* FALLTHROUGH */ case USB_ST_SETUP: tr_setup: - data = STAILQ_FIRST(&uc->uc_tx_pending); + data = STAILQ_FIRST(&uc->uc_tx_pending[qid]); if (data == NULL) { RTWN_DPRINTF(sc, RTWN_DEBUG_XMIT, "%s: empty pending queue\n", __func__); - sc->sc_tx_n_active = 0; + do_is_empty_check = true; goto finish; } - STAILQ_REMOVE_HEAD(&uc->uc_tx_pending, next); - STAILQ_INSERT_TAIL(&uc->uc_tx_active, data, next); + STAILQ_REMOVE_HEAD(&uc->uc_tx_pending[qid], next); + STAILQ_INSERT_TAIL(&uc->uc_tx_active[qid], data, next); /* * Note: if this is a beacon frame, ensure that it will go @@ -169,11 +178,17 @@ tr_setup: sc->sc_tx_n_active++; break; default: - data = STAILQ_FIRST(&uc->uc_tx_active); + data = STAILQ_FIRST(&uc->uc_tx_active[qid]); if (data == NULL) goto tr_setup; - STAILQ_REMOVE_HEAD(&uc->uc_tx_active, next); + STAILQ_REMOVE_HEAD(&uc->uc_tx_active[qid], next); rtwn_usb_txeof(uc, data, 1); + if (error != 0) + device_printf(sc->sc_dev, + "%s: called; txeof qid=%d, error=%s\n", + __func__, + qid, + usbd_errstr(error)); if (error != USB_ERR_CANCELLED) { usbd_xfer_set_stall(xfer); goto tr_setup; @@ -181,6 +196,19 @@ tr_setup: break; } finish: + + /* + * Clear sc_tx_n_active if all the pending transfers are 0. + * + * This is currently a crutch because net80211 doesn't provide + * a way to defer all the FF checks or one of the FF checks. + * Eventually this should just be tracked per-endpoint. + */ + for (i = RTWN_BULK_TX_FIRST; i < RTWN_BULK_EP_COUNT; i++) + if (STAILQ_FIRST(&uc->uc_tx_pending[i]) != NULL) + do_is_empty_check = false; + if (do_is_empty_check) + sc->sc_tx_n_active = 0; #ifdef IEEE80211_SUPPORT_SUPERG /* * If the TX active queue drops below a certain @@ -210,6 +238,34 @@ finish: rtwn_start(sc); } +void +rtwn_bulk_tx_callback_be(struct usb_xfer *xfer, usb_error_t error) +{ + + rtwn_bulk_tx_callback_qid(xfer, error, RTWN_BULK_TX_BE); +} + +void +rtwn_bulk_tx_callback_bk(struct usb_xfer *xfer, usb_error_t error) +{ + + rtwn_bulk_tx_callback_qid(xfer, error, RTWN_BULK_TX_BK); +} + +void +rtwn_bulk_tx_callback_vi(struct usb_xfer *xfer, usb_error_t error) +{ + + rtwn_bulk_tx_callback_qid(xfer, error, RTWN_BULK_TX_VI); +} + +void +rtwn_bulk_tx_callback_vo(struct usb_xfer *xfer, usb_error_t error) +{ + + rtwn_bulk_tx_callback_qid(xfer, error, RTWN_BULK_TX_VO); +} + static void rtwn_usb_tx_checksum(struct rtwn_tx_desc_common *txd) { @@ -226,6 +282,7 @@ rtwn_usb_tx_start(struct rtwn_softc *sc, struct ieee80211_node *ni, struct rtwn_data *data; struct usb_xfer *xfer; uint16_t ac; + int qid = 0; RTWN_ASSERT_LOCKED(sc); @@ -236,17 +293,23 @@ rtwn_usb_tx_start(struct rtwn_softc *sc, struct ieee80211_node *ni, if (data == NULL) return (ENOBUFS); + /* TODO: should really get a consistent AC/TID, ath(4) style */ ac = M_WME_GETAC(m); switch (type) { case IEEE80211_FC0_TYPE_CTL: case IEEE80211_FC0_TYPE_MGT: - xfer = uc->uc_xfer[RTWN_BULK_TX_VO]; + qid = RTWN_BULK_TX_VO; break; default: - xfer = uc->uc_xfer[wme2qid[ac]]; + qid = uc->wme2qid[ac]; break; } + xfer = uc->uc_xfer[qid]; + + RTWN_DPRINTF(sc, RTWN_DEBUG_XMIT, + "%s: called, ac=%d, qid=%d, xfer=%p\n", + __func__, ac, qid, xfer); txd = (struct rtwn_tx_desc_common *)tx_desc; txd->pktlen = htole16(m->m_pkthdr.len); @@ -264,6 +327,7 @@ rtwn_usb_tx_start(struct rtwn_softc *sc, struct ieee80211_node *ni, data->buflen = m->m_pkthdr.len + sc->txdesc_len; data->id = id; data->ni = ni; + data->qid = qid; if (data->ni != NULL) { data->m = m; #ifndef D4054 @@ -271,7 +335,7 @@ rtwn_usb_tx_start(struct rtwn_softc *sc, struct ieee80211_node *ni, #endif } - STAILQ_INSERT_TAIL(&uc->uc_tx_pending, data, next); + STAILQ_INSERT_TAIL(&uc->uc_tx_pending[qid], data, next); if (STAILQ_EMPTY(&uc->uc_tx_inactive)) sc->qfullmsk = 1; diff --git a/sys/dev/rtwn/usb/rtwn_usb_tx.h b/sys/dev/rtwn/usb/rtwn_usb_tx.h index 7b762cc01a00..193103f32707 100644 --- a/sys/dev/rtwn/usb/rtwn_usb_tx.h +++ b/sys/dev/rtwn/usb/rtwn_usb_tx.h @@ -17,7 +17,10 @@ #ifndef RTWN_USB_TX_H #define RTWN_USB_TX_H -void rtwn_bulk_tx_callback(struct usb_xfer *, usb_error_t); +void rtwn_bulk_tx_callback_bk(struct usb_xfer *, usb_error_t); +void rtwn_bulk_tx_callback_be(struct usb_xfer *, usb_error_t); +void rtwn_bulk_tx_callback_vi(struct usb_xfer *, usb_error_t); +void rtwn_bulk_tx_callback_vo(struct usb_xfer *, usb_error_t); int rtwn_usb_tx_start(struct rtwn_softc *, struct ieee80211_node *, struct mbuf *, uint8_t *, uint8_t, int); diff --git a/sys/dev/rtwn/usb/rtwn_usb_var.h b/sys/dev/rtwn/usb/rtwn_usb_var.h index bad697bfa1db..646dde66aeab 100644 --- a/sys/dev/rtwn/usb/rtwn_usb_var.h +++ b/sys/dev/rtwn/usb/rtwn_usb_var.h @@ -37,6 +37,7 @@ struct rtwn_data { uint8_t *buf; /* 'id' is meaningful for beacons only */ int id; + int qid; uint16_t buflen; struct mbuf *m; struct ieee80211_node *ni; @@ -50,15 +51,16 @@ enum { RTWN_BULK_TX_BK, /* = WME_AC_BK */ RTWN_BULK_TX_VI, /* = WME_AC_VI */ RTWN_BULK_TX_VO, /* = WME_AC_VO */ - RTWN_N_TRANSFER = 5, + RTWN_BULK_EP_COUNT = 5, }; #define RTWN_EP_QUEUES RTWN_BULK_RX +#define RTWN_BULK_TX_FIRST RTWN_BULK_TX_BE struct rtwn_usb_softc { struct rtwn_softc uc_sc; /* must be the first */ struct usb_device *uc_udev; - struct usb_xfer *uc_xfer[RTWN_N_TRANSFER]; + struct usb_xfer *uc_xfer[RTWN_BULK_EP_COUNT]; struct rtwn_data uc_rx[RTWN_USB_RX_LIST_COUNT]; rtwn_datahead uc_rx_active; @@ -70,14 +72,16 @@ struct rtwn_usb_softc { int uc_rx_off; struct rtwn_data uc_tx[RTWN_USB_TX_LIST_COUNT]; - rtwn_datahead uc_tx_active; + rtwn_datahead uc_tx_active[RTWN_BULK_EP_COUNT]; rtwn_datahead uc_tx_inactive; - rtwn_datahead uc_tx_pending; + rtwn_datahead uc_tx_pending[RTWN_BULK_EP_COUNT]; int (*uc_align_rx)(int, int); int ntx; int tx_agg_desc_num; + + uint8_t wme2qid[4]; }; #define RTWN_USB_SOFTC(sc) ((struct rtwn_usb_softc *)(sc))
