From: Stefan Sperling Subject: Re: update ieee80211_classify() for more responsive SSH To: tech@openbsd.org Date: Fri, 12 Dec 2025 11:53:30 +0100 I have not heard anything yet about this version of the diff. Any ok/test with this version? On Thu, Dec 04, 2025 at 12:47:38PM +0100, Stefan Sperling wrote: > On Wed, Dec 03, 2025 at 01:02:32PM -0700, Theo de Raadt wrote: > > I have a worry that as soon as our kernel does conversion from one > > priority classification, to a different level's priority conversion, > > we should possibly be responsible for doing some traffic management > > automatically. > > > > My gut feeling is we should be the same rate limiting back-pressure > > that downstream devices do. > > > > If we don't have that... hey, can I get a trivial knob to make my chrome > > set the DSCP_EF flag? Or can we make that the default? > > > > See, I'm saying there is a slippery slope to EF becoming the default, > > and I feel like that should be stopped by saying high priority obviously > > means a limited % of traffic on the port. > > Agreed. Thanks for pointing this out. > > In the default timings prescribed by the 802.11 standard a device which > wins a high-prio VO Tx opportunity can use up to 2ms to send N frames. > This includes meta frames such as RTS, CTS, and block-ack. > Details: https://inet.omnetpp.org/docs/showcases/wireless/txop/doc/index.html > > Access points usually send a beacon every 100ms, and everything else must > happen in-between beacons. Which means, with 2ms per Tx op, a channel will > start becoming completely blocked if an application can grab about 50 VO Tx > opportunities per beacon. > > To balance this with typing latency, we need to consider that users are > typing comfortably at a latency between 20-200ms. > https://www.researchgate.net/publication/375773571_Effects_of_Text_Input_Latency_on_Performance_and_Task_Load > > Based on this, let's aim for roughly 50ms typing latency. Which means we > want to allow up to 2 VO Tx OPs per 100ms. > The intention is that each VO Tx OP we allow corresponds to a keypress in > an application using EF. Beyond this limit we fall back on best-effort. > This leaves time on the channel for other types of traffic, preventing abuse. > > Another problem in the previous diff was that our mapping from EDCA access > categories to QoS TID was outdated. EDCA has 4 queues, and QoS defines 8. > The 802.11 standard provides a mapping table ("Table 10-1 UP-to-AC mappings") > which I have applied in the patch below. This needs a fix for Tx aggregation > where TIDs 4-7 had been left disabled. The only other direct consumer of this > is the bwfm(4) driver which already expects to see priority values 0-7. > > This patch is working well for me. > > SSH key-presses get assigned to TID 6 (VO 3 -> UP 6). > > Bulk transfers (tcpbench) remain on TID 1 (BK 1 -> UP 1). TID 1 has a lower > prio than TID 0 in the UP-to-AC mapping, so using TID 1 for tcpbench is better. > > There are ways for access points to customize TID mappings, but I don't have > any which actually do this. Defaults should be good enough for now. > > > M sys/net80211/ieee80211_node.c | 44+ 4- > M sys/net80211/ieee80211_output.c | 79+ 17- > M sys/net80211/ieee80211_var.h | 2+ 0- > > 3 files changed, 125 insertions(+), 21 deletions(-) > > commit - 320ac6b49a67888bf6863fdc1a6ef9a750bd3d38 > commit + 04d9073cc5e16df46562019fa6d9812a377713ab > blob - 8086ef87306c57fc2fe930c8e07695bd7aef6298 > blob + 3d15d132829d794d55cc359426023133292b0dea > --- sys/net80211/ieee80211_node.c > +++ sys/net80211/ieee80211_node.c > @@ -81,6 +81,10 @@ void ieee80211_node_addba_request_ac_be_to(void *); > void ieee80211_node_addba_request_ac_bk_to(void *); > void ieee80211_node_addba_request_ac_vi_to(void *); > void ieee80211_node_addba_request_ac_vo_to(void *); > +void ieee80211_node_addba_request_tid4(void *); > +void ieee80211_node_addba_request_tid5(void *); > +void ieee80211_node_addba_request_tid6(void *); > +void ieee80211_node_addba_request_tid7(void *); > void ieee80211_needs_auth(struct ieee80211com *, struct ieee80211_node *); > #ifndef IEEE80211_STA_ONLY > void ieee80211_node_join_ht(struct ieee80211com *, struct ieee80211_node *); > @@ -1798,6 +1802,14 @@ ieee80211_node_set_timeouts(struct ieee80211_node *ni) > ieee80211_node_addba_request_ac_vi_to, ni); > timeout_set(&ni->ni_addba_req_to[EDCA_AC_VO], > ieee80211_node_addba_request_ac_vo_to, ni); > + timeout_set(&ni->ni_addba_req_to[4], > + ieee80211_node_addba_request_tid4, ni); > + timeout_set(&ni->ni_addba_req_to[5], > + ieee80211_node_addba_request_tid5, ni); > + timeout_set(&ni->ni_addba_req_to[6], > + ieee80211_node_addba_request_tid6, ni); > + timeout_set(&ni->ni_addba_req_to[7], > + ieee80211_node_addba_request_tid7, ni); > for (i = 0; i < nitems(ni->ni_addba_req_intval); i++) > ni->ni_addba_req_intval[i] = 1; > } > @@ -2094,10 +2106,10 @@ ieee80211_ba_del(struct ieee80211_node *ni) > for (tid = 0; tid < nitems(ni->ni_tx_ba); tid++) > ieee80211_node_tx_ba_clear(ni, tid); > > - timeout_del(&ni->ni_addba_req_to[EDCA_AC_BE]); > - timeout_del(&ni->ni_addba_req_to[EDCA_AC_BK]); > - timeout_del(&ni->ni_addba_req_to[EDCA_AC_VI]); > - timeout_del(&ni->ni_addba_req_to[EDCA_AC_VO]); > + for (tid = 0; tid < IEEE80211_NUM_TID; tid++) { > + if (timeout_initialized(&ni->ni_addba_req_to[tid])) > + timeout_del(&ni->ni_addba_req_to[tid]); > + } > } > > void > @@ -2773,6 +2785,34 @@ ieee80211_node_addba_request_ac_vo_to(void *arg) > ieee80211_node_addba_request(ni, EDCA_AC_VO); > } > > +void > +ieee80211_node_addba_request_tid4(void *arg) > +{ > + struct ieee80211_node *ni = arg; > + ieee80211_node_addba_request(ni, 4); > +} > + > +void > +ieee80211_node_addba_request_tid5(void *arg) > +{ > + struct ieee80211_node *ni = arg; > + ieee80211_node_addba_request(ni, 5); > +} > + > +void > +ieee80211_node_addba_request_tid6(void *arg) > +{ > + struct ieee80211_node *ni = arg; > + ieee80211_node_addba_request(ni, 6); > +} > + > +void > +ieee80211_node_addba_request_tid7(void *arg) > +{ > + struct ieee80211_node *ni = arg; > + ieee80211_node_addba_request(ni, 7); > +} > + > #ifndef IEEE80211_STA_ONLY > /* > * This function is called to notify the 802.1X PACP machine that a new > blob - 1ffd981a6fe179bc9f0689caaaad2af30277b453 > blob + 14f5629cf84de8704e10d0b045a3e7ffcaac9923 > --- sys/net80211/ieee80211_output.c > +++ sys/net80211/ieee80211_output.c > @@ -416,6 +416,45 @@ ieee80211_up_to_ac(struct ieee80211com *ic, int up) > return ac; > } > > +enum ieee80211_edca_ac > +ieee80211_classify_limit(struct ieee80211com *ic, enum ieee80211_edca_ac ac) > +{ > + const suseconds_t txop_interval = 100 * 1000; /* 100 msec, in usec */ > + /* Maximum amounts of high-prio Tx opportunities, per 100ms. */ > + static const int txop_limit[EDCA_NUM_AC] = { > + 0, /* Background */ > + 0, /* Best Effort */ > + 4, /* Video */ > + 2 /* Voice */ > + }; > + > + if (txop_limit[ac] <= 0) /* not rate-limited */ > + return ac; > + > + if (ic->ic_ecda_txop_count[ac] < txop_limit[ac]) { > + if (ic->ic_ecda_txop_count[ac] == 0) > + getmicrouptime(&ic->ic_ecda_txop_time[ac]); > + ic->ic_ecda_txop_count[ac]++; > + } else { > + struct timeval now, delta; > + > + getmicrouptime(&now); > + timersub(&now, &ic->ic_ecda_txop_time[ac], &delta); > + > + /* > + * Fall back on best-effort if the limit has been exceeded > + * within the current rate-limiting window. > + */ > + if (delta.tv_sec == 0 && delta.tv_usec < txop_interval) > + return EDCA_AC_BE; > + > + ic->ic_ecda_txop_count[ac] = 1; > + ic->ic_ecda_txop_time[ac] = now; > + } > + > + return ac; > +} > + > /* > * Get mbuf's user-priority: if mbuf is not VLAN tagged, select user-priority > * based on the DSCP (Differentiated Services Codepoint) field. > @@ -425,16 +464,28 @@ ieee80211_classify(struct ieee80211com *ic, struct mbu > { > struct ether_header eh; > u_int8_t ds_field; > + enum ieee80211_edca_ac ac; > + > + /* Map ECDA categories (0-3) to User Priority TIDs (0-7) */ > + static const int ecda_to_up[EDCA_NUM_AC] = { > + 1, /* Background */ > + 0, /* Best Effort */ > + 5, /* Video (primary) */ > + 6 /* Voice (primary) */ > + }; > + > #if NVLAN > 0 > - if (m->m_flags & M_VLANTAG) /* use VLAN 802.1D user-priority */ > - return EVL_PRIOFTAG(m->m_pkthdr.ether_vtag); > + if (m->m_flags & M_VLANTAG) { /* use VLAN 802.1D user-priority */ > + ac = EVL_PRIOFTAG(m->m_pkthdr.ether_vtag); > + return ecda_to_up[ieee80211_classify_limit(ic, ac)]; > + } > #endif > m_copydata(m, 0, sizeof(eh), (caddr_t)&eh); > if (eh.ether_type == htons(ETHERTYPE_IP)) { > struct ip ip; > m_copydata(m, sizeof(eh), sizeof(ip), (caddr_t)&ip); > if (ip.ip_v != 4) > - return 0; > + return ecda_to_up[EDCA_AC_BE]; > ds_field = ip.ip_tos; > } > #ifdef INET6 > @@ -444,31 +495,42 @@ ieee80211_classify(struct ieee80211com *ic, struct mbu > m_copydata(m, sizeof(eh), sizeof(ip6), (caddr_t)&ip6); > flowlabel = ntohl(ip6.ip6_flow); > if ((flowlabel >> 28) != 6) > - return 0; > + return ecda_to_up[EDCA_AC_BE]; > ds_field = (flowlabel >> 20) & 0xff; > } > #endif /* INET6 */ > else /* neither IPv4 nor IPv6 */ > - return 0; > + return ecda_to_up[EDCA_AC_BE]; > > /* > - * Map Differentiated Services Codepoint field (see RFC2474). > + * Map Differentiated Services Codepoint field (see RFC8325). > * Preserves backward compatibility with IP Precedence field. > */ > switch (ds_field & 0xfc) { > - case IPTOS_PREC_PRIORITY: > - return EDCA_AC_VI; > - case IPTOS_PREC_IMMEDIATE: > - return EDCA_AC_BK; > - case IPTOS_PREC_FLASH: > - case IPTOS_PREC_FLASHOVERRIDE: > - case IPTOS_PREC_CRITIC_ECP: > - case IPTOS_PREC_INTERNETCONTROL: > - case IPTOS_PREC_NETCONTROL: > - return EDCA_AC_VO; > + case IPTOS_DSCP_EF: > + /* TODO: case IPTOS_DSCP_VA: */ > + ac = EDCA_AC_VO; > + break; > + case IPTOS_DSCP_CS5: > + case IPTOS_DSCP_AF41: > + case IPTOS_DSCP_AF42: > + case IPTOS_DSCP_AF43: > + case IPTOS_DSCP_CS4: > + case IPTOS_DSCP_AF31: > + case IPTOS_DSCP_AF32: > + case IPTOS_DSCP_AF33: > + case IPTOS_DSCP_CS3: > + ac = EDCA_AC_VI; > + break; > + case IPTOS_DSCP_CS1: > + ac = EDCA_AC_BK; > + break; > default: > - return EDCA_AC_BE; > + /* unused, or explicitly mapped to UP 0 */ > + return ecda_to_up[EDCA_AC_BE]; > } > + > + return ecda_to_up[ieee80211_classify_limit(ic, ac)]; > } > > int > blob - c87240ad7081167514225a3549611ed7a99a40f1 > blob + b77e585bf0d677d279047bd9d8d18ccbf64596a8 > --- sys/net80211/ieee80211_var.h > +++ sys/net80211/ieee80211_var.h > @@ -337,6 +337,8 @@ struct ieee80211com { > */ > struct ieee80211_edca_ac_params ic_edca_ac[EDCA_NUM_AC]; > u_int ic_edca_updtcount; > + u_int ic_ecda_txop_count[EDCA_NUM_AC]; > + struct timeval ic_ecda_txop_time[EDCA_NUM_AC]; > u_int16_t ic_tid_noack; > u_int8_t ic_globalcnt[EAPOL_KEY_NONCE_LEN]; > u_int8_t ic_nonce[EAPOL_KEY_NONCE_LEN]; > >