[PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29

[PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29

[Luca Coelho]

From: Luca Coelho <luciano.coelho@intel.com>

Hi,

A bunch of patches with mac80211 and cfg80211 changes from our
internal tree.

Please review, though you have already reviewed (or even written!)
most if not all of them. ;)

Thanks!

Cheers,
Luca.


Ayala Beker (1):
  cfg80211: Use the HE operation IE to determine a 6GHz BSS channel

Ilan Peer (6):
  cfg80211: Add support for notifying association comeback
  mac80211: Notify cfg80211 about association comeback
  cfg80211: Fix order of enum nl80211_band_iftype_attr documentation
  mac80211: Remove a couple of obsolete TODO
  mac80211: Fix the size used for building probe request
  cfg80211: Acquire wiphy mutex on regulatory work

Johannes Berg (7):
  mac80211: add more HT/VHT/HE state logging
  [BUGFIX] cfg80211: check fixed size before ieee80211_he_oper_size()
  mac80211: mark TX-during-stop for TX in in_reconfig
  mac80211: do drv_reconfig_complete() before restarting all
  cfg80211: simplify cfg80211_chandef_valid()
  mac80211: fix lookup when adding AddBA extension element
  mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

Mordechay Goodstein (1):
  mac80211: update channel context before station state

Nathan Errera (1):
  mac80211: introduce channel switch disconnect function

 include/net/cfg80211.h       | 12 +++++++
 include/net/mac80211.h       | 12 +++++++
 include/uapi/linux/nl80211.h | 10 ++++--
 net/mac80211/agg-rx.c        |  5 +--
 net/mac80211/agg-tx.c        | 10 ++++--
 net/mac80211/cfg.c           | 14 +++++++-
 net/mac80211/driver-ops.h    |  5 ++-
 net/mac80211/main.c          | 13 +++-----
 net/mac80211/mlme.c          | 54 +++++++++++++++++++++----------
 net/mac80211/sta_info.c      | 15 +++++----
 net/mac80211/util.c          | 16 +++++-----
 net/wireless/chan.c          | 62 +++++++++++++++++++-----------------
 net/wireless/nl80211.c       | 38 ++++++++++++++++++++++
 net/wireless/reg.c           |  8 +++--
 net/wireless/scan.c          | 48 +++++++++++++++++++++++++---
 net/wireless/trace.h         | 17 ++++++++++
 16 files changed, 256 insertions(+), 83 deletions(-)

-- 
2.33.1

[PATCH 01/16] mac80211: add more HT/VHT/HE state logging

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

Add more logging in places that affect HT/VHT/HE state, so
things get easier to debug.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/mlme.c | 46 +++++++++++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 54ab0e1ef6ca..ee5473b0a791 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -164,12 +164,15 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	chandef->freq1_offset = channel->freq_offset;
 
 	if (channel->band == NL80211_BAND_6GHZ) {
-		if (!ieee80211_chandef_he_6ghz_oper(sdata, he_oper, chandef))
+		if (!ieee80211_chandef_he_6ghz_oper(sdata, he_oper, chandef)) {
+			mlme_dbg(sdata,
+				 "bad 6 GHz operation, disabling HT/VHT/HE\n");
 			ret = IEEE80211_STA_DISABLE_HT |
 			      IEEE80211_STA_DISABLE_VHT |
 			      IEEE80211_STA_DISABLE_HE;
-		else
+		} else {
 			ret = 0;
+		}
 		vht_chandef = *chandef;
 		goto out;
 	} else if (sband->band == NL80211_BAND_S1GHZ) {
@@ -190,6 +193,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	ieee80211_apply_htcap_overrides(sdata, &sta_ht_cap);
 
 	if (!ht_oper || !sta_ht_cap.ht_supported) {
+		mlme_dbg(sdata, "HT operation missing / HT not supported\n");
 		ret = IEEE80211_STA_DISABLE_HT |
 		      IEEE80211_STA_DISABLE_VHT |
 		      IEEE80211_STA_DISABLE_HE;
@@ -223,6 +227,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	if (sta_ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) {
 		ieee80211_chandef_ht_oper(ht_oper, chandef);
 	} else {
+		mlme_dbg(sdata, "40 MHz not supported\n");
 		/* 40 MHz (and 80 MHz) must be supported for VHT */
 		ret = IEEE80211_STA_DISABLE_VHT;
 		/* also mark 40 MHz disabled */
@@ -231,6 +236,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (!vht_oper || !sband->vht_cap.vht_supported) {
+		mlme_dbg(sdata, "VHT operation missing / VHT not supported\n");
 		ret = IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
@@ -253,7 +259,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 						&vht_chandef)) {
 			if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE))
 				sdata_info(sdata,
-					   "HE AP VHT information is invalid, disable HE\n");
+					   "HE AP VHT information is invalid, disabling HE\n");
 			ret = IEEE80211_STA_DISABLE_HE;
 			goto out;
 		}
@@ -263,7 +269,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 					       &vht_chandef)) {
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
-				   "AP VHT information is invalid, disable VHT\n");
+				   "AP VHT information is invalid, disabling VHT\n");
 		ret = IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
@@ -271,7 +277,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	if (!cfg80211_chandef_valid(&vht_chandef)) {
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
-				   "AP VHT information is invalid, disable VHT\n");
+				   "AP VHT information is invalid, disabling VHT\n");
 		ret = IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
@@ -284,7 +290,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	if (!cfg80211_chandef_compatible(chandef, &vht_chandef)) {
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
-				   "AP VHT information doesn't match HT, disable VHT\n");
+				   "AP VHT information doesn't match HT, disabling VHT\n");
 		ret = IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
@@ -5036,19 +5042,23 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 
 	/* disable HT/VHT/HE if we don't support them */
 	if (!sband->ht_cap.ht_supported && !is_6ghz) {
+		mlme_dbg(sdata, "HT not supported, disabling HT/VHT/HE\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
 	}
 
 	if (!sband->vht_cap.vht_supported && is_5ghz) {
+		mlme_dbg(sdata, "VHT not supported, disabling VHT/HE\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
 	}
 
 	if (!ieee80211_get_he_iftype_cap(sband,
-					 ieee80211_vif_type_p2p(&sdata->vif)))
+					 ieee80211_vif_type_p2p(&sdata->vif))) {
+		mlme_dbg(sdata, "HE not supported, disabling it\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
+	}
 
 	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HT) && !is_6ghz) {
 		ht_oper = elems->ht_operation;
@@ -5072,6 +5082,9 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 		}
 
 		if (!elems->vht_cap_elem) {
+			if (vht_cap)
+				sdata_info(sdata,
+					   "bad VHT capabilities, disabling VHT\n");
 			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
 			vht_oper = NULL;
 		}
@@ -5119,8 +5132,10 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 		break;
 	}
 
