diff mbox series

[v2,6/7] wifi: mac80211: add support to call color_change and OBSS collision on a link

Message ID 20240422053412.2024075-7-quic_adisi@quicinc.com (mailing list archive)
State Accepted
Delegated to: Johannes Berg
Headers show
Series wifi: cfg80211/mac80211: add support for HE BSS color handling with Multi-Link Operation | expand

Commit Message

Aditya Kumar Singh April 22, 2024, 5:34 a.m. UTC 
Currently ieee80211_color_change_finish() function finalizes color change
by scheduling a finalizing worker using the deflink. Similarly, function
ieee80211_obss_color_collision_notify() notifies the mac80211 about color
collision on deflink. With MLO, there is a need to do it on a given link
basis.

Pass link ID of the link on which color change needs to be finalized or
OBSS color collision is detected.

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 drivers/net/wireless/ath/ath11k/mac.c         |  2 +-
 drivers/net/wireless/ath/ath11k/wmi.c         |  3 +-
 .../net/wireless/mediatek/mt76/mt7915/mcu.c   |  2 +-
 .../net/wireless/mediatek/mt76/mt7996/mcu.c   |  2 +-
 include/net/mac80211.h                        |  6 ++-
 net/mac80211/cfg.c                            | 43 ++++++++++++++++---
 net/mac80211/rx.c                             |  7 +--
 7 files changed, 50 insertions(+), 15 deletions(-)

Comments

