diff mbox series

[v4.19,1/2] mac80211: mark TX-during-stop for TX in in_reconfig

Message ID 20211217203550.54684-1-johannes@sipsolutions.net
State New
Headers show
Series [v4.19,1/2] mac80211: mark TX-during-stop for TX in in_reconfig | expand

Commit Message

Johannes Berg Dec. 17, 2021, 8:35 p.m. UTC 
From: Johannes Berg <johannes.berg@intel.com>

Commit db7205af049d230e7e0abf61c1e74c1aab40f390 upstream.

Mark TXQs as having seen transmit while they were stopped if
we bail out of drv_wake_tx_queue() due to reconfig, so that
the queue wake after this will make them catch up. This is
particularly necessary for when TXQs are used for management
packets since those TXQs won't see a lot of traffic that'd
make them catch up later.

Cc: stable@vger.kernel.org
Fixes: 4856bfd23098 ("mac80211: do not call driver wake_tx_queue op during reconfig")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Link: https://lore.kernel.org/r/iwlwifi.20211129152938.4573a221c0e1.I0d1d5daea3089be3fc0dccc92991b0f8c5677f0c@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
I'm not sure why you say it doesn't apply - it did for me?
---
 net/mac80211/driver-ops.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Johannes Berg Dec. 17, 2021, 8:42 p.m. UTC | #1 
On Fri, 2021-12-17 at 20:35 +0000, Johannes Berg wrote:
> 
> I'm not sure why you say it doesn't apply - it did for me?
> 

Oh, I'm an idiot, it doesn't *compile* afterwards since
IEEE80211_TXQ_STOP_NETIF_TX doesn't exist yet.

Let's ignore this patch for now, but please take 2/2.

I'll have to think if we even want to introduce this on the old kernels,
the bug that was reported required a firmware crash in the first place,
and it was reported on iwlwifi which doesn't even use TXQs on 4.19 yet.

johannes
gregkh@linuxfoundation.org Dec. 17, 2021, 11:36 p.m. UTC | #2 
On Fri, Dec 17, 2021 at 09:42:21PM +0100, Johannes Berg wrote:
> On Fri, 2021-12-17 at 20:35 +0000, Johannes Berg wrote:
> > 
> > I'm not sure why you say it doesn't apply - it did for me?
> > 
> 
> Oh, I'm an idiot, it doesn't *compile* afterwards since
> IEEE80211_TXQ_STOP_NETIF_TX doesn't exist yet.
> 
> Let's ignore this patch for now, but please take 2/2.
> 
> I'll have to think if we even want to introduce this on the old kernels,
> the bug that was reported required a firmware crash in the first place,
> and it was reported on iwlwifi which doesn't even use TXQs on 4.19 yet.

Yeah, I don't think it's needed, but as it was marked to go back really
far, I wanted to give you the option to do so or not.

Thanks for the 2/2 patch, I'll queue it up when I get a chance tomorrow.

greg k-h
Johannes Berg Dec. 18, 2021, 8:56 a.m. UTC | #3 
On Sat, 2021-12-18 at 00:36 +0100, Greg KH wrote:
> 
> Yeah, I don't think it's needed, but as it was marked to go back really
> far, I wanted to give you the option to do so or not.
> 

Yeah I marked it because I'm pretty sure that problem _was_ introduced
that far back with the TXQ code, but OTOH I'm not even sure initially it
had users there, and then users that would run into the restart issue
... probably not, otherwise I'd think we'd have heard about it earlier?

Backporting the IEEE80211_TXQ_STOP_NETIF_TX stuff doesn't seem that
problematic either, but I'm not convinced it's worth it, and I suppose
somebody whose driver actually uses it on those kernels would better
take a look - I'm not familiar with all the drivers in detail, and
iwlwifi doesn't use it yet there.

johannes
diff mbox series

Patch

diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 2123f6e90fc0..8f71c271653f 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1166,8 +1166,11 @@  static inline void drv_wake_tx_queue(struct ieee80211_local *local,
 {
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(txq->txq.vif);
 
-	if (local->in_reconfig)
+	/* In reconfig don't transmit now, but mark for waking later */
+	if (local->in_reconfig) {
+		set_bit(IEEE80211_TXQ_STOP_NETIF_TX, &txq->flags);
 		return;
+	}
 
 	if (!check_sdata_in_driver(sdata))
 		return;