-	if (!have_80mhz)
+	if (!have_80mhz) {
+		sdata_info(sdata, "80 MHz not supported, disabling VHT\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
+	}
 
 	if (sband->band == NL80211_BAND_S1GHZ) {
 		s1g_oper = elems->s1g_oper;
@@ -5684,12 +5699,14 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
 	else if (!is_6ghz)
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
 	vht_elem = ieee80211_bss_get_elem(req->bss, WLAN_EID_VHT_CAPABILITY);
-	if (vht_elem && vht_elem->datalen >= sizeof(struct ieee80211_vht_cap))
+	if (vht_elem && vht_elem->datalen >= sizeof(struct ieee80211_vht_cap)) {
 		memcpy(&assoc_data->ap_vht_cap, vht_elem->data,
 		       sizeof(struct ieee80211_vht_cap));
-	else if (is_5ghz)
+	} else if (is_5ghz) {
+		sdata_info(sdata, "VHT capa missing/short, disabling VHT/HE\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT |
 				IEEE80211_STA_DISABLE_HE;
+	}
 	rcu_read_unlock();
 
 	if (WARN((sdata->vif.driver_flags & IEEE80211_VIF_SUPPORTS_UAPSD) &&
@@ -5763,16 +5780,21 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (req->flags & ASSOC_REQ_DISABLE_HT) {
+		mlme_dbg(sdata, "HT disabled by flag, disabling HT/VHT/HE\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
 	}
 
-	if (req->flags & ASSOC_REQ_DISABLE_VHT)
+	if (req->flags & ASSOC_REQ_DISABLE_VHT) {
+		mlme_dbg(sdata, "VHT disabled by flag, disabling VHT\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
+	}
 
-	if (req->flags & ASSOC_REQ_DISABLE_HE)
+	if (req->flags & ASSOC_REQ_DISABLE_HE) {
+		mlme_dbg(sdata, "HE disabled by flag, disabling VHT\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
+	}
 
 	err = ieee80211_prep_connection(sdata, req->bss, true, override);
 	if (err)
-- 
2.33.1

[PATCH 02/16] cfg80211: Add support for notifying association comeback

[Luca Coelho]

From: Ilan Peer <ilan.peer@intel.com>

Thought the underline driver MLME can handle association temporal
rejection with comeback, it is still useful to notify this to
user space, as user space might want to handle the temporal
rejection differently. For example, in case the comeback time
is too long, user space can deauthenticate immediately and try
to associate with a different AP.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/cfg80211.h       | 12 ++++++++++++
 include/uapi/linux/nl80211.h |  7 +++++++
 net/wireless/nl80211.c       | 38 ++++++++++++++++++++++++++++++++++++
 net/wireless/trace.h         | 17 ++++++++++++++++
 4 files changed, 74 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 362da9f6bf39..b9b269504591 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -8269,6 +8269,18 @@ bool cfg80211_iftype_allowed(struct wiphy *wiphy, enum nl80211_iftype iftype,
 			     bool is_4addr, u8 check_swif);
 
 
+/**
+ * cfg80211_assoc_comeback - notification of association that was
+ * temporarly rejected with a comeback
+ * @netdev: network device
+ * @bss: the bss entry with which association is in progress.
+ * @timeout: timeout interval value TUs.
+ *
+ * this function may sleep. the caller must hold the corresponding wdev's mutex.
+ */
+void cfg80211_assoc_comeback(struct net_device *netdev,
+			     struct cfg80211_bss *bss, u32 timeout);
+
 /* Logging, debugging and troubleshooting/diagnostic helpers. */
 
 /* wiphy_printk helpers, similar to dev_printk */
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 3e734826792f..5ab616dc363e 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1232,6 +1232,11 @@
  *	&NL80211_ATTR_FILS_NONCES - for FILS Nonces
  *		(STA Nonce 16 bytes followed by AP Nonce 16 bytes)
  *
+ * @NL80211_CMD_ASSOC_COMEBACK: notification about an association
+ *      temporal rejection with comeback. The event includes %NL80211_ATTR_MAC
+ *      to describe the BSSID address of the AP and %NL80211_ATTR_TIMEOUT to
+ *      specify the timeout value.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1474,6 +1479,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_SET_FILS_AAD,
 
+	NL80211_CMD_ASSOC_COMEBACK,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 83a1ba96e172..249109848497 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -17040,6 +17040,44 @@ static void nl80211_send_remain_on_chan_event(
 	nlmsg_free(msg);
 }
 
+void cfg80211_assoc_comeback(struct net_device *netdev,
+			     struct cfg80211_bss *bss, u32 timeout)
+{
+	struct wireless_dev *wdev = netdev->ieee80211_ptr;
+	struct wiphy *wiphy = wdev->wiphy;
+	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
+	struct sk_buff *msg;
+	void *hdr;
+
+	trace_cfg80211_assoc_comeback(wdev, bss->bssid, timeout);
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!msg)
+		return;
+
+	hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_ASSOC_COMEBACK);
+	if (!hdr) {
+		nlmsg_free(msg);
+		return;
+	}
+
+	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
+	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
+	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bss->bssid) ||
+	    nla_put_u32(msg, NL80211_ATTR_TIMEOUT, timeout))
+		goto nla_put_failure;
+
+	genlmsg_end(msg, hdr);
+
+	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
+				NL80211_MCGRP_MLME, GFP_KERNEL);
+	return;
+
+ nla_put_failure:
+	nlmsg_free(msg);
+}
+EXPORT_SYMBOL(cfg80211_assoc_comeback);
+
 void cfg80211_ready_on_channel(struct wireless_dev *wdev, u64 cookie,
 			       struct ieee80211_channel *chan,
 			       unsigned int duration, gfp_t gfp)
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 0b27eaa14a18..e0a80349c5b3 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -3693,6 +3693,23 @@ TRACE_EVENT(rdev_set_radar_offchan,
 		  WIPHY_PR_ARG, CHAN_DEF_PR_ARG)
 );
 
+TRACE_EVENT(cfg80211_assoc_comeback,
+	TP_PROTO(struct wireless_dev *wdev, const u8 *bssid, u32 timeout),
+	TP_ARGS(wdev, bssid, timeout),
+	TP_STRUCT__entry(
+		WDEV_ENTRY
+		MAC_ENTRY(bssid)
+		__field(u32, timeout)
+	),
+	TP_fast_assign(
+		WDEV_ASSIGN;
+		MAC_ASSIGN(bssid, bssid);
+		__entry->timeout = timeout;
+	),
+	TP_printk(WDEV_PR_FMT ", " MAC_PR_FMT ", timeout: %u TUs",
+		  WDEV_PR_ARG, MAC_PR_ARG(bssid), __entry->timeout)
+);
+
 #endif /* !__RDEV_OPS_TRACE || TRACE_HEADER_MULTI_READ */
 
 #undef TRACE_INCLUDE_PATH
-- 
2.33.1

[PATCH 03/16] mac80211: Notify cfg80211 about association comeback

[Luca Coelho]

From: Ilan Peer <ilan.peer@intel.com>

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/mlme.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index ee5473b0a791..30d344a88d96 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3740,6 +3740,10 @@ static void ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
 	    elems->timeout_int &&
 	    elems->timeout_int->type == WLAN_TIMEOUT_ASSOC_COMEBACK) {
 		u32 tu, ms;
+
+		cfg80211_assoc_comeback(sdata->dev, assoc_data->bss,
+					le32_to_cpu(elems->timeout_int->value));
+
 		tu = le32_to_cpu(elems->timeout_int->value);
 		ms = tu * 1024 / 1000;
 		sdata_info(sdata,
-- 
2.33.1

[PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel

[Luca Coelho]

From: Ayala Beker <ayala.beker@intel.com>

A non-collocated AP whose primary channel is not a PSC channel
may transmit a duplicated beacon on the corresponding PSC channel
in which it would indicate its true primary channel.
Use this inforamtion contained in the HE operation IE to determine
the primary channel of the AP.
In case of invalid infomration ignore it and use the channel
the frame was received on.

Signed-off-by: Ayala Beker <ayala.beker@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/scan.c | 46 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 22e92be61938..3fd0757ead29 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1800,7 +1800,33 @@ int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
 	const u8 *tmp;
 	int channel_number = -1;
 
-	if (band == NL80211_BAND_S1GHZ) {
+	if (channel->band == NL80211_BAND_6GHZ) {
+		const struct element *elem;
+
+		elem = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, ie,
+					      ielen);
+		if (elem && elem->datalen >= ieee80211_he_oper_size(&elem->data[1])) {
+			struct ieee80211_he_operation *he_oper =
+				(void *)(&elem->data[1]);
+			const struct ieee80211_he_6ghz_oper *he_6ghz_oper;
+
+			he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper);
+			if (!he_6ghz_oper)
+				return channel;
+
+			freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
+							      NL80211_BAND_6GHZ);
+
+			/* duplicated beacon indication is relevant for beacons
+			 * only
+			 */
+			if (freq != channel->center_freq &&
+			    abs(freq - channel->center_freq) <= 80 &&
+			    (ftype != CFG80211_BSS_FTYPE_BEACON ||
+			     he_6ghz_oper->control & IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON))
+				channel_number = he_6ghz_oper->primary;
+		}
+	} else if (band == NL80211_BAND_S1GHZ) {
 		tmp = cfg80211_find_ie(WLAN_EID_S1G_OPERATION, ie, ielen);
 		if (tmp && tmp[1] >= sizeof(struct ieee80211_s1g_oper_ie)) {
 			struct ieee80211_s1g_oper_ie *s1gop = (void *)(tmp + 2);
@@ -1831,12 +1857,13 @@ EXPORT_SYMBOL(cfg80211_get_ies_channel_number);
  * from neighboring channels and the Beacon frames use the DSSS Parameter Set
  * element to indicate the current (transmitting) channel, but this might also
  * be needed on other bands if RX frequency does not match with the actual
- * operating channel of a BSS.
+ * operating channel of a BSS, or if the AP reports a different primary channel.
  */
 static struct ieee80211_channel *
 cfg80211_get_bss_channel(struct wiphy *wiphy, const u8 *ie, size_t ielen,
 			 struct ieee80211_channel *channel,
-			 enum nl80211_bss_scan_width scan_width)
+			 enum nl80211_bss_scan_width scan_width,
+			 enum cfg80211_bss_frame_type ftype)
 {
 	u32 freq;
 	int channel_number;
@@ -1911,7 +1938,7 @@ cfg80211_inform_single_bss_data(struct wiphy *wiphy,
 		return NULL;
 
 	channel = cfg80211_get_bss_channel(wiphy, ie, ielen, data->chan,
-					   data->scan_width);
+					   data->scan_width, ftype);
 	if (!channel)
 		return NULL;
 
@@ -2333,6 +2360,7 @@ cfg80211_inform_single_bss_frame_data(struct wiphy *wiphy,
 	size_t ielen, min_hdr_len = offsetof(struct ieee80211_mgmt,
 					     u.probe_resp.variable);
 	int bss_type;
+	enum cfg80211_bss_frame_type ftype;
 
 	BUILD_BUG_ON(offsetof(struct ieee80211_mgmt, u.probe_resp.variable) !=
 			offsetof(struct ieee80211_mgmt, u.beacon.variable));
@@ -2369,8 +2397,16 @@ cfg80211_inform_single_bss_frame_data(struct wiphy *wiphy,
 			variable = ext->u.s1g_beacon.variable;
 	}
 
+	if (ieee80211_is_beacon(mgmt->frame_control))
+		ftype = CFG80211_BSS_FTYPE_BEACON;
+	else if (ieee80211_is_probe_resp(mgmt->frame_control))
+		ftype = CFG80211_BSS_FTYPE_PRESP;
+	else
+		ftype = CFG80211_BSS_FTYPE_UNKNOWN;
+
 	channel = cfg80211_get_bss_channel(wiphy, variable,
-					   ielen, data->chan, data->scan_width);
+					   ielen, data->chan, data->scan_width,
+					   ftype);
 	if (!channel)
 		return NULL;
 
-- 
2.33.1

[PATCH 05/16] [BUGFIX] cfg80211: check fixed size before ieee80211_he_oper_size()

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

We need to check the fixed portion is present before calling
ieee80211_he_oper_size() so that we don't access fields in
the static portion that don't exist.

type=bugfix
ticket=none
fixes=I130f678e4aa390973ab39d838bbfe7b2d54bff8e

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Reviewed-on: https://git-amr-3.devtools.intel.com/gerrit/332428
automatic-review: ec ger unix iil jenkins <EC.GER.UNIX.IIL.JENKINS@INTEL.COM>
Tested-by: ec ger unix iil jenkins <EC.GER.UNIX.IIL.JENKINS@INTEL.COM>
Reviewed-by: Luciano Coelho <luciano.coelho@intel.com>
---
 net/wireless/scan.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 3fd0757ead29..fddcb60b5b60 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1802,14 +1802,16 @@ int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
 
 	if (channel->band == NL80211_BAND_6GHZ) {
 		const struct element *elem;
+		struct ieee80211_he_operation *he_oper;
 
 		elem = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, ie,
 					      ielen);
-		if (elem && elem->datalen >= ieee80211_he_oper_size(&elem->data[1])) {
-			struct ieee80211_he_operation *he_oper =
-				(void *)(&elem->data[1]);
+		if (elem && elem->datalen >= sizeof(*he_oper) &&
+		    elem->datalen >= ieee80211_he_oper_size(&elem->data[1])) {
 			const struct ieee80211_he_6ghz_oper *he_6ghz_oper;
 
+			he_oper = (void *)&elem->data[1];
+
 			he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper);
 			if (!he_6ghz_oper)
 				return channel;
-- 
2.33.1

[PATCH 06/16] mac80211: introduce channel switch disconnect function

[Luca Coelho]

From: Nathan Errera <nathan.errera@intel.com>

Introduce a disconnect function that can be used when a
channel switch error occurs. The channel switch can request to
block the tx, and so, we need to make sure we do not send a deauth
frame in this case.

Signed-off-by: Nathan Errera <nathan.errera@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/mac80211.h | 12 ++++++++++++
 net/mac80211/cfg.c     | 14 +++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 775dbb982654..6770e17a1d38 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6073,6 +6073,18 @@ void ieee80211_radar_detected(struct ieee80211_hw *hw);
  */
 void ieee80211_chswitch_done(struct ieee80211_vif *vif, bool success);
 
+/**
+ * ieee80211_channel_switch_disconnect - disconnect due to channel switch error
+ * @vif &struct ieee80211_vif pointer from the add_interface callback.
+ * @block_tx: if %true, do not send deauth frame.
+ *
+ * Instruct mac80211 to disconnect due to a channel switch error. The channel
+ * switch can request to block the tx and so, we need to make sure we do not send
+ * a deauth frame in this case.
+ */
+void ieee80211_channel_switch_disconnect(struct ieee80211_vif *vif,
+					 bool block_tx);
+
 /**
  * ieee80211_request_smps - request SM PS transition
  * @vif: &struct ieee80211_vif pointer from the add_interface callback.
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 45334d59fe06..23b35fe73a50 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -5,7 +5,7 @@
  * Copyright 2006-2010	Johannes Berg <johannes@sipsolutions.net>
  * Copyright 2013-2015  Intel Mobile Communications GmbH
  * Copyright (C) 2015-2017 Intel Deutschland GmbH
- * Copyright (C) 2018-2020 Intel Corporation
+ * Copyright (C) 2018-2021 Intel Corporation
  */
 
 #include <linux/ieee80211.h>
@@ -3198,6 +3198,18 @@ void ieee80211_csa_finish(struct ieee80211_vif *vif)
 }
 EXPORT_SYMBOL(ieee80211_csa_finish);
 
+void ieee80211_channel_switch_disconnect(struct ieee80211_vif *vif, bool block_tx)
+{
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+	struct ieee80211_local *local = sdata->local;
+
+	sdata->csa_block_tx = block_tx;
+	sdata_info(sdata, "channel switch failed, disconnecting\n");
+	ieee80211_queue_work(&local->hw, &ifmgd->csa_connection_drop_work);
+}
+EXPORT_SYMBOL(ieee80211_channel_switch_disconnect);
+
 static int ieee80211_set_after_csa_beacon(struct ieee80211_sub_if_data *sdata,
 					  u32 *changed)
 {
-- 
2.33.1

[PATCH 07/16] mac80211: mark TX-during-stop for TX in in_reconfig

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

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>
---
 net/mac80211/driver-ops.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index cd3731cbf6c6..c336267f4599 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1219,8 +1219,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;
-- 
2.33.1

[PATCH 08/16] mac80211: do drv_reconfig_complete() before restarting all

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

When we reconfigure, the driver might do some things to complete
the reconfiguration. It's strange and could be broken in some
cases because we restart other works (e.g. remain-on-channel and
TX) before this happens, yet only start queues later.

Change this to do the reconfig complete when reconfiguration is
actually complete, not when we've already started doing other
things again.

For iwlwifi, this should fix a race where the reconfig can race
with TX, for ath10k and ath11k that also use this it won't make
a difference because they just start queues there, and mac80211
also stopped the queues and will restart them later as before.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/util.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 43df2f0c5db9..4b102d5863cf 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2646,6 +2646,13 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 		mutex_unlock(&local->sta_mtx);
 	}
 
+	/*
+	 * If this is for hw restart things are still running.
+	 * We may want to change that later, however.
+	 */
+	if (local->open_count && (!suspended || reconfig_due_to_wowlan))
+		drv_reconfig_complete(local, IEEE80211_RECONFIG_TYPE_RESTART);
+
 	if (local->in_reconfig) {
 		local->in_reconfig = false;
 		barrier();
@@ -2664,13 +2671,6 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 					IEEE80211_QUEUE_STOP_REASON_SUSPEND,
 					false);
 
-	/*
-	 * If this is for hw restart things are still running.
-	 * We may want to change that later, however.
-	 */
-	if (local->open_count && (!suspended || reconfig_due_to_wowlan))
-		drv_reconfig_complete(local, IEEE80211_RECONFIG_TYPE_RESTART);
-
 	if (!suspended)
 		return 0;
 
-- 
2.33.1

[PATCH 09/16] cfg80211: Fix order of enum nl80211_band_iftype_attr documentation

[Luca Coelho]

From: Ilan Peer <ilan.peer@intel.com>

And fix the comment.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/uapi/linux/nl80211.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 5ab616dc363e..a8e20d25252b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3754,13 +3754,12 @@ enum nl80211_mpath_info {
  *     capabilities IE
  * @NL80211_BAND_IFTYPE_ATTR_HE_CAP_PPE: HE PPE thresholds information as
  *     defined in HE capabilities IE
- * @NL80211_BAND_IFTYPE_ATTR_MAX: highest band HE capability attribute currently
- *     defined
  * @NL80211_BAND_IFTYPE_ATTR_HE_6GHZ_CAPA: HE 6GHz band capabilities (__le16),
  *	given for all 6 GHz band channels
  * @NL80211_BAND_IFTYPE_ATTR_VENDOR_ELEMS: vendor element capabilities that are
  *	advertised on this band/for this iftype (binary)
  * @__NL80211_BAND_IFTYPE_ATTR_AFTER_LAST: internal use
+ * @NL80211_BAND_IFTYPE_ATTR_MAX: highest band attribute currently defined
  */
 enum nl80211_band_iftype_attr {
 	__NL80211_BAND_IFTYPE_ATTR_INVALID,
-- 
2.33.1

[PATCH 10/16] mac80211: update channel context before station state

[Luca Coelho]

From: Mordechay Goodstein <mordechay.goodstein@intel.com>

Currently channel context is updated only after station got an update about
new assoc state, this results in station using the old channel context.

Fix this by moving the update channel context before updating station,
enabling the driver to immediately use the updated channel context in
the new assoc state.

Signed-off-by: Mordechay Goodstein <mordechay.goodstein@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/sta_info.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 51b49f0d3ad4..9d496923fc19 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -667,6 +667,15 @@ static int sta_info_insert_finish(struct sta_info *sta) __acquires(RCU)
 
 	list_add_tail_rcu(&sta->list, &local->sta_list);
 
+	/* update channel context before notifying the driver about state
+	 * change, this enables driver using the updated channel context right away.
+	 */
+	if (sta->sta_state >= IEEE80211_STA_ASSOC) {
+		ieee80211_recalc_min_chandef(sta->sdata);
+		if (!sta->sta.support_p2p_ps)
+			ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
+	}
+
 	/* notify driver */
 	err = sta_info_insert_drv_state(local, sdata, sta);
 	if (err)
@@ -674,12 +683,6 @@ static int sta_info_insert_finish(struct sta_info *sta) __acquires(RCU)
 
 	set_sta_flag(sta, WLAN_STA_INSERTED);
 
-	if (sta->sta_state >= IEEE80211_STA_ASSOC) {
-		ieee80211_recalc_min_chandef(sta->sdata);
-		if (!sta->sta.support_p2p_ps)
-			ieee80211_recalc_p2p_go_ps_allowed(sta->sdata);
-	}
-
 	/* accept BA sessions now */
 	clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
 
-- 
2.33.1

[PATCH 11/16] cfg80211: simplify cfg80211_chandef_valid()

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

There are a lot of duplicate checks in this function to
check the delta between the control channel and CF1.
With the addition of 320 MHz, this will become even more.
Simplify the code so that the common checks are done
only once for multiple bandwidths.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/chan.c | 62 +++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 869c43d4414c..5b865c4719d1 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -245,19 +245,7 @@ bool cfg80211_chandef_valid(const struct cfg80211_chan_def *chandef)
 		    oper_freq - MHZ_TO_KHZ(oper_width) / 2)
 			return false;
 		break;
-	case NL80211_CHAN_WIDTH_40:
-		if (chandef->center_freq1 != control_freq + 10 &&
-		    chandef->center_freq1 != control_freq - 10)
-			return false;
-		if (chandef->center_freq2)
-			return false;
-		break;
 	case NL80211_CHAN_WIDTH_80P80:
-		if (chandef->center_freq1 != control_freq + 30 &&
-		    chandef->center_freq1 != control_freq + 10 &&
-		    chandef->center_freq1 != control_freq - 10 &&
-		    chandef->center_freq1 != control_freq - 30)
-			return false;
 		if (!chandef->center_freq2)
 			return false;
 		/* adjacent is not allowed -- that's a 160 MHz channel */
@@ -265,28 +253,42 @@ bool cfg80211_chandef_valid(const struct cfg80211_chan_def *chandef)
 		    chandef->center_freq2 - chandef->center_freq1 == 80)
 			return false;
 		break;
-	case NL80211_CHAN_WIDTH_80:
-		if (chandef->center_freq1 != control_freq + 30 &&
-		    chandef->center_freq1 != control_freq + 10 &&
-		    chandef->center_freq1 != control_freq - 10 &&
-		    chandef->center_freq1 != control_freq - 30)
-			return false;
+	default:
 		if (chandef->center_freq2)
 			return false;
 		break;
-	case NL80211_CHAN_WIDTH_160:
-		if (chandef->center_freq1 != control_freq + 70 &&
-		    chandef->center_freq1 != control_freq + 50 &&
-		    chandef->center_freq1 != control_freq + 30 &&
-		    chandef->center_freq1 != control_freq + 10 &&
-		    chandef->center_freq1 != control_freq - 10 &&
-		    chandef->center_freq1 != control_freq - 30 &&
-		    chandef->center_freq1 != control_freq - 50 &&
-		    chandef->center_freq1 != control_freq - 70)
-			return false;
-		if (chandef->center_freq2)
-			return false;
+	}
+
+	switch (chandef->width) {
+	case NL80211_CHAN_WIDTH_5:
+	case NL80211_CHAN_WIDTH_10:
+	case NL80211_CHAN_WIDTH_20:
+	case NL80211_CHAN_WIDTH_20_NOHT:
+	case NL80211_CHAN_WIDTH_1:
+	case NL80211_CHAN_WIDTH_2:
+	case NL80211_CHAN_WIDTH_4:
+	case NL80211_CHAN_WIDTH_8:
+	case NL80211_CHAN_WIDTH_16:
+		/* all checked above */
 		break;
+	case NL80211_CHAN_WIDTH_160:
+		if (chandef->center_freq1 == control_freq + 70 ||
+		    chandef->center_freq1 == control_freq + 50 ||
+		    chandef->center_freq1 == control_freq - 50 ||
+		    chandef->center_freq1 == control_freq - 70)
+			break;
+		fallthrough;
+	case NL80211_CHAN_WIDTH_80P80:
+	case NL80211_CHAN_WIDTH_80:
+		if (chandef->center_freq1 == control_freq + 30 ||
+		    chandef->center_freq1 == control_freq - 30)
+			break;
+		fallthrough;
+	case NL80211_CHAN_WIDTH_40:
+		if (chandef->center_freq1 == control_freq + 10 ||
+		    chandef->center_freq1 == control_freq - 10)
+			break;
+		fallthrough;
 	default:
 		return false;
 	}
-- 
2.33.1

[PATCH 12/16] mac80211: Remove a couple of obsolete TODO

[Luca Coelho]

From: Ilan Peer <ilan.peer@intel.com>

The HE capability IE is an extension IE so remove
an irrelevant comments.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/main.c | 13 +++++--------
 net/mac80211/mlme.c |  4 ----
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 45fb517591ee..5311c3cd3050 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -1131,17 +1131,14 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
 		local->scan_ies_len +=
 			2 + sizeof(struct ieee80211_vht_cap);
 
-	/* HE cap element is variable in size - set len to allow max size */
 	/*
-	 * TODO: 1 is added at the end of the calculation to accommodate for
-	 *	the temporary placing of the HE capabilities IE under EXT.
-	 *	Remove it once it is placed in the final place.
-	 */
-	if (supp_he)
+	 * HE cap element is variable in size - set len to allow max size */
+	if (supp_he) {
 		local->scan_ies_len +=
-			2 + sizeof(struct ieee80211_he_cap_elem) +
+			3 + sizeof(struct ieee80211_he_cap_elem) +
 			sizeof(struct ieee80211_he_mcs_nss_supp) +
-			IEEE80211_HE_PPE_THRES_MAX_LEN + 1;
+			IEEE80211_HE_PPE_THRES_MAX_LEN;
+	}
 
 	if (!local->ops->hw_scan) {
 		/* For hw_scan, driver needs to set these up. */
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 30d344a88d96..e6af62587e81 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -655,10 +655,6 @@ static void ieee80211_add_he_ie(struct ieee80211_sub_if_data *sdata,
 	if (!he_cap || !reg_cap)
 		return;
 
-	/*
-	 * TODO: the 1 added is because this temporarily is under the EXTENSION
-	 * IE. Get rid of it when it moves.
-	 */
 	he_cap_size =
 		2 + 1 + sizeof(he_cap->he_cap_elem) +
 		ieee80211_he_mcs_nss_size(&he_cap->he_cap_elem) +
-- 
2.33.1

[PATCH 13/16] mac80211: Fix the size used for building probe request

[Luca Coelho]

From: Ilan Peer <ilan.peer@intel.com>

Instead of using the hard-coded value of '100' use the correct
scan IEs length as calculated during HW registration to mac80211.

Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 4b102d5863cf..1e306ea2f625 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2063,7 +2063,7 @@ struct sk_buff *ieee80211_build_probe_req(struct ieee80211_sub_if_data *sdata,
 		chandef.chan = chan;
 
 	skb = ieee80211_probereq_get(&local->hw, src, ssid, ssid_len,
-				     100 + ie_len);
+				     local->scan_ies_len + ie_len);
 	if (!skb)
 		return NULL;
 
-- 
2.33.1

[PATCH 14/16] mac80211: fix lookup when adding AddBA extension element

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

We should be doing the HE capabilities lookup based on the full
interface type so if P2P doesn't have HE but client has it doesn't
get confused. Fix that.

Fixes: 2ab45876756f ("mac80211: add support for the ADDBA extension element")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/agg-rx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 470ff0ce3dc7..7d2925bb966e 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -9,7 +9,7 @@
  * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
  * Copyright 2007-2010, Intel Corporation
  * Copyright(c) 2015-2017 Intel Deutschland GmbH
- * Copyright (C) 2018-2020 Intel Corporation
+ * Copyright (C) 2018-2021 Intel Corporation
  */
 
 /**
@@ -191,7 +191,8 @@ static void ieee80211_add_addbaext(struct ieee80211_sub_if_data *sdata,
 	sband = ieee80211_get_sband(sdata);
 	if (!sband)
 		return;
-	he_cap = ieee80211_get_he_iftype_cap(sband, sdata->vif.type);
+	he_cap = ieee80211_get_he_iftype_cap(sband,
+					     ieee80211_vif_type_p2p(&sdata->vif));
 	if (!he_cap)
 		return;
 
-- 
2.33.1

[PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

When we call ieee80211_agg_start_txq(), that will in turn call
schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb()
this is done under sta->lock, which leads to certain circular
lock dependencies, as reported by Chris Murphy:
https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com

In general, ieee80211_agg_start_txq() is usually not called
with sta->lock held, only in this one place. But it's always
called with sta->ampdu_mlme.mtx held, and that's therefore
clearly sufficient.

Change ieee80211_stop_tx_ba_cb() to also call it without the
sta->lock held, by factoring it out of ieee80211_remove_tid_tx()
(which is only called in this one place).

This breaks the locking chain and makes it less likely that
we'll have similar locking chain problems in the future.

Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/mac80211/agg-tx.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 430a58587538..4dd56daed89b 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -9,7 +9,7 @@
  * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
  * Copyright 2007-2010, Intel Corporation
  * Copyright(c) 2015-2017 Intel Deutschland GmbH
- * Copyright (C) 2018 - 2020 Intel Corporation
+ * Copyright (C) 2018 - 2021 Intel Corporation
  */
 
 #include <linux/ieee80211.h>
@@ -213,6 +213,8 @@ ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
 	struct ieee80211_txq *txq = sta->sta.txq[tid];
 	struct txq_info *txqi;
 
+	lockdep_assert_held(&sta->ampdu_mlme.mtx);
+
 	if (!txq)
 		return;
 
@@ -290,7 +292,6 @@ static void ieee80211_remove_tid_tx(struct sta_info *sta, int tid)
 	ieee80211_assign_tid_tx(sta, tid, NULL);
 
 	ieee80211_agg_splice_finish(sta->sdata, tid);
-	ieee80211_agg_start_txq(sta, tid, false);
 
 	kfree_rcu(tid_tx, rcu_head);
 }
@@ -889,6 +890,7 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
 {
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
 	bool send_delba = false;
+	bool start_txq = false;
 
 	ht_dbg(sdata, "Stopping Tx BA session for %pM tid %d\n",
 	       sta->sta.addr, tid);
@@ -906,10 +908,14 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
 		send_delba = true;
 
 	ieee80211_remove_tid_tx(sta, tid);
+	start_txq = true;
 
  unlock_sta:
 	spin_unlock_bh(&sta->lock);
 
+	if (start_txq)
+		ieee80211_agg_start_txq(sta, tid, false);
+
 	if (send_delba)
 		ieee80211_send_delba(sdata, sta->sta.addr, tid,
 			WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE);
-- 
2.33.1

[PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work

[Luca Coelho]

From: Ilan Peer <ilan.peer@intel.com>

The function cfg80211_reg_can_beacon_relax() expects wiphy
mutex to be held when it is being called. However, when
reg_leave_invalid_chans() is called the mutex is not held.
Fix it by acquiring the lock before calling the function.

Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 net/wireless/reg.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index df87c7f3a049..6f6da1cedd80 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2338,6 +2338,7 @@ static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev)
 	struct cfg80211_chan_def chandef = {};
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
 	enum nl80211_iftype iftype;
+	bool ret;
 
 	wdev_lock(wdev);
 	iftype = wdev->iftype;
@@ -2387,7 +2388,11 @@ static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev)
 	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_P2P_GO:
 	case NL80211_IFTYPE_ADHOC:
-		return cfg80211_reg_can_beacon_relax(wiphy, &chandef, iftype);
+		wiphy_lock(wiphy);
+		ret = cfg80211_reg_can_beacon_relax(wiphy, &chandef, iftype);
+		wiphy_unlock(wiphy);
+
+		return ret;
 	case NL80211_IFTYPE_STATION:
 	case NL80211_IFTYPE_P2P_CLIENT:
 		return cfg80211_chandef_usable(wiphy, &chandef,
@@ -2409,7 +2414,6 @@ static void reg_leave_invalid_chans(struct wiphy *wiphy)
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
 
 	ASSERT_RTNL();
-
 	list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list)
 		if (!reg_wdev_chan_valid(wiphy, wdev))
 			cfg80211_leave(rdev, wdev);
-- 
2.33.1

Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Toke Høiland-Jørgensen]

Luca Coelho <luca@coelho.fi> writes:

> From: Johannes Berg <johannes.berg@intel.com>
>
> When we call ieee80211_agg_start_txq(), that will in turn call
> schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb()
> this is done under sta->lock, which leads to certain circular
> lock dependencies, as reported by Chris Murphy:
> https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com
>
> In general, ieee80211_agg_start_txq() is usually not called
> with sta->lock held, only in this one place. But it's always
> called with sta->ampdu_mlme.mtx held, and that's therefore
> clearly sufficient.
>
> Change ieee80211_stop_tx_ba_cb() to also call it without the
> sta->lock held, by factoring it out of ieee80211_remove_tid_tx()
> (which is only called in this one place).
>
> This breaks the locking chain and makes it less likely that
> we'll have similar locking chain problems in the future.
>
> Reported-by: Chris Murphy <lists@colorremedies.com>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>

Does this need a fixes: tag?

-Toke

Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Luca Coelho]

On Mon, 2021-11-29 at 14:54 +0100, Toke Høiland-Jørgensen wrote:
> Luca Coelho <luca@coelho.fi> writes:
> 
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > When we call ieee80211_agg_start_txq(), that will in turn call
> > schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb()
> > this is done under sta->lock, which leads to certain circular
> > lock dependencies, as reported by Chris Murphy:
> > https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com
> > 
> > In general, ieee80211_agg_start_txq() is usually not called
> > with sta->lock held, only in this one place. But it's always
> > called with sta->ampdu_mlme.mtx held, and that's therefore
> > clearly sufficient.
> > 
> > Change ieee80211_stop_tx_ba_cb() to also call it without the
> > sta->lock held, by factoring it out of ieee80211_remove_tid_tx()
> > (which is only called in this one place).
> > 
> > This breaks the locking chain and makes it less likely that
> > we'll have similar locking chain problems in the future.
> > 
> > Reported-by: Chris Murphy <lists@colorremedies.com>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> 
> Does this need a fixes: tag?

Hi Toke,

Neither Johannes nor Chris pointed to any specific patch that this
commit is fixing.  If you know the exact commit that broke this, I can
add the tag and send v2.

Thanks!

--
Cheers,
Luca.

Re: [PATCH 01/16] mac80211: add more HT/VHT/HE state logging

[Luca Coelho]

On Mon, 2021-11-29 at 15:32 +0200, Luca Coelho wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Add more logging in places that affect HT/VHT/HE state, so
> things get easier to debug.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---

Sorry, there was a merge mistake in this patch and it broke
compilation.  I'll send v2 in a sec.

--
Cheers,
Luca.

[PATCH v2] mac80211: add more HT/VHT/HE state logging

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

Add more logging in places that affect HT/VHT/HE state, so
things get easier to debug.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---

In v2:
  * removed "if (vht_cap)" in one of the changes.  Merge mistake.

net/mac80211/mlme.c | 45 +++++++++++++++++++++++++++++++++------------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 54ab0e1ef6ca..666955ef300f 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -164,12 +164,15 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	chandef->freq1_offset = channel->freq_offset;
 
 	if (channel->band == NL80211_BAND_6GHZ) {
-		if (!ieee80211_chandef_he_6ghz_oper(sdata, he_oper, chandef))
+		if (!ieee80211_chandef_he_6ghz_oper(sdata, he_oper, chandef)) {
+			mlme_dbg(sdata,
+				 "bad 6 GHz operation, disabling HT/VHT/HE\n");
 			ret = IEEE80211_STA_DISABLE_HT |
 			      IEEE80211_STA_DISABLE_VHT |
 			      IEEE80211_STA_DISABLE_HE;
-		else
+		} else {
 			ret = 0;
+		}
 		vht_chandef = *chandef;
 		goto out;
 	} else if (sband->band == NL80211_BAND_S1GHZ) {
@@ -190,6 +193,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	ieee80211_apply_htcap_overrides(sdata, &sta_ht_cap);
 
 	if (!ht_oper || !sta_ht_cap.ht_supported) {
+		mlme_dbg(sdata, "HT operation missing / HT not supported\n");
 		ret = IEEE80211_STA_DISABLE_HT |
 		      IEEE80211_STA_DISABLE_VHT |
 		      IEEE80211_STA_DISABLE_HE;
@@ -223,6 +227,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	if (sta_ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) {
 		ieee80211_chandef_ht_oper(ht_oper, chandef);
 	} else {
+		mlme_dbg(sdata, "40 MHz not supported\n");
 		/* 40 MHz (and 80 MHz) must be supported for VHT */
 		ret = IEEE80211_STA_DISABLE_VHT;
 		/* also mark 40 MHz disabled */
@@ -231,6 +236,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (!vht_oper || !sband->vht_cap.vht_supported) {
+		mlme_dbg(sdata, "VHT operation missing / VHT not supported\n");
 		ret = IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
@@ -253,7 +259,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 						&vht_chandef)) {
 			if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE))
 				sdata_info(sdata,
-					   "HE AP VHT information is invalid, disable HE\n");
+					   "HE AP VHT information is invalid, disabling HE\n");
 			ret = IEEE80211_STA_DISABLE_HE;
 			goto out;
 		}
@@ -263,7 +269,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 					       &vht_chandef)) {
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
-				   "AP VHT information is invalid, disable VHT\n");
+				   "AP VHT information is invalid, disabling VHT\n");
 		ret = IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
