All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-03 17:43 ` Borja Salazar
  0 siblings, 0 replies; 18+ messages in thread
From: Borja Salazar @ 2015-11-03 17:37 UTC (permalink / raw)
  To: linux-wireless; +Cc: ath9k-devel, Borja Salazar

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(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 3e3dac3..9b17a59 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-03 17:43 ` Borja Salazar
  0 siblings, 0 replies; 18+ messages in thread
From: Borja Salazar @ 2015-11-03 17:43 UTC (permalink / raw)
  To: ath9k-devel

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(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 3e3dac3..9b17a59 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] ath9k: incorrect queue may be stopped/awaken
  2015-11-03 17:43 ` [ath9k-devel] " Borja Salazar
@ 2015-11-12 19:57   ` Kalle Valo
  -1 siblings, 0 replies; 18+ messages in thread
From: Kalle Valo @ 2015-11-12 19:57 UTC (permalink / raw)
  To: Borja Salazar; +Cc: linux-wireless, ath9k-devel

Borja Salazar <borja.salazar@fon.com> writes:

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

Signed-off-by line is missing, I can't take this.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-12 19:57   ` Kalle Valo
  0 siblings, 0 replies; 18+ messages in thread
From: Kalle Valo @ 2015-11-12 19:57 UTC (permalink / raw)
  To: ath9k-devel

Borja Salazar <borja.salazar@fon.com> writes:

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

Signed-off-by line is missing, I can't take this.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] ath9k: incorrect queue may be stopped/awaken
  2015-11-03 17:43 ` [ath9k-devel] " Borja Salazar
@ 2015-11-13  6:31   ` Janusz Dziedzic
  -1 siblings, 0 replies; 18+ messages in thread
From: Janusz Dziedzic @ 2015-11-13  6:31 UTC (permalink / raw)
  To: Borja Salazar, Sujith Manoharan; +Cc: linux-wireless, ath9k-devel

On 3 November 2015 at 18:37, Borja Salazar <borja.salazar@fon.com> wrote:
> 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(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 3e3dac3..9b17a59 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -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
>
Hello, could you check it again?
I see such patch that enable hw_queues only for MCC and disable for
non-MCC mode.

ath9k: Enable HW queue control only for MCC

    Enabling HW queue control for normal (non-mcc) mode
    causes problems with queue management, resulting
    in traffic stall. Since it is mainly required for
    fairness in MCC mode, disable it for the general case.

    Bug: https://dev.openwrt.org/ticket/18164

BR
Janusz

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-13  6:31   ` Janusz Dziedzic
  0 siblings, 0 replies; 18+ messages in thread
From: Janusz Dziedzic @ 2015-11-13  6:31 UTC (permalink / raw)
  To: ath9k-devel

On 3 November 2015 at 18:37, Borja Salazar <borja.salazar@fon.com> wrote:
> 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(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 3e3dac3..9b17a59 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -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
>
Hello, could you check it again?
I see such patch that enable hw_queues only for MCC and disable for
non-MCC mode.

ath9k: Enable HW queue control only for MCC

    Enabling HW queue control for normal (non-mcc) mode
    causes problems with queue management, resulting
    in traffic stall. Since it is mainly required for
    fairness in MCC mode, disable it for the general case.

    Bug: https://dev.openwrt.org/ticket/18164

BR
Janusz

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2] ath9k: incorrect queue may be stopped/awaken
  2015-11-13  6:31   ` [ath9k-devel] " Janusz Dziedzic
@ 2015-11-13 10:41     ` Borja Salazar
  -1 siblings, 0 replies; 18+ messages in thread
From: Borja Salazar @ 2015-11-13 10:33 UTC (permalink / raw)
  To: linux-wireless
  Cc: janusz.dziedzic, sujith, ath9k-devel, ath9k-devel, Kalle Valo,
	Borja Salazar

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

Signed-off-by: Borja Salazar <borja.salazar@fon.com>
---
 drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 3e3dac3..9b17a59 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH v2] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-13 10:41     ` Borja Salazar
  0 siblings, 0 replies; 18+ messages in thread
From: Borja Salazar @ 2015-11-13 10:41 UTC (permalink / raw)
  To: ath9k-devel

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

Signed-off-by: Borja Salazar <borja.salazar@fon.com>
---
 drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 3e3dac3..9b17a59 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] ath9k: incorrect queue may be stopped/awaken
  2015-11-13 10:41     ` [ath9k-devel] " Borja Salazar
@ 2015-11-13 10:54       ` Borja Salazar
  -1 siblings, 0 replies; 18+ messages in thread
