All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29
@ 2021-11-29 13:32 Luca Coelho
  2021-11-29 13:32 ` [PATCH 01/16] mac80211: add more HT/VHT/HE state logging Luca Coelho
                   ` (15 more replies)
  0 siblings, 16 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 01/16] mac80211: add more HT/VHT/HE state logging
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-30  9:05   ` kernel test robot
                     ` (3 more replies)
  2021-11-29 13:32 ` [PATCH 02/16] cfg80211: Add support for notifying association comeback Luca Coelho
                   ` (14 subsequent siblings)
  15 siblings, 4 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/16] cfg80211: Add support for notifying association comeback
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
  2021-11-29 13:32 ` [PATCH 01/16] mac80211: add more HT/VHT/HE state logging Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 03/16] mac80211: Notify cfg80211 about " Luca Coelho
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/16] mac80211: Notify cfg80211 about association comeback
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
  2021-11-29 13:32 ` [PATCH 01/16] mac80211: add more HT/VHT/HE state logging Luca Coelho
  2021-11-29 13:32 ` [PATCH 02/16] cfg80211: Add support for notifying association comeback Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel Luca Coelho
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (2 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 03/16] mac80211: Notify cfg80211 about " Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-30  8:14   ` kernel test robot
                     ` (4 more replies)
  2021-11-29 13:32 ` [PATCH 05/16] [BUGFIX] cfg80211: check fixed size before ieee80211_he_oper_size() Luca Coelho
                   ` (11 subsequent siblings)
  15 siblings, 5 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/16] [BUGFIX] cfg80211: check fixed size before ieee80211_he_oper_size()
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (3 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 06/16] mac80211: introduce channel switch disconnect function Luca Coelho
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/16] mac80211: introduce channel switch disconnect function
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (4 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 05/16] [BUGFIX] cfg80211: check fixed size before ieee80211_he_oper_size() Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 07/16] mac80211: mark TX-during-stop for TX in in_reconfig Luca Coelho
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/16] mac80211: mark TX-during-stop for TX in in_reconfig
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (5 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 06/16] mac80211: introduce channel switch disconnect function Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 08/16] mac80211: do drv_reconfig_complete() before restarting all Luca Coelho
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/16] mac80211: do drv_reconfig_complete() before restarting all
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (6 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 07/16] mac80211: mark TX-during-stop for TX in in_reconfig Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 09/16] cfg80211: Fix order of enum nl80211_band_iftype_attr documentation Luca Coelho
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/16] cfg80211: Fix order of enum nl80211_band_iftype_attr documentation
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (7 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 08/16] mac80211: do drv_reconfig_complete() before restarting all Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 10/16] mac80211: update channel context before station state Luca Coelho
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/16] mac80211: update channel context before station state
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (8 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 09/16] cfg80211: Fix order of enum nl80211_band_iftype_attr documentation Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 11/16] cfg80211: simplify cfg80211_chandef_valid() Luca Coelho
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/16] cfg80211: simplify cfg80211_chandef_valid()
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (9 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 10/16] mac80211: update channel context before station state Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 12/16] mac80211: Remove a couple of obsolete TODO Luca Coelho
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 12/16] mac80211: Remove a couple of obsolete TODO
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (10 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 11/16] cfg80211: simplify cfg80211_chandef_valid() Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 13/16] mac80211: Fix the size used for building probe request Luca Coelho
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 13/16] mac80211: Fix the size used for building probe request
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (11 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 12/16] mac80211: Remove a couple of obsolete TODO Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 14/16] mac80211: fix lookup when adding AddBA extension element Luca Coelho
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 14/16] mac80211: fix lookup when adding AddBA extension element
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (12 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 13/16] mac80211: Fix the size used for building probe request Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:32 ` [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock Luca Coelho
  2021-11-29 13:32 ` [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work Luca Coelho
  15 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (13 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 14/16] mac80211: fix lookup when adding AddBA extension element Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-11-29 13:54   ` Toke Høiland-Jørgensen
  2021-12-02 13:26   ` [PATCH v2] " Luca Coelho
  2021-11-29 13:32 ` [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work Luca Coelho
  15 siblings, 2 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work
  2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
                   ` (14 preceding siblings ...)
  2021-11-29 13:32 ` [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock Luca Coelho
@ 2021-11-29 13:32 ` Luca Coelho
  2021-12-01 13:47   ` Kalle Valo
  2021-12-02 13:28   ` [PATCH v2] " Luca Coelho
  15 siblings, 2 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-29 13:32 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-29 13:32 ` [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock Luca Coelho
@ 2021-11-29 13:54   ` Toke Høiland-Jørgensen
  2021-11-30 11:12     ` Luca Coelho
  2021-12-02 13:26   ` [PATCH v2] " Luca Coelho
  1 sibling, 1 reply; 40+ messages in thread
From: Toke Høiland-Jørgensen @ 2021-11-29 13:54 UTC (permalink / raw)
  To: Luca Coelho, johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel
  2021-11-29 13:32 ` [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel Luca Coelho
@ 2021-11-30  8:14   ` kernel test robot
  2021-11-30 11:18     ` kernel test robot
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 40+ messages in thread
From: kernel test robot @ 2021-11-30  8:14 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 11886 bytes --]

Hi Luca,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jberg-mac80211-next/master]
[also build test ERROR on next-20211129]
[cannot apply to jberg-mac80211/master linus/master v5.16-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20211130/202111301626.sAKhr9zO-lkp(a)intel.com/config)
compiler: sh4-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/da233d0b8179f2db6b1f5e783da1c0b986cc8e96
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
        git checkout da233d0b8179f2db6b1f5e783da1c0b986cc8e96
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash drivers/clocksource/ net/wireless/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/wireless/scan.c: In function 'cfg80211_get_ies_channel_number':
>> net/wireless/scan.c:1801:13: error: 'channel' undeclared (first use in this function)
    1801 |         if (channel->band == NL80211_BAND_6GHZ) {
         |             ^~~~~~~
   net/wireless/scan.c:1801:13: note: each undeclared identifier is reported only once for each function it appears in
>> net/wireless/scan.c:1815:25: error: 'freq' undeclared (first use in this function); did you mean 'ifreq'?
    1815 |                         freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
         |                         ^~~~
         |                         ifreq
   In file included from include/linux/kernel.h:16,
                    from net/wireless/scan.c:10:
   include/linux/math.h:136:17: error: first argument to '__builtin_choose_expr' not a constant
     136 |                 __builtin_choose_expr(                                  \
         |                 ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:61: note: in definition of macro '__abs_choose_expr'
     144 |         ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                             ^~~~~
   include/linux/math.h:132:17: note: in expansion of macro '__abs_choose_expr'
     132 |                 __abs_choose_expr(x, long,                              \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:133:17: note: in expansion of macro '__abs_choose_expr'
     133 |                 __abs_choose_expr(x, int,                               \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:134:17: note: in expansion of macro '__abs_choose_expr'
     134 |                 __abs_choose_expr(x, short,                             \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:135:17: note: in expansion of macro '__abs_choose_expr'
     135 |                 __abs_choose_expr(x, char,                              \
         |                 ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:29: note: in expansion of macro 'abs'
    1822 |                             abs(freq - channel->center_freq) <= 80 &&
         |                             ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr(        \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:61: note: in definition of macro '__abs_choose_expr'
     144 |         ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                             ^~~~~
   include/linux/math.h:132:17: note: in expansion of macro '__abs_choose_expr'
     132 |                 __abs_choose_expr(x, long,                              \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:133:17: note: in expansion of macro '__abs_choose_expr'
     133 |                 __abs_choose_expr(x, int,                               \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:134:17: note: in expansion of macro '__abs_choose_expr'
     134 |                 __abs_choose_expr(x, short,                             \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:135:17: note: in expansion of macro '__abs_choose_expr'
     135 |                 __abs_choose_expr(x, char,                              \
         |                 ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:29: note: in expansion of macro 'abs'
    1822 |                             abs(freq - channel->center_freq) <= 80 &&
         |                             ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr(        \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:61: note: in definition of macro '__abs_choose_expr'
     144 |         ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                             ^~~~~
   include/linux/math.h:132:17: note: in expansion of macro '__abs_choose_expr'
     132 |                 __abs_choose_expr(x, long,                              \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:133:17: note: in expansion of macro '__abs_choose_expr'
     133 |                 __abs_choose_expr(x, int,                               \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:134:17: note: in expansion of macro '__abs_choose_expr'
     134 |                 __abs_choose_expr(x, short,                             \
         |                 ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:29: note: in expansion of macro 'abs'
    1822 |                             abs(freq - channel->center_freq) <= 80 &&
         |                             ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr(        \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:61: note: in definition of macro '__abs_choose_expr'
     144 |         ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                             ^~~~~
   include/linux/math.h:132:17: note: in expansion of macro '__abs_choose_expr'
     132 |                 __abs_choose_expr(x, long,                              \
         |                 ^~~~~~~~~~~~~~~~~
   include/linux/math.h:133:17: note: in expansion of macro '__abs_choose_expr'
     133 |                 __abs_choose_expr(x, int,                               \
         |                 ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:29: note: in expansion of macro 'abs'
    1822 |                             abs(freq - channel->center_freq) <= 80 &&
         |                             ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr(        \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:61: note: in definition of macro '__abs_choose_expr'
     144 |         ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                             ^~~~~
   include/linux/math.h:132:17: note: in expansion of macro '__abs_choose_expr'
     132 |                 __abs_choose_expr(x, long,                              \
         |                 ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:29: note: in expansion of macro 'abs'
    1822 |                             abs(freq - channel->center_freq) <= 80 &&
         |                             ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr(        \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:131:17: note: in expansion of macro '__abs_choose_expr'
     131 | #define abs(x)  __abs_choose_expr(x, long long,                         \
         |                 ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:29: note: in expansion of macro 'abs'
    1822 |                             abs(freq - channel->center_freq) <= 80 &&


vim +/channel +1801 net/wireless/scan.c

  1794	
  1795	int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
  1796					    enum nl80211_band band)
  1797	{
  1798		const u8 *tmp;
  1799		int channel_number = -1;
  1800	
> 1801		if (channel->band == NL80211_BAND_6GHZ) {
  1802			const struct element *elem;
  1803	
  1804			elem = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, ie,
  1805						      ielen);
  1806			if (elem && elem->datalen >= ieee80211_he_oper_size(&elem->data[1])) {
  1807				struct ieee80211_he_operation *he_oper =
  1808					(void *)(&elem->data[1]);
  1809				const struct ieee80211_he_6ghz_oper *he_6ghz_oper;
  1810	
  1811				he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper);
  1812				if (!he_6ghz_oper)
  1813					return channel;
  1814	
> 1815				freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
  1816								      NL80211_BAND_6GHZ);
  1817	
  1818				/* duplicated beacon indication is relevant for beacons
  1819				 * only
  1820				 */
  1821				if (freq != channel->center_freq &&
  1822				    abs(freq - channel->center_freq) <= 80 &&
  1823				    (ftype != CFG80211_BSS_FTYPE_BEACON ||
  1824				     he_6ghz_oper->control & IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON))
  1825					channel_number = he_6ghz_oper->primary;
  1826			}
  1827		} else if (band == NL80211_BAND_S1GHZ) {
  1828			tmp = cfg80211_find_ie(WLAN_EID_S1G_OPERATION, ie, ielen);
  1829			if (tmp && tmp[1] >= sizeof(struct ieee80211_s1g_oper_ie)) {
  1830				struct ieee80211_s1g_oper_ie *s1gop = (void *)(tmp + 2);
  1831	
  1832				channel_number = s1gop->primary_ch;
  1833			}
  1834		} else {
  1835			tmp = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ie, ielen);
  1836			if (tmp && tmp[1] == 1) {
  1837				channel_number = tmp[2];
  1838			} else {
  1839				tmp = cfg80211_find_ie(WLAN_EID_HT_OPERATION, ie, ielen);
  1840				if (tmp && tmp[1] >= sizeof(struct ieee80211_ht_operation)) {
  1841					struct ieee80211_ht_operation *htop = (void *)(tmp + 2);
  1842	
  1843					channel_number = htop->primary_chan;
  1844				}
  1845			}
  1846		}
  1847	
  1848		return channel_number;
  1849	}
  1850	EXPORT_SYMBOL(cfg80211_get_ies_channel_number);
  1851	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/16] mac80211: add more HT/VHT/HE state logging
  2021-11-29 13:32 ` [PATCH 01/16] mac80211: add more HT/VHT/HE state logging Luca Coelho
@ 2021-11-30  9:05   ` kernel test robot
  2021-11-30  9:26     ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 40+ messages in thread
From: kernel test robot @ 2021-11-30  9:05 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 9195 bytes --]

Hi Luca,

I love your patch! Yet something to improve:

[auto build test ERROR on jberg-mac80211-next/master]
[also build test ERROR on jberg-mac80211/master linus/master v5.16-rc3 next-20211129]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20211130/202111301700.jAkdP74U-lkp(a)intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/8275a35cfbfc5c94966b49077447b828f24c14d5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
        git checkout 8275a35cfbfc5c94966b49077447b828f24c14d5
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash arch/arm/common/ drivers/pinctrl/qcom/ net/mac80211/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/mac80211/mlme.c: In function 'ieee80211_prep_channel':
>> net/mac80211/mlme.c:5085:29: error: 'vht_cap' undeclared (first use in this function); did you mean 'ht_cap'?
    5085 |                         if (vht_cap)
         |                             ^~~~~~~
         |                             ht_cap
   net/mac80211/mlme.c:5085:29: note: each undeclared identifier is reported only once for each function it appears in


vim +5085 net/mac80211/mlme.c

  5005	
  5006	static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
  5007					  struct cfg80211_bss *cbss)
  5008	{
  5009		struct ieee80211_local *local = sdata->local;
  5010		struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
  5011		const struct ieee80211_ht_cap *ht_cap = NULL;
  5012		const struct ieee80211_ht_operation *ht_oper = NULL;
  5013		const struct ieee80211_vht_operation *vht_oper = NULL;
  5014		const struct ieee80211_he_operation *he_oper = NULL;
  5015		const struct ieee80211_s1g_oper_ie *s1g_oper = NULL;
  5016		struct ieee80211_supported_band *sband;
  5017		struct cfg80211_chan_def chandef;
  5018		bool is_6ghz = cbss->channel->band == NL80211_BAND_6GHZ;
  5019		bool is_5ghz = cbss->channel->band == NL80211_BAND_5GHZ;
  5020		struct ieee80211_bss *bss = (void *)cbss->priv;
  5021		struct ieee802_11_elems *elems;
  5022		const struct cfg80211_bss_ies *ies;
  5023		int ret;
  5024		u32 i;
  5025		bool have_80mhz;
  5026	
  5027		rcu_read_lock();
  5028	
  5029		ies = rcu_dereference(cbss->ies);
  5030		elems = ieee802_11_parse_elems(ies->data, ies->len, false,
  5031					       NULL, NULL);
  5032		if (!elems) {
  5033			rcu_read_unlock();
  5034			return -ENOMEM;
  5035		}
  5036	
  5037		sband = local->hw.wiphy->bands[cbss->channel->band];
  5038	
  5039		ifmgd->flags &= ~(IEEE80211_STA_DISABLE_40MHZ |
  5040				  IEEE80211_STA_DISABLE_80P80MHZ |
  5041				  IEEE80211_STA_DISABLE_160MHZ);
  5042	
  5043		/* disable HT/VHT/HE if we don't support them */
  5044		if (!sband->ht_cap.ht_supported && !is_6ghz) {
  5045			mlme_dbg(sdata, "HT not supported, disabling HT/VHT/HE\n");
  5046			ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5047			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5048			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5049		}
  5050	
  5051		if (!sband->vht_cap.vht_supported && is_5ghz) {
  5052			mlme_dbg(sdata, "VHT not supported, disabling VHT/HE\n");
  5053			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5054			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5055		}
  5056	
  5057		if (!ieee80211_get_he_iftype_cap(sband,
  5058						 ieee80211_vif_type_p2p(&sdata->vif))) {
  5059			mlme_dbg(sdata, "HE not supported, disabling it\n");
  5060			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5061		}
  5062	
  5063		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HT) && !is_6ghz) {
  5064			ht_oper = elems->ht_operation;
  5065			ht_cap = elems->ht_cap_elem;
  5066	
  5067			if (!ht_cap) {
  5068				ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5069				ht_oper = NULL;
  5070			}
  5071		}
  5072	
  5073		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT) && !is_6ghz) {
  5074			vht_oper = elems->vht_operation;
  5075			if (vht_oper && !ht_oper) {
  5076				vht_oper = NULL;
  5077				sdata_info(sdata,
  5078					   "AP advertised VHT without HT, disabling HT/VHT/HE\n");
  5079				ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5080				ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5081				ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5082			}
  5083	
  5084			if (!elems->vht_cap_elem) {
> 5085				if (vht_cap)
  5086					sdata_info(sdata,
  5087						   "bad VHT capabilities, disabling VHT\n");
  5088				ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5089				vht_oper = NULL;
  5090			}
  5091		}
  5092	
  5093		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE)) {
  5094			he_oper = elems->he_operation;
  5095	
  5096			if (is_6ghz) {
  5097				struct ieee80211_bss_conf *bss_conf;
  5098				u8 i, j = 0;
  5099	
  5100				bss_conf = &sdata->vif.bss_conf;
  5101	
  5102				if (elems->pwr_constr_elem)
  5103					bss_conf->pwr_reduction = *elems->pwr_constr_elem;
  5104	
  5105				BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
  5106					     ARRAY_SIZE(elems->tx_pwr_env));
  5107	
  5108				for (i = 0; i < elems->tx_pwr_env_num; i++) {
  5109					if (elems->tx_pwr_env_len[i] >
  5110					    sizeof(bss_conf->tx_pwr_env[j]))
  5111						continue;
  5112	
  5113					bss_conf->tx_pwr_env_num++;
  5114					memcpy(&bss_conf->tx_pwr_env[j], elems->tx_pwr_env[i],
  5115					       elems->tx_pwr_env_len[i]);
  5116					j++;
  5117				}
  5118			}
  5119	
  5120			if (!ieee80211_verify_sta_he_mcs_support(sdata, sband, he_oper))
  5121				ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5122		}
  5123	
  5124		/* Allow VHT if at least one channel on the sband supports 80 MHz */
  5125		have_80mhz = false;
  5126		for (i = 0; i < sband->n_channels; i++) {
  5127			if (sband->channels[i].flags & (IEEE80211_CHAN_DISABLED |
  5128							IEEE80211_CHAN_NO_80MHZ))
  5129				continue;
  5130	
  5131			have_80mhz = true;
  5132			break;
  5133		}
  5134	
  5135		if (!have_80mhz) {
  5136			sdata_info(sdata, "80 MHz not supported, disabling VHT\n");
  5137			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5138		}
  5139	
  5140		if (sband->band == NL80211_BAND_S1GHZ) {
  5141			s1g_oper = elems->s1g_oper;
  5142			if (!s1g_oper)
  5143				sdata_info(sdata,
  5144					   "AP missing S1G operation element?\n");
  5145		}
  5146	
  5147		ifmgd->flags |= ieee80211_determine_chantype(sdata, sband,
  5148							     cbss->channel,
  5149							     bss->vht_cap_info,
  5150							     ht_oper, vht_oper, he_oper,
  5151							     s1g_oper,
  5152							     &chandef, false);
  5153	
  5154		sdata->needed_rx_chains = min(ieee80211_ht_vht_rx_chains(sdata, cbss),
  5155					      local->rx_chains);
  5156	
  5157		rcu_read_unlock();
  5158		/* the element data was RCU protected so no longer valid anyway */
  5159		kfree(elems);
  5160		elems = NULL;
  5161	
  5162		if (ifmgd->flags & IEEE80211_STA_DISABLE_HE && is_6ghz) {
  5163			sdata_info(sdata, "Rejecting non-HE 6/7 GHz connection");
  5164			return -EINVAL;
  5165		}
  5166	
  5167		/* will change later if needed */
  5168		sdata->smps_mode = IEEE80211_SMPS_OFF;
  5169	
  5170		mutex_lock(&local->mtx);
  5171		/*
  5172		 * If this fails (possibly due to channel context sharing
  5173		 * on incompatible channels, e.g. 80+80 and 160 sharing the
  5174		 * same control channel) try to use a smaller bandwidth.
  5175		 */
  5176		ret = ieee80211_vif_use_channel(sdata, &chandef,
  5177						IEEE80211_CHANCTX_SHARED);
  5178	
  5179		/* don't downgrade for 5 and 10 MHz channels, though. */
  5180		if (chandef.width == NL80211_CHAN_WIDTH_5 ||
  5181		    chandef.width == NL80211_CHAN_WIDTH_10)
  5182			goto out;
  5183	
  5184		while (ret && chandef.width != NL80211_CHAN_WIDTH_20_NOHT) {
  5185			ifmgd->flags |= ieee80211_chandef_downgrade(&chandef);
  5186			ret = ieee80211_vif_use_channel(sdata, &chandef,
  5187							IEEE80211_CHANCTX_SHARED);
  5188		}
  5189	 out:
  5190		mutex_unlock(&local->mtx);
  5191		return ret;
  5192	}
  5193	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/16] mac80211: add more HT/VHT/HE state logging
  2021-11-29 13:32 ` [PATCH 01/16] mac80211: add more HT/VHT/HE state logging Luca Coelho
@ 2021-11-30  9:26     ` kernel test robot
  2021-11-30  9:26     ` kernel test robot
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 40+ messages in thread
From: kernel test robot @ 2021-11-30  9:26 UTC (permalink / raw)
  To: Luca Coelho; +Cc: llvm, kbuild-all

Hi Luca,

I love your patch! Yet something to improve:

[auto build test ERROR on jberg-mac80211-next/master]
[also build test ERROR on jberg-mac80211/master linus/master v5.16-rc3 next-20211130]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: riscv-randconfig-c006-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301728.gw8kkcVb-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/8275a35cfbfc5c94966b49077447b828f24c14d5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
        git checkout 8275a35cfbfc5c94966b49077447b828f24c14d5
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash net/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/mac80211/mlme.c:5085:8: error: use of undeclared identifier 'vht_cap'; did you mean 'ht_cap'?
                           if (vht_cap)
                               ^~~~~~~
                               ht_cap
   net/mac80211/mlme.c:5011:33: note: 'ht_cap' declared here
           const struct ieee80211_ht_cap *ht_cap = NULL;
                                          ^
   1 error generated.


vim +5085 net/mac80211/mlme.c

  5005	
  5006	static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
  5007					  struct cfg80211_bss *cbss)
  5008	{
  5009		struct ieee80211_local *local = sdata->local;
  5010		struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
  5011		const struct ieee80211_ht_cap *ht_cap = NULL;
  5012		const struct ieee80211_ht_operation *ht_oper = NULL;
  5013		const struct ieee80211_vht_operation *vht_oper = NULL;
  5014		const struct ieee80211_he_operation *he_oper = NULL;
  5015		const struct ieee80211_s1g_oper_ie *s1g_oper = NULL;
  5016		struct ieee80211_supported_band *sband;
  5017		struct cfg80211_chan_def chandef;
  5018		bool is_6ghz = cbss->channel->band == NL80211_BAND_6GHZ;
  5019		bool is_5ghz = cbss->channel->band == NL80211_BAND_5GHZ;
  5020		struct ieee80211_bss *bss = (void *)cbss->priv;
  5021		struct ieee802_11_elems *elems;
  5022		const struct cfg80211_bss_ies *ies;
  5023		int ret;
  5024		u32 i;
  5025		bool have_80mhz;
  5026	
  5027		rcu_read_lock();
  5028	
  5029		ies = rcu_dereference(cbss->ies);
  5030		elems = ieee802_11_parse_elems(ies->data, ies->len, false,
  5031					       NULL, NULL);
  5032		if (!elems) {
  5033			rcu_read_unlock();
  5034			return -ENOMEM;
  5035		}
  5036	
  5037		sband = local->hw.wiphy->bands[cbss->channel->band];
  5038	
  5039		ifmgd->flags &= ~(IEEE80211_STA_DISABLE_40MHZ |
  5040				  IEEE80211_STA_DISABLE_80P80MHZ |
  5041				  IEEE80211_STA_DISABLE_160MHZ);
  5042	
  5043		/* disable HT/VHT/HE if we don't support them */
  5044		if (!sband->ht_cap.ht_supported && !is_6ghz) {
  5045			mlme_dbg(sdata, "HT not supported, disabling HT/VHT/HE\n");
  5046			ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5047			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5048			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5049		}
  5050	
  5051		if (!sband->vht_cap.vht_supported && is_5ghz) {
  5052			mlme_dbg(sdata, "VHT not supported, disabling VHT/HE\n");
  5053			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5054			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5055		}
  5056	
  5057		if (!ieee80211_get_he_iftype_cap(sband,
  5058						 ieee80211_vif_type_p2p(&sdata->vif))) {
  5059			mlme_dbg(sdata, "HE not supported, disabling it\n");
  5060			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5061		}
  5062	
  5063		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HT) && !is_6ghz) {
  5064			ht_oper = elems->ht_operation;
  5065			ht_cap = elems->ht_cap_elem;
  5066	
  5067			if (!ht_cap) {
  5068				ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5069				ht_oper = NULL;
  5070			}
  5071		}
  5072	
  5073		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT) && !is_6ghz) {
  5074			vht_oper = elems->vht_operation;
  5075			if (vht_oper && !ht_oper) {
  5076				vht_oper = NULL;
  5077				sdata_info(sdata,
  5078					   "AP advertised VHT without HT, disabling HT/VHT/HE\n");
  5079				ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5080				ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5081				ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5082			}
  5083	
  5084			if (!elems->vht_cap_elem) {
> 5085				if (vht_cap)
  5086					sdata_info(sdata,
  5087						   "bad VHT capabilities, disabling VHT\n");
  5088				ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5089				vht_oper = NULL;
  5090			}
  5091		}
  5092	
  5093		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE)) {
  5094			he_oper = elems->he_operation;
  5095	
  5096			if (is_6ghz) {
  5097				struct ieee80211_bss_conf *bss_conf;
  5098				u8 i, j = 0;
  5099	
  5100				bss_conf = &sdata->vif.bss_conf;
  5101	
  5102				if (elems->pwr_constr_elem)
  5103					bss_conf->pwr_reduction = *elems->pwr_constr_elem;
  5104	
  5105				BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
  5106					     ARRAY_SIZE(elems->tx_pwr_env));
  5107	
  5108				for (i = 0; i < elems->tx_pwr_env_num; i++) {
  5109					if (elems->tx_pwr_env_len[i] >
  5110					    sizeof(bss_conf->tx_pwr_env[j]))
  5111						continue;
  5112	
  5113					bss_conf->tx_pwr_env_num++;
  5114					memcpy(&bss_conf->tx_pwr_env[j], elems->tx_pwr_env[i],
  5115					       elems->tx_pwr_env_len[i]);
  5116					j++;
  5117				}
  5118			}
  5119	
  5120			if (!ieee80211_verify_sta_he_mcs_support(sdata, sband, he_oper))
  5121				ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5122		}
  5123	
  5124		/* Allow VHT if at least one channel on the sband supports 80 MHz */
  5125		have_80mhz = false;
  5126		for (i = 0; i < sband->n_channels; i++) {
  5127			if (sband->channels[i].flags & (IEEE80211_CHAN_DISABLED |
  5128							IEEE80211_CHAN_NO_80MHZ))
  5129				continue;
  5130	
  5131			have_80mhz = true;
  5132			break;
  5133		}
  5134	
  5135		if (!have_80mhz) {
  5136			sdata_info(sdata, "80 MHz not supported, disabling VHT\n");
  5137			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5138		}
  5139	
  5140		if (sband->band == NL80211_BAND_S1GHZ) {
  5141			s1g_oper = elems->s1g_oper;
  5142			if (!s1g_oper)
  5143				sdata_info(sdata,
  5144					   "AP missing S1G operation element?\n");
  5145		}
  5146	
  5147		ifmgd->flags |= ieee80211_determine_chantype(sdata, sband,
  5148							     cbss->channel,
  5149							     bss->vht_cap_info,
  5150							     ht_oper, vht_oper, he_oper,
  5151							     s1g_oper,
  5152							     &chandef, false);
  5153	
  5154		sdata->needed_rx_chains = min(ieee80211_ht_vht_rx_chains(sdata, cbss),
  5155					      local->rx_chains);
  5156	
  5157		rcu_read_unlock();
  5158		/* the element data was RCU protected so no longer valid anyway */
  5159		kfree(elems);
  5160		elems = NULL;
  5161	
  5162		if (ifmgd->flags & IEEE80211_STA_DISABLE_HE && is_6ghz) {
  5163			sdata_info(sdata, "Rejecting non-HE 6/7 GHz connection");
  5164			return -EINVAL;
  5165		}
  5166	
  5167		/* will change later if needed */
  5168		sdata->smps_mode = IEEE80211_SMPS_OFF;
  5169	
  5170		mutex_lock(&local->mtx);
  5171		/*
  5172		 * If this fails (possibly due to channel context sharing
  5173		 * on incompatible channels, e.g. 80+80 and 160 sharing the
  5174		 * same control channel) try to use a smaller bandwidth.
  5175		 */
  5176		ret = ieee80211_vif_use_channel(sdata, &chandef,
  5177						IEEE80211_CHANCTX_SHARED);
  5178	
  5179		/* don't downgrade for 5 and 10 MHz channels, though. */
  5180		if (chandef.width == NL80211_CHAN_WIDTH_5 ||
  5181		    chandef.width == NL80211_CHAN_WIDTH_10)
  5182			goto out;
  5183	
  5184		while (ret && chandef.width != NL80211_CHAN_WIDTH_20_NOHT) {
  5185			ifmgd->flags |= ieee80211_chandef_downgrade(&chandef);
  5186			ret = ieee80211_vif_use_channel(sdata, &chandef,
  5187							IEEE80211_CHANCTX_SHARED);
  5188		}
  5189	 out:
  5190		mutex_unlock(&local->mtx);
  5191		return ret;
  5192	}
  5193	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/16] mac80211: add more HT/VHT/HE state logging
@ 2021-11-30  9:26     ` kernel test robot
  0 siblings, 0 replies; 40+ messages in thread
From: kernel test robot @ 2021-11-30  9:26 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 9313 bytes --]

Hi Luca,

I love your patch! Yet something to improve:

[auto build test ERROR on jberg-mac80211-next/master]
[also build test ERROR on jberg-mac80211/master linus/master v5.16-rc3 next-20211130]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: riscv-randconfig-c006-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301728.gw8kkcVb-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/8275a35cfbfc5c94966b49077447b828f24c14d5
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
        git checkout 8275a35cfbfc5c94966b49077447b828f24c14d5
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash net/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/mac80211/mlme.c:5085:8: error: use of undeclared identifier 'vht_cap'; did you mean 'ht_cap'?
                           if (vht_cap)
                               ^~~~~~~
                               ht_cap
   net/mac80211/mlme.c:5011:33: note: 'ht_cap' declared here
           const struct ieee80211_ht_cap *ht_cap = NULL;
                                          ^
   1 error generated.


vim +5085 net/mac80211/mlme.c

  5005	
  5006	static int ieee80211_prep_channel(struct ieee80211_sub_if_data *sdata,
  5007					  struct cfg80211_bss *cbss)
  5008	{
  5009		struct ieee80211_local *local = sdata->local;
  5010		struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
  5011		const struct ieee80211_ht_cap *ht_cap = NULL;
  5012		const struct ieee80211_ht_operation *ht_oper = NULL;
  5013		const struct ieee80211_vht_operation *vht_oper = NULL;
  5014		const struct ieee80211_he_operation *he_oper = NULL;
  5015		const struct ieee80211_s1g_oper_ie *s1g_oper = NULL;
  5016		struct ieee80211_supported_band *sband;
  5017		struct cfg80211_chan_def chandef;
  5018		bool is_6ghz = cbss->channel->band == NL80211_BAND_6GHZ;
  5019		bool is_5ghz = cbss->channel->band == NL80211_BAND_5GHZ;
  5020		struct ieee80211_bss *bss = (void *)cbss->priv;
  5021		struct ieee802_11_elems *elems;
  5022		const struct cfg80211_bss_ies *ies;
  5023		int ret;
  5024		u32 i;
  5025		bool have_80mhz;
  5026	
  5027		rcu_read_lock();
  5028	
  5029		ies = rcu_dereference(cbss->ies);
  5030		elems = ieee802_11_parse_elems(ies->data, ies->len, false,
  5031					       NULL, NULL);
  5032		if (!elems) {
  5033			rcu_read_unlock();
  5034			return -ENOMEM;
  5035		}
  5036	
  5037		sband = local->hw.wiphy->bands[cbss->channel->band];
  5038	
  5039		ifmgd->flags &= ~(IEEE80211_STA_DISABLE_40MHZ |
  5040				  IEEE80211_STA_DISABLE_80P80MHZ |
  5041				  IEEE80211_STA_DISABLE_160MHZ);
  5042	
  5043		/* disable HT/VHT/HE if we don't support them */
  5044		if (!sband->ht_cap.ht_supported && !is_6ghz) {
  5045			mlme_dbg(sdata, "HT not supported, disabling HT/VHT/HE\n");
  5046			ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5047			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5048			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5049		}
  5050	
  5051		if (!sband->vht_cap.vht_supported && is_5ghz) {
  5052			mlme_dbg(sdata, "VHT not supported, disabling VHT/HE\n");
  5053			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5054			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5055		}
  5056	
  5057		if (!ieee80211_get_he_iftype_cap(sband,
  5058						 ieee80211_vif_type_p2p(&sdata->vif))) {
  5059			mlme_dbg(sdata, "HE not supported, disabling it\n");
  5060			ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5061		}
  5062	
  5063		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HT) && !is_6ghz) {
  5064			ht_oper = elems->ht_operation;
  5065			ht_cap = elems->ht_cap_elem;
  5066	
  5067			if (!ht_cap) {
  5068				ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5069				ht_oper = NULL;
  5070			}
  5071		}
  5072	
  5073		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_VHT) && !is_6ghz) {
  5074			vht_oper = elems->vht_operation;
  5075			if (vht_oper && !ht_oper) {
  5076				vht_oper = NULL;
  5077				sdata_info(sdata,
  5078					   "AP advertised VHT without HT, disabling HT/VHT/HE\n");
  5079				ifmgd->flags |= IEEE80211_STA_DISABLE_HT;
  5080				ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5081				ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5082			}
  5083	
  5084			if (!elems->vht_cap_elem) {
> 5085				if (vht_cap)
  5086					sdata_info(sdata,
  5087						   "bad VHT capabilities, disabling VHT\n");
  5088				ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5089				vht_oper = NULL;
  5090			}
  5091		}
  5092	
  5093		if (!(ifmgd->flags & IEEE80211_STA_DISABLE_HE)) {
  5094			he_oper = elems->he_operation;
  5095	
  5096			if (is_6ghz) {
  5097				struct ieee80211_bss_conf *bss_conf;
  5098				u8 i, j = 0;
  5099	
  5100				bss_conf = &sdata->vif.bss_conf;
  5101	
  5102				if (elems->pwr_constr_elem)
  5103					bss_conf->pwr_reduction = *elems->pwr_constr_elem;
  5104	
  5105				BUILD_BUG_ON(ARRAY_SIZE(bss_conf->tx_pwr_env) !=
  5106					     ARRAY_SIZE(elems->tx_pwr_env));
  5107	
  5108				for (i = 0; i < elems->tx_pwr_env_num; i++) {
  5109					if (elems->tx_pwr_env_len[i] >
  5110					    sizeof(bss_conf->tx_pwr_env[j]))
  5111						continue;
  5112	
  5113					bss_conf->tx_pwr_env_num++;
  5114					memcpy(&bss_conf->tx_pwr_env[j], elems->tx_pwr_env[i],
  5115					       elems->tx_pwr_env_len[i]);
  5116					j++;
  5117				}
  5118			}
  5119	
  5120			if (!ieee80211_verify_sta_he_mcs_support(sdata, sband, he_oper))
  5121				ifmgd->flags |= IEEE80211_STA_DISABLE_HE;
  5122		}
  5123	
  5124		/* Allow VHT if at least one channel on the sband supports 80 MHz */
  5125		have_80mhz = false;
  5126		for (i = 0; i < sband->n_channels; i++) {
  5127			if (sband->channels[i].flags & (IEEE80211_CHAN_DISABLED |
  5128							IEEE80211_CHAN_NO_80MHZ))
  5129				continue;
  5130	
  5131			have_80mhz = true;
  5132			break;
  5133		}
  5134	
  5135		if (!have_80mhz) {
  5136			sdata_info(sdata, "80 MHz not supported, disabling VHT\n");
  5137			ifmgd->flags |= IEEE80211_STA_DISABLE_VHT;
  5138		}
  5139	
  5140		if (sband->band == NL80211_BAND_S1GHZ) {
  5141			s1g_oper = elems->s1g_oper;
  5142			if (!s1g_oper)
  5143				sdata_info(sdata,
  5144					   "AP missing S1G operation element?\n");
  5145		}
  5146	
  5147		ifmgd->flags |= ieee80211_determine_chantype(sdata, sband,
  5148							     cbss->channel,
  5149							     bss->vht_cap_info,
  5150							     ht_oper, vht_oper, he_oper,
  5151							     s1g_oper,
  5152							     &chandef, false);
  5153	
  5154		sdata->needed_rx_chains = min(ieee80211_ht_vht_rx_chains(sdata, cbss),
  5155					      local->rx_chains);
  5156	
  5157		rcu_read_unlock();
  5158		/* the element data was RCU protected so no longer valid anyway */
  5159		kfree(elems);
  5160		elems = NULL;
  5161	
  5162		if (ifmgd->flags & IEEE80211_STA_DISABLE_HE && is_6ghz) {
  5163			sdata_info(sdata, "Rejecting non-HE 6/7 GHz connection");
  5164			return -EINVAL;
  5165		}
  5166	
  5167		/* will change later if needed */
  5168		sdata->smps_mode = IEEE80211_SMPS_OFF;
  5169	
  5170		mutex_lock(&local->mtx);
  5171		/*
  5172		 * If this fails (possibly due to channel context sharing
  5173		 * on incompatible channels, e.g. 80+80 and 160 sharing the
  5174		 * same control channel) try to use a smaller bandwidth.
  5175		 */
  5176		ret = ieee80211_vif_use_channel(sdata, &chandef,
  5177						IEEE80211_CHANCTX_SHARED);
  5178	
  5179		/* don't downgrade for 5 and 10 MHz channels, though. */
  5180		if (chandef.width == NL80211_CHAN_WIDTH_5 ||
  5181		    chandef.width == NL80211_CHAN_WIDTH_10)
  5182			goto out;
  5183	
  5184		while (ret && chandef.width != NL80211_CHAN_WIDTH_20_NOHT) {
  5185			ifmgd->flags |= ieee80211_chandef_downgrade(&chandef);
  5186			ret = ieee80211_vif_use_channel(sdata, &chandef,
  5187							IEEE80211_CHANCTX_SHARED);
  5188		}
  5189	 out:
  5190		mutex_unlock(&local->mtx);
  5191		return ret;
  5192	}
  5193	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-29 13:54   ` Toke Høiland-Jørgensen
@ 2021-11-30 11:12     ` Luca Coelho
  2021-11-30 11:32       ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2021-11-30 11:12 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, johannes; +Cc: linux-wireless

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.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 01/16] mac80211: add more HT/VHT/HE state logging
  2021-11-29 13:32 ` [PATCH 01/16] mac80211: add more HT/VHT/HE state logging Luca Coelho
  2021-11-30  9:05   ` kernel test robot
  2021-11-30  9:26     ` kernel test robot
@ 2021-11-30 11:15   ` Luca Coelho
  2021-11-30 11:16   ` [PATCH v2] " Luca Coelho
  3 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-11-30 11:15 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

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.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2] mac80211: add more HT/VHT/HE state logging
  2021-11-29 13:32 ` [PATCH 01/16] mac80211: add more HT/VHT/HE state logging Luca Coelho
                     ` (2 preceding siblings ...)
  2021-11-30 11:15   ` Luca Coelho
@ 2021-11-30 11:16   ` Luca Coelho
  2021-11-30 15:50     ` Ben Greear
  3 siblings, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2021-11-30 11:16 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel
  2021-11-29 13:32 ` [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel Luca Coelho
@ 2021-11-30 11:18     ` kernel test robot
  2021-11-30 11:18     ` kernel test robot
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 40+ messages in thread
From: kernel test robot @ 2021-11-30 11:18 UTC (permalink / raw)
  To: Luca Coelho; +Cc: llvm, kbuild-all

Hi Luca,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jberg-mac80211-next/master]
[also build test ERROR on next-20211130]
[cannot apply to jberg-mac80211/master linus/master v5.16-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: riscv-randconfig-c006-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301955.BdSRnKyU-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/da233d0b8179f2db6b1f5e783da1c0b986cc8e96
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
        git checkout da233d0b8179f2db6b1f5e783da1c0b986cc8e96
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash net/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/wireless/scan.c:1801:6: error: use of undeclared identifier 'channel'
           if (channel->band == NL80211_BAND_6GHZ) {
               ^
   net/wireless/scan.c:1813:12: error: use of undeclared identifier 'channel'
                                   return channel;
                                          ^
>> net/wireless/scan.c:1815:4: error: use of undeclared identifier 'freq'
                           freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
                           ^
   net/wireless/scan.c:1822:12: error: use of undeclared identifier 'freq'
                               abs(freq - channel->center_freq) <= 80 &&
                                   ^
   net/wireless/scan.c:1822:19: error: use of undeclared identifier 'channel'
                               abs(freq - channel->center_freq) <= 80 &&
                                          ^
   net/wireless/scan.c:1822:12: error: use of undeclared identifier 'freq'
                               abs(freq - channel->center_freq) <= 80 &&
                                   ^
   net/wireless/scan.c:1822:19: error: use of undeclared identifier 'channel'
                               abs(freq - channel->center_freq) <= 80 &&
                                          ^
   net/wireless/scan.c:1821:8: error: use of undeclared identifier 'freq'
                           if (freq != channel->center_freq &&
                               ^
   net/wireless/scan.c:1821:16: error: use of undeclared identifier 'channel'
                           if (freq != channel->center_freq &&
                                       ^
>> net/wireless/scan.c:1823:9: error: use of undeclared identifier 'ftype'; did you mean '_ctype'?
                               (ftype != CFG80211_BSS_FTYPE_BEACON ||
                                ^~~~~
                                _ctype
   include/linux/ctype.h:21:28: note: '_ctype' declared here
   extern const unsigned char _ctype[];
                              ^
   10 errors generated.


vim +/channel +1801 net/wireless/scan.c

  1794	
  1795	int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
  1796					    enum nl80211_band band)
  1797	{
  1798		const u8 *tmp;
  1799		int channel_number = -1;
  1800	
> 1801		if (channel->band == NL80211_BAND_6GHZ) {
  1802			const struct element *elem;
  1803	
  1804			elem = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, ie,
  1805						      ielen);
  1806			if (elem && elem->datalen >= ieee80211_he_oper_size(&elem->data[1])) {
  1807				struct ieee80211_he_operation *he_oper =
  1808					(void *)(&elem->data[1]);
  1809				const struct ieee80211_he_6ghz_oper *he_6ghz_oper;
  1810	
  1811				he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper);
  1812				if (!he_6ghz_oper)
  1813					return channel;
  1814	
> 1815				freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
  1816								      NL80211_BAND_6GHZ);
  1817	
  1818				/* duplicated beacon indication is relevant for beacons
  1819				 * only
  1820				 */
  1821				if (freq != channel->center_freq &&
  1822				    abs(freq - channel->center_freq) <= 80 &&
> 1823				    (ftype != CFG80211_BSS_FTYPE_BEACON ||
  1824				     he_6ghz_oper->control & IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON))
  1825					channel_number = he_6ghz_oper->primary;
  1826			}
  1827		} else if (band == NL80211_BAND_S1GHZ) {
  1828			tmp = cfg80211_find_ie(WLAN_EID_S1G_OPERATION, ie, ielen);
  1829			if (tmp && tmp[1] >= sizeof(struct ieee80211_s1g_oper_ie)) {
  1830				struct ieee80211_s1g_oper_ie *s1gop = (void *)(tmp + 2);
  1831	
  1832				channel_number = s1gop->primary_ch;
  1833			}
  1834		} else {
  1835			tmp = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ie, ielen);
  1836			if (tmp && tmp[1] == 1) {
  1837				channel_number = tmp[2];
  1838			} else {
  1839				tmp = cfg80211_find_ie(WLAN_EID_HT_OPERATION, ie, ielen);
  1840				if (tmp && tmp[1] >= sizeof(struct ieee80211_ht_operation)) {
  1841					struct ieee80211_ht_operation *htop = (void *)(tmp + 2);
  1842	
  1843					channel_number = htop->primary_chan;
  1844				}
  1845			}
  1846		}
  1847	
  1848		return channel_number;
  1849	}
  1850	EXPORT_SYMBOL(cfg80211_get_ies_channel_number);
  1851	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel
@ 2021-11-30 11:18     ` kernel test robot
  0 siblings, 0 replies; 40+ messages in thread
From: kernel test robot @ 2021-11-30 11:18 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 6323 bytes --]

Hi Luca,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jberg-mac80211-next/master]
[also build test ERROR on next-20211130]
[cannot apply to jberg-mac80211/master linus/master v5.16-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: riscv-randconfig-c006-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301955.BdSRnKyU-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 25eb7fa01d7ebbe67648ea03841cda55b4239ab2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/da233d0b8179f2db6b1f5e783da1c0b986cc8e96
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
        git checkout da233d0b8179f2db6b1f5e783da1c0b986cc8e96
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash net/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/wireless/scan.c:1801:6: error: use of undeclared identifier 'channel'
           if (channel->band == NL80211_BAND_6GHZ) {
               ^
   net/wireless/scan.c:1813:12: error: use of undeclared identifier 'channel'
                                   return channel;
                                          ^
>> net/wireless/scan.c:1815:4: error: use of undeclared identifier 'freq'
                           freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
                           ^
   net/wireless/scan.c:1822:12: error: use of undeclared identifier 'freq'
                               abs(freq - channel->center_freq) <= 80 &&
                                   ^
   net/wireless/scan.c:1822:19: error: use of undeclared identifier 'channel'
                               abs(freq - channel->center_freq) <= 80 &&
                                          ^
   net/wireless/scan.c:1822:12: error: use of undeclared identifier 'freq'
                               abs(freq - channel->center_freq) <= 80 &&
                                   ^
   net/wireless/scan.c:1822:19: error: use of undeclared identifier 'channel'
                               abs(freq - channel->center_freq) <= 80 &&
                                          ^
   net/wireless/scan.c:1821:8: error: use of undeclared identifier 'freq'
                           if (freq != channel->center_freq &&
                               ^
   net/wireless/scan.c:1821:16: error: use of undeclared identifier 'channel'
                           if (freq != channel->center_freq &&
                                       ^
>> net/wireless/scan.c:1823:9: error: use of undeclared identifier 'ftype'; did you mean '_ctype'?
                               (ftype != CFG80211_BSS_FTYPE_BEACON ||
                                ^~~~~
                                _ctype
   include/linux/ctype.h:21:28: note: '_ctype' declared here
   extern const unsigned char _ctype[];
                              ^
   10 errors generated.


vim +/channel +1801 net/wireless/scan.c

  1794	
  1795	int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
  1796					    enum nl80211_band band)
  1797	{
  1798		const u8 *tmp;
  1799		int channel_number = -1;
  1800	
> 1801		if (channel->band == NL80211_BAND_6GHZ) {
  1802			const struct element *elem;
  1803	
  1804			elem = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, ie,
  1805						      ielen);
  1806			if (elem && elem->datalen >= ieee80211_he_oper_size(&elem->data[1])) {
  1807				struct ieee80211_he_operation *he_oper =
  1808					(void *)(&elem->data[1]);
  1809				const struct ieee80211_he_6ghz_oper *he_6ghz_oper;
  1810	
  1811				he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper);
  1812				if (!he_6ghz_oper)
  1813					return channel;
  1814	
> 1815				freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
  1816								      NL80211_BAND_6GHZ);
  1817	
  1818				/* duplicated beacon indication is relevant for beacons
  1819				 * only
  1820				 */
  1821				if (freq != channel->center_freq &&
  1822				    abs(freq - channel->center_freq) <= 80 &&
> 1823				    (ftype != CFG80211_BSS_FTYPE_BEACON ||
  1824				     he_6ghz_oper->control & IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON))
  1825					channel_number = he_6ghz_oper->primary;
  1826			}
  1827		} else if (band == NL80211_BAND_S1GHZ) {
  1828			tmp = cfg80211_find_ie(WLAN_EID_S1G_OPERATION, ie, ielen);
  1829			if (tmp && tmp[1] >= sizeof(struct ieee80211_s1g_oper_ie)) {
  1830				struct ieee80211_s1g_oper_ie *s1gop = (void *)(tmp + 2);
  1831	
  1832				channel_number = s1gop->primary_ch;
  1833			}
  1834		} else {
  1835			tmp = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ie, ielen);
  1836			if (tmp && tmp[1] == 1) {
  1837				channel_number = tmp[2];
  1838			} else {
  1839				tmp = cfg80211_find_ie(WLAN_EID_HT_OPERATION, ie, ielen);
  1840				if (tmp && tmp[1] >= sizeof(struct ieee80211_ht_operation)) {
  1841					struct ieee80211_ht_operation *htop = (void *)(tmp + 2);
  1842	
  1843					channel_number = htop->primary_chan;
  1844				}
  1845			}
  1846		}
  1847	
  1848		return channel_number;
  1849	}
  1850	EXPORT_SYMBOL(cfg80211_get_ies_channel_number);
  1851	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-30 11:12     ` Luca Coelho
@ 2021-11-30 11:32       ` Toke Høiland-Jørgensen
  2021-11-30 11:52         ` Johannes Berg
  0 siblings, 1 reply; 40+ messages in thread
From: Toke Høiland-Jørgensen @ 2021-11-30 11:32 UTC (permalink / raw)
  To: Luca Coelho, johannes; +Cc: linux-wireless

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


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel
  2021-11-29 13:32 ` [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel Luca Coelho
  2021-11-30  8:14   ` kernel test robot
  2021-11-30 11:18     ` kernel test robot
@ 2021-11-30 11:49   ` kernel test robot
  2021-12-02 12:28   ` Luca Coelho
  2021-12-02 12:36   ` [PATCH v2] " Luca Coelho
  4 siblings, 0 replies; 40+ messages in thread
From: kernel test robot @ 2021-11-30 11:49 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 10709 bytes --]

Hi Luca,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on jberg-mac80211-next/master]
[also build test ERROR on next-20211130]
[cannot apply to jberg-mac80211/master linus/master v5.16-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: i386-randconfig-s001-20211128 (https://download.01.org/0day-ci/archive/20211130/202111301920.kYgRFuAr-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/da233d0b8179f2db6b1f5e783da1c0b986cc8e96
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Luca-Coelho/cfg80211-mac80211-patches-from-our-internal-tree-2021-11-29/20211129-213704
        git checkout da233d0b8179f2db6b1f5e783da1c0b986cc8e96
        # save the config file to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/wireless/scan.c: In function 'cfg80211_get_ies_channel_number':
   net/wireless/scan.c:1801:6: error: 'channel' undeclared (first use in this function)
    1801 |  if (channel->band == NL80211_BAND_6GHZ) {
         |      ^~~~~~~
   net/wireless/scan.c:1801:6: note: each undeclared identifier is reported only once for each function it appears in
   net/wireless/scan.c:1815:4: error: 'freq' undeclared (first use in this function); did you mean 'ifreq'?
    1815 |    freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
         |    ^~~~
         |    ifreq
   In file included from include/linux/kernel.h:16,
                    from net/wireless/scan.c:10:
>> include/linux/math.h:136:3: error: first argument to '__builtin_choose_expr' not a constant
     136 |   __builtin_choose_expr(     \
         |   ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:54: note: in definition of macro '__abs_choose_expr'
     144 |  ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                      ^~~~~
   include/linux/math.h:132:3: note: in expansion of macro '__abs_choose_expr'
     132 |   __abs_choose_expr(x, long,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:133:3: note: in expansion of macro '__abs_choose_expr'
     133 |   __abs_choose_expr(x, int,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:134:3: note: in expansion of macro '__abs_choose_expr'
     134 |   __abs_choose_expr(x, short,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:135:3: note: in expansion of macro '__abs_choose_expr'
     135 |   __abs_choose_expr(x, char,    \
         |   ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:8: note: in expansion of macro 'abs'
    1822 |        abs(freq - channel->center_freq) <= 80 &&
         |        ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr( \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:54: note: in definition of macro '__abs_choose_expr'
     144 |  ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                      ^~~~~
   include/linux/math.h:132:3: note: in expansion of macro '__abs_choose_expr'
     132 |   __abs_choose_expr(x, long,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:133:3: note: in expansion of macro '__abs_choose_expr'
     133 |   __abs_choose_expr(x, int,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:134:3: note: in expansion of macro '__abs_choose_expr'
     134 |   __abs_choose_expr(x, short,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:135:3: note: in expansion of macro '__abs_choose_expr'
     135 |   __abs_choose_expr(x, char,    \
         |   ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:8: note: in expansion of macro 'abs'
    1822 |        abs(freq - channel->center_freq) <= 80 &&
         |        ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr( \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:54: note: in definition of macro '__abs_choose_expr'
     144 |  ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                      ^~~~~
   include/linux/math.h:132:3: note: in expansion of macro '__abs_choose_expr'
     132 |   __abs_choose_expr(x, long,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:133:3: note: in expansion of macro '__abs_choose_expr'
     133 |   __abs_choose_expr(x, int,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:134:3: note: in expansion of macro '__abs_choose_expr'
     134 |   __abs_choose_expr(x, short,    \
         |   ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:8: note: in expansion of macro 'abs'
    1822 |        abs(freq - channel->center_freq) <= 80 &&
         |        ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr( \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:54: note: in definition of macro '__abs_choose_expr'
     144 |  ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                      ^~~~~
   include/linux/math.h:132:3: note: in expansion of macro '__abs_choose_expr'
     132 |   __abs_choose_expr(x, long,    \
         |   ^~~~~~~~~~~~~~~~~
   include/linux/math.h:133:3: note: in expansion of macro '__abs_choose_expr'
     133 |   __abs_choose_expr(x, int,    \
         |   ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:8: note: in expansion of macro 'abs'
    1822 |        abs(freq - channel->center_freq) <= 80 &&
         |        ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr( \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:144:54: note: in definition of macro '__abs_choose_expr'
     144 |  ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
         |                                                      ^~~~~
   include/linux/math.h:132:3: note: in expansion of macro '__abs_choose_expr'
     132 |   __abs_choose_expr(x, long,    \
         |   ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:8: note: in expansion of macro 'abs'
    1822 |        abs(freq - channel->center_freq) <= 80 &&
         |        ^~~
   include/linux/math.h:141:43: error: first argument to '__builtin_choose_expr' not a constant
     141 | #define __abs_choose_expr(x, type, other) __builtin_choose_expr( \
         |                                           ^~~~~~~~~~~~~~~~~~~~~
   include/linux/math.h:131:16: note: in expansion of macro '__abs_choose_expr'
     131 | #define abs(x) __abs_choose_expr(x, long long,    \
         |                ^~~~~~~~~~~~~~~~~
   net/wireless/scan.c:1822:8: note: in expansion of macro 'abs'
    1822 |        abs(freq - channel->center_freq) <= 80 &&
         |        ^~~
>> net/wireless/scan.c:1823:9: error: 'ftype' undeclared (first use in this function); did you mean 'cftype'?
    1823 |        (ftype != CFG80211_BSS_FTYPE_BEACON ||
         |         ^~~~~
         |         cftype


vim +1823 net/wireless/scan.c

  1794	
  1795	int cfg80211_get_ies_channel_number(const u8 *ie, size_t ielen,
  1796					    enum nl80211_band band)
  1797	{
  1798		const u8 *tmp;
  1799		int channel_number = -1;
  1800	
  1801		if (channel->band == NL80211_BAND_6GHZ) {
  1802			const struct element *elem;
  1803	
  1804			elem = cfg80211_find_ext_elem(WLAN_EID_EXT_HE_OPERATION, ie,
  1805						      ielen);
  1806			if (elem && elem->datalen >= ieee80211_he_oper_size(&elem->data[1])) {
  1807				struct ieee80211_he_operation *he_oper =
  1808					(void *)(&elem->data[1]);
  1809				const struct ieee80211_he_6ghz_oper *he_6ghz_oper;
  1810	
  1811				he_6ghz_oper = ieee80211_he_6ghz_oper(he_oper);
  1812				if (!he_6ghz_oper)
  1813					return channel;
  1814	
  1815				freq = ieee80211_channel_to_frequency(he_6ghz_oper->primary,
  1816								      NL80211_BAND_6GHZ);
  1817	
  1818				/* duplicated beacon indication is relevant for beacons
  1819				 * only
  1820				 */
  1821				if (freq != channel->center_freq &&
  1822				    abs(freq - channel->center_freq) <= 80 &&
> 1823				    (ftype != CFG80211_BSS_FTYPE_BEACON ||
  1824				     he_6ghz_oper->control & IEEE80211_HE_6GHZ_OPER_CTRL_DUP_BEACON))
  1825					channel_number = he_6ghz_oper->primary;
  1826			}
  1827		} else if (band == NL80211_BAND_S1GHZ) {
  1828			tmp = cfg80211_find_ie(WLAN_EID_S1G_OPERATION, ie, ielen);
  1829			if (tmp && tmp[1] >= sizeof(struct ieee80211_s1g_oper_ie)) {
  1830				struct ieee80211_s1g_oper_ie *s1gop = (void *)(tmp + 2);
  1831	
  1832				channel_number = s1gop->primary_ch;
  1833			}
  1834		} else {
  1835			tmp = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ie, ielen);
  1836			if (tmp && tmp[1] == 1) {
  1837				channel_number = tmp[2];
  1838			} else {
  1839				tmp = cfg80211_find_ie(WLAN_EID_HT_OPERATION, ie, ielen);
  1840				if (tmp && tmp[1] >= sizeof(struct ieee80211_ht_operation)) {
  1841					struct ieee80211_ht_operation *htop = (void *)(tmp + 2);
  1842	
  1843					channel_number = htop->primary_chan;
  1844				}
  1845			}
  1846		}
  1847	
  1848		return channel_number;
  1849	}
  1850	EXPORT_SYMBOL(cfg80211_get_ies_channel_number);
  1851	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-30 11:32       ` Toke Høiland-Jørgensen
@ 2021-11-30 11:52         ` Johannes Berg
  2021-11-30 11:56           ` Toke Høiland-Jørgensen
  2021-11-30 11:57           ` Luca Coelho
  0 siblings, 2 replies; 40+ messages in thread
From: Johannes Berg @ 2021-11-30 11:52 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Luca Coelho; +Cc: linux-wireless

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

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-30 11:52         ` Johannes Berg
@ 2021-11-30 11:56           ` Toke Høiland-Jørgensen
  2021-11-30 11:57           ` Luca Coelho
  1 sibling, 0 replies; 40+ messages in thread
From: Toke Høiland-Jørgensen @ 2021-11-30 11:56 UTC (permalink / raw)
  To: Johannes Berg, Luca Coelho; +Cc: linux-wireless

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


^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-30 11:52         ` Johannes Berg
  2021-11-30 11:56           ` Toke Høiland-Jørgensen
@ 2021-11-30 11:57           ` Luca Coelho
  2021-11-30 12:08             ` Johannes Berg
  1 sibling, 1 reply; 40+ messages in thread
From: Luca Coelho @ 2021-11-30 11:57 UTC (permalink / raw)
  To: Johannes Berg, Toke Høiland-Jørgensen; +Cc: linux-wireless

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.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-30 11:57           ` Luca Coelho
@ 2021-11-30 12:08             ` Johannes Berg
  0 siblings, 0 replies; 40+ messages in thread
From: Johannes Berg @ 2021-11-30 12:08 UTC (permalink / raw)
  To: Luca Coelho, Toke Høiland-Jørgensen; +Cc: linux-wireless

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

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH v2] mac80211: add more HT/VHT/HE state logging
  2021-11-30 11:16   ` [PATCH v2] " Luca Coelho
@ 2021-11-30 15:50     ` Ben Greear
  0 siblings, 0 replies; 40+ messages in thread
