Re: Fix iwx wrong ac indexs.

[zxystd]

OK.


seems it is the only reason why I can see the difference.
Thank you.



---Original---
From: "Stefan Sperling"<s...@stsp.name&gt;
Date: Thu, Mar 18, 2021 17:07 PM
To: "zxystd"<zxy...@foxmail.com&gt;;
Cc: "tech"<tech@openbsd.org&gt;;
Subject: Re: Fix iwx wrong ac indexs.


On Thu, Mar 18, 2021 at 11:41:33AM +0800, zxystd wrote:
&gt; The indexes into the ac array in the iwx_mac_ctxt_cmd_common are from the
&gt; iwx_ac enum and not the txfs. The current code therefore puts the edca 
params
&gt; in the wrong indexes of the array, causing wrong priority for data-streams 
of
&gt; different ACs. This bug is not exists in iwm as its txf number is equal to
&gt; the corresponding ac in the iwx_ac enum.

Thank you!
This doesn't really cause any problem as far as I can see, since we
don't actually make use of QoS. But I agree it should be fixed.

&gt; Index: src/sys/dev/pci/if_iwx.c
&gt; ===================================================================
&gt; RCS file: /cvs/src/sys/dev/pci/if_iwx.c,v
&gt; retrieving revision 1.50
&gt; diff -u -p -u -r1.50 if_iwx.c
&gt; --- src/sys/dev/pci/if_iwx.c17 Mar 2021 15:59:27 -00001.50
&gt; +++ src/sys/dev/pci/if_iwx.c18 Mar 2021 03:28:04 -0000
&gt; @@ -5135,6 +5135,17 @@ iwx_ack_rates(struct iwx_softc *sc, stru
&gt;&nbsp; *ofdm_rates = ofdm;
&gt;&nbsp; }
&gt;&nbsp; 
&gt; +static uint8_t iwx_mac80211_ac_to_ucode_ac(enum ieee80211_edca_ac ac)
&gt; +{
&gt; +static const uint8_t iwx_ac_to_ucode_ac[] = {
&gt; +IWX_AC_VO,
&gt; +IWX_AC_VI,
&gt; +IWX_AC_BE,
&gt; +IWX_AC_BK
&gt; +};

I would prefer to use a global array without wrapping it inside a function.
Other code can simply index the array directly, see below.

&gt; +return iwx_ac_to_ucode_ac[ac];
&gt; +}
&gt; +
&gt;&nbsp; void
&gt;&nbsp; iwx_mac_ctxt_cmd_common(struct iwx_softc *sc, struct iwx_node *in,
&gt;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; struct iwx_mac_ctx_cmd *cmd, uint32_t action)
&gt; @@ -5181,12 +5192,13 @@ iwx_mac_ctxt_cmd_common(struct iwx_softc
&gt;&nbsp; for (i = 0; i < EDCA_NUM_AC; i++) {
&gt;&nbsp; struct ieee80211_edca_ac_params *ac = &amp;ic-&gt;ic_edca_ac[i];
&gt;&nbsp; int txf = iwx_ac_to_tx_fifo[i];
&gt; +uint8_t ucode_ac = iwx_mac80211_ac_to_ucode_ac(i);

Instead of calling a function, we could do an array lookup:

uint8_t ucode_ac = iwx_ac_to_ucode_ac[ac];
&gt;&nbsp; 
&gt; -cmd-&gt;ac[txf].cw_min = htole16(IWX_EXP2(ac-&gt;ac_ecwmin));
&gt; -cmd-&gt;ac[txf].cw_max = htole16(IWX_EXP2(ac-&gt;ac_ecwmax));
&gt; -cmd-&gt;ac[txf].aifsn = ac-&gt;ac_aifsn;
&gt; -cmd-&gt;ac[txf].fifos_mask = (1 << txf);
&gt; -cmd-&gt;ac[txf].edca_txop = htole16(ac-&gt;ac_txoplimit * 32);
&gt; +cmd-&gt;ac[ucode_ac].cw_min = htole16(IWX_EXP2(ac-&gt;ac_ecwmin));
&gt; +cmd-&gt;ac[ucode_ac].cw_max = htole16(IWX_EXP2(ac-&gt;ac_ecwmax));
&gt; +cmd-&gt;ac[ucode_ac].aifsn = ac-&gt;ac_aifsn;
&gt; +cmd-&gt;ac[ucode_ac].fifos_mask = (1 << txf);
&gt; +cmd-&gt;ac[ucode_ac].edca_txop = htole16(ac-&gt;ac_txoplimit * 32);
&gt;&nbsp; }
&gt;&nbsp; if (ni-&gt;ni_flags &amp; IEEE80211_NODE_QOS)
&gt;&nbsp; cmd-&gt;qos_flags |= htole32(IWX_MAC_QOS_FLG_UPDATE_EDCA);