From: Borja Salazar @ 2015-11-13 10:49 UTC (permalink / raw)
  To: linux-wireless
  Cc: janusz.dziedzic, sujith, ath9k-devel, ath9k-devel, Kalle Valo,
	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.

Regards,


On Fri, Nov 13, 2015 at 11:33 AM, Borja Salazar <borja.salazar@fon.com> wrote:
> When channel context is enabled, we could be
> stopping/awakening an incorrect queue.
>
> Signed-off-by: Borja Salazar <borja.salazar@fon.com>
> ---
>  drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 3e3dac3..9b17a59 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -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
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH v2] ath9k: incorrect queue may be stopped/awaken
  2015-11-13 10:41     ` [ath9k-devel] " Borja Salazar
  (?)
  (?)
@ 2015-11-13 10:54     ` Borja Salazar
  2015-11-13 11:17       ` Janusz Dziedzic
  -1 siblings, 1 reply; 18+ messages in thread
From: Borja Salazar @ 2015-11-13 10:54 UTC (permalink / raw)
  To: ath9k-devel

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.

Regards,

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

On Fri, Nov 13, 2015 at 11:33 AM, Borja Salazar <borja.salazar@fon.com>
wrote:

> When channel context is enabled, we could be
> stopping/awakening an incorrect queue.
>
> Signed-off-by: Borja Salazar <borja.salazar@fon.com>
> ---
>  drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c
> b/drivers/net/wireless/ath/ath9k/xmit.c
> index 3e3dac3..9b17a59 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20151113/451dc977/attachment-0001.htm 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH v2] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-13 10:54       ` Borja Salazar
  0 siblings, 0 replies; 18+ messages in thread
From: Borja Salazar @ 2015-11-13 10:54 UTC (permalink / raw)
  To: ath9k-devel

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.

Regards,


On Fri, Nov 13, 2015 at 11:33 AM, Borja Salazar <borja.salazar@fon.com> wrote:
> When channel context is enabled, we could be
> stopping/awakening an incorrect queue.
>
> Signed-off-by: Borja Salazar <borja.salazar@fon.com>
> ---
>  drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index 3e3dac3..9b17a59 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -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
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH v2] ath9k: incorrect queue may be stopped/awaken
  2015-11-13 10:54     ` Borja Salazar
@ 2015-11-13 11:17       ` Janusz Dziedzic
  2015-11-13 11:19           ` [ath9k-devel] " Janusz Dziedzic
  0 siblings, 1 reply; 18+ messages in thread
From: Janusz Dziedzic @ 2015-11-13 11:17 UTC (permalink / raw)
  To: ath9k-devel

On 13 November 2015 at 11:45, Borja Salazar <borja.salazar@fon.com> wrote:

> 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

+       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



> Regards,
>
> [image: Fon] <http://www.fon.com/>Borja SalazarFirmware teamAll
> information in this email is confidential
> <http://corp.fon.com/legal/email-disclaimer>
>
> On Fri, Nov 13, 2015 at 11:33 AM, Borja Salazar <borja.salazar@fon.com>
> wrote:
>
>> When channel context is enabled, we could be
>> stopping/awakening an incorrect queue.
>>
>> Signed-off-by: Borja Salazar <borja.salazar@fon.com>
>> ---
>>  drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c
>> b/drivers/net/wireless/ath/ath9k/xmit.c
>> index 3e3dac3..9b17a59 100644
>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>> @@ -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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20151113/0a70bcc2/attachment.htm 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] ath9k: incorrect queue may be stopped/awaken
  2015-11-13 11:17       ` Janusz Dziedzic