Jeff Johnson April 23, 2024, 6:46 p.m. UTC | #1 
On 4/21/2024 10:34 PM, Aditya Kumar Singh wrote:
> Currently ieee80211_color_change_finish() function finalizes color change
> by scheduling a finalizing worker using the deflink. Similarly, function
> ieee80211_obss_color_collision_notify() notifies the mac80211 about color
> collision on deflink. With MLO, there is a need to do it on a given link
> basis.
> 
> Pass link ID of the link on which color change needs to be finalized or
> OBSS color collision is detected.
> 
> Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
> ---
[...]
>  void
>  ieee80211_obss_color_collision_notify(struct ieee80211_vif *vif,
> -				      u64 color_bitmap)
> +				      u64 color_bitmap, u8 link_id)
>  {
>  	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> -	struct ieee80211_link_data *link = &sdata->deflink;
> +	struct ieee80211_link_data *link;
>  
> -	if (sdata->vif.bss_conf.color_change_active || sdata->vif.bss_conf.csa_active)
> +	if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
>  		return;
>  
> -	if (delayed_work_pending(&link->color_collision_detect_work))
> +	rcu_read_lock();

Johannes, how do you feel about the new cleanup.h functionality? I ask because
if we change this to just be:

	guard(rcu)();

then we can remove all of the explicit rcu_read_unlock() calls --
rcu_read_unlock() will be called automatically when the function goes out of
scope.

> +
> +	link = rcu_dereference(sdata->link[link_id]);
> +	if (WARN_ON(!link)) {
> +		rcu_read_unlock();
>  		return;
> +	}
> +
> +	if (link->conf->color_change_active || link->conf->csa_active) {
> +		rcu_read_unlock();
> +		return;
> +	}
> +
> +	if (delayed_work_pending(&link->color_collision_detect_work)) {
> +		rcu_read_unlock();
> +		return;
> +	}
>  
>  	link->color_bitmap = color_bitmap;
>  	/* queue the color collision detection event every 500 ms in order to
> @@ -4853,6 +4882,8 @@ ieee80211_obss_color_collision_notify(struct ieee80211_vif *vif,
>  	ieee80211_queue_delayed_work(&sdata->local->hw,
>  				     &link->color_collision_detect_work,
>  				     msecs_to_jiffies(500));
> +
> +	rcu_read_unlock();
>  }
>  EXPORT_SYMBOL_GPL(ieee80211_obss_color_collision_notify);
Johannes Berg April 23, 2024, 6:50 p.m. UTC | #2 
On Tue, 2024-04-23 at 11:46 -0700, Jeff Johnson wrote:
> 
> Johannes, how do you feel about the new cleanup.h functionality? I ask because
> if we change this to just be:
> 
> 	guard(rcu)();
> 
> then we can remove all of the explicit rcu_read_unlock() calls --
> rcu_read_unlock() will be called automatically when the function goes out of
> scope.

Sounds like a good idea to me.

Jakub doesn't like it so much, see this thread:
https://lore.kernel.org/netdev/20240325223905.100979-5-johannes@sipsolutions.net/

But ultimately I don't think he'll stop us from doing it here,
especially if we use it carefully enough that we actually do need the
guard for the whole scope.

johannes
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index c32be587000d..40c3ba6c445b 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -1659,7 +1659,7 @@  void ath11k_mac_bcn_tx_event(struct ath11k_vif *arvif)
 	if (vif->bss_conf.color_change_active &&
 	    ieee80211_beacon_cntdwn_is_complete(vif, 0)) {
 		arvif->bcca_zero_sent = true;
-		ieee80211_color_change_finish(vif);
+		ieee80211_color_change_finish(vif, 0);
 		return;
 	}
 
diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
index c74aa3f95658..3761dfce1f64 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.c
+++ b/drivers/net/wireless/ath/ath11k/wmi.c
@@ -4064,7 +4064,8 @@  ath11k_wmi_obss_color_collision_event(struct ath11k_base *ab, struct sk_buff *sk
 
 	switch (ev->evt_type) {
 	case WMI_BSS_COLOR_COLLISION_DETECTION:
-		ieee80211_obss_color_collision_notify(arvif->vif, ev->obss_color_bitmap);
+		ieee80211_obss_color_collision_notify(arvif->vif, ev->obss_color_bitmap,
+						      0);
 		ath11k_dbg(ab, ATH11K_DBG_WMI,
 			   "OBSS color collision detected vdev:%d, event:%d, bitmap:%08llx\n",
 			   ev->vdev_id, ev->evt_type, ev->obss_color_bitmap);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index d90f98c50039..6442e3e5d45b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -331,7 +331,7 @@  mt7915_mcu_cca_finish(void *priv, u8 *mac, struct ieee80211_vif *vif)
 	if (!vif->bss_conf.color_change_active || vif->type == NL80211_IFTYPE_STATION)
 		return;
 
-	ieee80211_color_change_finish(vif);
+	ieee80211_color_change_finish(vif, 0);
 }
 
 static void
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
index b44abe2acc81..163b822a477c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
@@ -418,7 +418,7 @@  mt7996_mcu_cca_finish(void *priv, u8 *mac, struct ieee80211_vif *vif)
 	if (!vif->bss_conf.color_change_active || vif->type == NL80211_IFTYPE_STATION)
 		return;
 
-	ieee80211_color_change_finish(vif);
+	ieee80211_color_change_finish(vif, 0);
 }
 
 static void
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 41f135a793ab..6be62cc2caf6 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5612,12 +5612,13 @@  bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif,
 /**
  * ieee80211_color_change_finish - notify mac80211 about color change
  * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @link_id: valid link_id during MLO or 0 for non-MLO
  *
  * After a color change announcement was scheduled and the counter in this
  * announcement hits 1, this function must be called by the driver to
  * notify mac80211 that the color can be changed
  */
-void ieee80211_color_change_finish(struct ieee80211_vif *vif);
+void ieee80211_color_change_finish(struct ieee80211_vif *vif, u8 link_id);
 
 /**
  * ieee80211_proberesp_get - retrieve a Probe Response template
@@ -7530,6 +7531,7 @@  ieee80211_get_unsol_bcast_probe_resp_tmpl(struct ieee80211_hw *hw,
 /**
  * ieee80211_obss_color_collision_notify - notify userland about a BSS color
  * collision.
+ * @link_id: valid link_id during MLO or 0 for non-MLO
  *
  * @vif: &struct ieee80211_vif pointer from the add_interface callback.
  * @color_bitmap: a 64 bit bitmap representing the colors that the local BSS is
@@ -7537,7 +7539,7 @@  ieee80211_get_unsol_bcast_probe_resp_tmpl(struct ieee80211_hw *hw,
  */
 void
 ieee80211_obss_color_collision_notify(struct ieee80211_vif *vif,
-				      u64 color_bitmap);
+				      u64 color_bitmap, u8 link_id);
 
 /**
  * ieee80211_is_tx_data - check if frame is a data frame
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b30d842e5068..b08e5d7687e3 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -4824,27 +4824,56 @@  void ieee80211_color_collision_detection_work(struct work_struct *work)
 					     link->link_id);
 }
 
-void ieee80211_color_change_finish(struct ieee80211_vif *vif)
+void ieee80211_color_change_finish(struct ieee80211_vif *vif, u8 link_id)
 {
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+	struct ieee80211_link_data *link;
+
+	if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
+		return;
+
+	rcu_read_lock();
+
+	link = rcu_dereference(sdata->link[link_id]);
+	if (WARN_ON(!link)) {
+		rcu_read_unlock();
+		return;
+	}
 
 	wiphy_work_queue(sdata->local->hw.wiphy,
-			 &sdata->deflink.color_change_finalize_work);
+			 &link->color_change_finalize_work);
+
+	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(ieee80211_color_change_finish);
 
 void
 ieee80211_obss_color_collision_notify(struct ieee80211_vif *vif,
-				      u64 color_bitmap)
+				      u64 color_bitmap, u8 link_id)
 {
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
-	struct ieee80211_link_data *link = &sdata->deflink;
+	struct ieee80211_link_data *link;
 
-	if (sdata->vif.bss_conf.color_change_active || sdata->vif.bss_conf.csa_active)
+	if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
 		return;
 
-	if (delayed_work_pending(&link->color_collision_detect_work))
+	rcu_read_lock();
+
+	link = rcu_dereference(sdata->link[link_id]);
+	if (WARN_ON(!link)) {
+		rcu_read_unlock();
 		return;
+	}
+
+	if (link->conf->color_change_active || link->conf->csa_active) {
+		rcu_read_unlock();
+		return;
+	}
+
+	if (delayed_work_pending(&link->color_collision_detect_work)) {
+		rcu_read_unlock();
+		return;
+	}
 
 	link->color_bitmap = color_bitmap;
 	/* queue the color collision detection event every 500 ms in order to
@@ -4853,6 +4882,8 @@  ieee80211_obss_color_collision_notify(struct ieee80211_vif *vif,
 	ieee80211_queue_delayed_work(&sdata->local->hw,
 				     &link->color_collision_detect_work,
 				     msecs_to_jiffies(500));
+
+	rcu_read_unlock();
 }
 EXPORT_SYMBOL_GPL(ieee80211_obss_color_collision_notify);
 
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 4b4cbd8bf35d..e2b561a9a751 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3361,7 +3361,7 @@  ieee80211_rx_check_bss_color_collision(struct ieee80211_rx_data *rx)
 	if (ieee80211_hw_check(&rx->local->hw, DETECTS_COLOR_COLLISION))
 		return;
 
-	if (rx->sdata->vif.bss_conf.csa_active)
+	if (rx->link->conf->csa_active)
 		return;
 
 	baselen = mgmt->u.beacon.variable - rx->skb->data;
@@ -3373,7 +3373,7 @@  ieee80211_rx_check_bss_color_collision(struct ieee80211_rx_data *rx)
 				    rx->skb->len - baselen);
 	if (ie && ie->datalen >= sizeof(struct ieee80211_he_operation) &&
 	    ie->datalen >= ieee80211_he_oper_size(ie->data + 1)) {
-		struct ieee80211_bss_conf *bss_conf = &rx->sdata->vif.bss_conf;
+		struct ieee80211_bss_conf *bss_conf = rx->link->conf;
 		const struct ieee80211_he_operation *he_oper;
 		u8 color;
 
@@ -3386,7 +3386,8 @@  ieee80211_rx_check_bss_color_collision(struct ieee80211_rx_data *rx)
 				      IEEE80211_HE_OPERATION_BSS_COLOR_MASK);
 		if (color == bss_conf->he_bss_color.color)
 			ieee80211_obss_color_collision_notify(&rx->sdata->vif,
-							      BIT_ULL(color));
+							      BIT_ULL(color),
+							      bss_conf->link_id);
 	}
 }