From: Ben Greear @ 2021-11-30 15:50 UTC (permalink / raw)
  To: Luca Coelho, johannes; +Cc: linux-wireless

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

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work
  2021-11-29 13:32 ` [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work Luca Coelho
@ 2021-12-01 13:47   ` Kalle Valo
  2021-12-02 13:27     ` Luca Coelho
  2021-12-02 13:28   ` [PATCH v2] " Luca Coelho
  1 sibling, 1 reply; 40+ messages in thread
From: Kalle Valo @ 2021-12-01 13:47 UTC (permalink / raw)
  To: Luca Coelho; +Cc: johannes, linux-wireless

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

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel
  2021-11-29 13:32 ` [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel Luca Coelho
                     ` (2 preceding siblings ...)
  2021-11-30 11:49   ` kernel test robot
@ 2021-12-02 12:28   ` Luca Coelho
  2021-12-02 12:36   ` [PATCH v2] " Luca Coelho
  4 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-12-02 12:28 UTC (permalink / raw)
  To: johannes; +Cc: linux-wireless

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.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel
  2021-11-29 13:32 ` [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel Luca Coelho
                     ` (3 preceding siblings ...)
  2021-12-02 12:28   ` Luca Coelho
@ 2021-12-02 12:36   ` Luca Coelho
  4 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-12-02 12:36 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH v2] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock
  2021-11-29 13:32 ` [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock Luca Coelho
  2021-11-29 13:54   ` Toke Høiland-Jørgensen
@ 2021-12-02 13:26   ` Luca Coelho
  1 sibling, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-12-02 13:26 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work
  2021-12-01 13:47   ` Kalle Valo
@ 2021-12-02 13:27     ` Luca Coelho
  0 siblings, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-12-02 13:27 UTC (permalink / raw)
  To: Kalle Valo; +Cc: johannes, linux-wireless

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.

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH v2] cfg80211: Acquire wiphy mutex on regulatory work
  2021-11-29 13:32 ` [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work Luca Coelho
  2021-12-01 13:47   ` Kalle Valo
@ 2021-12-02 13:28   ` Luca Coelho
  1 sibling, 0 replies; 40+ messages in thread
From: Luca Coelho @ 2021-12-02 13:28 UTC (permalink / raw)
  To: johannes; +Cc: luca, linux-wireless

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


^ permalink raw reply related	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2021-12-02 13:29 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-29 13:32 [PATCH 00/16] cfg80211/mac80211 patches from our internal tree 2021-11-29 Luca Coelho
2021-11-29 13:32 ` [PATCH 01/16] mac80211: add more HT/VHT/HE state logging Luca Coelho
2021-11-30  9:05   ` kernel test robot
2021-11-30  9:26   ` kernel test robot
2021-11-30  9:26     ` kernel test robot
2021-11-30 11:15   ` Luca Coelho
2021-11-30 11:16   ` [PATCH v2] " Luca Coelho
2021-11-30 15:50     ` Ben Greear
2021-11-29 13:32 ` [PATCH 02/16] cfg80211: Add support for notifying association comeback Luca Coelho
2021-11-29 13:32 ` [PATCH 03/16] mac80211: Notify cfg80211 about " Luca Coelho
2021-11-29 13:32 ` [PATCH 04/16] cfg80211: Use the HE operation IE to determine a 6GHz BSS channel Luca Coelho
2021-11-30  8:14   ` kernel test robot
2021-11-30 11:18   ` kernel test robot
2021-11-30 11:18     ` kernel test robot
2021-11-30 11:49   ` kernel test robot
2021-12-02 12:28   ` Luca Coelho
2021-12-02 12:36   ` [PATCH v2] " Luca Coelho
2021-11-29 13:32 ` [PATCH 05/16] [BUGFIX] cfg80211: check fixed size before ieee80211_he_oper_size() Luca Coelho
2021-11-29 13:32 ` [PATCH 06/16] mac80211: introduce channel switch disconnect function Luca Coelho
2021-11-29 13:32 ` [PATCH 07/16] mac80211: mark TX-during-stop for TX in in_reconfig Luca Coelho
2021-11-29 13:32 ` [PATCH 08/16] mac80211: do drv_reconfig_complete() before restarting all Luca Coelho
2021-11-29 13:32 ` [PATCH 09/16] cfg80211: Fix order of enum nl80211_band_iftype_attr documentation Luca Coelho
2021-11-29 13:32 ` [PATCH 10/16] mac80211: update channel context before station state Luca Coelho
2021-11-29 13:32 ` [PATCH 11/16] cfg80211: simplify cfg80211_chandef_valid() Luca Coelho
2021-11-29 13:32 ` [PATCH 12/16] mac80211: Remove a couple of obsolete TODO Luca Coelho
2021-11-29 13:32 ` [PATCH 13/16] mac80211: Fix the size used for building probe request Luca Coelho
2021-11-29 13:32 ` [PATCH 14/16] mac80211: fix lookup when adding AddBA extension element Luca Coelho
2021-11-29 13:32 ` [PATCH 15/16] mac80211: agg-tx: don't schedule_and_wake_txq() under sta->lock Luca Coelho
2021-11-29 13:54   ` Toke Høiland-Jørgensen
2021-11-30 11:12     ` Luca Coelho
2021-11-30 11:32       ` Toke Høiland-Jørgensen
2021-11-30 11:52         ` Johannes Berg
2021-11-30 11:56           ` Toke Høiland-Jørgensen
2021-11-30 11:57           ` Luca Coelho
2021-11-30 12:08             ` Johannes Berg
2021-12-02 13:26   ` [PATCH v2] " Luca Coelho
2021-11-29 13:32 ` [PATCH 16/16] cfg80211: Acquire wiphy mutex on regulatory work Luca Coelho
2021-12-01 13:47   ` Kalle Valo
2021-12-02 13:27     ` Luca Coelho
2021-12-02 13:28   ` [PATCH v2] " Luca Coelho

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.