@@ -271,7 +277,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	if (!cfg80211_chandef_valid(&vht_chandef)) {
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
-				   "AP VHT information is invalid, disable VHT\n");
+				   "AP VHT information is invalid, disabling VHT\n");
 		ret = IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
@@ -284,7 +290,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
 	if (!cfg80211_chandef_compatible(chandef, &vht_chandef)) {
 		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
 			sdata_info(sdata,
-				   "AP VHT information doesn't match HT, disable VHT\n");
+				   "AP VHT information doesn't match HT, disabling VHT\n");
 		ret = IEEE80211_STA_DISABLE_VHT;
 		goto out;
 	}
@@ -5036,19 +5042,23 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 
 	/* disable HT/VHT/HE if we don't support them */
 	if (!sband->ht_cap.ht_supported && !is_6ghz) {
+		mlme_dbg(sdata, "HT not supported, disabling HT/VHT/HE\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
 	}
 
 	if (!sband->vht_cap.vht_supported && is_5ghz) {
+		mlme_dbg(sdata, "VHT not supported, disabling VHT/HE\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
 	}
 
 	if (!ieee80211_get_he_iftype_cap(sband,
-					 ieee80211_vif_type_p2p(&sdata->vif)))
+					 ieee80211_vif_type_p2p(&sdata->vif))) {
+		mlme_dbg(sdata, "HE not supported, disabling it\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
+	}
 
 	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HT) && !is_6ghz) {
 		ht_oper = elems->ht_operation;
@@ -5072,6 +5082,8 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 		}
 
 		if (!elems->vht_cap_elem) {
+			sdata_info(sdata,
+				   "bad VHT capabilities, disabling VHT\n");
 			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
 			vht_oper = NULL;
 		}