@ 2015-11-13 11:19           ` Janusz Dziedzic
  0 siblings, 0 replies; 18+ messages in thread
From: Janusz Dziedzic @ 2015-11-13 11:19 UTC (permalink / raw)
  To: Borja Salazar
  Cc: linux-wireless, Sujith Manoharan, ath9k-devel, ath9k-devel, Kalle Valo

Send again as txt.


On 13 November 2015 at 12:17, Janusz Dziedzic <janusz.dziedzic@tieto.com> wrote:
>
>
>
> On 13 November 2015 at 11:45, Borja Salazar <borja.salazar@fon.com> wrote:
>>
>> 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
>
> +       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
>
>
>>
>> Regards,
>>
>> Borja Salazar
>> Firmware team
>> All information in this email is confidential
>>
>> On Fri, Nov 13, 2015 at 11:33 AM, Borja Salazar <borja.salazar@fon.com> wrote:
>>>
>>> When channel context is enabled, we could be
>>> stopping/awakening an incorrect queue.
>>>
>>> Signed-off-by: Borja Salazar <borja.salazar@fon.com>
>>> ---
>>>  drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
>>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>>> index 3e3dac3..9b17a59 100644
>>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>> @@ -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
>>>
>>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH v2] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-13 11:19           ` Janusz Dziedzic
  0 siblings, 0 replies; 18+ messages in thread
From: Janusz Dziedzic @ 2015-11-13 11:19 UTC (permalink / raw)
  To: ath9k-devel

Send again as txt.


On 13 November 2015 at 12:17, Janusz Dziedzic <janusz.dziedzic@tieto.com> wrote:
>
>
>
> On 13 November 2015 at 11:45, Borja Salazar <borja.salazar@fon.com> wrote:
>>
>> 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
>
> +       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
>
>
>>
>> Regards,
>>
>> Borja Salazar
>> Firmware team
>> All information in this email is confidential
>>
>> On Fri, Nov 13, 2015 at 11:33 AM, Borja Salazar <borja.salazar@fon.com> wrote:
>>>
>>> When channel context is enabled, we could be
>>> stopping/awakening an incorrect queue.
>>>
>>> Signed-off-by: Borja Salazar <borja.salazar@fon.com>
>>> ---
>>>  drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
>>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>>> index 3e3dac3..9b17a59 100644
>>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>> @@ -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
>>>
>>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] ath9k: incorrect queue may be stopped/awaken
  2015-11-13 11:19           ` [ath9k-devel] " Janusz Dziedzic
@ 2015-11-13 11:34             ` Borja Salazar
  -1 siblings, 0 replies; 18+ messages in thread
From: Borja Salazar @ 2015-11-13 11:27 UTC (permalink / raw)
  To: Janusz Dziedzic
  Cc: linux-wireless, Sujith Manoharan, ath9k-devel, ath9k-devel, Kalle Valo

arggg, you are right, we have it the right way as a patch for openwrt,
I made a mistake generating kernel patch, will send V3
Borja Salazar
Firmware team
All information in this email is confidential


