Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: update ieee80211_classify() for more responsive SSH
To:
tech@openbsd.org
Date:
Fri, 12 Dec 2025 11:53:30 +0100

Download raw body.

Thread
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];
> 
>