Subject: [PATCH 2/2] ath9k: Fix stop in tx date traffic after scan

When moving off channel during background scanning, driver
can not clear IEEE80211_QUEUE_STOP_REASON_DRIVER which was
set earliar due to a shortage in tx buffer, this will lead
to a situation where no data frames will be passed down to
driver when the hw is put back into operating channel. This
patch make sure no txq is stopped after moving to operating
channel after scan. This issue is seen only when NM is used
and exposed by the following commit

Author: Felix Fietkau <[email protected]>
Date: Tue Jun 1 21:33:13 2010 +0200
ath9k: fix queue stop/start based on the number of pending frames

Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---

This is not a clean fix for this issue, but best available
from driver. For a decent fix from driver, the following
changes in mac80211 may be required

1. Make ieee80211_wake_queue() callable when moving to off channel
also just to clear IEEE80211_QUEUE_STOP_REASON_DRIVER or

2. For drivers which does not support hw scan, add a way to
tell them if the hw is being configured into operating channel
(driver can detect this, but this would be a hack in driver)
so that the driver can wake up the stopped queues after the
hw is configured back to operating channel.

Johannes, if the above mac80211 changes make any sense, I'll send
a new set of patches to fix this problem.

---

drivers/net/wireless/ath/ath9k/ath9k.h | 1 +
drivers/net/wireless/ath/ath9k/main.c | 6 ++++++
drivers/net/wireless/ath/ath9k/xmit.c | 2 +-
3 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 998ae2c..d8d8804 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -692,4 +692,5 @@ bool ath_mac80211_start_queue(struct ath_softc *sc, u16 skb_queue);
void ath_start_rfkill_poll(struct ath_softc *sc);
extern void ath9k_rfkill_poll_state(struct ieee80211_hw *hw);

+void ath_wake_mac80211_queue(struct ath_softc *sc, struct ath_txq *txq);
#endif /* ATH9K_H */
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 6cf0410..7a0d633 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1491,6 +1491,7 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
struct ieee80211_conf *conf = &hw->conf;
struct ath_hw *ah = sc->sc_ah;
bool disable_radio;
+ int i;

mutex_lock(&sc->mutex);

@@ -1605,6 +1606,11 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
mutex_unlock(&sc->mutex);
return -EINVAL;
}
+
+ for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
+ if (ATH_TXQ_SETUP(sc, i))
+ ath_wake_mac80211_queue(sc, &sc->tx.txq[i]);
+ }
}

skip_chan_change:
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 21aa5bd..5bfec95 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2067,7 +2067,7 @@ static void ath_tx_rc_status(struct ath_buf *bf, struct ath_tx_status *ts,
tx_info->status.rates[tx_rateindex].count = ts->ts_longretry + 1;
}

-static void ath_wake_mac80211_queue(struct ath_softc *sc, struct ath_txq *txq)
+void ath_wake_mac80211_queue(struct ath_softc *sc, struct ath_txq *txq)
{
int qnum;

--
1.7.0.4



2010-07-22 10:14:59

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] ath9k: Fix stop in tx date traffic after scan

On Thu, 2010-07-22 at 15:41 +0530, Vasanthakumar Thiagarajan wrote:
> > Just go and implement flush() and all these issues will go away and you
> > will stop thinking that you need to touch queues from channel switching.
> > They have nothing to do with each other.
>
>
> I thought about it also, but i'll hit the same issue
> when ieee80211_scan_state_leave_oper_channel() flushes
> the hw tx queues where driver is not supposed to wake
> up the queues as drv_flush() is called only after stopping
> all queues.

I don't get it. The driver can start/stop queues at _any_ time it wants
to. Regardless of what mac80211 is doing, all that goes via
IEEE80211_QUEUE_STOP_REASON_DRIVER which is never touched by mac80211
itself.

johannes


Subject: Re: [PATCH 2/2] ath9k: Fix stop in tx date traffic after scan

> Just go and implement flush() and all these issues will go away and you
> will stop thinking that you need to touch queues from channel switching.
> They have nothing to do with each other.


I thought about it also, but i'll hit the same issue
when ieee80211_scan_state_leave_oper_channel() flushes
the hw tx queues where driver is not supposed to wake
up the queues as drv_flush() is called only after stopping
all queues.

