mac80211: Dont allow to wake up netif tx queues while on off channel
PATCH 1/2 mac80211: Dont allow to wake up netif tx queues while on off channel
Drivers are not supposed to call ieee80211_wake_queue() while operating on off channel during sw scanning, but there is no clear way for the driver to know that it is operating on off channel during scanning. There are cases (unavailablity/availability of tx buffers in ath9k, for example) where driver needs to stop/restart tx queues during background scanning state, this might result in waking up the corresponding netif tx queue when the device is on off channel which is not desired. This patches fixes this by checking SCAN_OFF_CHANNEL bit in scanning before restarting the tx queue.
--- net/mac80211/util.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
net/mac80211/util.c +2 -1
@@ -277,7 +277,8 @@ static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,__clear_bit (reason, &local->queue_stop_reasons[queue]); -if (local->queue_stop_reasons[queue] !=0 ) +if ((local->queue_stop_reasons[queue] !=0 ) || +test_bit (SCAN_OFF_CHANNEL , &local->scanning))/* someone still has this queue stopped */ return ;
Signature · 1 line
1.7.0.4
Johannes Berg
maintainerQuoted 20 lines · Vasanthakumar Thiagarajan
Drivers are not supposed to call
ieee80211_wake_queue()while operating on off channel during sw scanning, but there is no clear way for the driver to know that it is operating on off channel during scanning. There are cases (unavailablity/availability of tx buffers in ath9k, for example) where driver needs to stop/restart tx queues during background scanning state, this might result in waking up the corresponding netif tx queue when the device is on off channel which is not desired. This patches fixes this by checkingSCAN_OFF_CHANNELbit in scanning before restarting the tx queue.Signed-off-by Vasanthakumar Thiagarajan <[email protected]>--- net/mac80211/util.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)net/mac80211/util.c +0
-0
net/mac80211/util.c static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue, · line 283
Quoted 8 lines
net/mac80211/util.c +2
-1@@ -277,7 +277,8 @@ static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,__clear_bit (reason, &local->queue_stop_reasons[queue]); -if (local->queue_stop_reasons[queue] !=0 ) +if ((local->queue_stop_reasons[queue] !=0 ) || +test_bit (SCAN_OFF_CHANNEL , &local->scanning))/* someone still has this queue stopped */ return ;
That doesn't seem to make sense, since we treat driver and scan stop status separately via wake_queue_by_reason()
johannes
Johannes Berg
maintainerQuoted 6 lines · Johannes Berg
Quoted 2 lines · Vasanthakumar Thiagarajan
Drivers are not supposed to call
ieee80211_wake_queue()while operating on off channel during sw scanning,That doesn't seem to make sense, since we treat driver and scan stop status separately via
wake_queue_by_reason()
IOW, the above assertion would seem to be false already.
johannes
Quoted 9 lines · Johannes Berg
Quoted 6 lines · Johannes Berg
Quoted 2 lines · Vasanthakumar Thiagarajan
Drivers are not supposed to call
ieee80211_wake_queue()while operating on off channel during sw scanning,That doesn't seem to make sense, since we treat driver and scan stop status separately via
wake_queue_by_reason()IOW, the above assertion would seem to be false already.
Sorry, I mean the existing code assumes that ieee80211_wake_queue() while operating on off channel during scanning.
Vasanth
Quoted 6 lines · Vasanth Thiagarajan
Quoted 5 lines · Johannes Berg
Quoted 3 lines · Johannes Berg
Quoted 2 lines · Vasanthakumar Thiagarajan
Drivers are not supposed to call
ieee80211_wake_queue()while operating on off channel during sw scanning,
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
That doesn't seem to make sense, since we treat driver and scan stop status separately via
wake_queue_by_reason()IOW, the above assertion would seem to be false already.
Sorry, I mean the existing code assumes that
ieee80211_wake_queue()while operating on off channel during scanning.
I mean the existing code assumes that ieee80211_wake_queue() is not supposed to be called while operating on off channel during scanning as it is not right to pass packets to the drivers when hw is on different channel.
Vasanth
Quoted 24 lines · Johannes Berg
Quoted 23 lines · Vasanthakumar Thiagarajan
Drivers are not supposed to call
ieee80211_wake_queue()while operating on off channel during sw scanning, but there is no clear way for the driver to know that it is operating on off channel during scanning. There are cases (unavailablity/availability of tx buffers in ath9k, for example) where driver needs to stop/restart tx queues during background scanning state, this might result in waking up the corresponding netif tx queue when the device is on off channel which is not desired. This patches fixes this by checkingSCAN_OFF_CHANNELbit in scanning before restarting the tx queue.Signed-off-by Vasanthakumar Thiagarajan <[email protected]>--- net/mac80211/util.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)net/mac80211/util.c +0
-0@@ -277,7 +277,8 @@ static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,__clear_bit (reason, &local->queue_stop_reasons[queue]);
Quoted 8 lines
net/mac80211/util.c static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue, · line 283
Quoted 5 lines
net/mac80211/util.c +2
-1-if (local->queue_stop_reasons[queue] !=0 ) +if ((local->queue_stop_reasons[queue] !=0 ) || +test_bit (SCAN_OFF_CHANNEL , &local->scanning))/* someone still has this queue stopped */ return ;That doesn't seem to make sense, since we treat driver and scan stop status separately via
wake_queue_by_reason()
I dont know if I explained the issue properly. The issue here is waking up the queues by driver during scan, particularly when operating on off channel. With ath9k, there is a possibility that ieee80211_wake_queue() can be called while moving from operational channel (during channel set in driver). In this case driver still needs to be allowed to clear the bit in queue_stop_reasons[] but not wake up the tx queue.
Vasanth
Quoted 34 lines · Vasanth Thiagarajan
Quoted 24 lines · Johannes Berg
Quoted 23 lines · Vasanthakumar Thiagarajan
Drivers are not supposed to call
ieee80211_wake_queue()while operating on off channel during sw scanning, but there is no clear way for the driver to know that it is operating on off channel during scanning. There are cases (unavailablity/availability of tx buffers in ath9k, for example) where driver needs to stop/restart tx queues during background scanning state, this might result in waking up the corresponding netif tx queue when the device is on off channel which is not desired. This patches fixes this by checkingSCAN_OFF_CHANNELbit in scanning before restarting the tx queue.Signed-off-by Vasanthakumar Thiagarajan <[email protected]>--- net/mac80211/util.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)net/mac80211/util.c +0
-0@@ -277,7 +277,8 @@ static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,__clear_bit (reason, &local->queue_stop_reasons[queue]);Quoted 8 lines
net/mac80211/util.c static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue, · line 283
Quoted 5 lines
net/mac80211/util.c +2
-1-if (local->queue_stop_reasons[queue] !=0 ) +if ((local->queue_stop_reasons[queue] !=0 ) || +test_bit (SCAN_OFF_CHANNEL , &local->scanning))/* someone still has this queue stopped */ return ;That doesn't seem to make sense, since we treat driver and scan stop status separately via
wake_queue_by_reason()
Quoted 7 lines
I dont know if I explained the issue properly. The issue here is waking up the queues by driver during scan, particularly when operating on off channel. With ath9k, there is a possibility that
ieee80211_wake_queue()can be called while moving from operational channel (during channel set in driver). In this case driver still needs to be allowed to clear the bit in queue_stop_reasons[] but not wake up the tx queue.
The actual issue here is
- ath9k does a ieee80211_stop_queue() upon detecting the shortage in tx buffer.
- by the time it wakes up the queue, network manager issues a background scanning. tx buffers become available again by draining the driver's tx queues while configuring the hw with a non-operational channel. As driver can not really clear its queue stop request by calling ieee80211_wake_queue() during off channel, the stopped queue would remain stopped by driver for ever.
My patch makes ieee80211_wake_queue() callable any time by driver. This would just clear the driver's stop req bit in queue_stop_reasons[] when on non-operational channel so that the frames coming from that queue would be passed to driver when the interface is put back on operational channel. The commit description in that patch is misleading. If you are ok with this fix, I will resend the patch with proper commit message. I could not even come up with a decent workaround in ath9k as there is no clean way for driver to know if it is moving in/out of operational channel.
This issue is easily seen with ath9k from latest wireless-testing and NM. With a simple iperf traffic, tx from mac80211 to driver would stop with in 20 secs.
Vasanth