@@ -5119,8 +5131,10 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
 		break;
 	}
 
-	if (!have_80mhz)
+	if (!have_80mhz) {
+		sdata_info(sdata, "80 MHz not supported, disabling VHT\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
+	}
 
 	if (sband->band == NL80211_BAND_S1GHZ) {
 		s1g_oper = elems->s1g_oper;
@@ -5684,12 +5698,14 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
 	else if (!is_6ghz)
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
 	vht_elem = ieee80211_bss_get_elem(req->bss, WLAN_EID_VHT_CAPABILITY);
-	if (vht_elem && vht_elem->datalen >= sizeof(struct ieee80211_vht_cap))
+	if (vht_elem && vht_elem->datalen >= sizeof(struct ieee80211_vht_cap)) {
 		memcpy(&assoc_data->ap_vht_cap, vht_elem->data,
 		       sizeof(struct ieee80211_vht_cap));
-	else if (is_5ghz)
+	} else if (is_5ghz) {
+		sdata_info(sdata, "VHT capa missing/short, disabling VHT/HE\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT |
 				IEEE80211_STA_DISABLE_HE;
+	}
 	rcu_read_unlock();
 
 	if (WARN((sdata->vif.driver_flags & IEEE80211_VIF_SUPPORTS_UAPSD) &&
@@ -5763,16 +5779,21 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
 	}
 
 	if (req->flags & ASSOC_REQ_DISABLE_HT) {
+		mlme_dbg(sdata, "HT disabled by flag, disabling HT/VHT/HE\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
 	}
 
-	if (req->flags & ASSOC_REQ_DISABLE_VHT)
+	if (req->flags & ASSOC_REQ_DISABLE_VHT) {
+		mlme_dbg(sdata, "VHT disabled by flag, disabling VHT\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
+	}
 
-	if (req->flags & ASSOC_REQ_DISABLE_HE)
+	if (req->flags & ASSOC_REQ_DISABLE_HE) {
+		mlme_dbg(sdata, "HE disabled by flag, disabling VHT\n");
 		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
+	}
 
 	err = ieee80211_prep_connection(sdata, req->bss, true, override);
 	if (err)
-- 
2.33.1

Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Toke Høiland-Jørgensen]

Luca Coelho <luca@coelho.fi> writes:

> On Mon, 2021-11-29 at 14:54 +0100, Toke Høiland-Jørgensen wrote:
>> Luca Coelho <luca@coelho.fi> writes:
>> 
>> > From: Johannes Berg <johannes.berg@intel.com>
>> > 
>> > When we call ieee80211_agg_start_txq(), that will in turn call
>> > schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb()
>> > this is done under sta->lock, which leads to certain circular
>> > lock dependencies, as reported by Chris Murphy:
>> > https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com
>> > 
>> > In general, ieee80211_agg_start_txq() is usually not called
>> > with sta->lock held, only in this one place. But it's always
>> > called with sta->ampdu_mlme.mtx held, and that's therefore
>> > clearly sufficient.
>> > 
>> > Change ieee80211_stop_tx_ba_cb() to also call it without the
>> > sta->lock held, by factoring it out of ieee80211_remove_tid_tx()
>> > (which is only called in this one place).
>> > 
>> > This breaks the locking chain and makes it less likely that
>> > we'll have similar locking chain problems in the future.
>> > 
>> > Reported-by: Chris Murphy <lists@colorremedies.com>
>> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>> > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>> 
>> Does this need a fixes: tag?
>
> Hi Toke,
>
> Neither Johannes nor Chris pointed to any specific patch that this
> commit is fixing.  If you know the exact commit that broke this, I can
> add the tag and send v2.

Well, it looks like the code you're changing comes all the way back from:
ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation")

Maybe Johannes can comment on whether it's appropriate to include this,
or if the code changed too much in the meantime...

-Toke

Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Johannes Berg]

On Tue, 2021-11-30 at 12:32 +0100, Toke Høiland-Jørgensen wrote:
> Luca Coelho <luca@coelho.fi> writes:
> 
> > On Mon, 2021-11-29 at 14:54 +0100, Toke Høiland-Jørgensen wrote:
> > > Luca Coelho <luca@coelho.fi> writes:
> > > 
> > > > From: Johannes Berg <johannes.berg@intel.com>
> > > > 
> > > > When we call ieee80211_agg_start_txq(), that will in turn call
> > > > schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb()
> > > > this is done under sta->lock, which leads to certain circular
> > > > lock dependencies, as reported by Chris Murphy:
> > > > https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com
> > > > 
> > > > In general, ieee80211_agg_start_txq() is usually not called
> > > > with sta->lock held, only in this one place. But it's always
> > > > called with sta->ampdu_mlme.mtx held, and that's therefore
> > > > clearly sufficient.
> > > > 
> > > > Change ieee80211_stop_tx_ba_cb() to also call it without the
> > > > sta->lock held, by factoring it out of ieee80211_remove_tid_tx()
> > > > (which is only called in this one place).
> > > > 
> > > > This breaks the locking chain and makes it less likely that
> > > > we'll have similar locking chain problems in the future.
> > > > 
> > > > Reported-by: Chris Murphy <lists@colorremedies.com>
> > > > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > > 
> > > Does this need a fixes: tag?
> > 
> > Hi Toke,
> > 
> > Neither Johannes nor Chris pointed to any specific patch that this
> > commit is fixing.  If you know the exact commit that broke this, I can
> > add the tag and send v2.
> 
> Well, it looks like the code you're changing comes all the way back from:
> ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation")
> 
> Maybe Johannes can comment on whether it's appropriate to include this,
> or if the code changed too much in the meantime...
> 

I think that probably makes sense, and I guess I can include that tag
when I apply it.

I suspect the reason I didn't do it internally (we have a different tag
though, so that's no excuse) is that there we didn't care until iwlwifi
actually gained TXQ support.

johannes

Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Toke Høiland-Jørgensen]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Tue, 2021-11-30 at 12:32 +0100, Toke Høiland-Jørgensen wrote:
>> Luca Coelho <luca@coelho.fi> writes:
>> 
>> > On Mon, 2021-11-29 at 14:54 +0100, Toke Høiland-Jørgensen wrote:
>> > > Luca Coelho <luca@coelho.fi> writes:
>> > > 
>> > > > From: Johannes Berg <johannes.berg@intel.com>
>> > > > 
>> > > > When we call ieee80211_agg_start_txq(), that will in turn call
>> > > > schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb()
>> > > > this is done under sta->lock, which leads to certain circular
>> > > > lock dependencies, as reported by Chris Murphy:
>> > > > https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com
>> > > > 
>> > > > In general, ieee80211_agg_start_txq() is usually not called
>> > > > with sta->lock held, only in this one place. But it's always
>> > > > called with sta->ampdu_mlme.mtx held, and that's therefore
>> > > > clearly sufficient.
>> > > > 
>> > > > Change ieee80211_stop_tx_ba_cb() to also call it without the
>> > > > sta->lock held, by factoring it out of ieee80211_remove_tid_tx()
>> > > > (which is only called in this one place).
>> > > > 
>> > > > This breaks the locking chain and makes it less likely that
>> > > > we'll have similar locking chain problems in the future.
>> > > > 
>> > > > Reported-by: Chris Murphy <lists@colorremedies.com>
>> > > > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>> > > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>> > > 
>> > > Does this need a fixes: tag?
>> > 
>> > Hi Toke,
>> > 
>> > Neither Johannes nor Chris pointed to any specific patch that this
>> > commit is fixing.  If you know the exact commit that broke this, I can
>> > add the tag and send v2.
>> 
>> Well, it looks like the code you're changing comes all the way back from:
>> ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation")
>> 
>> Maybe Johannes can comment on whether it's appropriate to include this,
>> or if the code changed too much in the meantime...
>> 
>
> I think that probably makes sense, and I guess I can include that tag
> when I apply it.
>
> I suspect the reason I didn't do it internally (we have a different tag
> though, so that's no excuse) is that there we didn't care until iwlwifi
> actually gained TXQ support.

Right, makes sense - thanks! :)

-Toke

Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Luca Coelho]

On Tue, 2021-11-30 at 12:52 +0100, Johannes Berg wrote:
> On Tue, 2021-11-30 at 12:32 +0100, Toke Høiland-Jørgensen wrote:
> > Luca Coelho <luca@coelho.fi> writes:
> > 
> > > On Mon, 2021-11-29 at 14:54 +0100, Toke Høiland-Jørgensen wrote:
> > > > Luca Coelho <luca@coelho.fi> writes:
> > > > 
> > > > > From: Johannes Berg <johannes.berg@intel.com>
> > > > > 
> > > > > When we call ieee80211_agg_start_txq(), that will in turn call
> > > > > schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb()
> > > > > this is done under sta->lock, which leads to certain circular
> > > > > lock dependencies, as reported by Chris Murphy:
> > > > > https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com
> > > > > 
> > > > > In general, ieee80211_agg_start_txq() is usually not called
> > > > > with sta->lock held, only in this one place. But it's always
> > > > > called with sta->ampdu_mlme.mtx held, and that's therefore
> > > > > clearly sufficient.
> > > > > 
> > > > > Change ieee80211_stop_tx_ba_cb() to also call it without the
> > > > > sta->lock held, by factoring it out of ieee80211_remove_tid_tx()
> > > > > (which is only called in this one place).
> > > > > 
> > > > > This breaks the locking chain and makes it less likely that
> > > > > we'll have similar locking chain problems in the future.
> > > > > 
> > > > > Reported-by: Chris Murphy <lists@colorremedies.com>
> > > > > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> > > > > Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> > > > 
> > > > Does this need a fixes: tag?
> > > 
> > > Hi Toke,
> > > 
> > > Neither Johannes nor Chris pointed to any specific patch that this
> > > commit is fixing.  If you know the exact commit that broke this, I can
> > > add the tag and send v2.
> > 
> > Well, it looks like the code you're changing comes all the way back from:
> > ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation")
> > 
> > Maybe Johannes can comment on whether it's appropriate to include this,
> > or if the code changed too much in the meantime...
> > 
> 
> I think that probably makes sense, and I guess I can include that tag
> when I apply it.
> 
> I suspect the reason I didn't do it internally (we have a different tag
> though, so that's no excuse) is that there we didn't care until iwlwifi
> actually gained TXQ support.

Thanks, guys!

Johannes, do you want me to send v2 or do you want to add the tag
yourself? There is one more patch that is broken (ugh, sorry) that I
need to fix anyway...

--
Cheers,
Luca.

Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Johannes Berg]