Vasanth

2010-07-22 09:56:46

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] ath9k: Fix stop in tx date traffic after scan

On Thu, 2010-07-22 at 02:25 -0700, Vasanthakumar Thiagarajan wrote:
> When moving off channel during background scanning, driver
> can not clear IEEE80211_QUEUE_STOP_REASON_DRIVER which was
> set earliar due to a shortage in tx buffer, this will lead
> to a situation where no data frames will be passed down to
> driver when the hw is put back into operating channel. This
> patch make sure no txq is stopped after moving to operating
> channel after scan. This issue is seen only when NM is used
> and exposed by the following commit
>
> Author: Felix Fietkau <[email protected]>
> Date: Tue Jun 1 21:33:13 2010 +0200
> ath9k: fix queue stop/start based on the number of pending frames
>
> Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
> ---
>
> This is not a clean fix for this issue, but best available
> from driver. For a decent fix from driver, the following
> changes in mac80211 may be required
>
> 1. Make ieee80211_wake_queue() callable when moving to off channel
> also just to clear IEEE80211_QUEUE_STOP_REASON_DRIVER or
>
> 2. For drivers which does not support hw scan, add a way to
> tell them if the hw is being configured into operating channel
> (driver can detect this, but this would be a hack in driver)
> so that the driver can wake up the stopped queues after the
> hw is configured back to operating channel.
>
> Johannes, if the above mac80211 changes make any sense, I'll send
> a new set of patches to fix this problem.

Really, that all doesn't make a whole lot of sense to me.

Just go and implement flush() and all these issues will go away and you
will stop thinking that you need to touch queues from channel switching.
They have nothing to do with each other.

johannes


2010-07-22 10:32:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] ath9k: Fix stop in tx date traffic after scan

On Thu, 2010-07-22 at 15:50 +0530, Vasanthakumar Thiagarajan wrote:
> On Thu, Jul 22, 2010 at 03:44:54PM +0530, Johannes Berg wrote:
> > On Thu, 2010-07-22 at 15:41 +0530, Vasanthakumar Thiagarajan wrote:
> > > > Just go and implement flush() and all these issues will go away and you
> > > > will stop thinking that you need to touch queues from channel switching.
> > > > They have nothing to do with each other.
> > >
> > >
> > > I thought about it also, but i'll hit the same issue
> > > when ieee80211_scan_state_leave_oper_channel() flushes
> > > the hw tx queues where driver is not supposed to wake
> > > up the queues as drv_flush() is called only after stopping
> > > all queues.
> >
> > I don't get it. The driver can start/stop queues at _any_ time it wants
> > to. Regardless of what mac80211 is doing, all that goes via
> > IEEE80211_QUEUE_STOP_REASON_DRIVER which is never touched by mac80211
> > itself.
>
> My understanding is, if driver wakes up the queues when operating on
> a off-channel, it would get data frames from upper layer for
> transmission but it should not send out these frames as the hw is on
> non-operating channel.

Ok, that seems to be true, but only because we don't properly manage the
interface queue stop in mac80211. Should be an easy fix there.

johannes


Subject: Re: [PATCH 2/2] ath9k: Fix stop in tx date traffic after scan

On Thu, Jul 22, 2010 at 03:44:54PM +0530, Johannes Berg wrote:
> On Thu, 2010-07-22 at 15:41 +0530, Vasanthakumar Thiagarajan wrote:
> > > Just go and implement flush() and all these issues will go away and you
> > > will stop thinking that you need to touch queues from channel switching.
> > > They have nothing to do with each other.
> >
> >
> > I thought about it also, but i'll hit the same issue
> > when ieee80211_scan_state_leave_oper_channel() flushes
> > the hw tx queues where driver is not supposed to wake
> > up the queues as drv_flush() is called only after stopping
> > all queues.
>
> I don't get it. The driver can start/stop queues at _any_ time it wants
> to. Regardless of what mac80211 is doing, all that goes via
> IEEE80211_QUEUE_STOP_REASON_DRIVER which is never touched by mac80211
> itself.

My understanding is, if driver wakes up the queues when operating on
a off-channel, it would get data frames from upper layer for
transmission but it should not send out these frames as the hw is on
non-operating channel.

Vasanth