[2/2] wifi: mac80211: ageout BSS color bitmap

Commit Message

Yi-Chia Hsieh April 3, 2023, 9:04 p.m. UTC

Add a periodic work runs once every second to check BSS color.
OBSS BSS Color will be aged out if not seen for 10 seconds.

Signed-off-by: Yi-Chia Hsieh <[e-mail]>
---
 include/linux/ieee80211.h  |  5 +++++
 include/net/mac80211.h     |  1 +
 net/mac80211/cfg.c         | 29 +++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |  2 ++
 net/mac80211/iface.c       |  6 ++++++
 net/mac80211/link.c        |  1 +
 net/mac80211/rx.c          |  1 +
 7 files changed, 45 insertions(+)

Comments

Johannes Berg June 5, 2023, 11:33 a.m. UTC | #1

On Mon, 2023-04-03 at 14:04 -0700, Yi-Chia Hsieh wrote:

> +/* ageout time for OBSS BSS color */
> +#define IEEE80211_BSS_COLOR_AGEOUT_TIME		10

I don't think that should be in this file, it's implementation defined.
Also it might be nicer to just define it in jiffies, i.e. (10 * HZ),
directly.

> +++ b/include/net/mac80211.h
> @@ -750,6 +750,7 @@ struct ieee80211_bss_conf {
>  	bool color_change_active;
>  	u8 color_change_color;
>  	u64 used_color_bitmap;
> +	unsigned long color_last_seen[IEEE80211_BSS_COLOR_MAX + 1];

This is really big (512 bytes) and per link - might make sense to only
allocate when needed.

> +void ieee80211_color_aging_work(struct work_struct *work)
> +{
> +	struct ieee80211_sub_if_data *sdata =
> +		container_of(work, struct ieee80211_sub_if_data,
> +			     deflink.color_aging_work.work);
> +	struct ieee80211_bss_conf *bss_conf = &sdata->vif.bss_conf;
> +	int i;
> +
> +	sdata_lock(sdata);
> +
> +	if (!ieee80211_sdata_running(sdata))
> +		goto unlock;
> +
> +	for (i = 1; i < IEEE80211_BSS_COLOR_MAX + 1; i++) {
> +		/* ageout if not seen for a period */
> +		if ((bss_conf->used_color_bitmap & BIT_ULL(i)) &&
> +		    time_before(bss_conf->color_last_seen[i],
> +				jiffies - IEEE80211_BSS_COLOR_AGEOUT_TIME * HZ)) {
> +			bss_conf->used_color_bitmap &= ~BIT_ULL(i);
> +		}
> +	}
> +
> +	ieee80211_queue_delayed_work(&sdata->local->hw,
> +				     &sdata->deflink.color_aging_work, HZ);

What? No ... you really can't do this. Just schedule the work properly
at the right time, don't leave it running forever every 1 second!!


> @@ -1430,6 +1431,11 @@ int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
>  
>  	set_bit(SDATA_STATE_RUNNING, &sdata->state);
>  
> +	if (sdata->vif.type == NL80211_IFTYPE_AP)
> +		ieee80211_queue_delayed_work(&sdata->local->hw,
> +					     &sdata->deflink.color_aging_work,
> +					     HZ);

Again, no.

johannes

Patch

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index 2463bdd2a382..3e5a344fe9ce 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -307,6 +307,11 @@  static inline u16 ieee80211_sn_sub(u16 sn1, u16 sn2)
 #define IEEE80211_TRIGGER_TYPE_BQRP		0x6
 #define IEEE80211_TRIGGER_TYPE_NFRP		0x7
 
+/* max color index */
+#define IEEE80211_BSS_COLOR_MAX			63
+/* ageout time for OBSS BSS color */
+#define IEEE80211_BSS_COLOR_AGEOUT_TIME		10
+
 struct ieee80211_hdr {
 	__le16 frame_control;
 	__le16 duration_id;
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index f003e0dbc7c4..2471b5b9e11d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -750,6 +750,7 @@  struct ieee80211_bss_conf {
 	bool color_change_active;
 	u8 color_change_color;
 	u64 used_color_bitmap;
+	unsigned long color_last_seen[IEEE80211_BSS_COLOR_MAX + 1];
 
 	bool vht_su_beamformer;
 	bool vht_su_beamformee;
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 8eb342300868..f5a36b8b392d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -4759,6 +4759,35 @@  ieee80211_color_change(struct wiphy *wiphy, struct net_device *dev,
 	return err;
 }
 
+void ieee80211_color_aging_work(struct work_struct *work)
+{
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data,
+			     deflink.color_aging_work.work);
+	struct ieee80211_bss_conf *bss_conf = &sdata->vif.bss_conf;
+	int i;
+
+	sdata_lock(sdata);
+
+	if (!ieee80211_sdata_running(sdata))
+		goto unlock;
+
+	for (i = 1; i < IEEE80211_BSS_COLOR_MAX + 1; i++) {
+		/* ageout if not seen for a period */
+		if ((bss_conf->used_color_bitmap & BIT_ULL(i)) &&
+		    time_before(bss_conf->color_last_seen[i],
+				jiffies - IEEE80211_BSS_COLOR_AGEOUT_TIME * HZ)) {
+			bss_conf->used_color_bitmap &= ~BIT_ULL(i);
+		}
+	}
+
+	ieee80211_queue_delayed_work(&sdata->local->hw,
+				     &sdata->deflink.color_aging_work, HZ);
+
+unlock:
+	sdata_unlock(sdata);
+}
+
 static int
 ieee80211_set_radar_background(struct wiphy *wiphy,
 			       struct cfg80211_chan_def *chandef)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ecc232eb1ee8..5c4bd432bd60 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -974,6 +974,7 @@  struct ieee80211_link_data {
 
 	struct work_struct color_change_finalize_work;
 	struct delayed_work color_collision_detect_work;
+	struct delayed_work color_aging_work;
 	u64 color_bitmap;
 
 	/* context reservation -- protected with chanctx_mtx */
@@ -1931,6 +1932,7 @@  int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 /* color change handling */
 void ieee80211_color_change_finalize_work(struct work_struct *work);
 void ieee80211_color_collision_detection_work(struct work_struct *work);
+void ieee80211_color_aging_work(struct work_struct *work);
 
 /* interface handling */
 #define MAC80211_SUPPORTED_FEATURES_TX	(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | \
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 23ed13f15067..d0bba2c19b0e 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -541,6 +541,7 @@  static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
 	cancel_work_sync(&sdata->deflink.color_change_finalize_work);
 
 	cancel_delayed_work_sync(&sdata->deflink.dfs_cac_timer_work);
+	cancel_delayed_work_sync(&sdata->deflink.color_aging_work);
 
 	if (sdata->wdev.cac_started) {
 		chandef = sdata->vif.bss_conf.chandef;
@@ -1430,6 +1431,11 @@  int ieee80211_do_open(struct wireless_dev *wdev, bool coming_up)
 
 	set_bit(SDATA_STATE_RUNNING, &sdata->state);
 
+	if (sdata->vif.type == NL80211_IFTYPE_AP)
+		ieee80211_queue_delayed_work(&sdata->local->hw,
+					     &sdata->deflink.color_aging_work,
+					     HZ);
+
 	return 0;
  err_del_interface:
 	drv_remove_interface(local, sdata);
diff --git a/net/mac80211/link.c b/net/mac80211/link.c
index 8c8869cc1fb4..16c9b554c853 100644
--- a/net/mac80211/link.c
+++ b/net/mac80211/link.c
@@ -41,6 +41,7 @@  void ieee80211_link_init(struct ieee80211_sub_if_data *sdata,
 		  ieee80211_color_change_finalize_work);
 	INIT_DELAYED_WORK(&link->color_collision_detect_work,
 			  ieee80211_color_collision_detection_work);
+	INIT_DELAYED_WORK(&link->color_aging_work, ieee80211_color_aging_work);
 	INIT_LIST_HEAD(&link->assigned_chanctx_list);
 	INIT_LIST_HEAD(&link->reserved_chanctx_list);
 	INIT_DELAYED_WORK(&link->dfs_cac_timer_work,
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index e4a60992e4eb..da2de912b2fc 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3265,6 +3265,7 @@  ieee80211_rx_check_bss_color_collision(struct ieee80211_rx_data *rx)
 		color = le32_get_bits(he_oper->he_oper_params,
 				      IEEE80211_HE_OPERATION_BSS_COLOR_MASK);
 		bss_conf->used_color_bitmap |= BIT_ULL(color);
+		bss_conf->color_last_seen[color] = jiffies;
 
 		if (color == bss_conf->he_bss_color.color)
 			ieee80211_obss_color_collision_notify(&rx->sdata->vif,