* [ath9k-devel] queue priority mapping @ 2014-11-20 18:00 Hubert Feurstein 2014-11-24 9:10 ` M. Braun 0 siblings, 1 reply; 11+ messages in thread From: Hubert Feurstein @ 2014-11-20 18:00 UTC (permalink / raw) To: ath9k-devel Hi, in ath9k_init_queues are the AC levels mapped to the hardware-queues by this loop below. But doesn't this map the priorities in the wrong direction? The hardware queue 0 has the lowest priority, but is mapped to IEEE80211_AC_VO. And the hardware queue 3 (with higher priority) is mapped to IEEE80211_AC_BK. Shouldn't that be the other way around (as changed below) ? @@ -323,11 +323,12 @@ static int ath9k_init_queues(struct ath_softc *sc) sc->beacon.cabq = ath_txq_setup(sc, ATH9K_TX_QUEUE_CAB, 0); ath_cabq_update(sc); sc->tx.uapsdq = ath_txq_setup(sc, ATH9K_TX_QUEUE_UAPSD, 0); - for (i = 0; i < IEEE80211_NUM_ACS; i++) { + for (i = IEEE80211_NUM_ACS - 1; i >= 0; i--) { sc->tx.txq_map[i] = ath_txq_setup(sc, ATH9K_TX_QUEUE_DATA, i); sc->tx.txq_map[i]->mac80211_qnum = i; sc->tx.txq_max_pending[i] = ATH_MAX_QDEPTH; } return 0; Best regards Hubert -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20141120/43448825/attachment.htm ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-20 18:00 [ath9k-devel] queue priority mapping Hubert Feurstein @ 2014-11-24 9:10 ` M. Braun 2014-11-25 21:14 ` Hubert Feurstein 0 siblings, 1 reply; 11+ messages in thread From: M. Braun @ 2014-11-24 9:10 UTC (permalink / raw) To: ath9k-devel Hi, Am 20.11.2014 um 19:00 schrieb Hubert Feurstein: > in ath9k_init_queues are the AC levels mapped to the hardware-queues by > this loop below. But doesn't this map the priorities in the wrong > direction? The hardware queue 0 has the lowest priority, but is mapped to > IEEE80211_AC_VO. And the hardware queue 3 (with higher priority) is mapped > to IEEE80211_AC_BK. Shouldn't that be the other way around (as changed > below) ? I don't know about the hw priority stuff, but the change you propose does not change the resulting data structure at all. You'd likely need to change mac80211_qnum to be IEEE80211_NUM_ACS-1-i (and maybe others) to change the mapping. Regards, M. Braun > > @@ -323,11 +323,12 @@ static int ath9k_init_queues(struct ath_softc *sc) > sc->beacon.cabq = ath_txq_setup(sc, ATH9K_TX_QUEUE_CAB, 0); > ath_cabq_update(sc); > > sc->tx.uapsdq = ath_txq_setup(sc, ATH9K_TX_QUEUE_UAPSD, 0); > > - for (i = 0; i < IEEE80211_NUM_ACS; i++) { > + for (i = IEEE80211_NUM_ACS - 1; i >= 0; i--) { > sc->tx.txq_map[i] = ath_txq_setup(sc, ATH9K_TX_QUEUE_DATA, i); > sc->tx.txq_map[i]->mac80211_qnum = i; > sc->tx.txq_max_pending[i] = ATH_MAX_QDEPTH; > } > return 0; > > Best regards > Hubert > > > > _______________________________________________ > ath9k-devel mailing list > ath9k-devel at lists.ath9k.org > https://lists.ath9k.org/mailman/listinfo/ath9k-devel > ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-24 9:10 ` M. Braun @ 2014-11-25 21:14 ` Hubert Feurstein 2014-11-25 21:22 ` Dave Taht ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Hubert Feurstein @ 2014-11-25 21:14 UTC (permalink / raw) To: ath9k-devel Hi, 2014-11-24 10:10 GMT+01:00 M. Braun <michael-dev@fami-braun.de>: [...] > > I don't know about the hw priority stuff, but the change you propose > does not change the resulting data structure at all. > You'd likely need to change mac80211_qnum to be IEEE80211_NUM_ACS-1-i > (and maybe others) to change the mapping. 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. Regards Hubert ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-25 21:14 ` Hubert Feurstein @ 2014-11-25 21:22 ` Dave Taht 2014-11-26 7:46 ` M. Braun 2014-11-26 12:25 ` Sujith Manoharan 2 siblings, 0 replies; 11+ messages in thread From: Dave Taht @ 2014-11-25 21:22 UTC (permalink / raw) To: ath9k-devel 802.11e style priority mappings are just so wrong for 802.11n, anyway. All they do is add bufferbloat and eat txops where you are much better off aggregating more packets. On Tue, Nov 25, 2014 at 1:14 PM, Hubert Feurstein <h.feurstein@gmail.com> wrote: > Hi, > > 2014-11-24 10:10 GMT+01:00 M. Braun <michael-dev@fami-braun.de>: > [...] >> >> I don't know about the hw priority stuff, but the change you propose >> does not change the resulting data structure at all. >> You'd likely need to change mac80211_qnum to be IEEE80211_NUM_ACS-1-i >> (and maybe others) to change the mapping. > > 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. > > Regards > Hubert > _______________________________________________ > ath9k-devel mailing list > ath9k-devel at lists.ath9k.org > https://lists.ath9k.org/mailman/listinfo/ath9k-devel -- Dave T?ht thttp://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-25 21:14 ` Hubert Feurstein 2014-11-25 21:22 ` Dave Taht @ 2014-11-26 7:46 ` M. Braun 2014-11-26 12:25 ` Sujith Manoharan 2 siblings, 0 replies; 11+ messages in thread From: M. Braun @ 2014-11-26 7:46 UTC (permalink / raw) To: ath9k-devel Hi, Am 25.11.2014 um 22:14 schrieb Hubert Feurstein: > [...] > 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. sorry, I looked at the ATH_TXQ_SETUP macro in ath9k.h instead of the ath_txq_setup function in xmit.c. Regards, M. Braun ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-25 21:14 ` Hubert Feurstein 2014-11-25 21:22 ` Dave Taht 2014-11-26 7:46 ` M. Braun @ 2014-11-26 12:25 ` Sujith Manoharan 2014-11-27 12:04 ` Hubert Feurstein 2 siblings, 1 reply; 11+ messages in thread From: Sujith Manoharan @ 2014-11-26 12:25 UTC (permalink / raw) To: ath9k-devel 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." Sujith ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-26 12:25 ` Sujith Manoharan @ 2014-11-27 12:04 ` Hubert Feurstein 2014-11-30 11:30 ` Thomas Hühn 0 siblings, 1 reply; 11+ messages in thread From: Hubert Feurstein @ 2014-11-27 12:04 UTC (permalink / raw) To: ath9k-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-27 12:04 ` Hubert Feurstein @ 2014-11-30 11:30 ` Thomas Hühn 2014-11-30 18:18 ` Sergey Ryazanov 0 siblings, 1 reply; 11+ messages in thread From: Thomas Hühn @ 2014-11-30 11:30 UTC (permalink / raw) To: ath9k-devel Hi Hubert, From my point of view the call-path of the functions look like this: ath9k_init_queues <http://lxr.free-electrons.com/ident?i=ath9k_init_queues>(struct ath_softc <http://lxr.free-electrons.com/ident?i=ath_softc> *sc <http://lxr.free-electrons.com/ident?i=sc>) -> ath_txq_setup <http://lxr.free-electrons.com/ident?i=ath_txq_setup>(sc <http://lxr.free-electrons.com/ident?i=sc>, ATH9K_TX_QUEUE_DATA, i <http://lxr.free-electrons.com/ident?i=i>) with subtype-remapping via qi.tqi_subtype = subtype_txq_to_hwq <http://lxr.free-electrons.com/ident?i=subtype_txq_to_hwq>[subtype]; -> ath9k_hw_setuptxqueue <http://lxr.free-electrons.com/ident?i=ath9k_hw_setuptxqueue>(ah, qtype, &qi); And the subtyperemapping reflects the correct AC class mapping to ath9k hw queues. In xmit.c function ath_txq_setup <http://lxr.free-electrons.com/ident?i=ath_txq_setup>() defines and initializes the mapping with: static const int subtype_txq_to_hwq <http://lxr.free-electrons.com/ident?i=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 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L101> /** 102 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L102> * enum ieee80211_ac_numbers - AC numbers as used in mac80211 103 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L103> * @IEEE80211_AC_VO: voice 104 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L104> * @IEEE80211_AC_VI: video 105 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L105> * @IEEE80211_AC_BE: best effort 106 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L106> * @IEEE80211_AC_BK: background 107 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L107> */ 108 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L108> enum ieee80211_ac_numbers <http://lxr.free-electrons.com/ident?i=ieee80211_ac_numbers> { 109 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L109> IEEE80211_AC_VO = 0, 110 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L110> IEEE80211_AC_VI = 1, 111 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L111> IEEE80211_AC_BE = 2, 112 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L112> IEEE80211_AC_BK = 3, 113 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L113> }; 114 <http://lxr.free-electrons.com/source/include/net/mac80211.h#L114> #define IEEE80211_NUM_ACS <http://lxr.free-electrons.com/ident?i=IEEE80211_NUM_ACS> 4 from ath9k/hw.h: 218 <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath9k/hw.h#L218> enum ath_hw_txq_subtype <http://lxr.free-electrons.com/ident?i=ath_hw_txq_subtype> { 219 <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath9k/hw.h#L219> ATH_TXQ_AC_BE = 0, 220 <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath9k/hw.h#L220> ATH_TXQ_AC_BK = 1, 221 <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath9k/hw.h#L221> ATH_TXQ_AC_VI = 2, 222 <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath9k/hw.h#L222> ATH_TXQ_AC_VO = 3, 223 <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath9k/hw.h#L223> }; this translates to the following mapping of AC to hw queues: static const int subtype_txq_to_hwq <http://lxr.free-electrons.com/ident?i=subtype_txq_to_hwq>[] = { [2] = 0, [3] = 1, [1] = 2, [0] = 3, }; 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 ? 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 at msujith.org <mailto: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 > _______________________________________________ > ath9k-devel mailing list > ath9k-devel at lists.ath9k.org <mailto:ath9k-devel@lists.ath9k.org> > https://lists.ath9k.org/mailman/listinfo/ath9k-devel <https://lists.ath9k.org/mailman/listinfo/ath9k-devel> -------------- next part -------------- An HTML attachment was scrubbed... URL: http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20141130/aa6eeb3b/attachment.htm ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-30 11:30 ` Thomas Hühn @ 2014-11-30 18:18 ` Sergey Ryazanov 2014-11-30 21:57 ` Thomas Hühn 0 siblings, 1 reply; 11+ messages in thread From: Sergey Ryazanov @ 2014-11-30 18:18 UTC (permalink / raw) To: ath9k-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-30 18:18 ` Sergey Ryazanov @ 2014-11-30 21:57 ` Thomas Hühn 2014-11-30 22:11 ` Sergey Ryazanov 0 siblings, 1 reply; 11+ messages in thread From: Thomas Hühn @ 2014-11-30 21:57 UTC (permalink / raw) To: ath9k-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [ath9k-devel] queue priority mapping 2014-11-30 21:57 ` Thomas Hühn @ 2014-11-30 22:11 ` Sergey Ryazanov 0 siblings, 0 replies; 11+ messages in thread From: Sergey Ryazanov @ 2014-11-30 22:11 UTC (permalink / raw) To: ath9k-devel 2014-12-01 0:57 GMT+03:00 Thomas H?hn <thomas@net.t-labs.tu-berlin.de>: > Hi Sergey, > Hi Thomas, > 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. > Do you know any issues caused by this queues misuse? I actually do not know what to 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 > -- BR, Sergey ? ?????????? ??????????? ??????? ?????? ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-11-30 22:11 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-11-20 18:00 [ath9k-devel] queue priority mapping Hubert Feurstein 2014-11-24 9:10 ` M. Braun 2014-11-25 21:14 ` Hubert Feurstein 2014-11-25 21:22 ` Dave Taht 2014-11-26 7:46 ` M. Braun 2014-11-26 12:25 ` Sujith Manoharan 2014-11-27 12:04 ` Hubert Feurstein 2014-11-30 11:30 ` Thomas Hühn 2014-11-30 18:18 ` Sergey Ryazanov 2014-11-30 21:57 ` Thomas Hühn 2014-11-30 22:11 ` Sergey Ryazanov
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.