On Tue, 2021-11-30 at 13:57 +0200, Luca Coelho wrote:
> > > > 
> 
> Johannes, do you want me to send v2 or do you want to add the tag
> yourself? There is one more patch that is broken (ugh, sorry) that I
> need to fix anyway...
> 

Well if you resend I don't have to remember, so that wouldn't hurt
either ;-)

johannes

Re: [PATCH v2] mac80211: add more HT/VHT/HE state logging

[Ben Greear]

On 11/30/21 3:16 AM, Luca Coelho wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Add more logging in places that affect HT/VHT/HE state, so
> things get easier to debug.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
> 
> In v2:
>    * removed "if (vht_cap)" in one of the changes.  Merge mistake.
> 
> net/mac80211/mlme.c | 45 +++++++++++++++++++++++++++++++++------------
>   1 file changed, 33 insertions(+), 12 deletions(-)
> 
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index 54ab0e1ef6ca..666955ef300f 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -164,12 +164,15 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>   	chandef->freq1_offset = channel->freq_offset;
>   
>   	if (channel->band == NL80211_BAND_6GHZ) {
> -		if (!ieee80211_chandef_he_6ghz_oper(sdata, he_oper, chandef))
> +		if (!ieee80211_chandef_he_6ghz_oper(sdata, he_oper, chandef)) {
> +			mlme_dbg(sdata,
> +				 "bad 6 GHz operation, disabling HT/VHT/HE\n");
>   			ret = IEEE80211_STA_DISABLE_HT |
>   			      IEEE80211_STA_DISABLE_VHT |
>   			      IEEE80211_STA_DISABLE_HE;
> -		else
> +		} else {
>   			ret = 0;
> +		}
>   		vht_chandef = *chandef;
>   		goto out;
>   	} else if (sband->band == NL80211_BAND_S1GHZ) {
> @@ -190,6 +193,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>   	ieee80211_apply_htcap_overrides(sdata, &sta_ht_cap);
>   
>   	if (!ht_oper || !sta_ht_cap.ht_supported) {
> +		mlme_dbg(sdata, "HT operation missing / HT not supported\n");

In case you re-spin this, I prefer that you not only have text like that above, but
then also explain what is happening, for instance, add:   Disabling HT/VHT/HE,
and maybe print out ht_oper and sta_ht_cap.ht_supported so you can see exactly why
it hit this code path.

Similar suggestions for changes below...

Thanks,
Ben

>   		ret = IEEE80211_STA_DISABLE_HT |
>   		      IEEE80211_STA_DISABLE_VHT |
>   		      IEEE80211_STA_DISABLE_HE;
> @@ -223,6 +227,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>   	if (sta_ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40) {
>   		ieee80211_chandef_ht_oper(ht_oper, chandef);
>   	} else {
> +		mlme_dbg(sdata, "40 MHz not supported\n");
>   		/* 40 MHz (and 80 MHz) must be supported for VHT */
>   		ret = IEEE80211_STA_DISABLE_VHT;
>   		/* also mark 40 MHz disabled */
> @@ -231,6 +236,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>   	}
>   
>   	if (!vht_oper || !sband->vht_cap.vht_supported) {
> +		mlme_dbg(sdata, "VHT operation missing / VHT not supported\n");
>   		ret = IEEE80211_STA_DISABLE_VHT;
>   		goto out;
>   	}
> @@ -253,7 +259,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>   						&vht_chandef)) {
>   			if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE))
>   				sdata_info(sdata,
> -					   "HE AP VHT information is invalid, disable HE\n");
> +					   "HE AP VHT information is invalid, disabling HE\n");
>   			ret = IEEE80211_STA_DISABLE_HE;
>   			goto out;
>   		}
> @@ -263,7 +269,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>   					       &vht_chandef)) {
>   		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
>   			sdata_info(sdata,
> -				   "AP VHT information is invalid, disable VHT\n");
> +				   "AP VHT information is invalid, disabling VHT\n");
>   		ret = IEEE80211_STA_DISABLE_VHT;
>   		goto out;
>   	}
> @@ -271,7 +277,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>   	if (!cfg80211_chandef_valid(&vht_chandef)) {
>   		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
>   			sdata_info(sdata,
> -				   "AP VHT information is invalid, disable VHT\n");
> +				   "AP VHT information is invalid, disabling VHT\n");
>   		ret = IEEE80211_STA_DISABLE_VHT;
>   		goto out;
>   	}
> @@ -284,7 +290,7 @@ ieee80211_determine_chantype(struct ieee80211_sub_if_data *sdata,
>   	if (!cfg80211_chandef_compatible(chandef, &vht_chandef)) {
>   		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT))
>   			sdata_info(sdata,
> -				   "AP VHT information doesn't match HT, disable VHT\n");
> +				   "AP VHT information doesn't match HT, disabling VHT\n");
>   		ret = IEEE80211_STA_DISABLE_VHT;
>   		goto out;
>   	}
> @@ -5036,19 +5042,23 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
>   
>   	/* disable HT/VHT/HE if we don't support them */
>   	if (!sband->ht_cap.ht_supported && !is_6ghz) {
> +		mlme_dbg(sdata, "HT not supported, disabling HT/VHT/HE\n");
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
>   	}
>   
>   	if (!sband->vht_cap.vht_supported && is_5ghz) {
> +		mlme_dbg(sdata, "VHT not supported, disabling VHT/HE\n");
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
>   	}
>   
>   	if (!ieee80211_get_he_iftype_cap(sband,
> -					 ieee80211_vif_type_p2p(&sdata->vif)))
> +					 ieee80211_vif_type_p2p(&sdata->vif))) {
> +		mlme_dbg(sdata, "HE not supported, disabling it\n");
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
> +	}
>   
>   	if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HT) && !is_6ghz) {
>   		ht_oper = elems->ht_operation;
> @@ -5072,6 +5082,8 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
>   		}
>   
>   		if (!elems->vht_cap_elem) {
> +			sdata_info(sdata,
> +				   "bad VHT capabilities, disabling VHT\n");
>   			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
>   			vht_oper = NULL;
>   		}
> @@ -5119,8 +5131,10 @@ static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
>   		break;
>   	}
>   
> -	if (!have_80mhz)
> +	if (!have_80mhz) {
> +		sdata_info(sdata, "80 MHz not supported, disabling VHT\n");
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
> +	}
>   
>   	if (sband->band == NL80211_BAND_S1GHZ) {
>   		s1g_oper = elems->s1g_oper;
> @@ -5684,12 +5698,14 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
>   	else if (!is_6ghz)
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
>   	vht_elem = ieee80211_bss_get_elem(req->bss, WLAN_EID_VHT_CAPABILITY);
> -	if (vht_elem && vht_elem->datalen >= sizeof(struct ieee80211_vht_cap))
> +	if (vht_elem && vht_elem->datalen >= sizeof(struct ieee80211_vht_cap)) {
>   		memcpy(&assoc_data->ap_vht_cap, vht_elem->data,
>   		       sizeof(struct ieee80211_vht_cap));
> -	else if (is_5ghz)
> +	} else if (is_5ghz) {
> +		sdata_info(sdata, "VHT capa missing/short, disabling VHT/HE\n");
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT |
>   				IEEE80211_STA_DISABLE_HE;
> +	}
>   	rcu_read_unlock();
>   
>   	if (WARN((sdata->vif.driver_flags & IEEE80211_VIF_SUPPORTS_UAPSD) &&
> @@ -5763,16 +5779,21 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
>   	}
>   
>   	if (req->flags & ASSOC_REQ_DISABLE_HT) {
> +		mlme_dbg(sdata, "HT disabled by flag, disabling HT/VHT/HE\n");
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
>   	}
>   
> -	if (req->flags & ASSOC_REQ_DISABLE_VHT)
> +	if (req->flags & ASSOC_REQ_DISABLE_VHT) {
> +		mlme_dbg(sdata, "VHT disabled by flag, disabling VHT\n");
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
> +	}
>   
> -	if (req->flags & ASSOC_REQ_DISABLE_HE)
> +	if (req->flags & ASSOC_REQ_DISABLE_HE) {
> +		mlme_dbg(sdata, "HE disabled by flag, disabling VHT\n");
>   		ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
> +	}
>   
>   	err = ieee80211_prep_connection(sdata, req->bss, true, override);
>   	if (err)
> 


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work

[Kalle Valo]

Luca Coelho <luca@coelho.fi> writes:

> From: Ilan Peer <ilan.peer@intel.com>
>
> The function cfg80211_reg_can_beacon_relax() expects wiphy
> mutex to be held when it is being called. However, when
> reg_leave_invalid_chans() is called the mutex is not held.
> Fix it by acquiring the lock before calling the function.
>
> Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
> Signed-off-by: Ilan Peer <ilan.peer@intel.com>
> Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")

Duplicate Fixes tags.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel

[Luca Coelho]

On Mon, 2021-11-29 at 15:32 +0200, Luca Coelho wrote:
> From: Ayala Beker <ayala.beker@intel.com>
> 
> A non-collocated AP whose primary channel is not a PSC channel
> may transmit a duplicated beacon on the corresponding PSC channel
> in which it would indicate its true primary channel.
> Use this inforamtion contained in the HE operation IE to determine
> the primary channel of the AP.
> In case of invalid infomration ignore it and use the channel
> the frame was received on.
> 
> Signed-off-by: Ayala Beker <ayala.beker@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---

As you know already (but for the record), this is totally broken. 
There were some conflicts due to the refactor that happened in this
function and I accidentally ran the wrong script to test compilation
before sending this series out... :(

V2 coming up soon.

--
Cheers,
Luca.

[PATCH v2] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel

[Luca Coelho]

From: Ayala Beker <ayala.beker@intel.com>

A non-collocated AP whose primary channel is not a PSC channel
may transmit a duplicated beacon on the corresponding PSC channel
in which it would indicate its true primary channel.
Use this inforamtion contained in the HE operation IE to determine
the primary channel of the AP.
In case of invalid infomration ignore it and use the channel
the frame was received on.

Signed-off-by: Ayala Beker <ayala.beker@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---

In v2, I  changed the patch so it will fit the code after the refactor.  This includes:

    * add the ftype to cfg80211_get_ies_channel_number();
    * return the channel number if !beacon or it's a dup beacon;
    * move the other checks to the caller function.

include/net/cfg80211.h | 24 ++++++++++---------
 net/wireless/scan.c    | 54 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 60 insertions(+), 18 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index b9b269504591..28f135873286 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -6385,17 +6385,6 @@ static inline void cfg80211_gen_new_bssid(const u8 *bssid, u8 max_bssid,
 	u64_to_ether_addr(new_bssid_u64, new_bssid);
 }
 
-/**
- * cfg80211_get_ies_channel_number - returns the channel number from ies
- * @ie: IEs
- * @ielen: length of IEs
- * @band: enum nl80211_band of the channel
- *
- * Returns the channel number, or -1 if none could be determined.
- */
-int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
-				    enum nl80211_band band);
-
 /**
  * cfg80211_is_element_inherited - returns if element ID should be inherited
  * @element: element to check
@@ -6431,6 +6420,19 @@ enum cfg80211_bss_frame_type {
 	CFG80211_BSS_FTYPE_PRESP,
 };
 
+/**
+ * cfg80211_get_ies_channel_number - returns the channel number from ies
+ * @ie: IEs
+ * @ielen: length of IEs
+ * @band: enum nl80211_band of the channel
+ * @ftype: frame type
+ *
+ * Returns the channel number, or -1 if none could be determined.
+ */
+int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
+				    enum nl80211_band band,
+				    enum cfg80211_bss_frame_type ftype);
+
 /**
  * cfg80211_inform_bss_data - inform cfg80211 of a new BSS
  *
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 22e92be61938..350d90d7b01c 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -1795,12 +1795,31 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
 }
 
 int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
-				    enum nl80211_band band)
+				    enum nl80211_band band,
+				    enum cfg80211_bss_frame_type ftype)
 {
 	const u8 *tmp;
 	int channel_number = -1;
 
-	if (band == NL80211_BAND_S1GHZ) {
+	if (band == NL80211_BAND_6GHZ) {
+		const struct element *elem;
+
+		elem = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, ie,
+					      ielen);
+		if (elem && elem->datalen >= ieee80211_he_oper_size(&elem->data[1])) {
+			struct ieee80211_he_operation *he_oper =
+				(void *)(&elem->data[1]);
+			const struct ieee80211_he_6ghz_oper *he_6ghz_oper;
+
+			he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper);
+			if (!he_6ghz_oper)
+				return channel_number;
+
+			if (ftype != CFG80211_BSS_FTYPE_BEACON ||
+			    he_6ghz_oper->control & IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON)
+				channel_number = he_6ghz_oper->primary;
+		}
+	} else if (band == NL80211_BAND_S1GHZ) {
 		tmp = cfg80211_find_ie(WLAN_EID_S1G_OPERATION, ie, ielen);
 		if (tmp && tmp[1] >= sizeof(struct ieee80211_s1g_oper_ie)) {
 			struct ieee80211_s1g_oper_ie *s1gop = (void *)(tmp + 2);
@@ -1831,18 +1850,20 @@ EXPORT_SYMBOL(cfg80211_get_ies_channel_number);
  * from neighboring channels and the Beacon frames use the DSSS Parameter Set
  * element to indicate the current (transmitting) channel, but this might also
  * be needed on other bands if RX frequency does not match with the actual
- * operating channel of a BSS.
+ * operating channel of a BSS, or if the AP reports a different primary channel.
  */
 static struct ieee80211_channel *
 cfg80211_get_bss_channel(struct wiphy *wiphy, const u8 *ie, size_t ielen,
 			 struct ieee80211_channel *channel,
-			 enum nl80211_bss_scan_width scan_width)
+			 enum nl80211_bss_scan_width scan_width,
+			 enum cfg80211_bss_frame_type ftype)
 {
 	u32 freq;
 	int channel_number;
 	struct ieee80211_channel *alt_channel;
 
-	channel_number = cfg80211_get_ies_channel_number(ie, ielen, channel->band);
+	channel_number = cfg80211_get_ies_channel_number(ie, ielen,
+							 channel->band, ftype);
 
 	if (channel_number < 0) {
 		/* No channel information in frame payload */
@@ -1850,6 +1871,16 @@ cfg80211_get_bss_channel(struct wiphy *wiphy, const u8 *ie, size_t ielen,
 	}
 
 	freq = ieee80211_channel_to_freq_khz(channel_number, channel->band);
+
+	/*
+	 * In 6GHz, duplicated beacon indication is relevant for
+	 * beacons only.
+	 */
+	if (channel->band == NL80211_BAND_6GHZ &&
+	    (freq == channel->center_freq ||
+	     abs(freq - channel->center_freq) > 80))
+		return channel;
+
 	alt_channel = ieee80211_get_channel_khz(wiphy, freq);
 	if (!alt_channel) {
 		if (channel->band == NL80211_BAND_2GHZ) {
@@ -1911,7 +1942,7 @@ cfg80211_inform_single_bss_data(struct wiphy *wiphy,
 		return NULL;
 
 	channel = cfg80211_get_bss_channel(wiphy, ie, ielen, data->chan,
-					   data->scan_width);
+					   data->scan_width, ftype);
 	if (!channel)
 		return NULL;
 
@@ -2333,6 +2364,7 @@ cfg80211_inform_single_bss_frame_data(struct wiphy *wiphy,
 	size_t ielen, min_hdr_len = offsetof(struct ieee80211_mgmt,
 					     u.probe_resp.variable);
 	int bss_type;
+	enum cfg80211_bss_frame_type ftype;
 
 	BUILD_BUG_ON(offsetof(struct ieee80211_mgmt, u.probe_resp.variable) !=
 			offsetof(struct ieee80211_mgmt, u.beacon.variable));
@@ -2369,8 +2401,16 @@ cfg80211_inform_single_bss_frame_data(struct wiphy *wiphy,
 			variable = ext->u.s1g_beacon.variable;
 	}
 
+	if (ieee80211_is_beacon(mgmt->frame_control))
+		ftype = CFG80211_BSS_FTYPE_BEACON;
+	else if (ieee80211_is_probe_resp(mgmt->frame_control))
+		ftype = CFG80211_BSS_FTYPE_PRESP;
+	else
+		ftype = CFG80211_BSS_FTYPE_UNKNOWN;
+
 	channel = cfg80211_get_bss_channel(wiphy, variable,
-					   ielen, data->chan, data->scan_width);
+					   ielen, data->chan, data->scan_width,
+					   ftype);
 	if (!channel)
 		return NULL;
 
-- 
2.33.1

[PATCH v2] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock

[Luca Coelho]

From: Johannes Berg <johannes.berg@intel.com>

When we call ieee80211_agg_start_txq(), that will in turn call
schedule_and_wake_txq(). Called from ieee80211_stop_tx_ba_cb()
this is done under sta->lock, which leads to certain circular
lock dependencies, as reported by Chris Murphy:
https://lore.kernel.org/r/CAJCQCtSXJ5qA4bqSPY=oLRMbv-irihVvP7A2uGutEbXQVkoNaw@mail.gmail.com

In general, ieee80211_agg_start_txq() is usually not called
with sta->lock held, only in this one place. But it's always
called with sta->ampdu_mlme.mtx held, and that's therefore
clearly sufficient.

Change ieee80211_stop_tx_ba_cb() to also call it without the
sta->lock held, by factoring it out of ieee80211_remove_tid_tx()
(which is only called in this one place).

This breaks the locking chain and makes it less likely that
we'll have similar locking chain problems in the future.

Fixes: ba8c3d6f16a1 ("mac80211: add an intermediate software queue implementation")
Reported-by: Chris Murphy <lists@colorremedies.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---

In v2:
    * Added fixes tag.

net/mac80211/agg-tx.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index 430a58587538..4dd56daed89b 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -9,7 +9,7 @@
  * Copyright 2007, Michael Wu <flamingice@sourmilk.net>
  * Copyright 2007-2010, Intel Corporation
  * Copyright(c) 2015-2017 Intel Deutschland GmbH
- * Copyright (C) 2018 - 2020 Intel Corporation
+ * Copyright (C) 2018 - 2021 Intel Corporation
  */
 
 #include <linux/ieee80211.h>
@@ -213,6 +213,8 @@ ieee80211_agg_start_txq(struct sta_info *sta, int tid, bool enable)
 	struct ieee80211_txq *txq = sta->sta.txq[tid];
 	struct txq_info *txqi;
 
+	lockdep_assert_held(&sta->ampdu_mlme.mtx);
+
 	if (!txq)
 		return;
 
@@ -290,7 +292,6 @@ static void ieee80211_remove_tid_tx(struct sta_info *sta, int tid)
 	ieee80211_assign_tid_tx(sta, tid, NULL);
 
 	ieee80211_agg_splice_finish(sta->sdata, tid);
-	ieee80211_agg_start_txq(sta, tid, false);
 
 	kfree_rcu(tid_tx, rcu_head);
 }
@@ -889,6 +890,7 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
 {
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
 	bool send_delba = false;
+	bool start_txq = false;
 
 	ht_dbg(sdata, "Stopping Tx BA session for %pM tid %d\n",
 	       sta->sta.addr, tid);
@@ -906,10 +908,14 @@ void ieee80211_stop_tx_ba_cb(struct sta_info *sta, int tid,
 		send_delba = true;
 
 	ieee80211_remove_tid_tx(sta, tid);
+	start_txq = true;
 
  unlock_sta:
 	spin_unlock_bh(&sta->lock);
 
+	if (start_txq)
+		ieee80211_agg_start_txq(sta, tid, false);
+
 	if (send_delba)
 		ieee80211_send_delba(sdata, sta->sta.addr, tid,
 			WLAN_BACK_INITIATOR, WLAN_REASON_QSTA_NOT_USE);
-- 
2.33.1

Re: [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work

[Luca Coelho]

On Wed, 2021-12-01 at 15:47 +0200, Kalle Valo wrote:
> Luca Coelho <luca@coelho.fi> writes:
> 
> > From: Ilan Peer <ilan.peer@intel.com>
> > 
> > The function cfg80211_reg_can_beacon_relax() expects wiphy
> > mutex to be held when it is being called. However, when
> > reg_leave_invalid_chans() is called the mutex is not held.
> > Fix it by acquiring the lock before calling the function.
> > 
> > Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
> > Signed-off-by: Ilan Peer <ilan.peer@intel.com>
> > Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
> 
> Duplicate Fixes tags.

Thanks for noticing!

My script adds the tag automatically, but it doesn't check (yet)
whether the tag was already there... I'll send v2 without it.

--
Cheers,
Luca.

[PATCH v2] cfg80211: Acquire wiphy mutex on regulatory work

[Luca Coelho]

From: Ilan Peer <ilan.peer@intel.com>

The function cfg80211_reg_can_beacon_relax() expects wiphy
mutex to be held when it is being called. However, when
reg_leave_invalid_chans() is called the mutex is not held.
Fix it by acquiring the lock before calling the function.

Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---

In v2:
    * Remove duplicate Fixes tag.

net/wireless/reg.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index df87c7f3a049..6f6da1cedd80 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -2338,6 +2338,7 @@ static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev)
 	struct cfg80211_chan_def chandef = {};
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
 	enum nl80211_iftype iftype;
+	bool ret;
 
 	wdev_lock(wdev);
 	iftype = wdev->iftype;
@@ -2387,7 +2388,11 @@ static bool reg_wdev_chan_valid(struct wiphy *wiphy, struct wireless_dev *wdev)
 	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_P2P_GO:
 	case NL80211_IFTYPE_ADHOC:
-		return cfg80211_reg_can_beacon_relax(wiphy, &chandef, iftype);
+		wiphy_lock(wiphy);
+		ret = cfg80211_reg_can_beacon_relax(wiphy, &chandef, iftype);
+		wiphy_unlock(wiphy);
+
+		return ret;
 	case NL80211_IFTYPE_STATION:
 	case NL80211_IFTYPE_P2P_CLIENT:
 		return cfg80211_chandef_usable(wiphy, &chandef,
@@ -2409,7 +2414,6 @@ static void reg_leave_invalid_chans(struct wiphy *wiphy)
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wiphy);
 
 	ASSERT_RTNL();
-
 	list_for_each_entry(wdev, &rdev->wiphy.wdev_list, list)
 		if (!reg_wdev_chan_valid(wiphy, wdev))
 			cfg80211_leave(rdev, wdev);
-- 
2.33.1