From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Re: iwx(4) ampdu fix
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: stsp@openbsd.org, tech@openbsd.org
Date: Sun, 23 Feb 2025 17:12:35 +0100

On Sun, 23 Feb 2025 13:27:14 +0100,
Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> 
> My rev 83 "MLD" firmware iwx(4) doesn't work as well at home as it did
> in the hackroom in Valencia.  There is significant packet loss.  So I
> started looking again at the new code to see if I missed some things.
> One thing I missed is correctly setting the AMDPU density parameter.
> 
> I also noticed that the Linux driver uses a larger maximum aggregation
> length with a not that this is the maximum size supported by the
> firmware.  So I bumped that too.
> 
> I also fixed a missing htole32(), although that will make not
> difference on amd64.
> 
> The diff doesn't completely fix things; there is still significant
> packet loss (also in non-HT mode).  But it appears to make thuiings a
> little bit better.
> 
> ok?
>

Tested on amd64:

        iwx0 at pci0 dev 20 function 3 "Intel Wi-Fi 6 AX201" rev 0x00, msix
        iwx0: hw rev 0x350, fw 77.a20fb07d.0, address 98:8d:46:21:2b:6d

which is connected to Unifi 5G network. During the test here was only the
device which used the AP.

Without this diff:

        - - - - - - - - - - - - - - - - - - - - - - - - -
        [ ID] Interval           Transfer     Bitrate         Retr
        [  4]   0.00-10.02  sec   132 MBytes   111 Mbits/sec    0            sender
        [  4]   0.00-10.22  sec   133 MBytes   109 Mbits/sec                  receiver

With your diff:

        - - - - - - - - - - - - - - - - - - - - - - - - -
        [ ID] Interval           Transfer     Bitrate         Retr
        [  4]   0.00-10.02  sec   136 MBytes   114 Mbits/sec    0            sender
        [  4]   0.00-10.23  sec   136 MBytes   112 Mbits/sec                  receiver

> 
> Index: dev/pci/if_iwx.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/if_iwx.c,v
> retrieving revision 1.189
> diff -u -p -r1.189 if_iwx.c
> --- dev/pci/if_iwx.c	4 Feb 2025 09:15:04 -0000	1.189
> +++ dev/pci/if_iwx.c	23 Feb 2025 12:26:47 -0000
> @@ -6912,8 +6912,8 @@ iwx_mld_add_sta_cmd(struct iwx_softc *sc
>  	struct ieee80211com *ic = &sc->sc_ic;
>  	struct iwx_link_config_cmd link_cmd;
>  	struct iwx_mvm_sta_cfg_cmd sta_cmd;
> -	uint32_t aggsize;
> -	const uint32_t max_aggsize = (IWX_STA_FLG_MAX_AGG_SIZE_64K >>
> +	uint32_t aggsize, mpdu_dens;
> +	const uint32_t max_aggsize = (IWX_STA_FLG_MAX_AGG_SIZE_4M >>
>  		    IWX_STA_FLG_MAX_AGG_SIZE_SHIFT);
>  	int err, changes;
>  
> @@ -6970,6 +6970,9 @@ iwx_mld_add_sta_cmd(struct iwx_softc *sc
>  		if (iwx_mimo_enabled(sc))
>  			sta_cmd.mimo = htole32(1);
>  
> +		mpdu_dens = (in->in_ni.ni_ampdu_param &
> +		    IEEE80211_AMPDU_PARAM_SS) >> 2;
> +
>  		if (in->in_ni.ni_flags & IEEE80211_NODE_VHT) {
>  			aggsize = (in->in_ni.ni_vhtcaps &
>  			    IEEE80211_VHTCAP_MAX_AMPDU_LEN_MASK) >>
> @@ -6981,8 +6984,8 @@ iwx_mld_add_sta_cmd(struct iwx_softc *sc
>  		if (aggsize > max_aggsize)
>  			aggsize = max_aggsize;
>  
> -		sta_cmd.tx_ampdu_spacing = htole32(0);
> -		sta_cmd.tx_ampdu_max_size = aggsize;
> +		sta_cmd.tx_ampdu_spacing = htole32(mpdu_dens);
> +		sta_cmd.tx_ampdu_max_size = htole32(aggsize);
>  	}
>  
>  	return iwx_send_cmd_pdu(sc,
> 

-- 
wbr, Kirill