wifi: mac80211: Adjust CQM handling for MLO

Commit Message

Johannes Berg Feb. 28, 2024, 8:47 a.m. UTC

From: Ilan Peer <[e-mail]>

The CQM handling did not consider the MLO case and thus notified
a not-existing link with the CQM change. To fix this, propagate
the CQM notification to all the active links (handling both the
non-MLO and MLO cases).

TODO: this currently propagates the same configuration to all
links regardless of the band. This might not be the correct
approach as different links might need to be configured with
different values.

Signed-off-by: Ilan Peer <[e-mail]>
Reviewed-by: Miriam Rachel Korenblit <[e-mail]>
Signed-off-by: Johannes Berg <[e-mail]>
---
 net/mac80211/cfg.c | 79 +++++++++++++++++++++++++++++-----------------
 1 file changed, 50 insertions(+), 29 deletions(-)

Comments

Kalle Valo March 4, 2024, 3:03 p.m. UTC | #1

Johannes Berg <[e-mail]> writes:

> From: Ilan Peer <[e-mail]>
>
> The CQM handling did not consider the MLO case and thus notified
> a not-existing link with the CQM change. To fix this, propagate
> the CQM notification to all the active links (handling both the
> non-MLO and MLO cases).
>
> TODO: this currently propagates the same configuration to all
> links regardless of the band. This might not be the correct
> approach as different links might need to be configured with
> different values.

Is that TODO link there intentionally?

Johannes Berg March 4, 2024, 3:23 p.m. UTC | #2

On Mon, 2024-03-04 at 17:03 +0200, Kalle Valo wrote:
> Johannes Berg <[e-mail]> writes:
> 
> > From: Ilan Peer <[e-mail]>
> > 
> > The CQM handling did not consider the MLO case and thus notified
> > a not-existing link with the CQM change. To fix this, propagate
> > the CQM notification to all the active links (handling both the
> > non-MLO and MLO cases).
> > 
> > TODO: this currently propagates the same configuration to all
> > links regardless of the band. This might not be the correct
> > approach as different links might need to be configured with
> > different values.
> 
> Is that TODO link there intentionally?
> 

Yeah, we need to consider this in the context of what wpa_s wants to do,
etc. So this _helps_, but it may not appropriately resolve the issue of
what this API really does.

johannes

Patch

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 0744113f3535..e57ba4f7a589 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3256,33 +3256,57 @@  static int ieee80211_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
+static void ieee80211_set_cqm_rssi_link(struct ieee80211_sub_if_data *sdata,
+					struct ieee80211_link_data *link,
+					s32 rssi_thold, u32 rssi_hyst,
+					s32 rssi_low, s32 rssi_high)
+{
+	struct ieee80211_bss_conf *conf;
+
+	if (!link || !link->conf)
+		return;
+
+	conf = link->conf;
+
+	if (rssi_thold && rssi_hyst &&
+	    rssi_thold == conf->cqm_rssi_thold &&
+	    rssi_hyst == conf->cqm_rssi_hyst)
+		return;
+
+	conf->cqm_rssi_thold = rssi_thold;
+	conf->cqm_rssi_hyst = rssi_hyst;
+	conf->cqm_rssi_low = rssi_low;
+	conf->cqm_rssi_high = rssi_high;
+	link->u.mgd.last_cqm_event_signal = 0;
+
+	if (!ieee80211_vif_link_active(&sdata->vif, link->link_id))
+		return;
+
+	if (sdata->u.mgd.associated &&
+	    (sdata->vif.driver_flags & IEEE80211_VIF_SUPPORTS_CQM_RSSI))
+		ieee80211_link_info_change_notify(sdata, link, BSS_CHANGED_CQM);
+}
+
 static int ieee80211_set_cqm_rssi_config(struct wiphy *wiphy,
 					 struct net_device *dev,
 					 s32 rssi_thold, u32 rssi_hyst)
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_vif *vif = &sdata->vif;
-	struct ieee80211_bss_conf *bss_conf = &vif->bss_conf;
+	int link_id;
 
-	if (rssi_thold == bss_conf->cqm_rssi_thold &&
-	    rssi_hyst == bss_conf->cqm_rssi_hyst)
-		return 0;
-
-	if (sdata->vif.driver_flags & IEEE80211_VIF_BEACON_FILTER &&
-	    !(sdata->vif.driver_flags & IEEE80211_VIF_SUPPORTS_CQM_RSSI))
+	if (vif->driver_flags & IEEE80211_VIF_BEACON_FILTER &&
+	    !(vif->driver_flags & IEEE80211_VIF_SUPPORTS_CQM_RSSI))
 		return -EOPNOTSUPP;
 
-	bss_conf->cqm_rssi_thold = rssi_thold;
-	bss_conf->cqm_rssi_hyst = rssi_hyst;
-	bss_conf->cqm_rssi_low = 0;
-	bss_conf->cqm_rssi_high = 0;
-	sdata->deflink.u.mgd.last_cqm_event_signal = 0;
+	/* For MLD, handle CQM change on all the active links */
+	for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
+		struct ieee80211_link_data *link =
+			sdata_dereference(sdata->link[link_id], sdata);
 
-	/* tell the driver upon association, unless already associated */
-	if (sdata->u.mgd.associated &&
-	    sdata->vif.driver_flags & IEEE80211_VIF_SUPPORTS_CQM_RSSI)
-		ieee80211_link_info_change_notify(sdata, &sdata->deflink,
-						  BSS_CHANGED_CQM);
+		ieee80211_set_cqm_rssi_link(sdata, link, rssi_thold, rssi_hyst,
+					    0, 0);
+	}
 
 	return 0;
 }
@@ -3293,22 +3317,19 @@  static int ieee80211_set_cqm_rssi_range_config(struct wiphy *wiphy,
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_vif *vif = &sdata->vif;
-	struct ieee80211_bss_conf *bss_conf = &vif->bss_conf;
+	int link_id;
 
-	if (sdata->vif.driver_flags & IEEE80211_VIF_BEACON_FILTER)
+	if (vif->driver_flags & IEEE80211_VIF_BEACON_FILTER)
 		return -EOPNOTSUPP;
 
-	bss_conf->cqm_rssi_low = rssi_low;
-	bss_conf->cqm_rssi_high = rssi_high;
-	bss_conf->cqm_rssi_thold = 0;
-	bss_conf->cqm_rssi_hyst = 0;
-	sdata->deflink.u.mgd.last_cqm_event_signal = 0;
+	/* For MLD, handle CQM change on all the active links */
+	for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
+		struct ieee80211_link_data *link =
+			sdata_dereference(sdata->link[link_id], sdata);
 
-	/* tell the driver upon association, unless already associated */
-	if (sdata->u.mgd.associated &&
-	    sdata->vif.driver_flags & IEEE80211_VIF_SUPPORTS_CQM_RSSI)
-		ieee80211_link_info_change_notify(sdata, &sdata->deflink,
-						  BSS_CHANGED_CQM);
+		ieee80211_set_cqm_rssi_link(sdata, link, 0, 0,
+					    rssi_low, rssi_high);
+	}
 
 	return 0;
 }