[RFC PATCH 00/12] wifi: AP side locking improvements

[RFC PATCH 00/12] wifi: AP side locking improvements

[Johannes Berg]

This obviously goes on top of my (fixed) RTNL redux series.

The idea here is that wiphy mutex can be held for quite a bit
of time (e.g. waiting for firmware), but beacon updates (e.g.
with the critical update design I had proposed a long while
back) and other things should be fast.

So with this not all things require wiphy mutex. These are:

 - beacon and related template updates
   (this one needs driver opt-in),
 - mgmt frame TX,
 - control port TX, and
 - peer probe.

This does make the implementation slightly more complex, but
the added complexity is almost entirely in mac80211 (and some
in cfg80211), unless a driver wants to opt in to unlocked
beacon/template updates, which it has to implement itself for
obvious reasons.

I think we probably want this even without the critical update
design we're still debating, but I thought I'd throw this out
here because a lot of the discussion has been focused on the
latencies throughout the stack, and I never thought they were
fundamental. QED ;-)

johannes

[RFC PATCH 01/12] wifi: mac80211: protect AP template pointers with a spinlock

[Johannes Berg]

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

The AP's beacon, presp etc. templates are only updated under
wiphy mutex right now. Prepare for being able to update them
without the wiphy mutex by using a (per-link) spinlock for
them.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c         | 66 +++++++++++++-------------------------
 net/mac80211/ieee80211_i.h | 18 +++++++++++
 net/mac80211/link.c        |  1 +
 3 files changed, 42 insertions(+), 43 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index bd1a857c812d..41d62e199851 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1122,13 +1122,11 @@ ieee80211_set_probe_resp(struct ieee80211_sub_if_data *sdata,
 			 const struct ieee80211_color_change_settings *cca,
 			 struct ieee80211_link_data *link)
 {
-	struct probe_resp *new, *old;
+	struct probe_resp *new;
 
 	if (!resp || !resp_len)
 		return 1;
 
-	old = sdata_dereference(link->u.ap.probe_resp, sdata);
-
 	new = kzalloc(sizeof(struct probe_resp) + resp_len, GFP_KERNEL);
 	if (!new)
 		return -ENOMEM;
@@ -1143,9 +1141,7 @@ ieee80211_set_probe_resp(struct ieee80211_sub_if_data *sdata,
 	else if (cca)
 		new->cntdwn_counter_offsets[0] = cca->counter_offset_presp;
 
-	rcu_assign_pointer(link->u.ap.probe_resp, new);
-	if (old)
-		kfree_rcu(old, rcu_head);
+	ap_tmpl_replace(link, link->u.ap.probe_resp, new);
 
 	return 0;
 }
@@ -1156,7 +1152,7 @@ static int ieee80211_set_fils_discovery(struct ieee80211_sub_if_data *sdata,
 					struct ieee80211_bss_conf *link_conf,
 					u64 *changed)
 {
-	struct fils_discovery_data *new, *old = NULL;
+	struct fils_discovery_data *new = NULL;
 	struct ieee80211_fils_discovery *fd;
 
 	if (!params->update)
@@ -1166,20 +1162,15 @@ static int ieee80211_set_fils_discovery(struct ieee80211_sub_if_data *sdata,
 	fd->min_interval = params->min_interval;
 	fd->max_interval = params->max_interval;
 
-	old = sdata_dereference(link->u.ap.fils_discovery, sdata);
 	if (params->tmpl && params->tmpl_len) {
 		new = kzalloc(sizeof(*new) + params->tmpl_len, GFP_KERNEL);
 		if (!new)
 			return -ENOMEM;
 		new->len = params->tmpl_len;
 		memcpy(new->data, params->tmpl, params->tmpl_len);
-		rcu_assign_pointer(link->u.ap.fils_discovery, new);
-	} else {
-		RCU_INIT_POINTER(link->u.ap.fils_discovery, NULL);
 	}
 
-	if (old)
-		kfree_rcu(old, rcu_head);
+	ap_tmpl_replace(link, link->u.ap.fils_discovery, new);
 
 	*changed |= BSS_CHANGED_FILS_DISCOVERY;
 	return 0;
@@ -1192,28 +1183,22 @@ ieee80211_set_unsol_bcast_probe_resp(struct ieee80211_sub_if_data *sdata,
 				     struct ieee80211_bss_conf *link_conf,
 				     u64 *changed)
 {
-	struct unsol_bcast_probe_resp_data *new, *old = NULL;
+	struct unsol_bcast_probe_resp_data *new = NULL;
 
 	if (!params->update)
 		return 0;
 
 	link_conf->unsol_bcast_probe_resp_interval = params->interval;
 
-	old = sdata_dereference(link->u.ap.unsol_bcast_probe_resp, sdata);
-
 	if (params->tmpl && params->tmpl_len) {
 		new = kzalloc(sizeof(*new) + params->tmpl_len, GFP_KERNEL);
 		if (!new)
 			return -ENOMEM;
 		new->len = params->tmpl_len;
 		memcpy(new->data, params->tmpl, params->tmpl_len);
-		rcu_assign_pointer(link->u.ap.unsol_bcast_probe_resp, new);
-	} else {
-		RCU_INIT_POINTER(link->u.ap.unsol_bcast_probe_resp, NULL);
 	}
 
-	if (old)
-		kfree_rcu(old, rcu_head);
+	ap_tmpl_replace(link, link->u.ap.unsol_bcast_probe_resp, new);
 
 	*changed |= BSS_CHANGED_UNSOL_BCAST_PROBE_RESP;
 	return 0;
@@ -1613,12 +1598,9 @@ ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata,
 		_changed |= BSS_CHANGED_FTM_RESPONDER;
 	}
 
-	rcu_assign_pointer(link->u.ap.beacon, new);
+	ap_tmpl_replace(link, link->u.ap.beacon, new);
 	sdata->u.ap.active = true;
 
-	if (old)
-		kfree_rcu(old, rcu_head);
-
 	ieee80211_update_ap_bandwidth(link, params);
 
 	*changed |= _changed;
@@ -1920,11 +1902,7 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 
 	err = drv_start_ap(sdata->local, sdata, link_conf);
 	if (err) {
-		old = sdata_dereference(link->u.ap.beacon, sdata);
-
-		if (old)
-			kfree_rcu(old, rcu_head);
-		RCU_INIT_POINTER(link->u.ap.beacon, NULL);
+		ap_tmpl_replace(link, link->u.ap.beacon, NULL);
 
 		if (ieee80211_num_beaconing_links(sdata) == 0)
 			sdata->u.ap.active = false;
@@ -2052,16 +2030,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	old_beacon = sdata_dereference(link->u.ap.beacon, sdata);
-	if (!old_beacon)
+	if (!sdata_dereference(link->u.ap.beacon, sdata))
 		return -ENOENT;
-	old_probe_resp = sdata_dereference(link->u.ap.probe_resp,
-					   sdata);
-	old_fils_discovery = sdata_dereference(link->u.ap.fils_discovery,
-					       sdata);
-	old_unsol_bcast_probe_resp =
-		sdata_dereference(link->u.ap.unsol_bcast_probe_resp,
-				  sdata);
 	old_s1g_short_beacon =
 		sdata_dereference(link->u.ap.s1g_short_beacon, sdata);
 
@@ -2082,10 +2052,20 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
 	}
 
 	/* remove beacon and probe response */
-	RCU_INIT_POINTER(link->u.ap.beacon, NULL);
-	RCU_INIT_POINTER(link->u.ap.probe_resp, NULL);
-	RCU_INIT_POINTER(link->u.ap.fils_discovery, NULL);
-	RCU_INIT_POINTER(link->u.ap.unsol_bcast_probe_resp, NULL);
+	scoped_guard(spinlock, &link->ap_tmpl_lock) {
+		old_beacon = ap_tmpl_dereference(link, link->u.ap.beacon);
+		old_probe_resp = ap_tmpl_dereference(link,
+						     link->u.ap.probe_resp);
+		old_fils_discovery =
+			ap_tmpl_dereference(link, link->u.ap.fils_discovery);
+		old_unsol_bcast_probe_resp =
+			ap_tmpl_dereference(link,
+					    link->u.ap.unsol_bcast_probe_resp);
+		RCU_INIT_POINTER(link->u.ap.beacon, NULL);
+		RCU_INIT_POINTER(link->u.ap.probe_resp, NULL);
+		RCU_INIT_POINTER(link->u.ap.fils_discovery, NULL);
+		RCU_INIT_POINTER(link->u.ap.unsol_bcast_probe_resp, NULL);
+	}
 	RCU_INIT_POINTER(link->u.ap.s1g_short_beacon, NULL);
 	kfree_rcu(old_beacon, rcu_head);
 	if (old_probe_resp)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 1430527d216c..02e50dcef27e 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1151,6 +1151,9 @@ struct ieee80211_link_data {
 		struct ieee80211_link_data_ap ap;
 	} u;
 
+	/* protects replacing the u.ap template pointers, also without wiphy mutex */
+	spinlock_t ap_tmpl_lock;
+
 	struct ieee80211_tx_queue_params tx_conf[IEEE80211_NUM_ACS];
 
 	struct ieee80211_bss_conf *conf;
@@ -1165,6 +1168,21 @@ struct ieee80211_link_data {
 #endif
 };
 
+#define ap_tmpl_dereference(link, p)					\
+	rcu_dereference_protected(p, lockdep_is_held(&(link)->ap_tmpl_lock))
+
+#define ap_tmpl_replace(link, p, new)					\
+do {									\
+	typeof(ap_tmpl_dereference(link, p)) __old;			\
+									\
+	spin_lock(&(link)->ap_tmpl_lock);				\
+	__old = rcu_replace_pointer(p, new,				\
+				    lockdep_is_held(&(link)->ap_tmpl_lock));\
+	spin_unlock(&(link)->ap_tmpl_lock);				\
+	if (__old)							\
+		kfree_rcu(__old, rcu_head);				\
+} while (0)
+
 struct ieee80211_sub_if_data {
 	struct list_head list;
 
diff --git a/net/mac80211/link.c b/net/mac80211/link.c
index 931950a10508..ce1c95b40cd9 100644
--- a/net/mac80211/link.c
+++ b/net/mac80211/link.c
@@ -126,6 +126,7 @@ void ieee80211_link_init(struct ieee80211_sub_if_data *sdata,
 	link->ap_power_level = IEEE80211_UNSET_POWER_LEVEL;
 	link->user_power_level = sdata->local->user_power_level;
 	link_conf->txpower = INT_MIN;
+	spin_lock_init(&link->ap_tmpl_lock);
 
 	wiphy_work_init(&link->csa.finalize_work,
 			ieee80211_csa_finalize_work);
-- 
2.55.0

[RFC PATCH 02/12] wifi: nl80211: add NL80211_CMD_UPDATE_BEACON

[Johannes Berg]

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

Updating beacon and related templates with NL80211_CMD_SET_BEACON
requires the wiphy mutex, which can get contended, delaying the
updates.

Add a NL80211_CMD_UPDATE_BEACON operation that can only do beacon
template updates, not other configuration changes, but is called
without the wiphy mutex. If something requires the mutex then the
driver can return -EAGAIN, to go through the normal change_beacon
with wiphy mutex held.

Also with this support updating multiple beacons for an MLD at
the same time, just to further reduce round-trips if desired.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h       |   5 +
 include/uapi/linux/nl80211.h |  16 ++
 net/wireless/nl80211.c       | 312 +++++++++++++++++++++++++++++------
 net/wireless/rdev-ops.h      |  11 ++
 net/wireless/trace.h         |  14 +-
 5 files changed, 307 insertions(+), 51 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 76edfe5765c8..ba66fb45dc12 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4943,6 +4943,9 @@ struct mgmt_frame_regs {
  * @start_ap: Start acting in AP mode defined by the parameters.
  * @change_beacon: Change the beacon parameters for an access point mode
  *	interface. This should reject the call when AP mode wasn't started.
+ * @update_beacon: Update the beacon and other templates like @change_beacon,
+ *	but called without the wiphy mutex held. May return -EAGAIN to have
+ *	it fall back to @change_beacon with the wiphy mutex held.
  * @stop_ap: Stop being an AP, including stopping beaconing.
  *
  * @add_station: Add a new station.
@@ -5358,6 +5361,8 @@ struct cfg80211_ops {
 			    struct cfg80211_ap_settings *settings);
 	int	(*change_beacon)(struct wiphy *wiphy, struct net_device *dev,
 				 struct cfg80211_ap_update *info);
+	int	(*update_beacon)(struct wiphy *wiphy, struct net_device *dev,
+				 struct cfg80211_ap_update *info);
 	int	(*stop_ap)(struct wiphy *wiphy, struct net_device *dev,
 			   unsigned int link_id);
 
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 75d4c5d6a7a3..e3c137a7c4c8 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1458,6 +1458,16 @@
  *	keep their non-evacuable marking, removed channels lose it, and newly
  *	added channels are evacuable by default; issue this command again to
  *	change the non-evacuable set.
+ * @NL80211_CMD_UPDATE_BEACON: Update the beacon and other templates of an AP
+ *	interface, but only that. Same attributes as %NL80211_CMD_SET_BEACON,
+ *	%NL80211_ATTR_BEACON_HEAD and %NL80211_ATTR_BEACON_TAIL are required,
+ *	configuration changes such as %NL80211_ATTR_FTM_RESPONDER rejected.
+ *	This doesn't acquire the wiphy mutex internally, so it's faster, but
+ *	cannot update any other configuration.
+ *	For MLO a single link is set with %NL80211_ATTR_MLO_LINK_ID as usual
+ *	and multiple can be nested in %NL80211_ATTR_MLO_LINKS but then the
+ *	update isn't atomic if any failures happen.
+ *	Only supported with %NL80211_EXT_FEATURE_UPDATE_BEACON.
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -1738,6 +1748,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_NAN_SET_NON_EVAC_CHANNELS,
 
+	NL80211_CMD_UPDATE_BEACON,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -7173,6 +7185,9 @@ enum nl80211_feature_flags {
  *	offload in station mode, including Fast Transition or Opportunistic
  *	Key Caching.
  *
+ * @NL80211_EXT_FEATURE_UPDATE_BEACON: Driver supports
+ *	%NL80211_CMD_UPDATE_BEACON.
+ *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
  */
@@ -7256,6 +7271,7 @@ enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_SET_KEY_LTF_SEED,
 	NL80211_EXT_FEATURE_PROBE_AP,
 	NL80211_EXT_FEATURE_FAST_ROAM_OFFLOAD,
+	NL80211_EXT_FEATURE_UPDATE_BEACON,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2ed6dd84b42f..b342a7b133d5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7563,15 +7563,87 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 	return err;
 }
 
+static int nl80211_parse_ap_update(struct cfg80211_registered_device *rdev,
+				   struct nlattr *attrs[],
+				   struct ieee80211_channel *chan,
+				   struct cfg80211_ap_update *params,
+				   struct netlink_ext_ack *extack)
+{
+	struct nlattr *attr;
+	int err;
+
+	err = nl80211_parse_beacon(rdev, attrs, &params->beacon, chan, extack);
+	if (err)
+		return err;
+
+	attr = attrs[NL80211_ATTR_FILS_DISCOVERY];
+	if (attr) {
+		err = nl80211_parse_fils_discovery(rdev, attr,
+						   &params->fils_discovery);
+		if (err)
+			return err;
+	}
+
+	attr = attrs[NL80211_ATTR_UNSOL_BCAST_PROBE_RESP];
+	if (attr) {
+		err = nl80211_parse_unsol_bcast_probe_resp(rdev, attr,
+							   &params->unsol_bcast_probe_resp);
+		if (err)
+			return err;
+	}
+
+	attr = attrs[NL80211_ATTR_S1G_SHORT_BEACON];
+	if (attr) {
+		err = nl80211_parse_s1g_short_beacon(rdev, attr,
+						     &params->s1g_short_beacon);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static int nl80211_change_beacon(struct cfg80211_registered_device *rdev,
+				 struct net_device *dev,
+				 struct cfg80211_ap_update *params)
+{
+	struct cfg80211_beaconing_check_config beacon_check = {};
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	unsigned int link_id = params->beacon.link_id;
+	int err;
+
+	/* recheck beaconing is permitted with possibly changed power type */
+	beacon_check.iftype = wdev->iftype;
+	beacon_check.relax = true;
+	beacon_check.reg_power =
+		cfg80211_get_6ghz_power_type(params->beacon.tail,
+					     params->beacon.tail_len, 0);
+	if (!cfg80211_reg_check_beaconing(&rdev->wiphy,
+					  &wdev->links[link_id].ap.chandef,
+					  &beacon_check))
+		return -EINVAL;
+
+	err = rdev_change_beacon(rdev, dev, params);
+	if (!err)
+		wdev->links[link_id].ap.reg_power = beacon_check.reg_power;
+
+	return err;
+}
+
+static void nl80211_free_ap_update(struct cfg80211_ap_update *params)
+{
+	kfree(params->beacon.mbssid_ies);
+	kfree(params->beacon.rnr_ies);
+	kfree(params);
+}
+
 static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
-	struct cfg80211_beaconing_check_config beacon_check = {};
 	unsigned int link_id = nl80211_link_id(info->attrs);
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_ap_update *params;
-	struct nlattr *attr;
 	int err;
 
 	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
@@ -7588,57 +7660,190 @@ static int nl80211_set_beacon(struct sk_buff *skb, struct genl_info *info)
 	if (!params)
 		return -ENOMEM;
 
-	err = nl80211_parse_beacon(rdev, info->attrs, &params->beacon,
-				   wdev->links[link_id].ap.chandef.chan,
-				   info->extack);
-	if (err)
-		goto out;
-
-	/* recheck beaconing is permitted with possibly changed power type */
-	beacon_check.iftype = wdev->iftype;
-	beacon_check.relax = true;
-	beacon_check.reg_power =
-		cfg80211_get_6ghz_power_type(params->beacon.tail,
-					     params->beacon.tail_len, 0);
-	if (!cfg80211_reg_check_beaconing(&rdev->wiphy,
-					  &wdev->links[link_id].ap.chandef,
-					  &beacon_check)) {
-		err = -EINVAL;
-		goto out;
-	}
-
-	attr = info->attrs[NL80211_ATTR_FILS_DISCOVERY];
-	if (attr) {
-		err = nl80211_parse_fils_discovery(rdev, attr,
-						   &params->fils_discovery);
-		if (err)
-			goto out;
-	}
-
-	attr = info->attrs[NL80211_ATTR_UNSOL_BCAST_PROBE_RESP];
-	if (attr) {
-		err = nl80211_parse_unsol_bcast_probe_resp(rdev, attr,
-							   &params->unsol_bcast_probe_resp);
-		if (err)
-			goto out;
-	}
-
-	attr = info->attrs[NL80211_ATTR_S1G_SHORT_BEACON];
-	if (attr) {
-		err = nl80211_parse_s1g_short_beacon(rdev, attr,
-						     &params->s1g_short_beacon);
-		if (err)
-			goto out;
-	}
-
-	err = rdev_change_beacon(rdev, dev, params);
+	err = nl80211_parse_ap_update(rdev, info->attrs,
+				      wdev->links[link_id].ap.chandef.chan,
+				      params, info->extack);
 	if (!err)
-		wdev->links[link_id].ap.reg_power = beacon_check.reg_power;
+		err = nl80211_change_beacon(rdev, dev, params);
 
+	nl80211_free_ap_update(params);
+	return err;
+}
+
+static struct cfg80211_ap_update *
+nl80211_parse_link_update(struct cfg80211_registered_device *rdev,
+			  struct net_device *dev, struct nlattr *attrs[],
+			  struct netlink_ext_ack *extack)
+{
+	struct nlattr *link_attr = attrs[NL80211_ATTR_MLO_LINK_ID];
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	unsigned int link_id = nl80211_link_id(attrs);
+	u16 valid_links = READ_ONCE(wdev->valid_links);
+	struct cfg80211_ap_update *params;
+	struct ieee80211_channel *chan;
+	int err;
+
+	/* without locks this is racy, the driver needs to check again */
+	if (valid_links ? !link_attr || !(valid_links & BIT(link_id)) :
+			  !!link_attr)
+		return ERR_PTR(-EINVAL);
+
+	if (attrs[NL80211_ATTR_FTM_RESPONDER] ||
+	    attrs[NL80211_ATTR_HE_BSS_COLOR] ||
+	    attrs[NL80211_ATTR_S1G_SHORT_BEACON]) {
+		NL_SET_ERR_MSG(extack, "cannot change configuration in update");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!attrs[NL80211_ATTR_BEACON_HEAD] ||
+	    !attrs[NL80211_ATTR_BEACON_TAIL]) {
+		NL_SET_ERR_MSG(extack, "beacon head and tail are required");
+		return ERR_PTR(-EINVAL);
+	}
+
+	chan = READ_ONCE(wdev->links[link_id].ap.chandef.chan);
+	if (!chan || !READ_ONCE(wdev->links[link_id].ap.beacon_interval))
+		return ERR_PTR(-EINVAL);
+
+	params = kzalloc_obj(*params);
+	if (!params)
+		return ERR_PTR(-ENOMEM);
+
+	err = nl80211_parse_ap_update(rdev, attrs, chan, params, extack);
+	if (err) {
+		nl80211_free_ap_update(params);
+		return ERR_PTR(err);
+	}
+
+	return params;
+}
+
+static int nl80211_update_link_beacon(struct cfg80211_registered_device *rdev,
+				      struct net_device *dev,
+				      struct cfg80211_ap_update *params)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	unsigned int link_id = params->beacon.link_id;
+
+	/* a power type change needs regulatory checks under the wiphy mutex */
+	if (cfg80211_get_6ghz_power_type(params->beacon.tail,
+					 params->beacon.tail_len, 0) !=
+	    READ_ONCE(wdev->links[link_id].ap.reg_power))
+		return -EAGAIN;
+
+	return rdev_update_beacon(rdev, dev, params);
+}
+
+static int
+nl80211_update_link_beacon_locked(struct cfg80211_registered_device *rdev,
+				  struct net_device *dev,
+				  struct cfg80211_ap_update *params)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	unsigned int link_id = params->beacon.link_id;
+
+	lockdep_assert_wiphy(&rdev->wiphy);
+
+	if (!rdev->wiphy.registered || !wdev_running(wdev))
+		return -ENETDOWN;
+
+	if (wdev->valid_links ? !(wdev->valid_links & BIT(link_id)) : link_id)
+		return -ENOLINK;
+
+	if (!wdev->links[link_id].ap.beacon_interval)
+		return -EINVAL;
+
+	return nl80211_change_beacon(rdev, dev, params);
+}
+
+static int nl80211_update_beacon(struct sk_buff *skb, struct genl_info *info)
+{
+	struct cfg80211_ap_update *params[IEEE80211_MLD_MAX_NUM_LINKS] = {};
+	struct cfg80211_registered_device *rdev = info->user_ptr[0];
+	struct net_device *dev = info->user_ptr[1];
+	unsigned long need_lock = 0;
+	unsigned int n = 0, i;
+	struct nlattr *link;
+	u16 links = 0;
+	int rem, err = 0;
+
+	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
+	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
+		return -EOPNOTSUPP;
+
+	if (!rdev->ops->update_beacon || !rdev->ops->change_beacon ||
+	    !wiphy_ext_feature_isset(&rdev->wiphy,
+				     NL80211_EXT_FEATURE_UPDATE_BEACON))
+		return -EOPNOTSUPP;
+
+	if (!info->attrs[NL80211_ATTR_MLO_LINKS]) {
+		params[0] = nl80211_parse_link_update(rdev, dev, info->attrs,
+						      info->extack);
+		if (IS_ERR(params[0]))
+			return PTR_ERR(params[0]);
+		n = 1;
+	} else {
+		struct nlattr **attrs = kzalloc_objs(*attrs, NUM_NL80211_ATTR);
+
+		if (!attrs)
+			return -ENOMEM;
+
+		nla_for_each_nested(link, info->attrs[NL80211_ATTR_MLO_LINKS],
+				    rem) {
+			struct cfg80211_ap_update *p;
+
+			err = nla_parse_nested(attrs, NL80211_ATTR_MAX, link,
+					       NULL, info->extack);
+			if (err)
+				break;
+
+			p = nl80211_parse_link_update(rdev, dev, attrs,
+						      info->extack);
+			if (IS_ERR(p)) {
+				err = PTR_ERR(p);
+				break;
+			}
+
+			/* also ensures we don't overflow params[] */
+			if (links & BIT(p->beacon.link_id)) {
+				nl80211_free_ap_update(p);
+				err = -EINVAL;
+				break;
+			}
+			links |= BIT(p->beacon.link_id);
+			params[n++] = p;
+		}
+
+		kfree(attrs);
+		if (!err && !n)
+			err = -EINVAL;
+		if (err)
+			goto out;
+	}
+
+	/* try updates without wiphy mutex first */
+	for (i = 0; i < n; i++) {
+		err = nl80211_update_link_beacon(rdev, dev, params[i]);
+		if (err == -EAGAIN)
+			__set_bit(i, &need_lock);
+		else if (err)
+			goto out;
+	}
+
+	err = 0;
+	if (need_lock) {
+		wiphy_lock(&rdev->wiphy);
+		for_each_set_bit(i, &need_lock, n) {
+			err = nl80211_update_link_beacon_locked(rdev, dev,
+								params[i]);
+			if (err)
+				break;
+		}
+		wiphy_unlock(&rdev->wiphy);
+	}
 out:
-	kfree(params->beacon.mbssid_ies);
-	kfree(params->beacon.rnr_ies);
-	kfree(params);
+	for (i = 0; i < n; i++)
+		nl80211_free_ap_update(params[i]);
 	return err;
 }
 
@@ -20888,6 +21093,13 @@ static const struct genl_small_ops nl80211_small_ops[] = {
 		.flags = GENL_ADMIN_PERM,
 		.internal_flags = IFLAGS(NL80211_FLAG_NEED_WDEV_UP),
 	},
+	{
+		.cmd = NL80211_CMD_UPDATE_BEACON,
+		.doit = nl80211_update_beacon,
+		.flags = GENL_UNS_ADMIN_PERM,
+		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP |
+					 NL80211_FLAG_NO_WIPHY_MTX),
+	},
 };
 
 static struct genl_family nl80211_fam __ro_after_init = {
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 1f1311c20e94..95882c8972d2 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -183,6 +183,17 @@ static inline int rdev_change_beacon(struct cfg80211_registered_device *rdev,
 	return ret;
 }
 
+static inline int rdev_update_beacon(struct cfg80211_registered_device *rdev,
+				     struct net_device *dev,
+				     struct cfg80211_ap_update *info)
+{
+	int ret;
+	trace_rdev_update_beacon(&rdev->wiphy, dev, info);
+	ret = rdev->ops->update_beacon(&rdev->wiphy, dev, info);
+	trace_rdev_return_int(&rdev->wiphy, ret);
+	return ret;
+}
+
 static inline int rdev_stop_ap(struct cfg80211_registered_device *rdev,
 			       struct net_device *dev, unsigned int link_id)
 {
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 83007a824010..f5fa2fb76474 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -740,7 +740,7 @@ TRACE_EVENT(rdev_start_ap,
 		  __entry->inactivity_timeout, __entry->link_id)
 );
 
-TRACE_EVENT(rdev_change_beacon,
+DECLARE_EVENT_CLASS(rdev_ap_update,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
 		 struct cfg80211_ap_update *info),
 	TP_ARGS(wiphy, netdev, info),
@@ -788,6 +788,18 @@ TRACE_EVENT(rdev_change_beacon,
 		  WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->link_id)
 );
 
+DEFINE_EVENT(rdev_ap_update, rdev_change_beacon,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
+		 struct cfg80211_ap_update *info),
+	TP_ARGS(wiphy, netdev, info)
+);
+
+DEFINE_EVENT(rdev_ap_update, rdev_update_beacon,
+	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
+		 struct cfg80211_ap_update *info),
+	TP_ARGS(wiphy, netdev, info)
+);
+
 TRACE_EVENT(rdev_stop_ap,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
 		 unsigned int link_id),
-- 
2.55.0

[RFC PATCH 03/12] wifi: mac80211: implement lockless beacon updates

[Johannes Berg]

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

Implement the new update_beacon() method to update beacon
and related templates without the wiphy mutex. If anything
else were to change return -EAGAIN, same if there are any
active countdowns, and during HW restart.

This doesn't enable it for any drivers since it depends on
the driver behaviour, if it fetches each beacon and doesn't
use the other templates it can just set the feature flag
(NL80211_EXT_FEATURE_UPDATE_BEACON), otherwise it must
implement the new update_beacon() mac80211 method.

The regular beacon update can no longer reliably access
the old beacon across the allocation (or we'd have to do
all those atomically), so if userspace does both at the
same time, we may send a corrupt beacon due to min() in
the copy. This won't happen with hostapd even if it were
to race, since it always sends both head and tail.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/mac80211.h    |   7 +
 net/mac80211/cfg.c        | 316 +++++++++++++++++++++++++++++++-------
 net/mac80211/driver-ops.h |  16 ++
 net/mac80211/trace.h      |  28 ++++
 4 files changed, 310 insertions(+), 57 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index cb9f8e14b3b6..c64478dc7c53 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -4586,6 +4586,9 @@ struct ieee80211_prep_tx_info {
  *	just "paused" for scanning/ROC, which is indicated by the beacon being
  *	disabled/enabled via @bss_info_changed.
  * @stop_ap: Stop operation on the AP interface.
+ * @update_beacon: Update the templates indicated by @changed (beacon, probe
+ *	response, FILS discovery and/or unsolicitated bcast probe response)
+ *	without the wiphy mutex held. This callback must not sleep.
  *
  * @reconfig_complete: Called after a call to ieee80211_restart_hw() and
  *	during resume, when the reconfiguration has completed.
@@ -4783,6 +4786,10 @@ struct ieee80211_ops {
 			struct ieee80211_bss_conf *link_conf);
 	void (*stop_ap)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 			struct ieee80211_bss_conf *link_conf);
+	void (*update_beacon)(struct ieee80211_hw *hw,
+			      struct ieee80211_vif *vif,
+			      struct ieee80211_bss_conf *link_conf,
+			      u64 changed);
 
 	u64 (*prepare_multicast)(struct ieee80211_hw *hw,
 				 struct netdev_hw_addr_list *mc_list);
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 41d62e199851..aff9b1023d80 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1466,68 +1466,37 @@ static void ieee80211_update_ap_bandwidth(struct ieee80211_link_data *link,
 	ieee80211_recalc_chanctx_min_def(local, chanctx);
 }
 
-static int
-ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata,
-			struct ieee80211_link_data *link,
-			struct cfg80211_beacon_data *params,
-			const struct ieee80211_csa_settings *csa,
-			const struct ieee80211_color_change_settings *cca,
-			u64 *changed)
+static struct beacon_data *
+ieee80211_alloc_beacon(struct cfg80211_beacon_data *params,
+		       int head_len, int tail_len)
 {
-	struct cfg80211_mbssid_elems *mbssid = NULL;
-	struct cfg80211_rnr_elems *rnr = NULL;
-	struct beacon_data *new, *old;
-	int new_head_len, new_tail_len;
-	int size, err;
-	u64 _changed = BSS_CHANGED_BEACON;
-	struct ieee80211_bss_conf *link_conf = link->conf;
+	struct cfg80211_mbssid_elems *mbssid = params->mbssid_ies;
+	struct cfg80211_rnr_elems *rnr = mbssid ? params->rnr_ies : NULL;
+	struct beacon_data *new;
+	int size;
 
-	old = sdata_dereference(link->u.ap.beacon, sdata);
+	size = sizeof(*new) + head_len + tail_len;
 
-	/* Need to have a beacon head if we don't have one yet */
-	if (!params->head && !old)
-		return -EINVAL;
-
-	/* new or old head? */
-	if (params->head)
-		new_head_len = params->head_len;
-	else
-		new_head_len = old->head_len;
-
-	/* new or old tail? */
-	if (params->tail || !old)
-		/* params->tail_len will be zero for !params->tail */
-		new_tail_len = params->tail_len;
-	else
-		new_tail_len = old->tail_len;
-
-	size = sizeof(*new) + new_head_len + new_tail_len;
-
-	if (params->mbssid_ies) {
-		mbssid = params->mbssid_ies;
+	if (mbssid) {
 		size += struct_size(new->mbssid_ies, elem, mbssid->cnt);
-		if (params->rnr_ies) {
-			rnr = params->rnr_ies;
+		if (rnr)
 			size += struct_size(new->rnr_ies, elem, rnr->cnt);
-		}
 		size += ieee80211_get_mbssid_beacon_len(mbssid, rnr,
 							mbssid->cnt);
 	}
 
 	new = kzalloc(size, GFP_KERNEL);
 	if (!new)
-		return -ENOMEM;
-
-	/* start filling the new info now */
+		return NULL;
 
 	/*
 	 * pointers go into the block we allocated,
 	 * memory is | beacon_data | head | tail | mbssid_ies | rnr_ies
 	 */
 	new->head = ((u8 *) new) + sizeof(*new);
-	new->tail = new->head + new_head_len;
-	new->head_len = new_head_len;
-	new->tail_len = new_tail_len;
+	new->tail = new->head + head_len;
+	new->head_len = head_len;
+	new->tail_len = tail_len;
 	/* copy in optional mbssid_ies */
 	if (mbssid) {
 		u8 *pos = new->tail + new->tail_len;
@@ -1541,14 +1510,61 @@ ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata,
 			pos += struct_size(new->rnr_ies, elem, rnr->cnt);
 			ieee80211_copy_rnr_beacon(pos, new->rnr_ies, rnr);
 		}
-		/* update bssid_indicator */
-		if (new->mbssid_ies->cnt && new->mbssid_ies->elem[0].len > 2)
-			link_conf->bssid_indicator =
-					*(new->mbssid_ies->elem[0].data + 2);
-		else
-			link_conf->bssid_indicator = 0;
 	}
 
+	return new;
+}
+
+static u8 ieee80211_beacon_bssid_indicator(struct beacon_data *beacon)
+{
+	if (beacon->mbssid_ies->cnt && beacon->mbssid_ies->elem[0].len > 2)
+		return *(beacon->mbssid_ies->elem[0].data + 2);
+	return 0;
+}
+
+static int
+ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata,
+			struct ieee80211_link_data *link,
+			struct cfg80211_beacon_data *params,
+			const struct ieee80211_csa_settings *csa,
+			const struct ieee80211_color_change_settings *cca,
+			u64 *changed)
+{
+	struct beacon_data *new, *old;
+	int new_head_len, new_tail_len;
+	int err;
+	u64 _changed = BSS_CHANGED_BEACON;
+	struct ieee80211_bss_conf *link_conf = link->conf;
+
+	scoped_guard(spinlock, &link->ap_tmpl_lock) {
+		old = ap_tmpl_dereference(link, link->u.ap.beacon);
+
+		/* Need to have a beacon head if we don't have one yet */
+		if (!params->head && !old)
+			return -EINVAL;
+
+		/* new or old head? */
+		if (params->head)
+			new_head_len = params->head_len;
+		else
+			new_head_len = old->head_len;
+
+		/* new or old tail? */
+		if (params->tail || !old)
+			/* params->tail_len will be zero for !params->tail */
+			new_tail_len = params->tail_len;
+		else
+			new_tail_len = old->tail_len;
+	}
+
+	new = ieee80211_alloc_beacon(params, new_head_len, new_tail_len);
+	if (!new)
+		return -ENOMEM;
+
+	if (new->mbssid_ies)
+		link_conf->bssid_indicator =
+			ieee80211_beacon_bssid_indicator(new);
+
 	if (csa) {
 		new->cntdwn_current_counter = csa->count;
 		memcpy(new->cntdwn_counter_offsets, csa->counter_offsets_beacon,
@@ -1562,15 +1578,10 @@ ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata,
 	/* copy in head */
 	if (params->head)
 		memcpy(new->head, params->head, new_head_len);
-	else
-		memcpy(new->head, old->head, new_head_len);
 
 	/* copy in optional tail */
 	if (params->tail)
 		memcpy(new->tail, params->tail, new_tail_len);
-	else
-		if (old)
-			memcpy(new->tail, old->tail, new_tail_len);
 
 	err = ieee80211_set_probe_resp(sdata, params->probe_resp,
 				       params->probe_resp_len, csa, cca, link);
@@ -1598,9 +1609,28 @@ ieee80211_assign_beacon(struct ieee80211_sub_if_data *sdata,
 		_changed |= BSS_CHANGED_FTM_RESPONDER;
 	}
 
-	ap_tmpl_replace(link, link->u.ap.beacon, new);
+	scoped_guard(spinlock, &link->ap_tmpl_lock) {
+		old = ap_tmpl_dereference(link, link->u.ap.beacon);
+
+		/*
+		 * A lockless update (which always has head and tail) may have
+		 * replaced the old beacon since the lengths were taken, so it
+		 * might not match; the result is then wrong, but at least safe.
+		 */
+		if (!params->head)
+			memcpy(new->head, old->head,
+			       min(new_head_len, old->head_len));
+		if (!params->tail && old)
+			memcpy(new->tail, old->tail,
+			       min(new_tail_len, old->tail_len));
+
+		rcu_assign_pointer(link->u.ap.beacon, new);
+	}
 	sdata->u.ap.active = true;
 
+	if (old)
+		kfree_rcu(old, rcu_head);
+
 	ieee80211_update_ap_bandwidth(link, params);
 
 	*changed |= _changed;
@@ -1999,6 +2029,177 @@ static int ieee80211_change_beacon(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
+static int ieee80211_update_beacon(struct wiphy *wiphy, struct net_device *dev,
+				   struct cfg80211_ap_update *params)
+{
+	struct cfg80211_unsol_bcast_probe_resp *ubpr =
+		&params->unsol_bcast_probe_resp;
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct cfg80211_fils_discovery *fd = &params->fils_discovery;
+	struct unsol_bcast_probe_resp_data *new_ubpr = NULL, *old_ubpr = NULL;
+	struct cfg80211_beacon_data *beacon = &params->beacon;
+	struct fils_discovery_data *new_fd = NULL, *old_fd = NULL;
+	struct probe_resp *new_presp = NULL, *old_presp = NULL;
+	struct ieee80211_local *local = sdata->local;
+	struct beacon_data *new, *old = NULL;
+	struct ieee80211_bss_conf *link_conf;
+	struct ieee80211_link_data *link;
+	struct ieee80211_channel *chan;
+	u64 changed = BSS_CHANGED_BEACON;
+	int err = 0;
+
+	if (READ_ONCE(local->in_reconfig) || READ_ONCE(local->quiescing))
+		return -EAGAIN;
+
+	new = ieee80211_alloc_beacon(beacon, beacon->head_len,
+				     beacon->tail_len);
+	if (!new)
+		return -ENOMEM;
+	memcpy(new->head, beacon->head, beacon->head_len);
+	memcpy(new->tail, beacon->tail, beacon->tail_len);
+
+	if (beacon->probe_resp && beacon->probe_resp_len) {
+		new_presp = kzalloc(sizeof(*new_presp) + beacon->probe_resp_len,
+				    GFP_KERNEL);
+		if (!new_presp) {
+			err = -ENOMEM;
+			goto free;
+		}
+		new_presp->len = beacon->probe_resp_len;
+		memcpy(new_presp->data, beacon->probe_resp,
+		       beacon->probe_resp_len);
+		changed |= BSS_CHANGED_AP_PROBE_RESP;
+	}
+
+	if (fd->update && fd->tmpl && fd->tmpl_len) {
+		new_fd = kzalloc(sizeof(*new_fd) + fd->tmpl_len, GFP_KERNEL);
+		if (!new_fd) {
+			err = -ENOMEM;
+			goto free;
+		}
+		new_fd->len = fd->tmpl_len;
+		memcpy(new_fd->data, fd->tmpl, fd->tmpl_len);
+	}
+
+	if (ubpr->update && ubpr->tmpl && ubpr->tmpl_len) {
+		new_ubpr = kzalloc(sizeof(*new_ubpr) + ubpr->tmpl_len,
+				   GFP_KERNEL);
+		if (!new_ubpr) {
+			err = -ENOMEM;
+			goto free;
+		}
+		new_ubpr->len = ubpr->tmpl_len;
+		memcpy(new_ubpr->data, ubpr->tmpl, ubpr->tmpl_len);
+	}
+
+	rcu_read_lock();
+
+	/* do_stop() synchronizes RCU before the interface type can change */
+	if (!ieee80211_sdata_running(sdata) ||
+	    (sdata->vif.type != NL80211_IFTYPE_AP &&
+	     sdata->vif.type != NL80211_IFTYPE_P2P_GO)) {
+		err = -ENETDOWN;
+		goto unlock;
+	}
+
+	link = rcu_dereference(sdata->link[beacon->link_id]);
+	if (!link) {
+		err = -ENOLINK;
+		goto unlock;
+	}
+	link_conf = link->conf;
+
+	/* configuration changes need the wiphy mutex, let cfg80211 retry */
+	err = -EAGAIN;
+
+	if (READ_ONCE(link_conf->csa_active) ||
+	    READ_ONCE(link_conf->color_change_active))
+		goto unlock;
+
+	if (new->mbssid_ies &&
+	    ieee80211_beacon_bssid_indicator(new) !=
+			READ_ONCE(link_conf->bssid_indicator))
+		goto unlock;
+
+	if (fd->update &&
+	    (fd->min_interval != READ_ONCE(link_conf->fils_discovery.min_interval) ||
+	     fd->max_interval != READ_ONCE(link_conf->fils_discovery.max_interval)))
+		goto unlock;
+
+	if (ubpr->update &&
+	    ubpr->interval !=
+			READ_ONCE(link_conf->unsol_bcast_probe_resp_interval))
+		goto unlock;
+
+	chan = READ_ONCE(link_conf->chanreq.oper.chan);
+	if (!chan)
+		goto unlock;
+
+	if (chan->band != NL80211_BAND_S1GHZ) {
+		enum ieee80211_sta_rx_bandwidth he_and_lower;
+
+		he_and_lower = ieee80211_calc_ap_he_and_lower(beacon);
+		if (he_and_lower != READ_ONCE(link->bss_bw.he_and_lower) ||
+		    ieee80211_calc_ap_eht_bw(beacon, he_and_lower) !=
+				READ_ONCE(link->bss_bw.eht))
+			goto unlock;
+	}
+
+	spin_lock(&link->ap_tmpl_lock);
+	old = ap_tmpl_dereference(link, link->u.ap.beacon);
+	if (!old) {
+		err = -ENOENT;
+	} else if (READ_ONCE(old->cntdwn_current_counter)) {
+		/* a countdown just started, keep the offsets */
+		old = NULL;
+	} else {
+		rcu_assign_pointer(link->u.ap.beacon, new);
+		new = NULL;
+
+		if (new_presp)
+			old_presp = rcu_replace_pointer(link->u.ap.probe_resp,
+							new_presp,
+							lockdep_is_held(&link->ap_tmpl_lock));
+		new_presp = NULL;
+
+		if (fd->update) {
+			old_fd = rcu_replace_pointer(link->u.ap.fils_discovery,
+						     new_fd,
+						     lockdep_is_held(&link->ap_tmpl_lock));
+			changed |= BSS_CHANGED_FILS_DISCOVERY;
+		}
+		new_fd = NULL;
+
+		if (ubpr->update) {
+			old_ubpr = rcu_replace_pointer(link->u.ap.unsol_bcast_probe_resp,
+						       new_ubpr,
+						       lockdep_is_held(&link->ap_tmpl_lock));
+			changed |= BSS_CHANGED_UNSOL_BCAST_PROBE_RESP;
+		}
+		new_ubpr = NULL;
+
+		drv_update_beacon(local, sdata, link_conf, changed);
+		err = 0;
+	}
+	spin_unlock(&link->ap_tmpl_lock);
+unlock:
+	rcu_read_unlock();
+free:
+	kfree(new);
+	kfree(new_presp);
+	kfree(new_fd);
+	kfree(new_ubpr);
+	if (old)
+		kfree_rcu(old, rcu_head);
+	if (old_presp)
+		kfree_rcu(old_presp, rcu_head);
+	if (old_fd)
+		kfree_rcu(old_fd, rcu_head);
+	if (old_ubpr)
+		kfree_rcu(old_ubpr, rcu_head);
+	return err;
+}
+
 static void ieee80211_free_next_beacon(struct ieee80211_link_data *link)
 {
 	if (!link->u.ap.next_beacon)
@@ -6018,6 +6219,7 @@ const struct cfg80211_ops mac80211_config_ops = {
 	.set_default_beacon_key = ieee80211_config_default_beacon_key,
 	.start_ap = ieee80211_start_ap,
 	.change_beacon = ieee80211_change_beacon,
+	.update_beacon = ieee80211_update_beacon,
 	.stop_ap = ieee80211_stop_ap,
 	.add_station = ieee80211_add_station,
 	.del_station = ieee80211_del_station,
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index f1c0b87fddd5..accd89bbc1fb 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -1107,6 +1107,22 @@ static inline void drv_stop_ap(struct ieee80211_local *local,
 	trace_drv_return_void(local);
 }
 
+static inline void drv_update_beacon(struct ieee80211_local *local,
+				     struct ieee80211_sub_if_data *sdata,
+				     struct ieee80211_bss_conf *link_conf,
+				     u64 changed)
+{
+	/* can race with HW restart, so don't warn */
+	if (!(sdata->flags & IEEE80211_SDATA_IN_DRIVER))
+		return;
+
+	trace_drv_update_beacon(local, sdata, link_conf, changed);
+	if (local->ops->update_beacon)
+		local->ops->update_beacon(&local->hw, &sdata->vif, link_conf,
+					  changed);
+	trace_drv_return_void(local);
+}
+
 static inline void
 drv_reconfig_complete(struct ieee80211_local *local,
 		      enum ieee80211_reconfig_type reconfig_type)
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 562a4964afa3..ea46c3cca21a 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1924,6 +1924,34 @@ TRACE_EVENT(drv_stop_ap,
 	)
 );
 
+TRACE_EVENT(drv_update_beacon,
+	TP_PROTO(struct ieee80211_local *local,
+		 struct ieee80211_sub_if_data *sdata,
+		 struct ieee80211_bss_conf *link_conf,
+		 u64 changed),
+
+	TP_ARGS(local, sdata, link_conf, changed),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		VIF_ENTRY
+		__field(u32, link_id)
+		__field(u64, changed)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		VIF_ASSIGN;
+		__entry->link_id = link_conf->link_id;
+		__entry->changed = changed;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT  VIF_PR_FMT " link id %u changed:%#llx",
+		LOCAL_PR_ARG, VIF_PR_ARG, __entry->link_id, __entry->changed
+	)
+);
+
 TRACE_EVENT(drv_reconfig_complete,
 	TP_PROTO(struct ieee80211_local *local,
 		 enum ieee80211_reconfig_type reconfig_type),
-- 
2.55.0

Re: [RFC PATCH 03/12] wifi: mac80211: implement lockless beacon updates

[Johannes Berg]

On Sun, 2026-10-04 at 23:40 +0200, Johannes Berg wrote:
> The regular beacon update can no longer reliably access
> the old beacon across the allocation (or we'd have to do
> all those atomically), so if userspace does both at the
> same time, we may send a corrupt beacon due to min() in
> the copy. This won't happen with hostapd even if it were
> to race, since it always sends both head and tail.

Heh. Aloka got really lucky, I went to rebase this now and it conflicts
pretty badly - not just patch wise but semantically - with the MBSSID
old beacon data fix.

This should never happen, but if a normal locked update is running while
an unlocked update replaces the beacon (including head, tail and mbssid
data), then the locked update cannot use the old beacon head, tail or
mbssid data at the same time, it might go away concurrently.

For the head/tail I basically said above that doesn't matter, we'll send
garbage if hostapd is causing garbage (just use whichever head/tail we
find, min() on the size); for the MBSSID data that doesn't really work,
the MBSSID fix said:

    Hostapd passes two Beacon templates to kernel for CSA and CCA -
    (1) beacon_csa/beacon_color_change used during the countdown.
    (2) beacon_after/beacon_next used after the countdown completes.
    Hostapd relies on the kernel to include the old MBSSID elements
    while sending beacon_csa/beacon_color_change templates to the
    driver.

So ... where does that leave us here.

I think the only good solution is to say this race will never happen
anyway, hostapd clearly won't do update-beacon and set-beacon at the
same time for the same interface. But we have to protect memory safety
in the kernel anyway.

So I think I'll change this to track "is a locked beacon update in
progress", and then an unlocked beacon update just returns -EAGAIN if it
finds (under the spinlock) that to be true. Presumably that'll never
happen, but if it does then it'll be correct. It also means that
unlocked beacon updates always have to come with their own MBSSID data.

johannes

[RFC PATCH 04/12] wifi: mac80211_hwsim: support lockless beacon updates

[Johannes Berg]

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

We just use ieee80211_beacon_get_template_ema_list() or
ieee80211_beacon_get() here to transmit the beacons, so
there's no need to implement update_beacon() and we can
just set NL80211_EXT_FEATURE_UPDATE_BEACON.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/virtual/mac80211_hwsim_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/virtual/mac80211_hwsim_main.c b/drivers/net/wireless/virtual/mac80211_hwsim_main.c
index 59c540d2677b..c93c8d61d1c0 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim_main.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim_main.c
@@ -6010,6 +6010,7 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
 			      NL80211_EXT_FEATURE_EXT_KEY_ID);
 	wiphy_ext_feature_set(hw->wiphy,
 			      NL80211_EXT_FEATURE_ASSOC_FRAME_ENCRYPTION);
+	wiphy_ext_feature_set(hw->wiphy, NL80211_EXT_FEATURE_UPDATE_BEACON);
 
 	hw->wiphy->interface_modes = param->iftypes;
 
-- 
2.55.0

[RFC PATCH 05/12] wifi: cfg80211: make cookie counter atomic

[Johannes Berg]

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

The cookie counter is currently protected by the
wiphy mutex, but in order to be able to transmit
management frames without holding that, we need
to assign cookies atomically.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/core.h | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/wireless/core.h b/net/wireless/core.h
index dfcb9ed5035b..90d59bac4701 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -82,7 +82,9 @@ struct cfg80211_registered_device {
 	/* protected by RTNL only */
 	int num_running_ifaces;
 	int num_running_monitor_ifaces;
-	u64 cookie_counter;
+
+	/* atomic for unlocked users */
+	atomic64_t cookie_counter;
 
 	/* BSSes/scanning */
 	spinlock_t bss_lock;
@@ -170,10 +172,10 @@ cfg80211_rdev_free_wowlan(struct cfg80211_registered_device *rdev)
 
 static inline u64 cfg80211_assign_cookie(struct cfg80211_registered_device *rdev)
 {
-	u64 r = ++rdev->cookie_counter;
+	u64 r = atomic64_inc_return(&rdev->cookie_counter);
 
 	if (WARN_ON(r == 0))
-		r = ++rdev->cookie_counter;
+		r = atomic64_inc_return(&rdev->cookie_counter);
 
 	return r;
 }
-- 
2.55.0

[RFC PATCH 06/12] wifi: nl80211: allow mgmt frame TX without wiphy mutex

[Johannes Berg]

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

Management frame transmissions need the wiphy mutex,
but the normal transmit path doesn't, so most cases
shouldn't need it here either. Due to checks, limit
it to AP/GO for now.

To support it, drivers must implement the new method
mgmt_tx_unlocked(), and return -EAGAIN from that for
requests that cannot be done without the wiphy mutex
(e.g. off-channel in mac80211), then the operation
will be retried with the wiphy mutex held.

This requires some work in nl80211_pre_doit() since
the same operation might be called with a wdev or
a netdev, but only netdevs can safely not lock the
wiphy mutex - hence NL80211_FLAG_NO_WIPHY_MTX_NDEV.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h  |  7 ++++
 net/wireless/core.h     |  4 +++
 net/wireless/mlme.c     | 49 +++++++++++++++++++++----
 net/wireless/nl80211.c  | 79 ++++++++++++++++++++++++++++++++---------
 net/wireless/rdev-ops.h | 12 +++++++
 net/wireless/trace.h    | 14 +++++++-
 6 files changed, 141 insertions(+), 24 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ba66fb45dc12..f227b702b38a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5080,6 +5080,9 @@ struct mgmt_frame_regs {
  *	This allows the operation to be terminated prior to timeout based on
  *	the duration value.
  * @mgmt_tx: Transmit a management frame.
+ * @mgmt_tx_unlocked: Like @mgmt_tx, but called without the wiphy mutex,
+ *	for AP/GO only. Return -EAGAIN to get @mgmt_tx with the wiphy
+ *	mutex held for the same operation.
  * @mgmt_tx_cancel_wait: Cancel the wait time from transmitting a management
  *	frame on another channel
  *
@@ -5503,6 +5506,10 @@ struct cfg80211_ops {
 	int	(*mgmt_tx)(struct wiphy *wiphy, struct wireless_dev *wdev,
 			   struct cfg80211_mgmt_tx_params *params,
 			   u64 cookie);
+	int	(*mgmt_tx_unlocked)(struct wiphy *wiphy,
+				    struct wireless_dev *wdev,
+				    struct cfg80211_mgmt_tx_params *params,
+				    u64 cookie);
 	int	(*mgmt_tx_cancel_wait)(struct wiphy *wiphy,
 				       struct wireless_dev *wdev,
 				       u64 cookie);
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 90d59bac4701..664a759f876b 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -413,6 +413,10 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
 			  struct wireless_dev *wdev,
 			  struct cfg80211_mgmt_tx_params *params,
 			  u64 cookie);
+int cfg80211_mlme_mgmt_tx_unlocked(struct cfg80211_registered_device *rdev,
+				   struct wireless_dev *wdev,
+				   struct cfg80211_mgmt_tx_params *params,
+				   u64 cookie);
 void cfg80211_oper_and_ht_capa(struct ieee80211_ht_cap *ht_capa,
 			       const struct ieee80211_ht_cap *ht_capa_mask);
 void cfg80211_oper_and_vht_capa(struct ieee80211_vht_cap *vht_capa,
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 96fd14f2a305..e81dae5bf15b 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -892,15 +892,14 @@ static bool cfg80211_allowed_random_address(struct wireless_dev *wdev,
 	return false;
 }
 
-int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
-			  struct wireless_dev *wdev,
-			  struct cfg80211_mgmt_tx_params *params, u64 cookie)
+static int cfg80211_mgmt_tx_check(struct cfg80211_registered_device *rdev,
+				  struct wireless_dev *wdev,
+				  enum nl80211_iftype iftype,
+				  struct cfg80211_mgmt_tx_params *params)
 {
 	const struct ieee80211_mgmt *mgmt;
 	u16 stype;
 
-	lockdep_assert_wiphy(&rdev->wiphy);
-
 	if (!wdev->wiphy->mgmt_stypes)
 		return -EOPNOTSUPP;
 
@@ -917,14 +916,14 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
 		return -EINVAL;
 
 	stype = le16_to_cpu(mgmt->frame_control) & IEEE80211_FCTL_STYPE;
-	if (!(wdev->wiphy->mgmt_stypes[wdev->iftype].tx & BIT(stype >> 4)))
+	if (!(wdev->wiphy->mgmt_stypes[iftype].tx & BIT(stype >> 4)))
 		return -EINVAL;
 
 	if (ieee80211_is_action(mgmt->frame_control) &&
 	    mgmt->u.action.category != WLAN_CATEGORY_PUBLIC) {
 		int err = 0;
 
-		switch (wdev->iftype) {
+		switch (iftype) {
 		case NL80211_IFTYPE_ADHOC:
 			/*
 			 * check for IBSS DA must be done by driver as
@@ -1003,10 +1002,46 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
 	    !cfg80211_allowed_random_address(wdev, mgmt))
 		return -EINVAL;
 
+	return 0;
+}
+
+int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
+			  struct wireless_dev *wdev,
+			  struct cfg80211_mgmt_tx_params *params, u64 cookie)
+{
+	int err;
+
+	lockdep_assert_wiphy(&rdev->wiphy);
+
+	err = cfg80211_mgmt_tx_check(rdev, wdev, wdev->iftype, params);
+	if (err)
+		return err;
+
 	/* Transmit the management frame as requested by user space */
 	return rdev_mgmt_tx(rdev, wdev, params, cookie);
 }
 
+int cfg80211_mlme_mgmt_tx_unlocked(struct cfg80211_registered_device *rdev,
+				   struct wireless_dev *wdev,
+				   struct cfg80211_mgmt_tx_params *params,
+				   u64 cookie)
+{
+	/* can change concurrently, so read only once */
+	enum nl80211_iftype iftype = READ_ONCE(wdev->iftype);
+	int err;
+
+	/* the checks for other interface types need the wiphy mutex */
+	if (!rdev->ops->mgmt_tx_unlocked ||
+	    (iftype != NL80211_IFTYPE_AP && iftype != NL80211_IFTYPE_P2P_GO))
+		return -EAGAIN;
+
+	err = cfg80211_mgmt_tx_check(rdev, wdev, iftype, params);
+	if (err)
+		return err;
+
+	return rdev_mgmt_tx_unlocked(rdev, wdev, params, cookie);
+}
+
 bool cfg80211_rx_mgmt_ext(struct wireless_dev *wdev,
 			  struct cfg80211_rx_info *info)
 {
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b342a7b133d5..ae3f4da167c5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -15102,6 +15102,22 @@ static int nl80211_register_mgmt(struct sk_buff *skb, struct genl_info *info)
 					   info->extack);
 }
 
+static int nl80211_tx_mgmt_locked(struct cfg80211_registered_device *rdev,
+				  struct wireless_dev *wdev,
+				  struct cfg80211_mgmt_tx_params *params,
+				  u64 cookie)
+{
+	if (params->link_id >= 0 &&
+	    !(wdev->valid_links & BIT(params->link_id)))
+		return -EINVAL;
+
+	if (params->offchan &&
+	    !cfg80211_off_channel_oper_allowed(wdev, params->chan))
+		return -EBUSY;
+
+	return cfg80211_mlme_mgmt_tx(rdev, wdev, params, cookie);
+}
+
 static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
@@ -15187,18 +15203,14 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
 	if (!chandef.chan && params.offchan)
 		return -EINVAL;
 
-	if (params.offchan &&
-	    !cfg80211_off_channel_oper_allowed(wdev, chandef.chan))
-		return -EBUSY;
-
 	params.link_id = nl80211_link_id_or_invalid(info->attrs);
 	/*
-	 * This now races due to the unlock, but we cannot check
-	 * the valid links for the _station_ anyway, so that's up
-	 * to the driver.
+	 * This races (even with the wiphy mutex, since it's not held for
+	 * netdevs here), but we cannot check the valid links for the
+	 * _station_ anyway, so that's up to the driver.
 	 */
 	if (params.link_id >= 0 &&
-	    !(wdev->valid_links & BIT(params.link_id)))
+	    !(READ_ONCE(wdev->valid_links) & BIT(params.link_id)))
 		return -EINVAL;
 
 	params.no_sta =
@@ -15229,7 +15241,23 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
 
 	params.chan = chandef.chan;
 	cookie = cfg80211_assign_cookie(rdev);
-	err = cfg80211_mlme_mgmt_tx(rdev, wdev, &params, cookie);
+
+	if (wdev->netdev) {
+		/* pre_doit didn't lock the wiphy mutex, try without */
+		err = cfg80211_mlme_mgmt_tx_unlocked(rdev, wdev, &params,
+						     cookie);
+		if (err == -EAGAIN) {
+			wiphy_lock(&rdev->wiphy);
+			if (!rdev->wiphy.registered || !wdev_running(wdev))
+				err = -ENETDOWN;
+			else
+				err = nl80211_tx_mgmt_locked(rdev, wdev,
+							     &params, cookie);
+			wiphy_unlock(&rdev->wiphy);
+		}
+	} else {
+		err = nl80211_tx_mgmt_locked(rdev, wdev, &params, cookie);
+	}
 	if (err)
 		goto free_msg;
 
@@ -19796,6 +19824,8 @@ nl80211_epcs_cfg(struct sk_buff *skb, struct genl_info *info)
 #define NL80211_FLAG_NO_WIPHY_MTX	0x40
 #define NL80211_FLAG_MLO_VALID_LINK_ID	0x80
 #define NL80211_FLAG_MLO_UNSUPPORTED	0x100
+/* no wiphy mutex if there's a netdev */
+#define NL80211_FLAG_NO_WIPHY_MTX_NDEV	0x200
 
 #define INTERNAL_FLAG_SELECTORS(__sel)			\
 	SELECTOR(__sel, NONE, 0) /* must be first */	\
@@ -19863,7 +19893,10 @@ nl80211_epcs_cfg(struct sk_buff *skb, struct genl_info *info)
 	SELECTOR(__sel, WDEV_UP_RTNL_NOMTX,		\
 		 NL80211_FLAG_NEED_WDEV_UP |		\
 		 NL80211_FLAG_NO_WIPHY_MTX |		\
-		 NL80211_FLAG_NEED_RTNL)
+		 NL80211_FLAG_NEED_RTNL)		\
+	SELECTOR(__sel, WDEV_UP_NOMTX_NETDEV,		\
+		 NL80211_FLAG_NEED_WDEV_UP |		\
+		 NL80211_FLAG_NO_WIPHY_MTX_NDEV)
 
 /*
  * Note: a
@@ -19889,6 +19922,14 @@ static u32 nl80211_internal_flags[] = {
 #undef SELECTOR
 };
 
+static bool nl80211_need_wiphy_mtx(u32 internal_flags, struct net_device *dev)
+{
+	if (internal_flags & NL80211_FLAG_NO_WIPHY_MTX)
+		return false;
+
+	return !(internal_flags & NL80211_FLAG_NO_WIPHY_MTX_NDEV && dev);
+}
+
 static int nl80211_pre_doit(const struct genl_split_ops *ops,
 			    struct sk_buff *skb,
 			    struct genl_info *info)
@@ -19946,7 +19987,7 @@ static int nl80211_pre_doit(const struct genl_split_ops *ops,
 		}
 	}
 
-	if (rdev && !(internal_flags & NL80211_FLAG_NO_WIPHY_MTX)) {
+	if (rdev && nl80211_need_wiphy_mtx(internal_flags, dev)) {
 		if (need_rtnl) {
 			wiphy_lock(&rdev->wiphy);
 		} else {
@@ -20047,19 +20088,24 @@ static void nl80211_post_doit(const struct genl_split_ops *ops,
 			      struct genl_info *info)
 {
 	u32 internal_flags = nl80211_internal_flags[ops->internal_flags];
+	struct net_device *dev = NULL;
+	bool locked;
 
 	if (info->user_ptr[1]) {
 		if (internal_flags & NL80211_FLAG_NEED_WDEV) {
 			struct wireless_dev *wdev = info->user_ptr[1];
 
-			dev_put(wdev->netdev);
+			dev = wdev->netdev;
 		} else {
-			dev_put(info->user_ptr[1]);
+			dev = info->user_ptr[1];
 		}
 	}
 
-	if (info->user_ptr[0] &&
-	    !(internal_flags & NL80211_FLAG_NO_WIPHY_MTX)) {
+	locked = info->user_ptr[0] &&
+		 nl80211_need_wiphy_mtx(internal_flags, dev);
+	dev_put(dev);
+
+	if (locked) {
 		struct cfg80211_registered_device *rdev = info->user_ptr[0];
 
 		/* we kept the mutex locked since pre_doit */
@@ -20601,7 +20647,8 @@ static const struct genl_small_ops nl80211_small_ops[] = {
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = nl80211_tx_mgmt,
 		.flags = GENL_UNS_ADMIN_PERM,
-		.internal_flags = IFLAGS(NL80211_FLAG_NEED_WDEV_UP),
+		.internal_flags = IFLAGS(NL80211_FLAG_NEED_WDEV_UP |
+					 NL80211_FLAG_NO_WIPHY_MTX_NDEV),
 	},
 	{
 		.cmd = NL80211_CMD_FRAME_WAIT_CANCEL,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 95882c8972d2..5502cd947844 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -785,6 +785,18 @@ static inline int rdev_mgmt_tx(struct cfg80211_registered_device *rdev,
 	return ret;
 }
 
+static inline int
+rdev_mgmt_tx_unlocked(struct cfg80211_registered_device *rdev,
+		      struct wireless_dev *wdev,
+		      struct cfg80211_mgmt_tx_params *params, u64 cookie)
+{
+	int ret;
+	trace_rdev_mgmt_tx_unlocked(&rdev->wiphy, wdev, params);
+	ret = rdev->ops->mgmt_tx_unlocked(&rdev->wiphy, wdev, params, cookie);
+	trace_rdev_return_int_cookie(&rdev->wiphy, ret, cookie);
+	return ret;
+}
+
 static inline int rdev_tx_control_port(struct cfg80211_registered_device *rdev,
 				       struct net_device *dev,
 				       const void *buf, size_t len,
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index f5fa2fb76474..1f3c1e0eb26a 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2236,7 +2236,7 @@ TRACE_EVENT(rdev_cancel_remain_on_channel,
 		  WIPHY_PR_ARG, WDEV_PR_ARG, __entry->cookie)
 );
 
-TRACE_EVENT(rdev_mgmt_tx,
+DECLARE_EVENT_CLASS(rdev_mgmt_tx_evt,
 	TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
 		 struct cfg80211_mgmt_tx_params *params),
 	TP_ARGS(wiphy, wdev, params),
@@ -2266,6 +2266,18 @@ TRACE_EVENT(rdev_mgmt_tx,
 		  BOOL_TO_STR(__entry->dont_wait_for_ack))
 );
 
+DEFINE_EVENT(rdev_mgmt_tx_evt, rdev_mgmt_tx,
+	TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+		 struct cfg80211_mgmt_tx_params *params),
+	TP_ARGS(wiphy, wdev, params)
+);
+
+DEFINE_EVENT(rdev_mgmt_tx_evt, rdev_mgmt_tx_unlocked,
+	TP_PROTO(struct wiphy *wiphy, struct wireless_dev *wdev,
+		 struct cfg80211_mgmt_tx_params *params),
+	TP_ARGS(wiphy, wdev, params)
+);
+
 TRACE_EVENT(rdev_tx_control_port,
 	TP_PROTO(struct wiphy *wiphy, struct net_device *netdev,
 		 const u8 *buf, size_t len, const u8 *dest, __be16 proto,
-- 
2.55.0

[RFC PATCH 07/12] wifi: mac80211: refactor mgmt frames TX

[Johannes Berg]

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

Refactor ieee80211_mgmt_tx() into on-channel and frame
building helper functions so we can later implement the
new mgmt_tx_unlocked() method.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/offchannel.c | 225 +++++++++++++++++++++-----------------
 1 file changed, 122 insertions(+), 103 deletions(-)

diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index c46cb81fab22..27ca2984826f 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -814,6 +814,124 @@ int ieee80211_cancel_remain_on_channel(struct wiphy *wiphy,
 	return ieee80211_cancel_roc(local, cookie, false);
 }
 
+static bool
+ieee80211_mgmt_tx_need_offchan(struct ieee80211_sub_if_data *sdata,
+			       struct cfg80211_mgmt_tx_params *params,
+			       const struct ieee80211_mgmt *mgmt,
+			       bool mlo_sta, int *link_id)
+{
+	struct ieee80211_chanctx_conf *chanctx_conf = NULL;
+	int i;
+
+	guard(rcu)();
+
+	/* Check all the links first */
+	for (i = 0; i < ARRAY_SIZE(sdata->vif.link_conf); i++) {
+		struct ieee80211_bss_conf *conf;
+
+		conf = rcu_dereference(sdata->vif.link_conf[i]);
+		if (!conf)
+			continue;
+
+		chanctx_conf = rcu_dereference(conf->chanctx_conf);
+		if (!chanctx_conf)
+			continue;
+
+		if (mlo_sta && params->chan == chanctx_conf->def.chan &&
+		    ether_addr_equal(sdata->vif.addr, mgmt->sa)) {
+			*link_id = i;
+			break;
+		}
+
+		if (ether_addr_equal(conf->addr, mgmt->sa)) {
+			/* If userspace requested Tx on a specific link
+			 * use the same link id if the link bss is matching
+			 * the requested chan.
+			 */
+			if (sdata->vif.valid_links &&
+			    params->link_id >= 0 && params->link_id == i &&
+			    params->chan == chanctx_conf->def.chan)
+				*link_id = i;
+
+			break;
+		}
+
+		chanctx_conf = NULL;
+	}
+
+	if (!chanctx_conf)
+		return true;
+
+	return params->chan && params->chan != chanctx_conf->def.chan;
+}
+
+static struct sk_buff *
+ieee80211_mgmt_tx_skb(struct ieee80211_sub_if_data *sdata,
+		      struct cfg80211_mgmt_tx_params *params,
+		      u64 cookie, gfp_t gfp)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct sk_buff *skb;
+	u32 flags;
+	u8 *data;
+
+	if (params->dont_wait_for_ack)
+		flags = IEEE80211_TX_CTL_NO_ACK;
+	else
+		flags = IEEE80211_TX_INTFL_NL80211_FRAME_TX |
+			IEEE80211_TX_CTL_REQ_TX_STATUS;
+
+	if (params->no_cck)
+		flags |= IEEE80211_TX_CTL_NO_CCK_RATE;
+
+	skb = dev_alloc_skb(local->hw.extra_tx_headroom + params->len);
+	if (!skb)
+		return NULL;
+	skb_reserve(skb, local->hw.extra_tx_headroom);
+
+	data = skb_put_data(skb, params->buf, params->len);
+
+	/* Update CSA counters */
+	if (sdata->vif.bss_conf.csa_active &&
+	    (sdata->vif.type == NL80211_IFTYPE_AP ||
+	     sdata->vif.type == NL80211_IFTYPE_MESH_POINT ||
+	     sdata->vif.type == NL80211_IFTYPE_ADHOC) &&
+	    params->n_csa_offsets) {
+		int i;
+		struct beacon_data *beacon = NULL;
+
+		rcu_read_lock();
+
+		if (sdata->vif.type == NL80211_IFTYPE_AP)
+			beacon = rcu_dereference(sdata->deflink.u.ap.beacon);
+		else if (sdata->vif.type == NL80211_IFTYPE_ADHOC)
+			beacon = rcu_dereference(sdata->u.ibss.presp);
+		else if (ieee80211_vif_is_mesh(&sdata->vif))
+			beacon = rcu_dereference(sdata->u.mesh.beacon);
+
+		if (beacon)
+			for (i = 0; i < params->n_csa_offsets; i++)
+				data[params->csa_offsets[i]] =
+					beacon->cntdwn_current_counter;
+
+		rcu_read_unlock();
+	}
+
+	IEEE80211_SKB_CB(skb)->flags = flags;
+	IEEE80211_SKB_CB(skb)->control.flags |= IEEE80211_TX_CTRL_DONT_USE_RATE_MASK;
+
+	skb->dev = sdata->dev;
+
+	/* make a copy to preserve the frame contents in case of encryption */
+	if (!params->dont_wait_for_ack &&
+	    ieee80211_attach_ack_skb(local, skb, &cookie, gfp)) {
+		kfree_skb(skb);
+		return NULL;
+	}
+
+	return skb;
+}
+
 int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		      struct cfg80211_mgmt_tx_params *params, u64 cookie)
 {
@@ -825,21 +943,10 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 	bool need_offchan = false;
 	bool mlo_sta = false;
 	int link_id = -1;
-	u32 flags;
 	int ret;
-	u8 *data;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	if (params->dont_wait_for_ack)
-		flags = IEEE80211_TX_CTL_NO_ACK;
-	else
-		flags = IEEE80211_TX_INTFL_NL80211_FRAME_TX |
-			IEEE80211_TX_CTL_REQ_TX_STATUS;
-
-	if (params->no_cck)
-		flags |= IEEE80211_TX_CTL_NO_CCK_RATE;
-
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_ADHOC:
 		if (!sdata->vif.cfg.ibss_joined)
@@ -927,52 +1034,9 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		   sdata->vif.type == NL80211_IFTYPE_NAN_DATA) {
 		/* Frames can be sent during NAN schedule */
 	} else if (!need_offchan) {
-		struct ieee80211_chanctx_conf *chanctx_conf = NULL;
-		int i;
-
-		rcu_read_lock();
-		/* Check all the links first */
-		for (i = 0; i < ARRAY_SIZE(sdata->vif.link_conf); i++) {
-			struct ieee80211_bss_conf *conf;
-
-			conf = rcu_dereference(sdata->vif.link_conf[i]);
-			if (!conf)
-				continue;
-
-			chanctx_conf = rcu_dereference(conf->chanctx_conf);
-			if (!chanctx_conf)
-				continue;
-
-			if (mlo_sta && params->chan == chanctx_conf->def.chan &&
-			    ether_addr_equal(sdata->vif.addr, mgmt->sa)) {
-				link_id = i;
-				break;
-			}
-
-			if (ether_addr_equal(conf->addr, mgmt->sa)) {
-				/* If userspace requested Tx on a specific link
-				 * use the same link id if the link bss is matching
-				 * the requested chan.
-				 */
-				if (sdata->vif.valid_links &&
-				    params->link_id >= 0 && params->link_id == i &&
-				    params->chan == chanctx_conf->def.chan)
-					link_id = i;
-
-				break;
-			}
-
-			chanctx_conf = NULL;
-		}
-
-		if (chanctx_conf) {
-			need_offchan = params->chan &&
-				       (params->chan !=
-					chanctx_conf->def.chan);
-		} else {
-			need_offchan = true;
-		}
-		rcu_read_unlock();
+		need_offchan = ieee80211_mgmt_tx_need_offchan(sdata, params,
+							      mgmt, mlo_sta,
+							      &link_id);
 	}
 
 	if (need_offchan && !params->offchan) {
@@ -980,56 +1044,11 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		goto out_unlock;
 	}
 
-	skb = dev_alloc_skb(local->hw.extra_tx_headroom + params->len);
+	skb = ieee80211_mgmt_tx_skb(sdata, params, cookie, GFP_KERNEL);
 	if (!skb) {
 		ret = -ENOMEM;
 		goto out_unlock;
 	}
-	skb_reserve(skb, local->hw.extra_tx_headroom);
-
-	data = skb_put_data(skb, params->buf, params->len);
-
-	/* Update CSA counters */
-	if (sdata->vif.bss_conf.csa_active &&
-	    (sdata->vif.type == NL80211_IFTYPE_AP ||
-	     sdata->vif.type == NL80211_IFTYPE_MESH_POINT ||
-	     sdata->vif.type == NL80211_IFTYPE_ADHOC) &&
-	    params->n_csa_offsets) {
-		int i;
-		struct beacon_data *beacon = NULL;
-
-		rcu_read_lock();
-
-		if (sdata->vif.type == NL80211_IFTYPE_AP)
-			beacon = rcu_dereference(sdata->deflink.u.ap.beacon);
-		else if (sdata->vif.type == NL80211_IFTYPE_ADHOC)
-			beacon = rcu_dereference(sdata->u.ibss.presp);
-		else if (ieee80211_vif_is_mesh(&sdata->vif))
-			beacon = rcu_dereference(sdata->u.mesh.beacon);
-
-		if (beacon)
-			for (i = 0; i < params->n_csa_offsets; i++)
-				data[params->csa_offsets[i]] =
-					beacon->cntdwn_current_counter;
-
-		rcu_read_unlock();
-	}
-
-	IEEE80211_SKB_CB(skb)->flags = flags;
-	IEEE80211_SKB_CB(skb)->control.flags |= IEEE80211_TX_CTRL_DONT_USE_RATE_MASK;
-
-	skb->dev = sdata->dev;
-
-	if (!params->dont_wait_for_ack) {
-		/* make a copy to preserve the frame contents
-		 * in case of encryption.
-		 */
-		ret = ieee80211_attach_ack_skb(local, skb, &cookie, GFP_KERNEL);
-		if (ret) {
-			kfree_skb(skb);
-			goto out_unlock;
-		}
-	}
 
 	if (!need_offchan) {
 		ieee80211_tx_skb_tid(sdata, skb,
-- 
2.55.0

[RFC PATCH 08/12] wifi: mac80211: implement mgmt_tx_unlocked()

[Johannes Berg]

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

Implement the mgmt_tx_unlocked() method and accept frames
that are transmitted on the operating channel with it, as
they don't need further handling and can go out directly.

For off-channel etc. just return -EAGAIN to fall back to
the locked version.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c         |  1 +
 net/mac80211/ieee80211_i.h |  3 ++
 net/mac80211/offchannel.c  | 87 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index aff9b1023d80..56328bfffa84 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -6270,6 +6270,7 @@ const struct cfg80211_ops mac80211_config_ops = {
 	.remain_on_channel = ieee80211_remain_on_channel,
 	.cancel_remain_on_channel = ieee80211_cancel_remain_on_channel,
 	.mgmt_tx = ieee80211_mgmt_tx,
+	.mgmt_tx_unlocked = ieee80211_mgmt_tx_unlocked,
 	.mgmt_tx_cancel_wait = ieee80211_mgmt_tx_cancel_wait,
 	.set_cqm_rssi_config = ieee80211_set_cqm_rssi_config,
 	.set_cqm_rssi_range_config = ieee80211_set_cqm_rssi_range_config,
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 02e50dcef27e..2f0d7a7b6685 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2189,6 +2189,9 @@ int ieee80211_cancel_remain_on_channel(struct wiphy *wiphy,
 				       struct wireless_dev *wdev, u64 cookie);
 int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		      struct cfg80211_mgmt_tx_params *params, u64 cookie);
+int ieee80211_mgmt_tx_unlocked(struct wiphy *wiphy, struct wireless_dev *wdev,
+			       struct cfg80211_mgmt_tx_params *params,
+			       u64 cookie);
 int ieee80211_mgmt_tx_cancel_wait(struct wiphy *wiphy,
 				  struct wireless_dev *wdev, u64 cookie);
 
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 27ca2984826f..730be15b4299 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -1074,6 +1074,93 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 	return ret;
 }
 
+int ieee80211_mgmt_tx_unlocked(struct wiphy *wiphy, struct wireless_dev *wdev,
+			       struct cfg80211_mgmt_tx_params *params,
+			       u64 cookie)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_WDEV_TO_SUB_IF(wdev);
+	const struct ieee80211_mgmt *mgmt = (void *)params->buf;
+	struct ieee80211_local *local = sdata->local;
+	struct sta_info *sta = NULL;
+	enum nl80211_band band;
+	struct sk_buff *skb;
+	int link_id = -1, i;
+
+	if (READ_ONCE(local->in_reconfig) || READ_ONCE(local->quiescing))
+		return -EAGAIN;
+
+	guard(rcu)();
+
+	/* do_stop() synchronizes RCU before the interface type can change */
+	if (!ieee80211_sdata_running(sdata) ||
+	    (sdata->vif.type != NL80211_IFTYPE_AP &&
+	     sdata->vif.type != NL80211_IFTYPE_P2P_GO) ||
+	    !sdata->bss->active)
+		return -EAGAIN;
+
+	if (!params->no_sta)
+		sta = sta_info_get_bss(sdata, mgmt->da);
+
+	if (ieee80211_is_action(mgmt->frame_control) &&
+	    mgmt->u.action.category != WLAN_CATEGORY_PUBLIC &&
+	    mgmt->u.action.category != WLAN_CATEGORY_SELF_PROTECTED &&
+	    mgmt->u.action.category != WLAN_CATEGORY_SPECTRUM_MGMT) {
+		if (!sta)
+			return -ENOLINK;
+		if (params->link_id >= 0 &&
+		    !(sta->sta.valid_links & BIT(params->link_id)))
+			return -ENOLINK;
+		link_id = params->link_id;
+	}
+
+	if ((params->chan || !sta || !sta->sta.mlo) &&
+	    ieee80211_mgmt_tx_need_offchan(sdata, params, mgmt,
+					   sta && sta->sta.mlo, &link_id))
+		return -EAGAIN;
+
+	/* things may have changed, avoid WARNs in ieee80211_tx_skb_tid() */
+	if (ieee80211_vif_is_mld(&sdata->vif)) {
+		band = 0;
+
+		if (link_id < 0 &&
+		    !ether_addr_equal(sdata->vif.addr, mgmt->sa)) {
+			for (i = 0; i < ARRAY_SIZE(sdata->vif.link_conf); i++) {
+				struct ieee80211_bss_conf *conf;
+
+				conf = rcu_dereference(sdata->vif.link_conf[i]);
+				if (conf &&
+				    ether_addr_equal(conf->addr, mgmt->sa)) {
+					link_id = i;
+					break;
+				}
+			}
+			if (link_id < 0)
+				return -EAGAIN;
+		}
+
+		if (link_id >= 0 &&
+		    !(READ_ONCE(sdata->vif.active_links) & BIT(link_id)))
+			return -EAGAIN;
+	} else {
+		struct ieee80211_chanctx_conf *chanctx_conf;
+
+		chanctx_conf =
+			rcu_dereference(sdata->vif.bss_conf.chanctx_conf);
+		if (!chanctx_conf)
+			return -EAGAIN;
+		band = chanctx_conf->def.chan->band;
+	}
+
+	skb = ieee80211_mgmt_tx_skb(sdata, params, cookie, GFP_ATOMIC);
+	if (!skb)
+		return -ENOMEM;
+
+	__ieee80211_tx_skb_tid_band(sdata, skb,
+				    params->no_sta ? ERR_PTR(-ENOENT) : sta,
+				    7, link_id, band);
+	return 0;
+}
+
 int ieee80211_mgmt_tx_cancel_wait(struct wiphy *wiphy,
 				  struct wireless_dev *wdev, u64 cookie)
 {
-- 
2.55.0

[RFC PATCH 09/12] wifi: mac80211: don't require wiphy mutex for probe_peer

[Johannes Berg]

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

The comment about wiphy mutex is stale since the cookie
cleanup, and everything else can be under RCU, we just
have to drop the warning in case it races AP stop.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 56328bfffa84..078947055b17 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -5179,11 +5179,17 @@ static int ieee80211_probe_peer(struct wiphy *wiphy, struct net_device *dev,
 	int size;
 	int ret;
 
-	/* the lock is needed to assign the cookie later */
-	lockdep_assert_wiphy(local->hw.wiphy);
+	guard(rcu)();
+
+	/* do_stop() synchronizes RCU before the interface type can change */
+	if (!ieee80211_sdata_running(sdata))
+		return -ENETDOWN;
 
 	switch (ieee80211_vif_type_p2p(&sdata->vif)) {
 	case NL80211_IFTYPE_AP:
+		/* NULL if the interface was a station when checked */
+		if (!peer)
+			return -EINVAL;
 		fromds = true;
 		break;
 	case NL80211_IFTYPE_STATION:
@@ -5209,8 +5215,7 @@ static int ieee80211_probe_peer(struct wiphy *wiphy, struct net_device *dev,
 			 * per-link address for the client's link.
 			 */
 			link_id = sta->deflink.link_id;
-			conf = wiphy_dereference(local->hw.wiphy,
-						 sdata->vif.link_conf[link_id]);
+			conf = rcu_dereference(sdata->vif.link_conf[link_id]);
 			if (!conf)
 				return -ENOLINK;
 			src_addr = conf->addr;
@@ -5225,9 +5230,8 @@ static int ieee80211_probe_peer(struct wiphy *wiphy, struct net_device *dev,
 		/* MLD transmissions must not rely on the band */
 		band = 0;
 	} else {
-		chanctx_conf = wiphy_dereference(local->hw.wiphy,
-						 sdata->vif.bss_conf.chanctx_conf);
-		if (WARN_ON(!chanctx_conf))
+		chanctx_conf = rcu_dereference(sdata->vif.bss_conf.chanctx_conf);
+		if (!chanctx_conf)
 			return -EINVAL;
 		band = chanctx_conf->def.chan->band;
 		link_id = 0;
@@ -5274,9 +5278,7 @@ static int ieee80211_probe_peer(struct wiphy *wiphy, struct net_device *dev,
 	}
 
 	local_bh_disable();
-	rcu_read_lock();
 	ieee80211_xmit(sdata, sta, skb);
-	rcu_read_unlock();
 	local_bh_enable();
 
 	return 0;
-- 
2.55.0

[RFC PATCH 10/12] wifi: nl80211: probe peers without wiphy mutex

[Johannes Berg]

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

None of the drivers implementing probe_peer (mac80211,
wil6210, nxpwifi) need the wiphy mutex for it, so we
can just not take it. Document that as well.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h | 5 ++++-
 net/wireless/nl80211.c | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f227b702b38a..5c33e31bea49 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5138,7 +5138,10 @@ struct mgmt_frame_regs {
  *
  * @probe_peer: probe a connected peer (AP: STA MAC required; STA: no MAC),
  *	must use the @cookie as provided which is later passed to
- *	cfg80211_probe_status().
+ *	cfg80211_probe_status(). Called without the wiphy mutex, so the
+ *	interface type may have changed/be changing and the MAC may be
+ *	%NULL even in AP mode (if runtime type changes are supported),
+ *	drivers must check for that.
  *
  * @set_noack_map: Set the NoAck Map for the TIDs.
  *
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ae3f4da167c5..b8da2df8d9cb 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -20765,7 +20765,8 @@ static const struct genl_small_ops nl80211_small_ops[] = {
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = nl80211_probe_peer,
 		.flags = GENL_UNS_ADMIN_PERM,
-		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
+		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP |
+					 NL80211_FLAG_NO_WIPHY_MTX),
 	},
 	{
 		.cmd = NL80211_CMD_REGISTER_BEACONS,
-- 
2.55.0

[RFC PATCH 11/12] wifi: mac80211: don't require wiphy mutex for control port TX

[Johannes Berg]

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

The wiphy mutex was only required for the cookie counter,
but that's been in cfg80211 for a while. The rest is just
datapath, so can work under RCU, just need to be careful
to handle interface type changes.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/tx.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 1953ec35c783..46134bda1e8f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -6600,9 +6600,6 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
 	u32 flags = 0;
 	int err;
 
-	/* mutex lock is only needed for incrementing the cookie counter */
-	lockdep_assert_wiphy(local->hw.wiphy);
-
 	/* Only accept CONTROL_PORT_PROTOCOL configured in CONNECT/ASSOCIATE
 	 * or Pre-Authentication
 	 */
@@ -6661,6 +6658,14 @@ int ieee80211_tx_control_port(struct wiphy *wiphy, struct net_device *dev,
 	 * AF_PACKET
 	 */
 	rcu_read_lock();
+
+	/* do_stop() synchronizes RCU before the interface type can change */
+	if (!ieee80211_sdata_running(sdata)) {
+		dev_kfree_skb(skb);
+		rcu_read_unlock();
+		return -ENETDOWN;
+	}
+
 	err = ieee80211_lookup_ra_sta(sdata, skb, &sta, true);
 	if (err) {
 		dev_kfree_skb(skb);
-- 
2.55.0

[RFC PATCH 12/12] wifi: nl80211: transmit control port frames without wiphy mutex

[Johannes Berg]

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

Only mac80211 currently implements tx_control_port, and it
no longer needs the wiphy mutex, so don't take it here.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h | 1 +
 net/wireless/nl80211.c | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5c33e31bea49..72f9ce3ad51b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5260,6 +5260,7 @@ struct mgmt_frame_regs {
  * @tx_control_port: TX a control port frame (EAPoL).  The noencrypt parameter
  *	tells the driver that the frame should not be encrypted. A @cookie
  *	value of 0 means the caller does not want TX status reporting.
+ *	Called without the wiphy mutex.
  *
  * @get_ftm_responder_stats: Retrieve FTM responder statistics, if available.
  *	Statistics should be cumulative, currently no way to reset is provided.
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b8da2df8d9cb..92210b0de08e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -21000,7 +21000,8 @@ static const struct genl_small_ops nl80211_small_ops[] = {
 		.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
 		.doit = nl80211_tx_control_port,
 		.flags = GENL_UNS_ADMIN_PERM,
-		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
+		.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP |
+					 NL80211_FLAG_NO_WIPHY_MTX),
 	},
 	{
 		.cmd = NL80211_CMD_GET_FTM_RESPONDER_STATS,
-- 
2.55.0

Re: [RFC PATCH 00/12] wifi: AP side locking improvements

[Johannes Berg]

On Sun, 2026-10-04 at 23:40 +0200, Johannes Berg wrote:
> This obviously goes on top of my (fixed) RTNL redux series.
> 
> The idea here is that wiphy mutex can be held for quite a bit
> of time (e.g. waiting for firmware), but beacon updates (e.g.
> with the critical update design I had proposed a long while
> back) and other things should be fast.
> 
> So with this not all things require wiphy mutex. These are:
> 
>  - beacon and related template updates
>    (this one needs driver opt-in),
>  - mgmt frame TX,
>  - control port TX, and
>  - peer probe.
> 
> This does make the implementation slightly more complex, but
> the added complexity is almost entirely in mac80211 (and some
> in cfg80211), unless a driver wants to opt in to unlocked
> beacon/template updates, which it has to implement itself for
> obvious reasons.

Thinking about this some more, there are a couple of corner cases I
don't like:

 - the semantics of "only for netdevs" and the get_device() in the
   implementation are strange

 - because of that, we can't do this for non-AP, and NAN would benefit

If we add a refcount_t to wdevs for this, and then use wdev_hold()
instead of dev_hold() (which uses the netdev dev_hold if it's there)
then it starts working for everything and the implementation gets nicer
too.

I'll rework it accordingly for the next round.

johannes

Re: [RFC PATCH 00/12] wifi: AP side locking improvements

[Jeff Johnson]

On 10/4/2026 2:40 PM, Johannes Berg wrote:
> This obviously goes on top of my (fixed) RTNL redux series.
> 
> The idea here is that wiphy mutex can be held for quite a bit
> of time (e.g. waiting for firmware), but beacon updates (e.g.
> with the critical update design I had proposed a long while
> back) and other things should be fast.
> 
> So with this not all things require wiphy mutex. These are:
> 
>  - beacon and related template updates
>    (this one needs driver opt-in),
>  - mgmt frame TX,
>  - control port TX, and
>  - peer probe.
> 
> This does make the implementation slightly more complex, but
> the added complexity is almost entirely in mac80211 (and some
> in cfg80211), unless a driver wants to opt in to unlocked
> beacon/template updates, which it has to implement itself for
> obvious reasons.
> 
> I think we probably want this even without the critical update
> design we're still debating, but I thought I'd throw this out
> here because a lot of the discussion has been focused on the
> latencies throughout the stack, and I never thought they were
> fundamental. QED ;-)
> 
> johannes

With both the redux and this series applied, I prompted my review agent:
prior to the 2 patchsets (18 patches and 12 patches) ath12k relied upon the
wiphy mutex to serialize many of the mac80211 operations. are any ath12k
object references now subject to new race conditions with this entire set of
30 patches applied?

The reply:
<LLM>
New Race: ath12k_reg_notifier vs ath12k_mac_op_start/ath12k_mac_op_stop on
ah->state

Before the series: ath12k_reg_notifier was called from
notify_self_managed_wiphys(), which ran inside reg_todo() under RTNL.
ath12k_mac_op_start() and ath12k_mac_op_stop() both assert
lockdep_assert_wiphy() at entry — and mac80211 calls them holding the wiphy
lock. The wiphy lock was acquired under RTNL (rtnl_lock() → wiphy_lock()), so
RTNL serialized the notifier against start/stop.

After the series (patch 18, 7fccd0f981b5): reg_todo() now holds cfg80211_mutex
instead of RTNL. notify_self_managed_wiphys() calls ath12k_reg_notifier()
holding only cfg80211_mutex — no wiphy lock, no RTNL.
ath12k_mac_op_start()/ath12k_mac_op_stop() hold only the wiphy lock — no
cfg80211_mutex. These two locks are disjoint.

Inside ath12k_reg_notifier(), the first thing it does is read ah->state
without holding ah->hw_mutex:

if (ah->state != ATH12K_HW_STATE_ON)   /* plain read, no hw_mutex */
    return;

ah->state is written exclusively under ah->hw_mutex (in ath12k_mac_op_start(),
ath12k_mac_op_stop(), and ath12k_mac_op_reconfig_complete()), none of which
hold cfg80211_mutex. The reg notifier (on CPU A, holding cfg80211_mutex) and
mac_op_stop (on CPU B, holding wiphy lock + hw_mutex) can now execute
concurrently. The ah->state read in the notifier and the ah->state =
ATH12K_HW_STATE_OFF write in stop share no common lock. This is a new data
race.

The same applies to the ah->regd_updated = false write further down in the
notifier and the ah->regd_updated = true write in ath12k_regd_update() (called
from a workqueue with no lock), and the ah->regd_updated read in
ath12k_regd_update() — none of these are serialized.

Before the series, RTNL acting as a global serializer masked all of this.

Pre-existing but Newly Exposed: ath12k_regd_update vs ath12k_reg_notifier on
ah->regd_updated

ath12k_regd_update() is called from ath12k_regd_update_work (a workqueue) and
reads then writes ah->regd_updated with no lock at all. ath12k_reg_notifier()
writes ah->regd_updated = false also with no lock. These two were already
concurrent before the series — workqueue items are not serialized by RTNL — so
this is not a new problem introduced by the patches. However, RTNL previously
prevented the reg notifier from being called while a mac op was running,
which reduced the window. That window is now wider.
</LLM>

So we'll probably need to address those issue.

/jeff

Re: [RFC PATCH 00/12] wifi: AP side locking improvements

[Johannes Berg]

On Mon, 2026-10-05 at 19:05 -0700, Jeff Johnson wrote:
> With both the redux and this series applied, I prompted my review agent:
> prior to the 2 patchsets (18 patches and 12 patches) ath12k relied upon the
> wiphy mutex to serialize many of the mac80211 operations. are any ath12k
> object references now subject to new race conditions with this entire set of
> 30 patches applied?

:)

> The reply:
> <LLM>
> New Race: ath12k_reg_notifier vs ath12k_mac_op_start/ath12k_mac_op_stop on
> ah->state

That's a fair point, but easy to resolve - we can just lock the wiphy in
the loop of notify_self_managed_wiphys(). It already can't go away since
we have cfg80211_mutex. Looking slightly deeper, we should guarantee
that we hold the wiphy mutex for a bunch of these operations to the
driver, and that needs more than a single line of code, but looks
totally doable.

> Pre-existing but Newly Exposed: ath12k_regd_update vs ath12k_reg_notifier on
> ah->regd_updated
> 
> ath12k_regd_update() is called from ath12k_regd_update_work (a workqueue) and
> reads then writes ah->regd_updated with no lock at all. ath12k_reg_notifier()
> writes ah->regd_updated = false also with no lock. These two were already
> concurrent before the series — workqueue items are not serialized by RTNL — so
> this is not a new problem introduced by the patches. However, RTNL previously
> prevented the reg notifier from being called while a mac op was running,
> which reduced the window. That window is now wider.

I don't think it's correct about that widening very much, "RTNL ...
prevented ... while a mac op was running" isn't true for most
operations, only start/stop (roughly), and e.g. sta_state changes the
state here.

But you'll need to fix that locally in the driver anyway.


I'm working also on adding clang context analysis to all of this, but
I've only _just_ (yesterday) worked out our internal build systems to
have clang 23 to be able to actually validate it, and will obviously
need to redo the (very small) adjustments in cfg80211 on top of this
series. Then we can annotate such things with clang and the right lock.

johannes

Re: [RFC PATCH 00/12] wifi: AP side locking improvements

[Johannes Berg]

On Tue, 2026-10-06 at 09:14 +0200, Johannes Berg wrote:
> On Mon, 2026-10-05 at 19:05 -0700, Jeff Johnson wrote:
> > With both the redux and this series applied, I prompted my review agent:
> > prior to the 2 patchsets (18 patches and 12 patches) ath12k relied upon the
> > wiphy mutex to serialize many of the mac80211 operations. are any ath12k
> > object references now subject to new race conditions with this entire set of
> > 30 patches applied?
> 
> :)
> 
> > The reply:
> > <LLM>
> > New Race: ath12k_reg_notifier vs ath12k_mac_op_start/ath12k_mac_op_stop on
> > ah->state
> 
> That's a fair point,

Actually, I think it was wrong: ah->state is only used within the
NL80211_REGDOM_SET_BY_DRIVER block, which is already only called in
reg_process_self_managed_hint(), which already holds wiphy mutex. It
might be correct about ah->regd_updated in this case, but then it found
that was _already_ racy.

> but easy to resolve - we can just lock the wiphy in
> the loop of notify_self_managed_wiphys(). It already can't go away since
> we have cfg80211_mutex. Looking slightly deeper, we should guarantee
> that we hold the wiphy mutex for a bunch of these operations to the
> driver, and that needs more than a single line of code, but looks
> totally doable.

That's probably still desired so we have _any_ locking, but needs to
drop the wiphy mutex in ath12k and rtw89.

johannes