From: Stefan Sperling <stsp@stsp.name>
Subject: Re: sys/ieee80211: support of uAPSD; sys/iwx: enable uAPSD when supported by AP
To: tech@openbsd.org
Date: Mon, 18 May 2026 10:40:08 +0200

This looks great. I mostly have small cosmetic concerns which could
be fixed after commit. And I spotted one potential problem which
could cause regressions in older drivers. See below.

On Mon, May 18, 2026 at 01:08:00AM +0200, Kirill A. Korinsky wrote:
> +static uint8_t
> +iwx_uapsd_ac_flags(struct ieee80211_node *ni)
> +{
> +	uint8_t ac_flags = 0;
> +
> +	if (ni->ni_uapsd_ac & IEEE80211_WMM_IE_STA_QOSINFO_AC_VO)
> +		ac_flags |= 1 << 0;

Could we please avoid magic numbers by using EDCA_AC_BE here instead of 0?
And similarly for the following lines:

> +	if (ni->ni_uapsd_ac & IEEE80211_WMM_IE_STA_QOSINFO_AC_VI)
> +		ac_flags |= 1 << 1;
> +	if (ni->ni_uapsd_ac & IEEE80211_WMM_IE_STA_QOSINFO_AC_BE)
> +		ac_flags |= 1 << 2;
> +	if (ni->ni_uapsd_ac & IEEE80211_WMM_IE_STA_QOSINFO_AC_BK)
> +		ac_flags |= 1 << 3;
> +
> +	return ac_flags;
> +}

> @@ -6673,6 +6740,21 @@ iwx_set_pslevel(struct iwx_softc *sc, in
>  		    IWX_POWER_FLAGS_POWER_MANAGEMENT_ENA_MSK);
>  		mcmd.rx_data_timeout = htole32(pmgt->rxtimeout * 1024);
>  		mcmd.tx_data_timeout = htole32(pmgt->txtimeout * 1024);
> +		if (ni->ni_uapsd &&
> +		    (sc->sc_capaflags & IWX_UCODE_TLV_FLAGS_UAPSD_SUPPORT)) {

For clarity, could we rename ni->ni_uapsd to ni->ni_use_uapsd or similar?
Or make it a flag such as ni->ni_flags & IEEE80211_NODE_UAPSD?

This would provide a very obvious indication to readers of this code
that ni_uapsd is a configuration flag, rather than some bit of data which
was parsed from the node's beacon (or assoc request in hostap mode).

> @@ -1601,6 +1611,9 @@ ieee80211_get_assoc_req(struct ieee80211
>  		    IEEE80211_HE_MCS_NSS_SIZE(ic->ic_he_phy_cap[0]);
>  	}
>  
> +	addwme = (ni->ni_flags & IEEE80211_NODE_QOS) ||
> +	    (ic->ic_flags & IEEE80211_F_HTON);

Shouldn't this be && instead of || ?

So far we've only enabled WME in 11n and 11ac modes.
I don't think enabling WME in 11a/b/g only mode (and drivers) makes sense,
even if the peer supports QoS. Because this could break some of the older
drivers, which are not being tested very often.