All of lore.kernel.org
 help / color / mirror / Atom feed
From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
To: linux-wireless@vger.kernel.org
Cc: Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
	Manikanta Pubbisetty <mpubbise@codeaurora.org>
Subject: [RFC] mac80211: fix deauth TX when we disconnect
Date: Tue,  6 Nov 2018 14:30:40 +0200	[thread overview]
Message-ID: <1541507440-21008-1-git-send-email-emmanuel.grumbach@intel.com> (raw)

The iTXQs stop/wake queue mechanism involves a whole bunch
of locks and this is probably why the call to
ieee80211_wake_txqs is deferred to a tasklet when called from
__ieee80211_wake_queue.

Another advantage of that is that ieee80211_wake_txqs might
call the wake_tx_queue() callback and then the driver may
call mac80211 which will call it back in the same context.

The bug I saw is that when we send a deauth frame as a
station we do:

flush(drop=1)
tx deauth
flush(drop=0)

While we flush we stop the queues and wake them up
immediately after we finished flushing. The problem here is
that the tasklet that de-facto enables the queue may not have
run until we send the deauth. Then the deauth frame is sent
to the driver (which is surprising by itself), but the driver
won't get anything useful from ieee80211_tx_dequeue because
the queue is stopped (or more precisely because
vif->txqs_stopped[0] is true).
Then the deauth is not sent. Later on, the tasklet will run,
but that'll be too late. We'll already have removed all the
vif etc...

There are 2 options I see here:
1) do what I did in this patch (which is pretty awful because
   of the if (lock) thing. If we wake up the queue because of
   a reason which is internal to mac80211, call
   ieee80211_wake_txqs synchronously since we know we are not
   coming from the driver.
2) we could set vif->txqs_stopped to false synchrously and do
   the rest of the job in a tasklet. This would allow the code
   that sends the Tx to put the frame in the queue and the
   driver would actually get it.

Maybe others have better ideas?

Change-Id: I5d3bd20ec765becb91944741c35e35f6e3888981
Cc: Manikanta Pubbisetty <mpubbise@codeaurora.org>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/mac80211/util.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index c9cb00a..c731ddb 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -299,16 +299,16 @@ out:
 	spin_unlock_bh(&fq->lock);
 }
 
-void ieee80211_wake_txqs(unsigned long data)
+static void _ieee80211_wake_txqs(struct ieee80211_local *local, bool lock)
 {
-	struct ieee80211_local *local = (struct ieee80211_local *)data;
 	struct ieee80211_sub_if_data *sdata;
 	int n_acs = IEEE80211_NUM_ACS;
 	unsigned long flags;
 	int i;
 
 	rcu_read_lock();
-	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
+	if (lock)
+		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 
 	if (local->hw.queues < IEEE80211_NUM_ACS)
 		n_acs = 1;
@@ -332,10 +332,18 @@ void ieee80211_wake_txqs(unsigned long data)
 		spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
 	}
 
-	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+	if (lock)
+		spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
 	rcu_read_unlock();
 }
 
+void ieee80211_wake_txqs(unsigned long data)
+{
+	struct ieee80211_local *local = (struct ieee80211_local *)data;
+
+	_ieee80211_wake_txqs(local, true);
+}
+
 void ieee80211_propagate_queue_wake(struct ieee80211_local *local, int queue)
 {
 	struct ieee80211_sub_if_data *sdata;
@@ -405,8 +413,12 @@ static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,
 	} else
 		tasklet_schedule(&local->tx_pending_tasklet);
 
-	if (local->ops->wake_tx_queue)
-		tasklet_schedule(&local->wake_txqs_tasklet);
+	if (local->ops->wake_tx_queue) {
+		if (reason == IEEE80211_QUEUE_STOP_REASON_DRIVER)
+			tasklet_schedule(&local->wake_txqs_tasklet);
+		else
+			_ieee80211_wake_txqs(local, false);
+	}
 }
 
 void ieee80211_wake_queue_by_reason(struct ieee80211_hw *hw, int queue,
-- 
2.7.4


             reply	other threads:[~2018-11-06 12:30 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-06 12:30 Emmanuel Grumbach [this message]
2018-11-06 13:19 ` [RFC] mac80211: fix deauth TX when we disconnect Emmanuel Grumbach

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1541507440-21008-1-git-send-email-emmanuel.grumbach@intel.com \
    --to=emmanuel.grumbach@intel.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mpubbise@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.