ath9k: incorrect queue may be stopped/awaken

When channel context is enabled, we could be
stopping/awakening an incorrect queue.

When channel context is enabled, we could be
stopping/awakening an incorrect queue.

---
 drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

drivers/net/wireless/ath/ath9k/xmit.c +12 -11

index 3e3dac3..9b17a59 100644
@@ -147,7 +147,12 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
 {
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
        struct ath_frame_info *fi = get_frame_info(skb);
-       int q = fi->txq;
+       int q;
+
+       if (ath9k_is_chanctx_enabled())
+               q = fi->txq;
+       else
+               q = info->hw_queue;
 
        if (q < 0)
                return;

@@ -158,10 +163,7 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
 
        if (txq->stopped &&
            txq->pending_frames < sc->tx.txq_max_pending[q]) {
-               if (ath9k_is_chanctx_enabled())
-                       ieee80211_wake_queue(sc->hw, info->hw_queue);
-               else
-                       ieee80211_wake_queue(sc->hw, q);
+               ieee80211_wake_queue(sc->hw, q);
                txq->stopped = false;
        }
 }

@@ -2299,17 +2301,17 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
         * info are no longer valid (overwritten by the ath_frame_info data.
         */
 
-       q = skb_get_queue_mapping(skb);
+       if (ath9k_is_chanctx_enabled())
+               q = skb_get_queue_mapping(skb);
+       else
+               q = info->hw_queue;
 
        ath_txq_lock(sc, txq);
        if (txq == sc->tx.txq_map[q]) {
                fi->txq = q;
                if (++txq->pending_frames > sc->tx.txq_max_pending[q] &&
                    !txq->stopped) {
-                       if (ath9k_is_chanctx_enabled())
-                               ieee80211_stop_queue(sc->hw, info->hw_queue);
-                       else
-                               ieee80211_stop_queue(sc->hw, q);
+                       ieee80211_stop_queue(sc->hw, q);
                        txq->stopped = true;
                }
        }
--

2.3.6

When channel context is enabled, we could be
stopping/awakening an incorrect queue.

Signed-off-by Borja Salazar <[email protected]>

---
 drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

drivers/net/wireless/ath/ath9k/xmit.c +12 -11

index 3e3dac3..9b17a59 100644
@@ -147,7 +147,12 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
 {
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
        struct ath_frame_info *fi = get_frame_info(skb);
-       int q = fi->txq;
+       int q;
+
+       if (ath9k_is_chanctx_enabled())
+               q = fi->txq;
+       else
+               q = info->hw_queue;
 
        if (q < 0)
                return;

@@ -158,10 +163,7 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
 
        if (txq->stopped &&
            txq->pending_frames < sc->tx.txq_max_pending[q]) {
-               if (ath9k_is_chanctx_enabled())
-                       ieee80211_wake_queue(sc->hw, info->hw_queue);
-               else
-                       ieee80211_wake_queue(sc->hw, q);
+               ieee80211_wake_queue(sc->hw, q);
                txq->stopped = false;
        }
 }

@@ -2299,17 +2301,17 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
         * info are no longer valid (overwritten by the ath_frame_info data.
         */
 
-       q = skb_get_queue_mapping(skb);
+       if (ath9k_is_chanctx_enabled())
+               q = skb_get_queue_mapping(skb);
+       else
+               q = info->hw_queue;
 
        ath_txq_lock(sc, txq);
        if (txq == sc->tx.txq_map[q]) {
                fi->txq = q;
                if (++txq->pending_frames > sc->tx.txq_max_pending[q] &&
                    !txq->stopped) {
-                       if (ath9k_is_chanctx_enabled())
-                               ieee80211_stop_queue(sc->hw, info->hw_queue);
-                       else
-                               ieee80211_stop_queue(sc->hw, q);
+                       ieee80211_stop_queue(sc->hw, q);
                        txq->stopped = true;
                }
        }
--

2.3.6

Let me explain the patch more thoroughly. We are testing MCC and we've found some issues with the queues handlers, the main problem is that when we transmit a multicast/broadcast frame, where hw_queue is 8 (CAB), we check queue status, which is 2(q), and if it is full and we have to stop it, we end stopping the wrong queue, 8, which is not full. From this point onwards stations are unable to connect to the AP.

When channel context is enabled, we could be
stopping/awakening an incorrect queue.

Signed-off-by Borja Salazar <[email protected]>

---
 drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

drivers/net/wireless/ath/ath9k/xmit.c +6 -1

index 3e3dac3..9b17a59 100644
@@ -147,7 +147,12 @@ static void ath_txq_skb_done(struct ath_softc *sc,
 struct ath_txq *txq,
 {
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
        struct ath_frame_info *fi = get_frame_info(skb);
-       int q = fi->txq;
+       int q;
+
+       if (ath9k_is_chanctx_enabled())
+               q = fi->txq;
+       else
+               q = info->hw_queue;
 
        if (q < 0)

                return;

drivers/net/wireless/ath/ath9k/xmit.c +1 -4

@@ -158,10 +163,7 @@ static void ath_txq_skb_done(struct ath_softc *sc,
 struct ath_txq *txq,
 
        if (txq->stopped &&
            txq->pending_frames < sc->tx.txq_max_pending[q]) {
-               if (ath9k_is_chanctx_enabled())
-                       ieee80211_wake_queue(sc->hw, info->hw_queue);
-               else
-                       ieee80211_wake_queue(sc->hw, q);
+               ieee80211_wake_queue(sc->hw, q);
                txq->stopped = false;
        }

}

drivers/net/wireless/ath/ath9k/xmit.c +0 -0

@@ -2299,17 +2301,17 @@ int ath_tx_start(struct ieee80211_hw *hw, struct
 sk_buff *skb,
         * info are no longer valid (overwritten by the ath_frame_info

data.

         */

drivers/net/wireless/ath/ath9k/xmit.c +4 -3

-       q = skb_get_queue_mapping(skb);
+       if (ath9k_is_chanctx_enabled())
+               q = skb_get_queue_mapping(skb);
+       else
+               q = info->hw_queue;
 
        ath_txq_lock(sc, txq);
        if (txq == sc->tx.txq_map[q]) {
                fi->txq = q;
                if (++txq->pending_frames > sc->tx.txq_max_pending[q] &&
                    !txq->stopped) {
-                       if (ath9k_is_chanctx_enabled())
-                               ieee80211_stop_queue(sc->hw,

info->hw_queue);

drivers/net/wireless/ath/ath9k/xmit.c +1 -3

-                       else
-                               ieee80211_stop_queue(sc->hw, q);
+                       ieee80211_stop_queue(sc->hw, q);
                        txq->stopped = true;
                }
        }
--

2.3.6

Hi,

Let me explain the patch more thoroughly. We are testing MCC and we've found some issues with the queues handlers, the main problem is that when we transmit a multicast/broadcast frame, where hw_queue is 8 (CAB), we check queue status, which is 2(q), and if it is full and we have to stop it, we end stopping the wrong queue, 8, which is not full. From this point onwards stations are unable to connect to the AP.

Let me know if something is not clear.

I am interested if you reintroduce bug that Sujith already fix in commit:

Regards,

[image: Fon] <http://www.fon.com/>Borja SalazarFirmware teamAll information in this email is confidential
<http://corp.fon.com/legal/email-disclaimer>

Quoted 63 lines · Borja Salazar

When channel context is enabled, we could be
stopping/awakening an incorrect queue.

Signed-off-by Borja Salazar <[email protected]>

---
 drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

drivers/net/wireless/ath/ath9k/xmit.c +6 -1

index 3e3dac3..9b17a59 100644
@@ -147,7 +147,12 @@ static void ath_txq_skb_done(struct ath_softc *sc,
 struct ath_txq *txq,
 {
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
        struct ath_frame_info *fi = get_frame_info(skb);
-       int q = fi->txq;
+       int q;
+
+       if (ath9k_is_chanctx_enabled())
+               q = fi->txq;
+       else
+               q = info->hw_queue;
 
        if (q < 0)

                return;

drivers/net/wireless/ath/ath9k/xmit.c +1 -4

@@ -158,10 +163,7 @@ static void ath_txq_skb_done(struct ath_softc *sc,
 struct ath_txq *txq,
 
        if (txq->stopped &&
            txq->pending_frames < sc->tx.txq_max_pending[q]) {
-               if (ath9k_is_chanctx_enabled())
-                       ieee80211_wake_queue(sc->hw, info->hw_queue);
-               else
-                       ieee80211_wake_queue(sc->hw, q);
+               ieee80211_wake_queue(sc->hw, q);
                txq->stopped = false;
        }

}

drivers/net/wireless/ath/ath9k/xmit.c +0 -0

@@ -2299,17 +2301,17 @@ int ath_tx_start(struct ieee80211_hw *hw, struct
 sk_buff *skb,
         * info are no longer valid (overwritten by the ath_frame_info

data.

         */

drivers/net/wireless/ath/ath9k/xmit.c +4 -3

-       q = skb_get_queue_mapping(skb);
+       if (ath9k_is_chanctx_enabled())
+               q = skb_get_queue_mapping(skb);
+       else
+               q = info->hw_queue;
 
        ath_txq_lock(sc, txq);
        if (txq == sc->tx.txq_map[q]) {
                fi->txq = q;
                if (++txq->pending_frames > sc->tx.txq_max_pending[q] &&
                    !txq->stopped) {
-                       if (ath9k_is_chanctx_enabled())
-                               ieee80211_stop_queue(sc->hw,

info->hw_queue);

drivers/net/wireless/ath/ath9k/xmit.c +0 -2

-                       else
-                               ieee80211_stop_queue(sc->hw, q);

Quoted 8 lines

+                       ieee80211_stop_queue(sc->hw, q);
                        txq->stopped = true;
                }
        }

--
2.3.6

Quoted 7 lines · Borja Salazar

Hi,

Let me explain the patch more thoroughly. We are testing MCC and we've found some issues with the queues handlers, the main problem is that when we transmit a multicast/broadcast frame, where hw_queue is 8 (CAB), we check queue status, which is 2(q), and if it is full and we have to stop it, we end stopping the wrong queue, 8, which is not full. From this point onwards stations are unable to connect to the AP.

Let me know if something is not clear.

I am interested if you reintroduce bug that Sujith already fix in commit:
ath9k: Enable HW queue control only for MCC

While as I understand correctly this patch disable hw queues for non-mcc (also clear IEEE80211_HW_QUEUE_CONTROL) and your

drivers/net/wireless/ath/ath9k/xmit.c +4 -0

+       if (ath9k_is_chanctx_enabled())
+               q = fi->txq;
+       else
+               q = info->hw_queue;

Use again hw_queue for standard non-mcc operation.

Please check this, I am sure while did only simple check :)

BR
Janusz

Quoted 74 lines

Regards,

Borja Salazar
Firmware team
All information in this email is confidential

Quoted 65 lines · Borja Salazar

When channel context is enabled, we could be
stopping/awakening an incorrect queue.

Signed-off-by Borja Salazar <[email protected]>

---
 drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

drivers/net/wireless/ath/ath9k/xmit.c +12 -11

index 3e3dac3..9b17a59 100644
@@ -147,7 +147,12 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
 {
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
        struct ath_frame_info *fi = get_frame_info(skb);
-       int q = fi->txq;
+       int q;
+
+       if (ath9k_is_chanctx_enabled())
+               q = fi->txq;
+       else
+               q = info->hw_queue;
 
        if (q < 0)
                return;

@@ -158,10 +163,7 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
 
        if (txq->stopped &&
            txq->pending_frames < sc->tx.txq_max_pending[q]) {
-               if (ath9k_is_chanctx_enabled())
-                       ieee80211_wake_queue(sc->hw, info->hw_queue);
-               else
-                       ieee80211_wake_queue(sc->hw, q);
+               ieee80211_wake_queue(sc->hw, q);
                txq->stopped = false;
        }
 }

@@ -2299,17 +2301,17 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
         * info are no longer valid (overwritten by the ath_frame_info data.
         */
 
-       q = skb_get_queue_mapping(skb);
+       if (ath9k_is_chanctx_enabled())
+               q = skb_get_queue_mapping(skb);
+       else
+               q = info->hw_queue;
 
        ath_txq_lock(sc, txq);
        if (txq == sc->tx.txq_map[q]) {
                fi->txq = q;
                if (++txq->pending_frames > sc->tx.txq_max_pending[q] &&
                    !txq->stopped) {
-                       if (ath9k_is_chanctx_enabled())
-                               ieee80211_stop_queue(sc->hw, info->hw_queue);
-                       else
-                               ieee80211_stop_queue(sc->hw, q);
+                       ieee80211_stop_queue(sc->hw, q);
                        txq->stopped = true;
                }
        }
--

2.3.6

Send again as txt.

Quoted 104 lines · Janusz Dziedzic

Quoted 7 lines · Borja Salazar

Hi,

Let me explain the patch more thoroughly. We are testing MCC and we've found some issues with the queues handlers, the main problem is that when we transmit a multicast/broadcast frame, where hw_queue is 8 (CAB), we check queue status, which is 2(q), and if it is full and we have to stop it, we end stopping the wrong queue, 8, which is not full. From this point onwards stations are unable to connect to the AP.

Let me know if something is not clear.

I am interested if you reintroduce bug that Sujith already fix in commit:
ath9k: Enable HW queue control only for MCC

While as I understand correctly this patch disable hw queues for non-mcc (also clear IEEE80211_HW_QUEUE_CONTROL) and your

drivers/net/wireless/ath/ath9k/xmit.c +4 -0

+       if (ath9k_is_chanctx_enabled())
+               q = fi->txq;
+       else
+               q = info->hw_queue;

Use again hw_queue for standard non-mcc operation.

Please check this, I am sure while did only simple check :)

BR
Janusz

Quoted 74 lines

Regards,

Borja Salazar
Firmware team
All information in this email is confidential

Quoted 65 lines · Borja Salazar

When channel context is enabled, we could be
stopping/awakening an incorrect queue.

Signed-off-by Borja Salazar <[email protected]>

---
 drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

drivers/net/wireless/ath/ath9k/xmit.c +12 -11

index 3e3dac3..9b17a59 100644
@@ -147,7 +147,12 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
 {
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
        struct ath_frame_info *fi = get_frame_info(skb);
-       int q = fi->txq;
+       int q;
+
+       if (ath9k_is_chanctx_enabled())
+               q = fi->txq;
+       else
+               q = info->hw_queue;
 
        if (q < 0)
                return;

@@ -158,10 +163,7 @@ static void ath_txq_skb_done(struct ath_softc *sc, struct ath_txq *txq,
 
        if (txq->stopped &&
            txq->pending_frames < sc->tx.txq_max_pending[q]) {
-               if (ath9k_is_chanctx_enabled())
-                       ieee80211_wake_queue(sc->hw, info->hw_queue);
-               else
-                       ieee80211_wake_queue(sc->hw, q);
+               ieee80211_wake_queue(sc->hw, q);
                txq->stopped = false;
        }
 }

@@ -2299,17 +2301,17 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
         * info are no longer valid (overwritten by the ath_frame_info data.
         */
 
-       q = skb_get_queue_mapping(skb);
+       if (ath9k_is_chanctx_enabled())
+               q = skb_get_queue_mapping(skb);
+       else
+               q = info->hw_queue;
 
        ath_txq_lock(sc, txq);
        if (txq == sc->tx.txq_map[q]) {
                fi->txq = q;
                if (++txq->pending_frames > sc->tx.txq_max_pending[q] &&
                    !txq->stopped) {
-                       if (ath9k_is_chanctx_enabled())
-                               ieee80211_stop_queue(sc->hw, info->hw_queue);
-                       else
-                               ieee80211_stop_queue(sc->hw, q);
+                       ieee80211_stop_queue(sc->hw, q);
                        txq->stopped = true;
                }
        }
--

2.3.6