On Fri, Nov 13, 2015 at 12:19 PM, Janusz Dziedzic
<janusz.dziedzic@tieto.com> wrote:
> Send again as txt.
>
>
> On 13 November 2015 at 12:17, Janusz Dziedzic <janusz.dziedzic@tieto.com> wrote:
>>
>>
>>
>> On 13 November 2015 at 11:45, Borja Salazar <borja.salazar@fon.com> wrote:
>>>
>>> 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
>>
>> +       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
>>
>>
>>>
>>> Regards,
>>>
>>> Borja Salazar
>>> Firmware team
>>> All information in this email is confidential
>>>
>>> On Fri, Nov 13, 2015 at 11:33 AM, Borja Salazar <borja.salazar@fon.com> wrote:
>>>>
>>>> When channel context is enabled, we could be
>>>> stopping/awakening an incorrect queue.
>>>>
>>>> Signed-off-by: Borja Salazar <borja.salazar@fon.com>
>>>> ---
>>>>  drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
>>>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>>>> index 3e3dac3..9b17a59 100644
>>>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>>>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>>> @@ -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
>>>>
>>>
>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH v2] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-13 11:34             ` Borja Salazar
  0 siblings, 0 replies; 18+ messages in thread
From: Borja Salazar @ 2015-11-13 11:34 UTC (permalink / raw)
  To: ath9k-devel

arggg, you are right, we have it the right way as a patch for openwrt,
I made a mistake generating kernel patch, will send V3
Borja Salazar
Firmware team
All information in this email is confidential


On Fri, Nov 13, 2015 at 12:19 PM, Janusz Dziedzic
<janusz.dziedzic@tieto.com> wrote:
> Send again as txt.
>
>
> On 13 November 2015 at 12:17, Janusz Dziedzic <janusz.dziedzic@tieto.com> wrote:
>>
>>
>>
>> On 13 November 2015 at 11:45, Borja Salazar <borja.salazar@fon.com> wrote:
>>>
>>> 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
>>
>> +       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
>>
>>
>>>
>>> Regards,
>>>
>>> Borja Salazar
>>> Firmware team
>>> All information in this email is confidential
>>>
>>> On Fri, Nov 13, 2015 at 11:33 AM, Borja Salazar <borja.salazar@fon.com> wrote:
>>>>
>>>> When channel context is enabled, we could be
>>>> stopping/awakening an incorrect queue.
>>>>
>>>> Signed-off-by: Borja Salazar <borja.salazar@fon.com>
>>>> ---
>>>>  drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++++++++----------
>>>>  1 file changed, 12 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
>>>> index 3e3dac3..9b17a59 100644
>>>> --- a/drivers/net/wireless/ath/ath9k/xmit.c
>>>> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
>>>> @@ -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
>>>>
>>>
>>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2] ath9k: incorrect queue may be stopped/awaken
  2015-11-13 10:54       ` [ath9k-devel] " Borja Salazar
@ 2015-11-13 11:37         ` Kalle Valo
  -1 siblings, 0 replies; 18+ messages in thread
From: Kalle Valo @ 2015-11-13 11:37 UTC (permalink / raw)
  To: Borja Salazar
  Cc: linux-wireless, janusz.dziedzic, sujith, ath9k-devel, ath9k-devel

Borja Salazar <borja.salazar@fon.com> writes:

> 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.

This information should be in the commit log.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ath9k-devel] [PATCH v2] ath9k: incorrect queue may be stopped/awaken
@ 2015-11-13 11:37         ` Kalle Valo
  0 siblings, 0 replies; 18+ messages in thread
From: Kalle Valo @ 2015-11-13 11:37 UTC (permalink / raw)
  To: ath9k-devel

Borja Salazar <borja.salazar@fon.com> writes:

> 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.

This information should be in the commit log.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2015-11-13 11:37 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-03 17:37 [PATCH] ath9k: incorrect queue may be stopped/awaken Borja Salazar
2015-11-03 17:43 ` [ath9k-devel] " Borja Salazar
2015-11-12 19:57 ` Kalle Valo
2015-11-12 19:57   ` [ath9k-devel] " Kalle Valo
2015-11-13  6:31 ` Janusz Dziedzic
2015-11-13  6:31   ` [ath9k-devel] " Janusz Dziedzic
2015-11-13 10:33   ` [PATCH v2] " Borja Salazar
2015-11-13 10:41     ` [ath9k-devel] " Borja Salazar
2015-11-13 10:49     ` Borja Salazar
2015-11-13 10:54       ` [ath9k-devel] " Borja Salazar
2015-11-13 11:37       ` Kalle Valo
2015-11-13 11:37         ` [ath9k-devel] " Kalle Valo
2015-11-13 10:54     ` Borja Salazar
2015-11-13 11:17       ` Janusz Dziedzic
2015-11-13 11:19         ` Janusz Dziedzic
2015-11-13 11:19           ` [ath9k-devel] " Janusz Dziedzic
2015-11-13 11:27           ` Borja Salazar
2015-11-13 11:34             ` [ath9k-devel] " Borja Salazar

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.