[ath9k-devel] queue priority mapping

["Thomas Hühn" <thomas@net.t-labs.tu-berlin.de>]

Hi Sergey,

You are right, the wrong mapping was caused by the call order.
Felix has already sent 3 patches to fix that bug. So lets test.

Greetings from Berlin
Thomas




> Am 30.11.2014 um 19:18 schrieb Sergey Ryazanov <ryazanov.s.a@gmail.com>:
> 
> Hi Thomas,
> 
> 2014-11-30 14:30 GMT+03:00 Thomas H?hn <thomas@net.t-labs.tu-berlin.de>:
>> From my point of view the call-path of the functions look like this:
>> 
>> ath9k_init_queues(struct ath_softc *sc)
>> 
>>       -> ath_txq_setup(sc, ATH9K_TX_QUEUE_DATA, i) with subtype-remapping
>> via
>> 
>> qi.tqi_subtype = subtype_txq_to_hwq[subtype];
>> 
>> -> ath9k_hw_setuptxqueue(ah, qtype, &qi);
>> 
>> And the subtyperemapping reflects the correct AC class mapping to ath9k hw
>> queues.
> 
> tqi_subtype field doesn't define hw queue. Quick grep show only 6
> lines in the whole driver, where this field is accessed. Actual hw
> queue number stored in axq_qnum field of ath_txq as documented in
> ath9k.h
> 
>> In xmit.c function ath_txq_setup() defines and initializes the mapping with:
>> 
>> static const int subtype_txq_to_hwq[] = {
>> 
>> [IEEE80211_AC_BE] = ATH_TXQ_AC_BE,
>> [IEEE80211_AC_BK] = ATH_TXQ_AC_BK,
>> [IEEE80211_AC_VI] = ATH_TXQ_AC_VI,
>> [IEEE80211_AC_VO] = ATH_TXQ_AC_VO,
>> };
>> 
>> from mac80211.h:
>> 
>> 101 /**
>> 102  * enum ieee80211_ac_numbers - AC numbers as used in mac80211
>> 103  * @IEEE80211_AC_VO: voice
>> 104  * @IEEE80211_AC_VI: video
>> 105  * @IEEE80211_AC_BE: best effort
>> 106  * @IEEE80211_AC_BK: background
>> 107  */
>> 108 enum ieee80211_ac_numbers {
>> 109         IEEE80211_AC_VO         = 0,
>> 110         IEEE80211_AC_VI         = 1,
>> 111         IEEE80211_AC_BE         = 2,
>> 112         IEEE80211_AC_BK         = 3,
>> 113 };
>> 114 #define IEEE80211_NUM_ACS       4
>> 
>> from ath9k/hw.h:
>> 
>> 218 enum ath_hw_txq_subtype {
>> 219         ATH_TXQ_AC_BE = 0,
>> 220         ATH_TXQ_AC_BK = 1,
>> 221         ATH_TXQ_AC_VI = 2,
>> 222         ATH_TXQ_AC_VO = 3,
>> 223 };
>> 
>> this translates to the following mapping of AC to hw queues:
>> 
>> static const int subtype_txq_to_hwq[] = {
>> [2] = 0,
>> [3] = 1,
>> [1] = 2,
>> [0] = 3,
>> };
>> 
> 
> This table confuse me too. But if you trace where the table and the
> tqi_subtype field is used, then you face, that this table means
> nothing for hw queue selection.
> 
>> 
>> That is how I understand the source code and yes it looks a bit complicated
>> but function wise seems to be correct.
>> Or do I miss something here ?
>> 
> 
> Actual hw queue selection occurs inside the ath9k_hw_setuptxqueue()
> function (in mac.c). Stripped version looks like:
> 
> int ath9k_hw_setuptxqueue(struct ath_hw *ah, enum ath9k_tx_queue type,
>                          const struct ath9k_tx_queue_info *qinfo)
> {
>        int q;
> [stripped]
>                for (q = 0; q < ATH9K_NUM_TX_QUEUES; q++)
>                        if (ah->txq[q].tqi_type ==
>                            ATH9K_TX_QUEUE_INACTIVE)
>                                break;
> [stripped]
>        return q;
> }
> 
> So, as Huber wrote, hw queue number really depends on call order. And
> existing call order gives us the following result:
> 
> ~# cat /sys/kernel/debug/ieee80211/phy0/ath9k/queues
> (VO):  qnum: 0 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
> (VI):  qnum: 1 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
> (BE):  qnum: 2 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
> (BK):  qnum: 3 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
> (CAB): qnum: 8 qdepth:  0 ampdu-depth:  0 pending:   0 stopped: 0
> 
> Just in case, I checked ath5k driver and it contains exactly the same code.
> 
> Let's wait a comment from maintainers.
> 
>> Greetings from Berlin
>> Thomas
>> 
>> 
>> 
>> 
>> Am 27.11.2014 um 13:04 schrieb Hubert Feurstein <h.feurstein@gmail.com>:
>> 
>> Hi Sujith,
>> 
>> 2014-11-26 13:25 GMT+01:00 Sujith Manoharan <sujith@msujith.org>:
>> 
>> Hubert Feurstein wrote:
>> 
>> Well, in fact it does. To understand my post you have to know the
>> behaviour of ath_txq_setup. The point here is that on the first call
>> of ath_txq_setup(..ATH9K_TX_QUEUE_DATA..), hw-queue 0 is returned. On
>> the second call hw-queue 1 is returned, ... . And IEEE80211_AC_VO = 0,
>> so hw-queue 0 is assigned, which is wrong in my opinion. But the right
>> place to fix this would be ath_txq_setup itself.
>> 
>> 
>> The mapping has been changed recently and I think it might be a problem,
>> since this is what the datasheet says:
>> 
>> "The mapping of physical DCUs to absolute channel access priorities is fixed
>> and
>> cannot be altered by software."
>> 
>> 
>> Yes I know, and isn't this the reason why there is
>> sc->tx.txq_map[<AC>] in place, to map the AC to the proper HW queue in
>> software. So sc->tx.txq_map[IEEE80211_AC_VO] should return the
>> higher-prio-queue 3 and not the low-prio-queue 0. In FreeBSD it is
>> that way, in Linux it is the opposite, because in Linux
>> IEEE80211_AC_VO=0 and in FreeBSD WME_AC_VO=3.
>> 
>> Hubert
> 
> -- 
> BR,
> Sergey
> _______________________________________________
> ath9k-devel mailing list
> ath9k-devel at lists.ath9k.org
> https://lists.ath9k.org/mailman/listinfo/ath9k-devel