[PATCH v8 0/5] wifi: cfg80211/mac80211: add link_id handling in AP channel switch during Multi-Link Operation

[PATCH v8 0/5] wifi: cfg80211/mac80211: add link_id handling in AP channel switch during Multi-Link Operation

[Aditya Kumar Singh]

Currently, during channel switch, deflink (or link_id 0) is always
considered. However, with Multi-Link Operation (MLO), there is a
need to handle link specific data structures based on the actual
operational link_id during channel switch operation.

Hence, add support for the same. Non-MLO based operations will use
link_id as 0 or deflink member as applicable.

While at it, beacon count down now needs to be updated on proper
link_id's beacon, do that as well.

Aditya Kumar Singh (5):
  wifi: cfg80211: send link id in channel_switch ops
  wifi: mac80211: update beacon counters per link basis
  wifi: mac80211: handle set csa/after_csa beacon on per link basis
  wifi: mac80211: start and finalize channel switch on link basis
  wifi: mac80211: add support to call csa_finish on a link
---
v8: * fixed space error in documentation in 1/5
    * minor commit message rephrasing in 3/5 and 5/5.

v7: * fixed proper link id validation in 2/5 and 5/5. (>=)
    * added comma in trace print in 1/5

v6: * splitted v5 2/3 into v6 3/5, 4/5 and 5/5
    * rephrased commit mesage in 1/5

v5: * fixed compilation issue reported by kernel test robot.

v4: * fixed compilation issue reported by kernel test robot.
    * rebased on latest ToT.
    * moved link_id arguement into csa_params struct in [1/3]

v3: * splitted [v2 1/2] into [v3 1/3] and [v3 2/3] having simple cfg80211
      changes in 1/3 for easy review. Rest in 2/3 [Johannes]
    * used wiphy_dereference() instead of sdata_dereference() [Johannes]

v2: * reabsed on ToT
    * removed unwanted locking sequence during cancelling CSA work handler
      since now locking is moved to wiphy level, that part is uncessary now.
---
 drivers/net/wireless/ath/ath10k/mac.c         |   4 +-
 drivers/net/wireless/ath/ath10k/wmi.c         |   2 +-
 drivers/net/wireless/ath/ath11k/mac.c         |   2 +-
 drivers/net/wireless/ath/ath11k/wmi.c         |   2 +-
 drivers/net/wireless/ath/ath12k/wmi.c         |   2 +-
 drivers/net/wireless/ath/ath9k/beacon.c       |   2 +-
 .../net/wireless/ath/ath9k/htc_drv_beacon.c   |   2 +-
 .../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c |   6 +-
 .../wireless/intel/iwlwifi/mvm/time-event.c   |   2 +-
 drivers/net/wireless/mediatek/mt76/mac80211.c |   2 +-
 .../net/wireless/mediatek/mt76/mt7615/mcu.c   |   2 +-
 .../net/wireless/mediatek/mt76/mt7915/mcu.c   |   2 +-
 .../net/wireless/mediatek/mt76/mt7996/mcu.c   |   2 +-
 .../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c |   2 +-
 drivers/net/wireless/ti/wlcore/event.c        |   2 +-
 drivers/net/wireless/virtual/mac80211_hwsim.c |   2 +-
 include/net/cfg80211.h                        |   3 +
 include/net/mac80211.h                        |   7 +-
 net/mac80211/cfg.c                            | 108 +++++++++++-------
 net/mac80211/link.c                           |   2 +
 net/mac80211/tx.c                             |  14 ++-
 net/wireless/nl80211.c                        |   1 +
 net/wireless/trace.h                          |   7 +-
 23 files changed, 112 insertions(+), 68 deletions(-)


base-commit: 28b3df1fe6ba2cb439ba109f095aa841fef3a54f
-- 
2.25.1

[PATCH v8 1/5] wifi: cfg80211: send link id in channel_switch ops

[Aditya Kumar Singh]

Currently, during channel switch, no link id information is passed down.
In order to support channel switch during Multi Link Operation, it is
required to pass link id as well.

Add changes to pass link id in the channel_switch cfg80211_ops.

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 include/net/cfg80211.h | 3 +++
 net/wireless/nl80211.c | 1 +
 net/wireless/trace.h   | 7 +++++--
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 51b9e6fa12f8..60d99336751a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1531,6 +1531,8 @@ struct cfg80211_ap_update {
  * @punct_bitmap: Preamble puncturing bitmap. Each bit represents
  *	a 20 MHz channel, lowest bit corresponding to the lowest channel.
  *	Bit set to 1 indicates that the channel is punctured.
+ * @link_id: defines the link on which channel switch is expected during
+ *	MLO. 0 in case of non-MLO.
  */
 struct cfg80211_csa_settings {
 	struct cfg80211_chan_def chandef;
@@ -1544,6 +1546,7 @@ struct cfg80211_csa_settings {
 	bool block_tx;
 	u8 count;
 	u16 punct_bitmap;
+	u8 link_id;
 };
 
 /**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e4f41f86e295..7f58e11eba58 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10232,6 +10232,7 @@ static int nl80211_channel_switch(struct sk_buff *skb, struct genl_info *info)
 			goto free;
 	}
 
+	params.link_id = link_id;
 	err = rdev_channel_switch(rdev, dev, &params);
 
 free:
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 1f374c8a17a5..77972f7b774f 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2324,6 +2324,7 @@ TRACE_EVENT(rdev_channel_switch,
 		__field(u8, count)
 		__dynamic_array(u16, bcn_ofs, params->n_counter_offsets_beacon)
 		__dynamic_array(u16, pres_ofs, params->n_counter_offsets_presp)
+		__field(u8, link_id)
 	),
 	TP_fast_assign(
 		WIPHY_ASSIGN;
@@ -2341,11 +2342,13 @@ TRACE_EVENT(rdev_channel_switch,
 			memcpy(__get_dynamic_array(pres_ofs),
 			       params->counter_offsets_presp,
 			       params->n_counter_offsets_presp * sizeof(u16));
+		__entry->link_id = params->link_id;
 	),
 	TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT
-		  ", block_tx: %d, count: %u, radar_required: %d",
+		  ", block_tx: %d, count: %u, radar_required: %d, link_id: %d",
 		  WIPHY_PR_ARG, NETDEV_PR_ARG, CHAN_DEF_PR_ARG,
-		  __entry->block_tx, __entry->count, __entry->radar_required)
+		  __entry->block_tx, __entry->count, __entry->radar_required,
+		  __entry->link_id)
 );
 
 TRACE_EVENT(rdev_set_qos_map,
-- 
2.25.1

[PATCH v8 2/5] wifi: mac80211: update beacon counters per link basis

[Aditya Kumar Singh]

Currently, function to update beacon counter uses deflink to fetch
the beacon and then update the counter. However, with MLO, there is
a need to update the counter for the beacon in a particular link.

Add support to use link_id in order to fetch the beacon from a particular
link data during beacon update counter.

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 drivers/net/wireless/ath/ath10k/mac.c             |  2 +-
 drivers/net/wireless/ath/ath11k/mac.c             |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c |  2 +-
 include/net/mac80211.h                            |  4 +++-
 net/mac80211/tx.c                                 | 14 +++++++++++---
 5 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 090bcf148d0c..e32e8c70986d 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2035,7 +2035,7 @@ static void ath10k_mac_vif_ap_csa_count_down(struct ath10k_vif *arvif)
 		return;
 
 	if (!ieee80211_beacon_cntdwn_is_complete(vif)) {
-		ieee80211_beacon_update_cntdwn(vif);
+		ieee80211_beacon_update_cntdwn(vif, 0);
 
 		ret = ath10k_mac_setup_bcn_tmpl(arvif);
 		if (ret)
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index b13525bbbb80..0ea2ae40f2ad 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -1589,7 +1589,7 @@ void ath11k_mac_bcn_tx_event(struct ath11k_vif *arvif)
 	arvif->bcca_zero_sent = false;
 
 	if (vif->bss_conf.color_change_active)
-		ieee80211_beacon_update_cntdwn(vif);
+		ieee80211_beacon_update_cntdwn(vif, 0);
 	ath11k_mac_setup_bcn_tmpl(arvif);
 }
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
index c4f96125cf33..581c2d3fc534 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
@@ -1467,7 +1467,7 @@ static void iwl_mvm_csa_count_down(struct iwl_mvm *mvm,
 	mvmvif->csa_countdown = true;
 
 	if (!ieee80211_beacon_cntdwn_is_complete(csa_vif)) {
-		int c = ieee80211_beacon_update_cntdwn(csa_vif);
+		int c = ieee80211_beacon_update_cntdwn(csa_vif, 0);
 
 		iwl_mvm_mac_ctxt_beacon_changed(mvm, csa_vif,
 						&csa_vif->bss_conf);
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 8d6ae22c09bf..ad70dea96a46 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5507,6 +5507,7 @@ static inline struct sk_buff *ieee80211_beacon_get(struct ieee80211_hw *hw,
 /**
  * ieee80211_beacon_update_cntdwn - request mac80211 to decrement the beacon countdown
  * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @link_id: valid link_id during MLO or 0 for non-MLO
  *
  * The beacon counter should be updated after each beacon transmission.
  * This function is called implicitly when
@@ -5516,7 +5517,8 @@ static inline struct sk_buff *ieee80211_beacon_get(struct ieee80211_hw *hw,
  *
  * Return: new countdown value
  */
-u8 ieee80211_beacon_update_cntdwn(struct ieee80211_vif *vif);
+u8 ieee80211_beacon_update_cntdwn(struct ieee80211_vif *vif,
+				  unsigned int link_id);
 
 /**
  * ieee80211_beacon_set_cntdwn - request mac80211 to set beacon countdown
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 68a48abc7287..ac5bdacf4b8c 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -5030,16 +5030,24 @@ static u8 __ieee80211_beacon_update_cntdwn(struct beacon_data *beacon)
 	return beacon->cntdwn_current_counter;
 }
 
-u8 ieee80211_beacon_update_cntdwn(struct ieee80211_vif *vif)
+u8 ieee80211_beacon_update_cntdwn(struct ieee80211_vif *vif, unsigned int link_id)
 {
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+	struct ieee80211_link_data *link;
 	struct beacon_data *beacon = NULL;
 	u8 count = 0;
 
+	if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
+		return 0;
+
 	rcu_read_lock();
 
+	link = rcu_dereference(sdata->link[link_id]);
+	if (!link)
+		goto unlock;
+
 	if (sdata->vif.type == NL80211_IFTYPE_AP)
-		beacon = rcu_dereference(sdata->deflink.u.ap.beacon);
+		beacon = rcu_dereference(link->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))
@@ -5280,7 +5288,7 @@ ieee80211_beacon_get_ap(struct ieee80211_hw *hw,
 
 	if (beacon->cntdwn_counter_offsets[0]) {
 		if (!is_template)
-			ieee80211_beacon_update_cntdwn(vif);
+			ieee80211_beacon_update_cntdwn(vif, link->link_id);
 
 		ieee80211_set_beacon_cntdwn(sdata, beacon, link);
 	}
-- 
2.25.1

[PATCH v8 3/5] wifi: mac80211: handle set csa/after_csa beacon on per link basis

[Aditya Kumar Singh]

In order to support CSA with MLO, there is a need to handle the functions
ieee80211_set_csa_beacon() and ieee80211_set_after_csa_beacon() on a per
link basis.

Implement this by making the function argument accept the the link data
instead of the sdata.

Currently, deflink would only be passed. Proper link data will be passed in
a subsequent patch.

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 net/mac80211/cfg.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index dd237081dbe3..a5d510932cbe 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3586,20 +3586,21 @@ void ieee80211_channel_switch_disconnect(struct ieee80211_vif *vif, bool block_t
 }
 EXPORT_SYMBOL(ieee80211_channel_switch_disconnect);
 
-static int ieee80211_set_after_csa_beacon(struct ieee80211_sub_if_data *sdata,
+static int ieee80211_set_after_csa_beacon(struct ieee80211_link_data *link_data,
 					  u64 *changed)
 {
+	struct ieee80211_sub_if_data *sdata = link_data->sdata;
 	int err;
 
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_AP:
-		if (!sdata->deflink.u.ap.next_beacon)
+		if (!link_data->u.ap.next_beacon)
 			return -EINVAL;
 
-		err = ieee80211_assign_beacon(sdata, &sdata->deflink,
-					      sdata->deflink.u.ap.next_beacon,
+		err = ieee80211_assign_beacon(sdata, link_data,
+					      link_data->u.ap.next_beacon,
 					      NULL, NULL, changed);
-		ieee80211_free_next_beacon(&sdata->deflink);
+		ieee80211_free_next_beacon(link_data);
 
 		if (err < 0)
 			return err;
@@ -3658,7 +3659,7 @@ static int __ieee80211_csa_finalize(struct ieee80211_link_data *link_data)
 
 	sdata->vif.bss_conf.csa_active = false;
 
-	err = ieee80211_set_after_csa_beacon(sdata, &changed);
+	err = ieee80211_set_after_csa_beacon(&sdata->deflink, &changed);
 	if (err)
 		return err;
 
@@ -3717,18 +3718,19 @@ void ieee80211_csa_finalize_work(struct wiphy *wiphy, struct wiphy_work *work)
 	ieee80211_csa_finalize(link);
 }
 
-static int ieee80211_set_csa_beacon(struct ieee80211_sub_if_data *sdata,
+static int ieee80211_set_csa_beacon(struct ieee80211_link_data *link_data,
 				    struct cfg80211_csa_settings *params,
 				    u64 *changed)
 {
+	struct ieee80211_sub_if_data *sdata = link_data->sdata;
 	struct ieee80211_csa_settings csa = {};
 	int err;
 
 	switch (sdata->vif.type) {
 	case NL80211_IFTYPE_AP:
-		sdata->deflink.u.ap.next_beacon =
+		link_data->u.ap.next_beacon =
 			cfg80211_beacon_dup(&params->beacon_after);
-		if (!sdata->deflink.u.ap.next_beacon)
+		if (!link_data->u.ap.next_beacon)
 			return -ENOMEM;
 
 		/*
@@ -3754,7 +3756,7 @@ static int ieee80211_set_csa_beacon(struct ieee80211_sub_if_data *sdata,
 		     IEEE80211_MAX_CNTDWN_COUNTERS_NUM) ||
 		    (params->n_counter_offsets_presp >
 		     IEEE80211_MAX_CNTDWN_COUNTERS_NUM)) {
-			ieee80211_free_next_beacon(&sdata->deflink);
+			ieee80211_free_next_beacon(link_data);
 			return -EINVAL;
 		}
 
@@ -3764,11 +3766,11 @@ static int ieee80211_set_csa_beacon(struct ieee80211_sub_if_data *sdata,
 		csa.n_counter_offsets_presp = params->n_counter_offsets_presp;
 		csa.count = params->count;
 
-		err = ieee80211_assign_beacon(sdata, &sdata->deflink,
+		err = ieee80211_assign_beacon(sdata, link_data,
 					      &params->beacon_csa, &csa,
 					      NULL, changed);
 		if (err < 0) {
-			ieee80211_free_next_beacon(&sdata->deflink);
+			ieee80211_free_next_beacon(link_data);
 			return err;
 		}
 
@@ -3928,7 +3930,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 	if (sdata->vif.bss_conf.color_change_active)
 		ieee80211_color_change_abort(sdata);
 
-	err = ieee80211_set_csa_beacon(sdata, params, &changed);
+	err = ieee80211_set_csa_beacon(&sdata->deflink, params, &changed);
 	if (err) {
 		ieee80211_link_unreserve_chanctx(&sdata->deflink);
 		goto out;
-- 
2.25.1

[PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

Add changes to start a channel switch as well as finalize it on link basis
in order to support CSA with MLO as well.

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 net/mac80211/cfg.c  | 69 ++++++++++++++++++++++++++-------------------
 net/mac80211/link.c |  2 ++
 2 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a5d510932cbe..4427259154e2 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3629,6 +3629,7 @@ static int __ieee80211_csa_finalize(struct ieee80211_link_data *link_data)
 {
 	struct ieee80211_sub_if_data *sdata = link_data->sdata;
 	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_bss_conf *link_conf = link_data->conf;
 	u64 changed = 0;
 	int err;
 
@@ -3650,22 +3651,21 @@ static int __ieee80211_csa_finalize(struct ieee80211_link_data *link_data)
 		if (link_data->reserved_ready)
 			return 0;
 
-		return ieee80211_link_use_reserved_context(&sdata->deflink);
+		return ieee80211_link_use_reserved_context(link_data);
 	}
 
-	if (!cfg80211_chandef_identical(&link_data->conf->chandef,
+	if (!cfg80211_chandef_identical(&link_conf->chandef,
 					&link_data->csa_chandef))
 		return -EINVAL;
 
-	sdata->vif.bss_conf.csa_active = false;
+	link_conf->csa_active = false;
 
-	err = ieee80211_set_after_csa_beacon(&sdata->deflink, &changed);
+	err = ieee80211_set_after_csa_beacon(link_data, &changed);
 	if (err)
 		return err;
 
-	if (sdata->vif.bss_conf.eht_puncturing != sdata->vif.bss_conf.csa_punct_bitmap) {
-		sdata->vif.bss_conf.eht_puncturing =
-					sdata->vif.bss_conf.csa_punct_bitmap;
+	if (link_conf->eht_puncturing != link_conf->csa_punct_bitmap) {
+		link_conf->eht_puncturing = link_conf->csa_punct_bitmap;
 		changed |= BSS_CHANGED_EHT_PUNCTURING;
 	}
 
@@ -3693,7 +3693,8 @@ static void ieee80211_csa_finalize(struct ieee80211_link_data *link_data)
 	struct ieee80211_sub_if_data *sdata = link_data->sdata;
 
 	if (__ieee80211_csa_finalize(link_data)) {
-		sdata_info(sdata, "failed to finalize CSA, disconnecting\n");
+		sdata_info(sdata, "failed to finalize CSA on link %d, disconnecting\n",
+			   link_data->link_id);
 		cfg80211_stop_iface(sdata->local->hw.wiphy, &sdata->wdev,
 				    GFP_KERNEL);
 	}
@@ -3869,7 +3870,10 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 	struct ieee80211_channel_switch ch_switch;
 	struct ieee80211_chanctx_conf *conf;
 	struct ieee80211_chanctx *chanctx;
+	struct ieee80211_bss_conf *link_conf;
+	struct ieee80211_link_data *link_data;
 	u64 changed = 0;
+	u8 link_id = params->link_id;
 	int err;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
@@ -3880,16 +3884,23 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 	if (sdata->wdev.cac_started)
 		return -EBUSY;
 
-	if (cfg80211_chandef_identical(&params->chandef,
-				       &sdata->vif.bss_conf.chandef))
+	if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
+		return -EINVAL;
+
+	link_data = wiphy_dereference(wiphy, sdata->link[link_id]);
+	if (!link_data)
+		return -ENOLINK;
+
+	link_conf = link_data->conf;
+
+	if (cfg80211_chandef_identical(&params->chandef, &link_conf->chandef))
 		return -EINVAL;
 
 	/* don't allow another channel switch if one is already active. */
-	if (sdata->vif.bss_conf.csa_active)
+	if (link_conf->csa_active)
 		return -EBUSY;
 
-	conf = rcu_dereference_protected(sdata->vif.bss_conf.chanctx_conf,
-					 lockdep_is_held(&local->hw.wiphy->mtx));
+	conf = wiphy_dereference(wiphy, link_conf->chanctx_conf);
 	if (!conf) {
 		err = -EBUSY;
 		goto out;
@@ -3913,7 +3924,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 	if (err)
 		goto out;
 
-	err = ieee80211_link_reserve_chanctx(&sdata->deflink, &params->chandef,
+	err = ieee80211_link_reserve_chanctx(link_data, &params->chandef,
 					     chanctx->mode,
 					     params->radar_required);
 	if (err)
@@ -3922,44 +3933,44 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
 	/* if reservation is invalid then this will fail */
 	err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0);
 	if (err) {
-		ieee80211_link_unreserve_chanctx(&sdata->deflink);
+		ieee80211_link_unreserve_chanctx(link_data);
 		goto out;
 	}
 
 	/* if there is a color change in progress, abort it */
-	if (sdata->vif.bss_conf.color_change_active)
+	if (link_conf->color_change_active)
 		ieee80211_color_change_abort(sdata);
 
-	err = ieee80211_set_csa_beacon(&sdata->deflink, params, &changed);
+	err = ieee80211_set_csa_beacon(link_data, params, &changed);
 	if (err) {
-		ieee80211_link_unreserve_chanctx(&sdata->deflink);
+		ieee80211_link_unreserve_chanctx(link_data);
 		goto out;
 	}
 
-	if (params->punct_bitmap && !sdata->vif.bss_conf.eht_support)
+	if (params->punct_bitmap && link_conf->eht_support)
 		goto out;
 
-	sdata->deflink.csa_chandef = params->chandef;
-	sdata->deflink.csa_block_tx = params->block_tx;
-	sdata->vif.bss_conf.csa_active = true;
-	sdata->vif.bss_conf.csa_punct_bitmap = params->punct_bitmap;
+	link_data->csa_chandef = params->chandef;
+	link_data->csa_block_tx = params->block_tx;
+	link_conf->csa_active = true;
+	link_conf->csa_punct_bitmap = params->punct_bitmap;
 
-	if (sdata->deflink.csa_block_tx)
+	if (link_data->csa_block_tx)
 		ieee80211_stop_vif_queues(local, sdata,
 					  IEEE80211_QUEUE_STOP_REASON_CSA);
 
 	cfg80211_ch_switch_started_notify(sdata->dev,
-					  &sdata->deflink.csa_chandef, 0,
+					  &link_data->csa_chandef, link_id,
 					  params->count, params->block_tx,
-					  sdata->vif.bss_conf.csa_punct_bitmap);
+					  link_conf->csa_punct_bitmap);
 
 	if (changed) {
-		ieee80211_link_info_change_notify(sdata, &sdata->deflink,
-						  changed);
+		ieee80211_link_info_change_notify(sdata, link_data, changed);
+		/* link_id to be passed here? */
 		drv_channel_switch_beacon(sdata, &params->chandef);
 	} else {
 		/* if the beacon didn't change, we can finalize immediately */
-		ieee80211_csa_finalize(&sdata->deflink);
+		ieee80211_csa_finalize(link_data);
 	}
 
 out:
diff --git a/net/mac80211/link.c b/net/mac80211/link.c
index d4f86955afa6..0a1d0ef09d9f 100644
--- a/net/mac80211/link.c
+++ b/net/mac80211/link.c
@@ -73,6 +73,8 @@ void ieee80211_link_stop(struct ieee80211_link_data *link)
 		ieee80211_mgd_stop_link(link);
 
 	cancel_delayed_work_sync(&link->color_collision_detect_work);
+	wiphy_work_cancel(link->sdata->local->hw.wiphy,
+			  &link->csa_finalize_work);
 	ieee80211_link_release_channel(link);
 }
 
-- 
2.25.1

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote:
> Add changes to start a channel switch as well as finalize it on link basis
> in order to support CSA with MLO as well.
> 

FYI, this had a number of conflicts with my other work - please check
the tree now.

johannes

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote:
> On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote:
> > Add changes to start a channel switch as well as finalize it on link basis
> > in order to support CSA with MLO as well.
> > 
> 
> FYI, this had a number of conflicts with my other work - please check
> the tree now.
> 

Also here btw, some hostap test with hwsim would be nice - though again
don't know how well hostapd supports all this yet.

johannes

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/8/24 19:35, Johannes Berg wrote:
> On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote:
>> On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote:
>>> Add changes to start a channel switch as well as finalize it on link basis
>>> in order to support CSA with MLO as well.
>>>
>>
>> FYI, this had a number of conflicts with my other work - please check
>> the tree now.
>>
> 
> Also here btw, some hostap test with hwsim would be nice - though again
> don't know how well hostapd supports all this yet.
> 
> johannes

Sure, I will rebase on latest ToT and will add some hwsim test cases (if 
supported well in hostapd) as well.

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/12/24 12:10, Aditya Kumar Singh wrote:
> On 2/8/24 19:35, Johannes Berg wrote:
>> On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote:
>>> On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote:
>>>> Add changes to start a channel switch as well as finalize it on link 
>>>> basis
>>>> in order to support CSA with MLO as well.
>>>>
>>>
>>> FYI, this had a number of conflicts with my other work - please check
>>> the tree now.
>>>
>>
>> Also here btw, some hostap test with hwsim would be nice - though again
>> don't know how well hostapd supports all this yet.
>>
>> johannes
> 
> Sure, I will rebase on latest ToT and will add some hwsim test cases (if 
> supported well in hostapd) as well.
> 
> 

My bad! I see you have applied the changes. Thanks :)
Looks good to me. Let me see on the hwsim test cases and if possible 
send it soon for review to Jouni.

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Mon, 2024-02-12 at 12:48 +0530, Aditya Kumar Singh wrote:

> My bad! I see you have applied the changes. Thanks :)

Right, sorry that wasn't clear.

> Looks good to me.

OK, thanks for checking.

> Let me see on the hwsim test cases and if possible 
> send it soon for review to Jouni.

So ... I'm looking at the client side, and thinking about that.
According to the spec, multi-link element should be present in beacons
of APs affiliated with the same MLD if one of the (other) links is doing
CSA, and then have also the CSA counters of course, relative to the
target link's TBTT (of course.)

Theoretically, both mac80211 and hostapd today support different beacon
interval on different links, I believe.

This makes the whole thing of including CSA for link A on beacons/probe
responses transmitted on link B very tricky, because you have to know
the timing, etc.

For the CSA counter of a link _itself_ we have counter_offsets_beacon
and counter_offsets_presp (for probe response offload) in struct
cfg80211_csa_settings, and also counter offsets in struct
cfg80211_mgmt_tx_params for sending probe responses.

But ... for the cross-link information in the MLE this gets way more
tricky? Especially if the beacon interval is different - we couldn't
just count down, we'd have to fill in the information when we know where
the frame is transmitted. For probe responses maybe we can afford to not
be perfect, but for beacons it better be right - so we have to calculate
the right counter value(s) for (all of) the switching link(s) according
to the current TSF, TSF offset and TBTT - not all of which we even have
in the driver?

I can see a few ways of implementing this:

 a) Punt to firmware and it parses the multi-link element etc. to
    find the places to update. But that's awful, it requires parsing
    possibly fragmented MLE with fragmented subelements containing the
    CSA elements inside ...

 b) Punt to firmware and give it a (possibly long) list of offsets K_N
    where to put the N'th counter for link K when transmitting the
    frame.

 c) Require the get_tsf operation and add an operation to retrieve TSF
    offset (**), and then calculate the TBTT for each link when
    beacon_get is called with the per-link beacon intervals and update
    the values correctly if CSA is in progress on any link ... requires
    against parsing like in (a) or offset information like in (b), but
    at least now it's in software?
    For probe responses this could be a bit off I guess, but maybe that
    doesn't matter as much - probe responses are not authenticated so a
    client probably shouldn't use them for real CSA anyway.

    (**) which I guess we need anyway for hostapd to put it into beacons
    etc.?

 d) Require beacon intervals to be the same, and then just count down?
    Still requires the offset information etc., but that's also not
    great because if the configuration happens during a TBTT on any of
    the links (some before, some after), it'll be very wrong. So not
    very inclined to do this one ...


Do you have any plans for any of this?

I'm mostly asking right now because I want a reliable way to test the
work I'm trying to do on the client side though, so I could also live
with some hacks (inject through debugfs?), but having it for real would
be nice.

johannes

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/12/24 20:16, Johannes Berg wrote:
> So ... I'm looking at the client side, and thinking about that.
> According to the spec, multi-link element should be present in beacons
> of APs affiliated with the same MLD if one of the (other) links is doing
> CSA, and then have also the CSA counters of course, relative to the
> target link's TBTT (of course.)
> 

Yes correct, you are referring to critical update(s) right?

> Theoretically, both mac80211 and hostapd today support different beacon
> interval on different links, I believe.
> 
> This makes the whole thing of including CSA for link A on beacons/probe
> responses transmitted on link B very tricky, because you have to know
> the timing, etc.

At least handling for probe response seems to be a bit easier since 
there we need not maintain any timer as such (as you have said too - we 
need not be perfect there). But beacons, as you said, it is indeed bit 
tricky to handle full support considering both links could beacon at 
different intervals.

> 
> For the CSA counter of a link _itself_ we have counter_offsets_beacon
> and counter_offsets_presp (for probe response offload) in struct
> cfg80211_csa_settings, and also counter offsets in struct
> cfg80211_mgmt_tx_params for sending probe responses.
> 
> But ... for the cross-link information in the MLE this gets way more
> tricky? Especially if the beacon interval is different - we couldn't
> just count down, we'd have to fill in the information when we know where
> the frame is transmitted. For probe responses maybe we can afford to not
> be perfect, but for beacons it better be right - so we have to calculate
> the right counter value(s) for (all of) the switching link(s) according
> to the current TSF, TSF offset and TBTT - not all of which we even have
> in the driver?
> 

Yes correct :)

> I can see a few ways of implementing this:
> 
>   a) Punt to firmware and it parses the multi-link element etc. to
>      find the places to update. But that's awful, it requires parsing
>      possibly fragmented MLE with fragmented subelements containing the
>      CSA elements inside ...
> 
>   b) Punt to firmware and give it a (possibly long) list of offsets K_N
>      where to put the N'th counter for link K when transmitting the
>      frame.
> 

At least for beacons if firmware takes care of it then it will be good. 
Firmware could maintain the counter (for the affected link) and (I 
assume it will be aware of the partner links) so whenever partner link 
transmits a beacon it could add the CSA (or such IEs) in per STA profile 
of the reporting link. It could get the contents from the affected link 
but actual value of the counter could be filled from the global counter 
being maintained. But not sure whether we can force every driver's 
firmware to do so?

Let aside actual drivers, adding the test case for mac80211_hw_sim would 
also be tricky since there beacon Tx is handled in software only.


>   c) Require the get_tsf operation and add an operation to retrieve TSF
>      offset (**), and then calculate the TBTT for each link when
>      beacon_get is called with the per-link beacon intervals and update
>      the values correctly if CSA is in progress on any link ... requires
>      against parsing like in (a) or offset information like in (b), but
>      at least now it's in software?
>      For probe responses this could be a bit off I guess, but maybe that
>      doesn't matter as much - probe responses are not authenticated so a
>      client probably shouldn't use them for real CSA anyway.
> 
>      (**) which I guess we need anyway for hostapd to put it into beacons
>      etc.?

Yes I'm too anticipating hostapd changes to support the critical update.

> 
>   d) Require beacon intervals to be the same, and then just count down?
>      Still requires the offset information etc., but that's also not
>      great because if the configuration happens during a TBTT on any of
>      the links (some before, some after), it'll be very wrong. So not
>      very inclined to do this one ...
> 

Agreed. Can't enforce the intervals to be same.

> 
> Do you have any plans for any of this?

I do have some idea (possibly half baked code) for handling critical 
updates in probe responses. Beacons, I assume firmware takes care hence 
I have changes to mark it in the link_conf so that drivers could read 
that and give it to its firmware.
Handling in beacon at kernel level, I need to think of it since anyways 
hw_sim test case is good to have :)

Do we have any data right now, how many actual drivers have offloaded 
beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)?
If we could see majority are using firmware then we could first add 
support for such drivers then we can think about handling it in kernel?

> 
> I'm mostly asking right now because I want a reliable way to test the
> work I'm trying to do on the client side though, so I could also live
> with some hacks (inject through debugfs?), but having it for real would
> be nice.

Okay. I'm right now working to add basic test case in hw_sim for MLO CSA 
(on first link) as we discussed. There is need of a few mac80211 changes 
in order to support for other links. I'm checking those now. Once done 
with these, then I'll take up critical updates.

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote:
> On 2/12/24 20:16, Johannes Berg wrote:
> > So ... I'm looking at the client side, and thinking about that.
> > According to the spec, multi-link element should be present in beacons
> > of APs affiliated with the same MLD if one of the (other) links is doing
> > CSA, and then have also the CSA counters of course, relative to the
> > target link's TBTT (of course.)
> 
> Yes correct, you are referring to critical update(s) right?

I was actually thinking the critical update flag is rather not all that
interesting, but the inclusion of the CSA-related elements in a
(partial) per-STA profile in the Multi-Link Element is. Those contain
the countdown fields, so need to be updated - according to the TBTT of
the link doing the CSA, not the link sending it.

> > Theoretically, both mac80211 and hostapd today support different beacon
> > interval on different links, I believe.
> > 
> > This makes the whole thing of including CSA for link A on beacons/probe
> > responses transmitted on link B very tricky, because you have to know
> > the timing, etc.
> 
> At least handling for probe response seems to be a bit easier since 
> there we need not maintain any timer as such (as you have said too - we 
> need not be perfect there).

OTOH, if we already need some infrastructure for beacons, we can also
use that infrastructure to fill in probe responses? Might be easier
overall, because otherwise you'd have to start timers in hostapd etc. if
you actually wanted to fill that in hostapd, and that's messy ...

And as a complete aside: link removal _also_ has a countdown, but I'm
not sure that affects cross-link, but if it does we might want a more
general cross-link counter infrastructure? Though that might not mesh
well with how (your) firmware handles it?

> But beacons, as you said, it is indeed bit 
> tricky to handle full support considering both links could beacon at 
> different intervals.

Indeed.

> > For the CSA counter of a link _itself_ we have counter_offsets_beacon
> > and counter_offsets_presp (for probe response offload) in struct
> > cfg80211_csa_settings, and also counter offsets in struct
> > cfg80211_mgmt_tx_params for sending probe responses.
> > 
> > But ... for the cross-link information in the MLE this gets way more
> > tricky? Especially if the beacon interval is different - we couldn't
> > just count down, we'd have to fill in the information when we know where
> > the frame is transmitted. For probe responses maybe we can afford to not
> > be perfect, but for beacons it better be right - so we have to calculate
> > the right counter value(s) for (all of) the switching link(s) according
> > to the current TSF, TSF offset and TBTT - not all of which we even have
> > in the driver?
> > 
> 
> Yes correct :)

:)

> > I can see a few ways of implementing this:
> > 
> >   a) Punt to firmware and it parses the multi-link element etc. to
> >      find the places to update. But that's awful, it requires parsing
> >      possibly fragmented MLE with fragmented subelements containing the
> >      CSA elements inside ...
> > 
> >   b) Punt to firmware and give it a (possibly long) list of offsets K_N
> >      where to put the N'th counter for link K when transmitting the
> >      frame.
> > 
> 
> At least for beacons if firmware takes care of it then it will be good. 

Note that I'm basically asking you, Mediatek and Realtek how this should
work. We're almost certainly not going to support multi-link AP in our
device any time soon, and we could adjust the firmware to it anyway if
needed.

So while I agree that punting it to firmware would be somewhat nice, we
still have to agree on (a) or (b), because (b) requires more information
from hostapd etc., but (a) requires _substantially_ more (and very
complex) parsing in firmware... If you wanted to support multi-BSSID,
then it's even more complex, which would argue for (b)?

I was trying to keep opinions out of this, but personally, I really
don't like (a), it puts too much complexity in the hands of firmware
engineers ;-)

> Firmware could maintain the counter (for the affected link) and (I 
> assume it will be aware of the partner links) so whenever partner link 
> transmits a beacon it could add the CSA (or such IEs) in per STA profile 
> of the reporting link. It could get the contents from the affected link 
> but actual value of the counter could be filled from the global counter 
> being maintained. But not sure whether we can force every driver's 
> firmware to do so?

I'm not sure I understand that description - how would firmware even
know which elements to take from what partner link etc.? I think hostapd
still has to add the elements, and firmware would just populate the
counter field(s)? But yeah that requires knowing from which partner link
to populate.

If you really wanted the firmware to handle the partner link information
completely it'd not only have to parse the Multi-Link Element on other
links but also _modify_, and know what to do, etc. - I don't think
that'll really work so well.

Think about it some more: On the link doing the CSA you'd have to parse
the beacon template and understand where the CSA-related elements are
(CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them
aside. Then on the partner link(s) parse the Multi-Link Element, see if
there's a partial STA profile already for some reason, merge the CSA-
related elements into that, update the counters; all while first
removing the 2 levels of fragmentation, and then re-doing them ...

I'm not even sure how I'd implement that in hwsim, and there I already
have all the helper functions and pretty much unlimited memory/code
size, so doing it in firmware seems awful?

Not to mention that it would never extend for future stuff, since new
elements wouldn't be copied over, firmware wouldn't necessarily know the
right order to merge them with existing elements in the per-STA profile,
etc.


> Let aside actual drivers, adding the test case for mac80211_hw_sim would 
> also be tricky since there beacon Tx is handled in software only.

Yeah but that's easy, especially if we go with (b) approach, even
mac80211 basically has all the necessary information to handle hwsim's
model of beacon transmission, we could add a little bit of additional
help there too (e.g. pass the TBTT to beacon_get).

> >   c) Require the get_tsf operation and add an operation to retrieve TSF
> >      offset (**), and then calculate the TBTT for each link when
> >      beacon_get is called with the per-link beacon intervals and update
> >      the values correctly if CSA is in progress on any link ... requires
> >      against parsing like in (a) or offset information like in (b), but
> >      at least now it's in software?
> >      For probe responses this could be a bit off I guess, but maybe that
> >      doesn't matter as much - probe responses are not authenticated so a
> >      client probably shouldn't use them for real CSA anyway.
> > 
> >      (**) which I guess we need anyway for hostapd to put it into beacons
> >      etc.?
> 
> Yes I'm too anticipating hostapd changes to support the critical update.

Oh hostapd will have to make changes here for everything - the (**)
footnote was about how hostapd knows the TSF offset between links?
Unless we're planning to always keep the TSF offset zero?

> > Do you have any plans for any of this?
> 
> I do have some idea (possibly half baked code) for handling critical 
> updates in probe responses. Beacons, I assume firmware takes care hence 
> I have changes to mark it in the link_conf so that drivers could read 
> that and give it to its firmware.
> Handling in beacon at kernel level, I need to think of it since anyways 
> hw_sim test case is good to have :)
> 
> Do we have any data right now, how many actual drivers have offloaded 
> beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)?
> If we could see majority are using firmware then we could first add 
> support for such drivers then we can think about handling it in kernel?

I agree that we can go with one approach for now, but I think we should
agree on the first model of how firmware does this. I'm not convinced
that your description above where firmware even magically handles
propagating the CSA elements to partner links will really work? But see
above.

> > I'm mostly asking right now because I want a reliable way to test the
> > work I'm trying to do on the client side though, so I could also live
> > with some hacks (inject through debugfs?), but having it for real would
> > be nice.
> 
> Okay. I'm right now working to add basic test case in hw_sim for MLO CSA 
> (on first link) as we discussed. There is need of a few mac80211 changes 
> in order to support for other links. I'm checking those now. Once done 
> with these, then I'll take up critical updates.
> 
> 
> 

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

Uh, sorry, accidentally sent too soon.

> > > I'm mostly asking right now because I want a reliable way to test the
> > > work I'm trying to do on the client side though, so I could also live
> > > with some hacks (inject through debugfs?), but having it for real would
> > > be nice.
> > 
> > Okay. I'm right now working to add basic test case in hw_sim for MLO CSA 
> > (on first link) as we discussed. There is need of a few mac80211 changes 
> > in order to support for other links. I'm checking those now. Once done 
> > with these, then I'll take up critical updates.

OK. I also have a few patches that I was working on. Mostly client
related, but maybe these would be relevant for you as well:

https://p.sipsolutions.net/f9e976055a616eda.txt
https://p.sipsolutions.net/054dcb0502247890.txt

Sorry about the format - once I've fully tested them etc. I'll send them
out.

johannes

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/13/24 14:12, Johannes Berg wrote:
> On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote:
>> On 2/12/24 20:16, Johannes Berg wrote:
>>> So ... I'm looking at the client side, and thinking about that.
>>> According to the spec, multi-link element should be present in beacons
>>> of APs affiliated with the same MLD if one of the (other) links is doing
>>> CSA, and then have also the CSA counters of course, relative to the
>>> target link's TBTT (of course.)
>>
>> Yes correct, you are referring to critical update(s) right?
> 
> I was actually thinking the critical update flag is rather not all that
> interesting, but the inclusion of the CSA-related elements in a
> (partial) per-STA profile in the Multi-Link Element is. Those contain
> the countdown fields, so need to be updated - according to the TBTT of
> the link doing the CSA, not the link sending it.
> 

Yes correct, this CSA thing is interesting but there is one more thing - 
handling of the BSS param change count (BPCC). Let's say the affected 
link undergoes CSA, then in its Basic Multi Link IE, BPCC should be 
incremented. But at the same time, in RNR of its partner links, where 
this affected link is there, the BPCC should also be incremented.
All though here it is not incremented with every beacon Tx, but still 
change in one affects beacon of other partner link(s) at least once.


>>> Theoretically, both mac80211 and hostapd today support different beacon
>>> interval on different links, I believe.
>>>
>>> This makes the whole thing of including CSA for link A on beacons/probe
>>> responses transmitted on link B very tricky, because you have to know
>>> the timing, etc.
>>
>> At least handling for probe response seems to be a bit easier since
>> there we need not maintain any timer as such (as you have said too - we
>> need not be perfect there).
> 
> OTOH, if we already need some infrastructure for beacons, we can also
> use that infrastructure to fill in probe responses? Might be easier
> overall, because otherwise you'd have to start timers in hostapd etc. if
> you actually wanted to fill that in hostapd, and that's messy ...
> 

Correct!

> And as a complete aside: link removal _also_ has a countdown, but I'm
> not sure that affects cross-link, but if it does we might want a more
> general cross-link counter infrastructure? Though that might not mesh
> well with how (your) firmware handles it?
> 

Yes.

>> But beacons, as you said, it is indeed bit
>> tricky to handle full support considering both links could beacon at
>> different intervals.
> 
> Indeed.
> 
>>> For the CSA counter of a link _itself_ we have counter_offsets_beacon
>>> and counter_offsets_presp (for probe response offload) in struct
>>> cfg80211_csa_settings, and also counter offsets in struct
>>> cfg80211_mgmt_tx_params for sending probe responses.
>>>
>>> But ... for the cross-link information in the MLE this gets way more
>>> tricky? Especially if the beacon interval is different - we couldn't
>>> just count down, we'd have to fill in the information when we know where
>>> the frame is transmitted. For probe responses maybe we can afford to not
>>> be perfect, but for beacons it better be right - so we have to calculate
>>> the right counter value(s) for (all of) the switching link(s) according
>>> to the current TSF, TSF offset and TBTT - not all of which we even have
>>> in the driver?
>>>
>>
>> Yes correct :)
> 
> :)
> 
>>> I can see a few ways of implementing this:
>>>
>>>    a) Punt to firmware and it parses the multi-link element etc. to
>>>       find the places to update. But that's awful, it requires parsing
>>>       possibly fragmented MLE with fragmented subelements containing the
>>>       CSA elements inside ...
>>>
>>>    b) Punt to firmware and give it a (possibly long) list of offsets K_N
>>>       where to put the N'th counter for link K when transmitting the
>>>       frame.
>>>
>>
>> At least for beacons if firmware takes care of it then it will be good.
> 
> Note that I'm basically asking you, Mediatek and Realtek how this should
> work. We're almost certainly not going to support multi-link AP in our
> device any time soon, and we could adjust the firmware to it anyway if
> needed.
> 
> So while I agree that punting it to firmware would be somewhat nice, we
> still have to agree on (a) or (b), because (b) requires more information
> from hostapd etc., but (a) requires _substantially_ more (and very
> complex) parsing in firmware... If you wanted to support multi-BSSID,
> then it's even more complex, which would argue for (b)?
> 

Correct. Both has got its own advantages and disadvantages. Need to 
agree on any one so that all can be benefited. Let's hear from other folks.


> I was trying to keep opinions out of this, but personally, I really
> don't like (a), it puts too much complexity in the hands of firmware
> engineers ;-)
> 
>> Firmware could maintain the counter (for the affected link) and (I
>> assume it will be aware of the partner links) so whenever partner link
>> transmits a beacon it could add the CSA (or such IEs) in per STA profile
>> of the reporting link. It could get the contents from the affected link
>> but actual value of the counter could be filled from the global counter
>> being maintained. But not sure whether we can force every driver's
>> firmware to do so?
> 
> I'm not sure I understand that description - how would firmware even
> know which elements to take from what partner link etc.? I think hostapd
> still has to add the elements, and firmware would just populate the
> counter field(s)? But yeah that requires knowing from which partner link
> to populate.
> 

Yeah so underlying assumption/thought was -
1. Firmware is aware of partner links
2. Host driver just needs to update the beacon template of the affected 
link and some how indicate to firmware that this is critical update.
3. As per spec, there are around 21-22 changes classified as critical 
update and among those only handful of those which has adding the IE in 
per STA profile. So firmware could possibly maintain a table to see if 
the affected link's beacon template has such IEs which if present needs 
to be added in partner links, it could add it as and when partner link 
transmits a beacon.
4. Once CSA is finished, the host would update the beacon template which 
will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to 
per STA profile as well of the reporting links.

But yeah adding to per STA profile when already elements are there and 
then first de-fragment and then add and then later fragment would be a 
tedious job :)


> If you really wanted the firmware to handle the partner link information
> completely it'd not only have to parse the Multi-Link Element on other
> links but also _modify_, and know what to do, etc. - I don't think
> that'll really work so well.
> 
> Think about it some more: On the link doing the CSA you'd have to parse
> the beacon template and understand where the CSA-related elements are
> (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them
> aside. Then on the partner link(s) parse the Multi-Link Element, see if
> there's a partial STA profile already for some reason, merge the CSA-
> related elements into that, update the counters; all while first
> removing the 2 levels of fragmentation, and then re-doing them ...
> 

Yeah complex on firmware level. But even if we solve this on kernel 
level, existing drivers who have offloaded beacon to firmware from 11ax 
or before itself, still need to handle it? It would looks ugly that till 
11ax it is offloaded and from 11be it is again put back to kernel?

> I'm not even sure how I'd implement that in hwsim, and there I already
> have all the helper functions and pretty much unlimited memory/code
> size, so doing it in firmware seems awful?
> 

Yes.

> Not to mention that it would never extend for future stuff, since new
> elements wouldn't be copied over, firmware wouldn't necessarily know the
> right order to merge them with existing elements in the per-STA profile,
> etc.
> 

Yeah so if we go with handle on firmware level, as and when spec 
changes, firmware changes are required too.

> 
>> Let aside actual drivers, adding the test case for mac80211_hw_sim would
>> also be tricky since there beacon Tx is handled in software only.
> 
> Yeah but that's easy, especially if we go with (b) approach, even
> mac80211 basically has all the necessary information to handle hwsim's
> model of beacon transmission, we could add a little bit of additional
> help there too (e.g. pass the TBTT to beacon_get).
> 
>>>    c) Require the get_tsf operation and add an operation to retrieve TSF
>>>       offset (**), and then calculate the TBTT for each link when
>>>       beacon_get is called with the per-link beacon intervals and update
>>>       the values correctly if CSA is in progress on any link ... requires
>>>       against parsing like in (a) or offset information like in (b), but
>>>       at least now it's in software?
>>>       For probe responses this could be a bit off I guess, but maybe that
>>>       doesn't matter as much - probe responses are not authenticated so a
>>>       client probably shouldn't use them for real CSA anyway.
>>>
>>>       (**) which I guess we need anyway for hostapd to put it into beacons
>>>       etc.?
>>
>> Yes I'm too anticipating hostapd changes to support the critical update.
> 
> Oh hostapd will have to make changes here for everything - the (**)
> footnote was about how hostapd knows the TSF offset between links?
> Unless we're planning to always keep the TSF offset zero?
> 
>>> Do you have any plans for any of this?
>>
>> I do have some idea (possibly half baked code) for handling critical
>> updates in probe responses. Beacons, I assume firmware takes care hence
>> I have changes to mark it in the link_conf so that drivers could read
>> that and give it to its firmware.
>> Handling in beacon at kernel level, I need to think of it since anyways
>> hw_sim test case is good to have :)
>>
>> Do we have any data right now, how many actual drivers have offloaded
>> beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)?
>> If we could see majority are using firmware then we could first add
>> support for such drivers then we can think about handling it in kernel?
> 
> I agree that we can go with one approach for now, but I think we should
> agree on the first model of how firmware does this. I'm not convinced
> that your description above where firmware even magically handles
> propagating the CSA elements to partner links will really work? But see
> above.
> 
>>> I'm mostly asking right now because I want a reliable way to test the
>>> work I'm trying to do on the client side though, so I could also live
>>> with some hacks (inject through debugfs?), but having it for real would
>>> be nice.
>>
>> Okay. I'm right now working to add basic test case in hw_sim for MLO CSA
>> (on first link) as we discussed. There is need of a few mac80211 changes
>> in order to support for other links. I'm checking those now. Once done
>> with these, then I'll take up critical updates.
>>
>>
>>

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Tue, 2024-02-13 at 15:48 +0530, Aditya Kumar Singh wrote:
> On 2/13/24 14:12, Johannes Berg wrote:
> > On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote:
> > > On 2/12/24 20:16, Johannes Berg wrote:
> > > > So ... I'm looking at the client side, and thinking about that.
> > > > According to the spec, multi-link element should be present in beacons
> > > > of APs affiliated with the same MLD if one of the (other) links is doing
> > > > CSA, and then have also the CSA counters of course, relative to the
> > > > target link's TBTT (of course.)
> > > 
> > > Yes correct, you are referring to critical update(s) right?
> > 
> > I was actually thinking the critical update flag is rather not all that
> > interesting, but the inclusion of the CSA-related elements in a
> > (partial) per-STA profile in the Multi-Link Element is. Those contain
> > the countdown fields, so need to be updated - according to the TBTT of
> > the link doing the CSA, not the link sending it.
> > 
> 
> Yes correct, this CSA thing is interesting but there is one more thing - 
> handling of the BSS param change count (BPCC). Let's say the affected 
> link undergoes CSA, then in its Basic Multi Link IE, BPCC should be 
> incremented. But at the same time, in RNR of its partner links, where 
> this affected link is there, the BPCC should also be incremented.
> All though here it is not incremented with every beacon Tx, but still 
> change in one affects beacon of other partner link(s) at least once.

Yes, I know, but IMHO that's not all that interesting, I'd think hostapd
could handle that.

> > Note that I'm basically asking you, Mediatek and Realtek how this should
> > work. We're almost certainly not going to support multi-link AP in our
> > device any time soon, and we could adjust the firmware to it anyway if
> > needed.
> > 
> > So while I agree that punting it to firmware would be somewhat nice, we
> > still have to agree on (a) or (b), because (b) requires more information
> > from hostapd etc., but (a) requires _substantially_ more (and very
> > complex) parsing in firmware... If you wanted to support multi-BSSID,
> > then it's even more complex, which would argue for (b)?
> > 
> 
> Correct. Both has got its own advantages and disadvantages. Need to 
> agree on any one so that all can be benefited. Let's hear from other folks.

Do you _have_ firmware for this already, perhaps?

I mean, I'm happy to be designing this "greenfield", but ... it seems
unlikely at this point?

Also what about ath12k with the multi-device changes, would the driver
have to handle it? Or I guess you have to sync the TSF somehow anyway,
so you could handle it based on TSF/TBTT?


> > I was trying to keep opinions out of this, but personally, I really
> > don't like (a), it puts too much complexity in the hands of firmware
> > engineers ;-)
> > 
> > > Firmware could maintain the counter (for the affected link) and (I
> > > assume it will be aware of the partner links) so whenever partner link
> > > transmits a beacon it could add the CSA (or such IEs) in per STA profile
> > > of the reporting link. It could get the contents from the affected link
> > > but actual value of the counter could be filled from the global counter
> > > being maintained. But not sure whether we can force every driver's
> > > firmware to do so?
> > 
> > I'm not sure I understand that description - how would firmware even
> > know which elements to take from what partner link etc.? I think hostapd
> > still has to add the elements, and firmware would just populate the
> > counter field(s)? But yeah that requires knowing from which partner link
> > to populate.
> > 
> 
> Yeah so underlying assumption/thought was -
> 1. Firmware is aware of partner links

Sure.

> 2. Host driver just needs to update the beacon template of the affected 
> link and some how indicate to firmware that this is critical update.

Sure.

> 3. As per spec, there are around 21-22 changes classified as critical 
> update and among those only handful of those which has adding the IE in 
> per STA profile. So firmware could possibly maintain a table to see if 
> the affected link's beacon template has such IEs which if present needs 
> to be added in partner links, it could add it as and when partner link 
> transmits a beacon.

That seems messy for all the reasons I mentioned, since it involves
parsing/defragmenting/merging/fragmenting per-STA profiles etc.

Do you really want to go there?

> 4. Once CSA is finished, the host would update the beacon template which 
> will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to 
> per STA profile as well of the reporting links.

Yeah I guess that's just a corollary of 3 :)

> But yeah adding to per STA profile when already elements are there and 
> then first de-fragment and then add and then later fragment would be a 
> tedious job :)

Yeah...

> > If you really wanted the firmware to handle the partner link information
> > completely it'd not only have to parse the Multi-Link Element on other
> > links but also _modify_, and know what to do, etc. - I don't think
> > that'll really work so well.
> > 
> > Think about it some more: On the link doing the CSA you'd have to parse
> > the beacon template and understand where the CSA-related elements are
> > (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them
> > aside. Then on the partner link(s) parse the Multi-Link Element, see if
> > there's a partial STA profile already for some reason, merge the CSA-
> > related elements into that, update the counters; all while first
> > removing the 2 levels of fragmentation, and then re-doing them ...
> > 
> 
> Yeah complex on firmware level. But even if we solve this on kernel 
> level, existing drivers who have offloaded beacon to firmware from 11ax 
> or before itself, still need to handle it? It would looks ugly that till 
> 11ax it is offloaded and from 11be it is again put back to kernel?

Not sure what you mean here - we can still offload beaconing with a 
template, just have to fill in the template a bit more (with partner
link CSA counters rather than just own CSA counters)?

I'm basically thinking this:

1. Firmware is aware of partner link CSA counters/TSF offset/TBTT

2. Host driver (hostapd) sets beacon template of affected link to
include CSA elements, and already today we include in the setting the
offset to the countdown fields, so it doesn't matter where those are.

3. Host driver (hostapd) *also* updates beacon templates of partner
links, but instead of saying "here's the CSA counter to update" it says
"here's the CSA counter for your partner link M to set" (OK, so there
potentially are multiple of those, but that's OK)

4. Firmware doesn't need to parse anything, it just needs to go through
the beacon when transmitting it (for any link) and set the CSA counter
fields for the link itself and/or for the partner links according to
what the correct counter is per TBTT rules, TSF offset, beacon interval
etc.


Note that this could easily be implemented even in software for hwsim or
drivers that don't offload beacon template to the device (like ath9k
though obviously that has no multi-link), *iff* you know the TSF offset
and TBTT of the beacon being created.
Currently the CSA counters are just decremented every time you request a
beacon, but the partner link counters would have to be calculated - but
if you know
 - the intended TBTT of the beacon to transmit
 - the TSF offset to the link undergoing CSA
 - the beacon interval of the link undergoing CSA
you can calculate
 - the TSF of the link undergoing CSA at the TBTT of the beacon we're
   creating,
 - the previous TBTT of the link undergoing CSA by the TSF (some
   calculation involving TSF modulo the beacon interval)
 - therefore the CSA counter of the link undergoing CSA to fill in

In hwsim it's even easier because we only request the beacon at the
TBTT, and the TSF offset is 0 ...

johannes

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/13/24 16:25, Johannes Berg wrote:
> On Tue, 2024-02-13 at 15:48 +0530, Aditya Kumar Singh wrote:
>> On 2/13/24 14:12, Johannes Berg wrote:
>>> On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote:
>>>> On 2/12/24 20:16, Johannes Berg wrote:
>>>>> So ... I'm looking at the client side, and thinking about that.
>>>>> According to the spec, multi-link element should be present in beacons
>>>>> of APs affiliated with the same MLD if one of the (other) links is doing
>>>>> CSA, and then have also the CSA counters of course, relative to the
>>>>> target link's TBTT (of course.)
>>>>
>>>> Yes correct, you are referring to critical update(s) right?
>>>
>>> I was actually thinking the critical update flag is rather not all that
>>> interesting, but the inclusion of the CSA-related elements in a
>>> (partial) per-STA profile in the Multi-Link Element is. Those contain
>>> the countdown fields, so need to be updated - according to the TBTT of
>>> the link doing the CSA, not the link sending it.
>>>
>>
>> Yes correct, this CSA thing is interesting but there is one more thing -
>> handling of the BSS param change count (BPCC). Let's say the affected
>> link undergoes CSA, then in its Basic Multi Link IE, BPCC should be
>> incremented. But at the same time, in RNR of its partner links, where
>> this affected link is there, the BPCC should also be incremented.
>> All though here it is not incremented with every beacon Tx, but still
>> change in one affects beacon of other partner link(s) at least once.
> 
> Yes, I know, but IMHO that's not all that interesting, I'd think hostapd
> could handle that.
> 
>>> Note that I'm basically asking you, Mediatek and Realtek how this should
>>> work. We're almost certainly not going to support multi-link AP in our
>>> device any time soon, and we could adjust the firmware to it anyway if
>>> needed.
>>>
>>> So while I agree that punting it to firmware would be somewhat nice, we
>>> still have to agree on (a) or (b), because (b) requires more information
>>> from hostapd etc., but (a) requires _substantially_ more (and very
>>> complex) parsing in firmware... If you wanted to support multi-BSSID,
>>> then it's even more complex, which would argue for (b)?
>>>
>>
>> Correct. Both has got its own advantages and disadvantages. Need to
>> agree on any one so that all can be benefited. Let's hear from other folks.
> 
> Do you _have_ firmware for this already, perhaps?
> 

Yes, private firmware is there. Not yet upstreamed since the 
infrastructure in mac80211 and driver is not there. AFAIK it is in 
pipeline. Internal testing have so far been good and did not see any 
issues. So at least from Qualcomm perspective, we are fine with 
offloading (the above we discussed) to firmware.


> I mean, I'm happy to be designing this "greenfield", but ... it seems
> unlikely at this point?
> 
> Also what about ath12k with the multi-device changes, would the driver
> have to handle it? Or I guess you have to sync the TSF somehow anyway,
> so you could handle it based on TSF/TBTT?
> 

Ath12k firmware also supports beacon offload and hence firmware is 
taking care to add the respective IEs in per-STA profile of the 
reporting links while host just sends the template for the affected link 
via beacon template update and indicating that critical update has happened.

> 
>>> I was trying to keep opinions out of this, but personally, I really
>>> don't like (a), it puts too much complexity in the hands of firmware
>>> engineers ;-)
>>>
>>>> Firmware could maintain the counter (for the affected link) and (I
>>>> assume it will be aware of the partner links) so whenever partner link
>>>> transmits a beacon it could add the CSA (or such IEs) in per STA profile
>>>> of the reporting link. It could get the contents from the affected link
>>>> but actual value of the counter could be filled from the global counter
>>>> being maintained. But not sure whether we can force every driver's
>>>> firmware to do so?
>>>
>>> I'm not sure I understand that description - how would firmware even
>>> know which elements to take from what partner link etc.? I think hostapd
>>> still has to add the elements, and firmware would just populate the
>>> counter field(s)? But yeah that requires knowing from which partner link
>>> to populate.
>>>
>>
>> Yeah so underlying assumption/thought was -
>> 1. Firmware is aware of partner links
> 
> Sure.
> 
>> 2. Host driver just needs to update the beacon template of the affected
>> link and some how indicate to firmware that this is critical update.
> 
> Sure.
> 
>> 3. As per spec, there are around 21-22 changes classified as critical
>> update and among those only handful of those which has adding the IE in
>> per STA profile. So firmware could possibly maintain a table to see if
>> the affected link's beacon template has such IEs which if present needs
>> to be added in partner links, it could add it as and when partner link
>> transmits a beacon.
> 
> That seems messy for all the reasons I mentioned, since it involves
> parsing/defragmenting/merging/fragmenting per-STA profiles etc.
> 
> Do you really want to go there?
> 

I understand things looks to be messy. But at least internally we have 
done in that way and things look simpler (on kernel level). Not sure 
whether all firmwares would agree to that.

>> 4. Once CSA is finished, the host would update the beacon template which
>> will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to
>> per STA profile as well of the reporting links.
> 
> Yeah I guess that's just a corollary of 3 :)
> 
>> But yeah adding to per STA profile when already elements are there and
>> then first de-fragment and then add and then later fragment would be a
>> tedious job :)
> 
> Yeah...
> 
>>> If you really wanted the firmware to handle the partner link information
>>> completely it'd not only have to parse the Multi-Link Element on other
>>> links but also _modify_, and know what to do, etc. - I don't think
>>> that'll really work so well.
>>>
>>> Think about it some more: On the link doing the CSA you'd have to parse
>>> the beacon template and understand where the CSA-related elements are
>>> (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them
>>> aside. Then on the partner link(s) parse the Multi-Link Element, see if
>>> there's a partial STA profile already for some reason, merge the CSA-
>>> related elements into that, update the counters; all while first
>>> removing the 2 levels of fragmentation, and then re-doing them ...
>>>
>>
>> Yeah complex on firmware level. But even if we solve this on kernel
>> level, existing drivers who have offloaded beacon to firmware from 11ax
>> or before itself, still need to handle it? It would looks ugly that till
>> 11ax it is offloaded and from 11be it is again put back to kernel?
> 
> Not sure what you mean here - we can still offload beaconing with a
> template, just have to fill in the template a bit more (with partner
> link CSA counters rather than just own CSA counters)?
> > I'm basically thinking this:
> 
> 1. Firmware is aware of partner link CSA counters/TSF offset/TBTT
> 
> 2. Host driver (hostapd) sets beacon template of affected link to
> include CSA elements, and already today we include in the setting the
> offset to the countdown fields, so it doesn't matter where those are.
> 
> 3. Host driver (hostapd) *also* updates beacon templates of partner
> links, but instead of saying "here's the CSA counter to update" it says
> "here's the CSA counter for your partner link M to set" (OK, so there
> potentially are multiple of those, but that's OK)
> 
> 4. Firmware doesn't need to parse anything, it just needs to go through
> the beacon when transmitting it (for any link) and set the CSA counter
> fields for the link itself and/or for the partner links according to
> what the correct counter is per TBTT rules, TSF offset, beacon interval
> etc.
> 

Hmm.. I see your point. So basically there is tradeoff - complexity of 
handling the parsing and all in firmware or complexity of handling those 
many offsets in kernel. I believe when hostapd updates the templates for 
the partner links, it would have to send offsets so we may need to 
extend our NL infrastructure for MLO in order to accommodate all those 
list of offsets during beacon template update.

> 
> Note that this could easily be implemented even in software for hwsim or
> drivers that don't offload beacon template to the device (like ath9k
> though obviously that has no multi-link), *iff* you know the TSF offset
> and TBTT of the beacon being created.
> Currently the CSA counters are just decremented every time you request a
> beacon, but the partner link counters would have to be calculated - but
> if you know
>   - the intended TBTT of the beacon to transmit
>   - the TSF offset to the link undergoing CSA
>   - the beacon interval of the link undergoing CSA
> you can calculate
>   - the TSF of the link undergoing CSA at the TBTT of the beacon we're
>     creating,
>   - the previous TBTT of the link undergoing CSA by the TSF (some
>     calculation involving TSF modulo the beacon interval)
>   - therefore the CSA counter of the link undergoing CSA to fill in
> 
> In hwsim it's even easier because we only request the beacon at the
> TBTT, and the TSF offset is 0 ...
> 

Yep, agree.

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Tue, 2024-02-13 at 18:11 +0530, Aditya Kumar Singh wrote:
> > > Correct. Both has got its own advantages and disadvantages. Need to
> > > agree on any one so that all can be benefited. Let's hear from other folks.
> > 
> > Do you _have_ firmware for this already, perhaps?
> > 
> 
> Yes, private firmware is there. Not yet upstreamed since the 
> infrastructure in mac80211 and driver is not there. AFAIK it is in 
> pipeline. Internal testing have so far been good and did not see any 
> issues. So at least from Qualcomm perspective, we are fine with 
> offloading (the above we discussed) to firmware.

Ah, OK.

Note that if we have all the offsets I guess you could still do all the
parsing? Though adding the per-STA profile in cross-link scenarios would
be a difference if hostapd has to do it in the template vs. firmware
copying the elements ...

For the record, I really don't like copying the elements and not sure
I'd want to implement that in mac80211/hwsim...

> > I mean, I'm happy to be designing this "greenfield", but ... it seems
> > unlikely at this point?
> > 
> > Also what about ath12k with the multi-device changes, would the driver
> > have to handle it? Or I guess you have to sync the TSF somehow anyway,
> > so you could handle it based on TSF/TBTT?
> > 
> 
> Ath12k firmware also supports beacon offload and hence firmware is 
> taking care to add the respective IEs in per-STA profile of the 
> reporting links while host just sends the template for the affected link 
> via beacon template update and indicating that critical update has happened.

OK. So I guess we have to support that? Or are there any chances we
could agree on something here overall and then the firmware can be
changed to support another mode?

> > That seems messy for all the reasons I mentioned, since it involves
> > parsing/defragmenting/merging/fragmenting per-STA profiles etc.
> > 
> > Do you really want to go there?
> > 
> 
> I understand things looks to be messy. But at least internally we have 
> done in that way and things look simpler (on kernel level). Not sure 
> whether all firmwares would agree to that.

And hwsim, but yeah :)

> > Not sure what you mean here - we can still offload beaconing with a
> > template, just have to fill in the template a bit more (with partner
> > link CSA counters rather than just own CSA counters)?
> > > I'm basically thinking this:
> > 
> > 1. Firmware is aware of partner link CSA counters/TSF offset/TBTT
> > 
> > 2. Host driver (hostapd) sets beacon template of affected link to
> > include CSA elements, and already today we include in the setting the
> > offset to the countdown fields, so it doesn't matter where those are.
> > 
> > 3. Host driver (hostapd) *also* updates beacon templates of partner
> > links, but instead of saying "here's the CSA counter to update" it says
> > "here's the CSA counter for your partner link M to set" (OK, so there
> > potentially are multiple of those, but that's OK)
> > 
> > 4. Firmware doesn't need to parse anything, it just needs to go through
> > the beacon when transmitting it (for any link) and set the CSA counter
> > fields for the link itself and/or for the partner links according to
> > what the correct counter is per TBTT rules, TSF offset, beacon interval
> > etc.
> > 
> 
> Hmm.. I see your point. So basically there is tradeoff - complexity of 
> handling the parsing and all in firmware or complexity of handling those 
> many offsets in kernel. I believe when hostapd updates the templates for 
> the partner links, it would have to send offsets so we may need to 
> extend our NL infrastructure for MLO in order to accommodate all those 
> list of offsets during beacon template update.

Yes, of course. But that doesn't seem _too_ complicated? Even supporting
max # of links (14) and ~3 counters each isn't a huge list... netlink
doesn't care, and drivers have lower link limits anyway.

Maybe I'll do a prototype later? But for now

 a) I need to test the client-side code I'm writing in some other way
    (element injection or something)

 b) let's wait for Realtek to comment also after the Lunar New Year :)

Thanks!

johannes

RE: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Ping-Ke Shih]

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Tuesday, February 13, 2024 8:49 PM
> To: Aditya Kumar Singh <quic_adisi@quicinc.com>
> Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ping-Ke Shih <pkshih@realtek.com>; Ryder Lee
> <ryder.lee@mediatek.com>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> 
>  b) let's wait for Realtek to comment also after the Lunar New Year :)
> 

Sorry for the late. 

Realtek firmware can update partner links' CSA/ECSA by given offset of
beacon template, which matches the conclusion of this thread, and Realtek vendor
(out of tree) driver has verified the firmware interface. So that will
work to Realtek WiFi 7 chips. 

Ping-Ke 

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

> 
> Sorry for the late. 
> 
> Realtek firmware can update partner links' CSA/ECSA by given offset of
> beacon template, which matches the conclusion of this thread, and Realtek vendor
> (out of tree) driver has verified the firmware interface. So that will
> work to Realtek WiFi 7 chips. 

Thanks!

I guess that'd also apply to probe responses? Or does it not send those
at all? But we discussed before that maybe we don't have to be perfect
there, so I guess we can find some solution to that.

So we have, so far

Realtek:
 - uses offset in beacon template to set partner counter,
 - requires host to set CSA/ECSA elements

Qualcomm:
 - copies and updates CSA/ECSA elements all by itself
 - btw, not sure here about probe responses, does it do that too?

hwsim:
 - personally, I'd prefer it works like Realtek, for less complexity

Intel:
 - not implemented, probably not going to happen any time soon, but I
   think we might prefer the Realtek way too if we ever do this


Mediatek? Broadcom? Anyone else?


I guess then for Qualcomm we'll need to just add an extended feature
flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
update the parter link templates, or so?

Thanks,
johannes

RE: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Ping-Ke Shih]

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Wednesday, February 21, 2024 4:10 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> 
> I guess that'd also apply to probe responses? Or does it not send those
> at all? But we discussed before that maybe we don't have to be perfect
> there, so I guess we can find some solution to that.

Unfortunately, Realtek firmware doesn't send probe responses at all. Still
need hostapd to reply those. 

Ping-Ke 

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote:
> > -----Original Message-----
> > From: Johannes Berg <johannes@sipsolutions.net>
> > Sent: Wednesday, February 21, 2024 4:10 PM
> > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org>
> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > 
> > I guess that'd also apply to probe responses? Or does it not send those
> > at all? But we discussed before that maybe we don't have to be perfect
> > there, so I guess we can find some solution to that.
> 
> Unfortunately, Realtek firmware doesn't send probe responses at all. Still
> need hostapd to reply those. 
> 

Right, but filling in the CSA counters?

johannes

RE: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Ping-Ke Shih]

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Wednesday, February 21, 2024 4:20 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> 
> On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote:
> > > -----Original Message-----
> > > From: Johannes Berg <johannes@sipsolutions.net>
> > > Sent: Wednesday, February 21, 2024 4:10 PM
> > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org>
> > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > >
> > > I guess that'd also apply to probe responses? Or does it not send those
> > > at all? But we discussed before that maybe we don't have to be perfect
> > > there, so I guess we can find some solution to that.
> >
> > Unfortunately, Realtek firmware doesn't send probe responses at all. Still
> > need hostapd to reply those.
> >
> 
> Right, but filling in the CSA counters?
> 

No, firmware doesn't modify content of probe response frame.

Ping-Ke

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Wed, 2024-02-21 at 08:28 +0000, Ping-Ke Shih wrote:
> 
> > -----Original Message-----
> > From: Johannes Berg <johannes@sipsolutions.net>
> > Sent: Wednesday, February 21, 2024 4:20 PM
> > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org>
> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > 
> > On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote:
> > > > -----Original Message-----
> > > > From: Johannes Berg <johannes@sipsolutions.net>
> > > > Sent: Wednesday, February 21, 2024 4:10 PM
> > > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> > > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> > > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> > > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org>
> > > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > > > 
> > > > I guess that'd also apply to probe responses? Or does it not send those
> > > > at all? But we discussed before that maybe we don't have to be perfect
> > > > there, so I guess we can find some solution to that.
> > > 
> > > Unfortunately, Realtek firmware doesn't send probe responses at all. Still
> > > need hostapd to reply those.
> > > 
> > 
> > Right, but filling in the CSA counters?
> > 
> 
> No, firmware doesn't modify content of probe response frame.
> 

Can you get that fixed? ;-)

With differing beacon intervals etc., I don't know there's a good way to
keep the counters even with a semblance of correctness, especially if we
don't know when the beacon was transmitted?

Or maybe just fill it in the driver since you probably have some beacon
timing data more easily accessible?

johannes

RE: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Ping-Ke Shih]

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Wednesday, February 21, 2024 4:35 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@nbd.name>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> 
> On Wed, 2024-02-21 at 08:28 +0000, Ping-Ke Shih wrote:
> >
> > > -----Original Message-----
> > > From: Johannes Berg <johannes@sipsolutions.net>
> > > Sent: Wednesday, February 21, 2024 4:20 PM
> > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org>
> > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > >
> > > On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote:
> > > > > -----Original Message-----
> > > > > From: Johannes Berg <johannes@sipsolutions.net>
> > > > > Sent: Wednesday, February 21, 2024 4:10 PM
> > > > > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> > > > > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> > > > > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> > > > > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@openwrt.org>
> > > > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > > > >
> > > > > I guess that'd also apply to probe responses? Or does it not send those
> > > > > at all? But we discussed before that maybe we don't have to be perfect
> > > > > there, so I guess we can find some solution to that.
> > > >
> > > > Unfortunately, Realtek firmware doesn't send probe responses at all. Still
> > > > need hostapd to reply those.
> > > >
> > >
> > > Right, but filling in the CSA counters?
> > >
> >
> > No, firmware doesn't modify content of probe response frame.
> >
> 
> Can you get that fixed? ;-)
> 
> With differing beacon intervals etc., I don't know there's a good way to
> keep the counters even with a semblance of correctness, especially if we
> don't know when the beacon was transmitted?
> 
> Or maybe just fill it in the driver since you probably have some beacon
> timing data more easily accessible?

If driver can get CSA or ECSA offset simply, I probably can fill a reasonable
CSA counter (not sure if I can get 100% accurate counter for now), but
seemingly neither ieee80211_tx_control nor ieee80211_tx_info (SKB_CB) doesn't
have these offsets. Any suggestions?

I wonder our out-of-tree driver generates probe response itself. Let's me
check how it does.

Ping-Ke

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Wed, 2024-02-21 at 08:57 +0000, Ping-Ke Shih wrote:
> > > 
> > > No, firmware doesn't modify content of probe response frame.
> > > 
> > 
> > Can you get that fixed? ;-)
> > 
> > With differing beacon intervals etc., I don't know there's a good way to
> > keep the counters even with a semblance of correctness, especially if we
> > don't know when the beacon was transmitted?
> > 
> > Or maybe just fill it in the driver since you probably have some beacon
> > timing data more easily accessible?
> 
> If driver can get CSA or ECSA offset simply, I probably can fill a reasonable
> CSA counter (not sure if I can get 100% accurate counter for now), but
> seemingly neither ieee80211_tx_control nor ieee80211_tx_info (SKB_CB) doesn't
> have these offsets. Any suggestions?

We're not there yet! This thread is debating how we want to handle it.

Although .. you have a point, we have that issue now already, and we
don't pass the CSA offsets in probe responses if hostapd is filling them
in. I guess we also have work to do on this.

> I wonder our out-of-tree driver generates probe response itself. Let's me
> check how it does.

You _could_ also actually just implement probe response "offload" in the
driver, then you get the template from hostapd/mac80211, and that should
come with the offsets to fill in. Might be easier, overall.

johannes

RE: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Ping-Ke Shih]

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Wednesday, February 21, 2024 5:00 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@nbd.name>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> 
> You _could_ also actually just implement probe response "offload" in the
> driver, then you get the template from hostapd/mac80211, and that should
> come with the offsets to fill in. Might be easier, overall.

Agree. That will be easier. What I only concern is hostapd can still work well
without probe request, which is dropped by driver implementing offload. 

Ping-Ke

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

On Wed, 2024-02-21 at 09:17 +0000, Ping-Ke Shih wrote:
> 
> > -----Original Message-----
> > From: Johannes Berg <johannes@sipsolutions.net>
> > Sent: Wednesday, February 21, 2024 5:00 PM
> > To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> > Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> > <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> > <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@nbd.name>
> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > 
> > You _could_ also actually just implement probe response "offload" in the
> > driver, then you get the template from hostapd/mac80211, and that should
> > come with the offsets to fill in. Might be easier, overall.
> 
> Agree. That will be easier. What I only concern is hostapd can still work well
> without probe request, which is dropped by driver implementing offload. 

That's what happens today, so yeah, that should work?

You need to not answer&drop certain P2P probe requests though. But since
we're talking about this in WiFi7 AP context, you could just make it
different depending on that, I think we get the templates all the time
in the kernel and just ignore them if not handled.

johannes

RE: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Ping-Ke Shih]

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Wednesday, February 21, 2024 5:20 PM
> To: Ping-Ke Shih <pkshih@realtek.com>; Aditya Kumar Singh <quic_adisi@quicinc.com>
> Cc: linux-wireless@vger.kernel.org; Jeff Johnson <quic_jjohnson@quicinc.com>; Ilan Peer
> <ilan.peer@intel.com>; Jouni Malinen <j@w1.fi>; Ryder Lee <ryder.lee@mediatek.com>; Arend van Spriel
> <arend.vanspriel@broadcom.com>; Felix Fietkau <nbd@nbd.name>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> 
> You need to not answer&drop certain P2P probe requests though. But since
> we're talking about this in WiFi7 AP context, you could just make it
> different depending on that, I think we get the templates all the time
> in the kernel and just ignore them if not handled.
> 

Got it! 

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

> I guess then for Qualcomm we'll need to just add an extended feature
> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
> update the parter link templates, or so?
> 
Or then again ... Maybe the other way around, since it works for
Qualcomm now, assuming it all works at all? Doesn't really matter, we
can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted
logic.

johannes

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/21/24 15:12, Johannes Berg wrote:
> 
>> I guess then for Qualcomm we'll need to just add an extended feature
>> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
>> update the parter link templates, or so?
>>
> Or then again ... Maybe the other way around, since it works for
> Qualcomm now, assuming it all works at all? Doesn't really matter, we
> can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted
> logic.
> 

Yeah anything is fine. Basically this flag should be visible till 
hostapd (so probably some wiphy level flag), so that it may skip forming 
beacon templates of partner links in CSA case. Based on the flag, those 
drivers which want it, it should form and send.

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/21/24 15:12, Johannes Berg wrote:
> 
>> I guess then for Qualcomm we'll need to just add an extended feature
>> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
>> update the parter link templates, or so?
>>
> Or then again ... Maybe the other way around, since it works for
> Qualcomm now, assuming it all works at all? Doesn't really matter, we
> can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted
> logic.
> 
> johannes

Yeah anything is fine. Basically this flag should be visible till 
hostapd (so probably some wiphy level flag), so that it may skip forming 
beacon templates of partner links in CSA case. Based on the flag, those 
drivers which want it, it should form and send.

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/21/24 13:39, Johannes Berg wrote:
> Qualcomm:
>   - copies and updates CSA/ECSA elements all by itself
>   - btw, not sure here about probe responses, does it do that too?

We had a thought about keeping this CSA/ECSA handling at host/kernel 
level only. But the major point of concern is _synchronization_ among 
firmware of each of the links participating in the MLD.

* Even if we ignore TSF/TBTT synchronization for a moment, how firmware 
will know when to transmit the beacon with a particular counter or when 
CSA has finished on other link? If rely on host's update then there is 
room for further delay and hence errors.
      - This is because, counter value on the reported link depends on 
the last beacon transmitted by the affected link.

* Host can send the template on all links but how to ensure that first 
template is reached on the affected link and then only on the partner 
links? Host will queue the command properly but reaping of the command 
on n (no of links) independent firmware can not be guaranteed in the 
same order in which host has filled. It depends how busy each of host to 
firmware path is.

* And then obviously, considering TSF/TBTT will be again complicating 
the synchronization part and making it more difficult to manage just via 
host.

Hence there is a strong urge to let firmware handle all this for beacons.

As far as how firmware will _magically_ communicate among themselves is 
concerned, we have *IPC* in place to achieve that. One link firmware can 
talk to other link firmware when required.


				Kernel Level
____________________________________________________________________________
  -------------- 	      -------------- 		 --------------
|   Firmware 1 |  <- IPC ->  |   Firmware 2 | <- IPC -> |   Firmware 3 |
|    on HW 1   |	     |    on HW 2   |           |    on HW 3   |
  -------------- 	      --------------             --------------


Hence, host just needs to update template of the affected link and 
indicate to firmware that it is a critical update. This firmware then 
can indicate other link firmware(s) to append CSA/ECSA IE with a given 
counter value to its beacon via this IPC. Parsing the IE and 
de-fragmenting and fragmenting it again can be done by firmware itself. 
(Agree that it is bit complex but when comparing with complexity of 
maintaining synchronicity across links, this looks more doable)

Hence we have taken "offloading beacons fully to firmware" approach.

For probe responses, it is handled in host/kernel only. Firmware sends 
back the last transmitted count in beacon to host. So we have the last 
transmitted count info. Per STA profile generation logic is also there. 
So we manage via that.

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Johannes Berg]

Hi,

On Wed, 2024-02-21 at 18:22 +0530, Aditya Kumar Singh wrote:
> On 2/21/24 13:39, Johannes Berg wrote:
> > Qualcomm:
> >   - copies and updates CSA/ECSA elements all by itself
> >   - btw, not sure here about probe responses, does it do that too?
> 
> We had a thought about keeping this CSA/ECSA handling at host/kernel 
> level only. But the major point of concern is _synchronization_ among 
> firmware of each of the links participating in the MLD.

Sure.

> * Even if we ignore TSF/TBTT synchronization for a moment, how firmware 
> will know when to transmit the beacon with a particular counter or when 
> CSA has finished on other link? If rely on host's update then there is 
> room for further delay and hence errors.
>       - This is because, counter value on the reported link depends on 
> the last beacon transmitted by the affected link.

Sure.

I don't think anyone suggested that the host will put the exact counter
value there, just to have the template.

> * Host can send the template on all links

Right.

> but how to ensure that first 
> template is reached on the affected link and then only on the partner 
> links? Host will queue the command properly but reaping of the command 
> on n (no of links) independent firmware can not be guaranteed in the 
> same order in which host has filled. It depends how busy each of host to 
> firmware path is.

True, and there's a potential for race conditions there I suppose, but I
suppose in the Intel, Realtek and hwsim case at least we wouldn't have
*different* firmwares running multiple links, but a single one.

In any case, you could solve this even with multiple, by applying the
new template only after you have the CSA'ing link's new beacon template,
if it requires filling in CSA counters, or such.

> * And then obviously, considering TSF/TBTT will be again complicating 
> the synchronization part and making it more difficult to manage just via 
> host.

Again, not suggesting that it is managed completely by the host, just
the templates.

> Hence there is a strong urge to let firmware handle all this for beacons.

Sure, that's fine, your call :)

> As far as how firmware will _magically_ communicate among themselves is 
> concerned, we have *IPC* in place to achieve that. One link firmware can 
> talk to other link firmware when required.

:-)

It probably doesn't actually help you make it race-free though, so does
it really matter? But again, it's your call how you want to do it, and
we'll just have to handle it in software appropriately. While I'd prefer
to have _one_ way of doing things, at least so far we've basically seen
one way of having the host involved and ath12k not having the host
involved, so it's all still really simple.

> 				Kernel Level
> ____________________________________________________________________________
>   -------------- 	      -------------- 		 --------------
> >   Firmware 1 |  <- IPC ->  |   Firmware 2 | <- IPC -> |   Firmware 3 |
> >    on HW 1   |	     |    on HW 2   |           |    on HW 3   |
>   -------------- 	      --------------             --------------
> 
> 
> Hence, host just needs to update template of the affected link and 
> indicate to firmware that it is a critical update. This firmware then 
> can indicate other link firmware(s) to append CSA/ECSA IE with a given 
> counter value to its beacon via this IPC.

Sure. You still have a race, because if you send a message over IPC
saying that the CSA needs to be included and it's just before the TBTT,
chances are the TBTT event will still happen without that. So in a sense
it's similar to the host updating the partner link's beacon template
"too late". You can have a situation where the CSA link's template is
updated just before _its_ TBTT, and the partner link's TBTT is just a
little bit later, but the update is delayed ...

You could probably solve that by making your IPC synchronous but then
you risk your TBTT timings in cases like this.

Arguably, I'm not sure it matters. I'm thinking we'll enforce that the
CSA must be in progress when updating the partner links beacon/probe
response templates referring to it (*), but ... ultimately,  I think we
can accept that the partner link updates the CSA one beacon later if
their TBTTs are very close.


(*) and now that I think about it, that might have to immediately come
with a separate template to use _after_ the switch, like CSA does for
the CSA link

>  Parsing the IE and 
> de-fragmenting and fragmenting it again can be done by firmware itself. 
> (Agree that it is bit complex but when comparing with complexity of 
> maintaining synchronicity across links, this looks more doable)

That sync maintenance is because of your hardware design though, others
don't necessarily have that because multiple links are handled by the
same NIC, not separate ones.

> Hence we have taken "offloading beacons fully to firmware" approach.

Sure, fair enough.

> For probe responses, it is handled in host/kernel only. Firmware sends 
> back the last transmitted count in beacon to host. So we have the last 
> transmitted count info. Per STA profile generation logic is also there. 
> So we manage via that.

So I think like in Realtek's case I'd probably advocate doing that in
the driver with the offsets given by hostapd/software stack, although
that requires having *two* feature flags, one for beacons and one for
probe responses ... since you lack the "magic" for probe responses.

johannes

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Aditya Kumar Singh]

On 2/21/24 18:38, Johannes Berg wrote:
> Hi,
> 
> On Wed, 2024-02-21 at 18:22 +0530, Aditya Kumar Singh wrote:
>> On 2/21/24 13:39, Johannes Berg wrote:
>>> Qualcomm:
>>>    - copies and updates CSA/ECSA elements all by itself
>>>    - btw, not sure here about probe responses, does it do that too?
>>
>> We had a thought about keeping this CSA/ECSA handling at host/kernel
>> level only. But the major point of concern is _synchronization_ among
>> firmware of each of the links participating in the MLD.
> 
> Sure.
> 
>> * Even if we ignore TSF/TBTT synchronization for a moment, how firmware
>> will know when to transmit the beacon with a particular counter or when
>> CSA has finished on other link? If rely on host's update then there is
>> room for further delay and hence errors.
>>        - This is because, counter value on the reported link depends on
>> the last beacon transmitted by the affected link.
> 
> Sure.
> 
> I don't think anyone suggested that the host will put the exact counter
> value there, just to have the template.
> 
>> * Host can send the template on all links
> 
> Right.
> 
>> but how to ensure that first
>> template is reached on the affected link and then only on the partner
>> links? Host will queue the command properly but reaping of the command
>> on n (no of links) independent firmware can not be guaranteed in the
>> same order in which host has filled. It depends how busy each of host to
>> firmware path is.
> 
> True, and there's a potential for race conditions there I suppose, but I
> suppose in the Intel, Realtek and hwsim case at least we wouldn't have
> *different* firmwares running multiple links, but a single one.

oh okay, makes sense.

> 
> In any case, you could solve this even with multiple, by applying the
> new template only after you have the CSA'ing link's new beacon template,
> if it requires filling in CSA counters, or such.
> 
>> * And then obviously, considering TSF/TBTT will be again complicating
>> the synchronization part and making it more difficult to manage just via
>> host.
> 
> Again, not suggesting that it is managed completely by the host, just
> the templates.
> 
>> Hence there is a strong urge to let firmware handle all this for beacons.
> 
> Sure, that's fine, your call :)
> 
>> As far as how firmware will _magically_ communicate among themselves is
>> concerned, we have *IPC* in place to achieve that. One link firmware can
>> talk to other link firmware when required.
> 
> :-)
> 
> It probably doesn't actually help you make it race-free though, so does
> it really matter? But again, it's your call how you want to do it, and
> we'll just have to handle it in software appropriately. While I'd prefer
> to have _one_ way of doing things, at least so far we've basically seen
> one way of having the host involved and ath12k not having the host
> involved, so it's all still really simple.

Yes!

> 
>> 				Kernel Level
>> ____________________________________________________________________________
>>    -------------- 	      -------------- 		 --------------
>>>    Firmware 1 |  <- IPC ->  |   Firmware 2 | <- IPC -> |   Firmware 3 |
>>>     on HW 1   |	     |    on HW 2   |           |    on HW 3   |
>>    -------------- 	      --------------             --------------
>>
>>
>> Hence, host just needs to update template of the affected link and
>> indicate to firmware that it is a critical update. This firmware then
>> can indicate other link firmware(s) to append CSA/ECSA IE with a given
>> counter value to its beacon via this IPC.
> 
> Sure. You still have a race, because if you send a message over IPC
> saying that the CSA needs to be included and it's just before the TBTT,
> chances are the TBTT event will still happen without that. So in a sense
> it's similar to the host updating the partner link's beacon template
> "too late". You can have a situation where the CSA link's template is
> updated just before _its_ TBTT, and the partner link's TBTT is just a
> little bit later, but the update is delayed ...
> 
> You could probably solve that by making your IPC synchronous but then
> you risk your TBTT timings in cases like this.
> 
> Arguably, I'm not sure it matters. I'm thinking we'll enforce that the
> CSA must be in progress when updating the partner links beacon/probe
> response templates referring to it (*), but ... ultimately,  I think we
> can accept that the partner link updates the CSA one beacon later if
> their TBTTs are very close.
> 
> 
> (*) and now that I think about it, that might have to immediately come
> with a separate template to use _after_ the switch, like CSA does for
> the CSA link

True. So each link will have to send two templates. beacon_csa and 
beacon_after

> 
>>   Parsing the IE and
>> de-fragmenting and fragmenting it again can be done by firmware itself.
>> (Agree that it is bit complex but when comparing with complexity of
>> maintaining synchronicity across links, this looks more doable)
> 
> That sync maintenance is because of your hardware design though, others
> don't necessarily have that because multiple links are handled by the
> same NIC, not separate ones.
> 
>> Hence we have taken "offloading beacons fully to firmware" approach.
> 
> Sure, fair enough.
> 
>> For probe responses, it is handled in host/kernel only. Firmware sends
>> back the last transmitted count in beacon to host. So we have the last
>> transmitted count info. Per STA profile generation logic is also there.
>> So we manage via that.
> 
> So I think like in Realtek's case I'd probably advocate doing that in
> the driver with the offsets given by hostapd/software stack, although
> that requires having *two* feature flags, one for beacons and one for
> probe responses ... since you lack the "magic" for probe responses.
> 
:) I guess yeah. So this approach we discussed is fine right?

Have some _feature flag_ to indicate whether host would like templates 
(with offsets) for reporting links or not. If it does then hostapd can 
send those, otherwise it won't. Hosts can further handle as per their need.

Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis

[Michael-cy Lee (李峻宇)]

On Wed, 2024-02-21 at 09:09 +0100, Johannes Berg wrote:
> 
> I guess that'd also apply to probe responses? Or does it not send
> those
> at all? But we discussed before that maybe we don't have to be
> perfect
> there, so I guess we can find some solution to that.
> 
> So we have, so far
> 
> Realtek:
>  - uses offset in beacon template to set partner counter,
>  - requires host to set CSA/ECSA elements
> 
> Qualcomm:
>  - copies and updates CSA/ECSA elements all by itself
>  - btw, not sure here about probe responses, does it do that too?
> 
> hwsim:
>  - personally, I'd prefer it works like Realtek, for less complexity
> 
> Intel:
>  - not implemented, probably not going to happen any time soon, but I
>    think we might prefer the Realtek way too if we ever do this
> 
> 
> Mediatek? Broadcom? Anyone else?
> 
> 
> I guess then for Qualcomm we'll need to just add an extended feature
> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
> update the parter link templates, or so?
> 
> Thanks,

Hi,
 
Sorry for the late reply.
 
In summary, MediaTek firmware behaves similarly to Realtek's firmware,
so we agree on the conclusion of this thread.
 
By the way, this behavior does not apply to probe response in MediaTek
firmware. In other words, MediaTek firmware does not modify the probe
response and it requires hostapd to fill all CSA-related Elements.
I think this could be fixed if hostapd provides CSA offsets when
sending the probe response.
 
Best,
Michael
> johannes

[PATCH v8 5/5] wifi: mac80211: add support to call csa_finish on a link

[Aditya Kumar Singh]

Currently ieee80211_csa_finish() function finalizes CSA by scheduling a
finalizing worker using the deflink. With MLO, there is a need to do it
on a given link basis.

Pass link ID of the link on which CSA needs to be finalized.

Signed-off-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
---
 drivers/net/wireless/ath/ath10k/mac.c             |  2 +-
 drivers/net/wireless/ath/ath10k/wmi.c             |  2 +-
 drivers/net/wireless/ath/ath11k/wmi.c             |  2 +-
 drivers/net/wireless/ath/ath12k/wmi.c             |  2 +-
 drivers/net/wireless/ath/ath9k/beacon.c           |  2 +-
 drivers/net/wireless/ath/ath9k/htc_drv_beacon.c   |  2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c |  4 ++--
 .../net/wireless/intel/iwlwifi/mvm/time-event.c   |  2 +-
 drivers/net/wireless/mediatek/mt76/mac80211.c     |  2 +-
 drivers/net/wireless/mediatek/mt76/mt7615/mcu.c   |  2 +-
 drivers/net/wireless/mediatek/mt76/mt7915/mcu.c   |  2 +-
 drivers/net/wireless/mediatek/mt76/mt7996/mcu.c   |  2 +-
 .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c |  2 +-
 drivers/net/wireless/ti/wlcore/event.c            |  2 +-
 drivers/net/wireless/virtual/mac80211_hwsim.c     |  2 +-
 include/net/mac80211.h                            |  3 ++-
 net/mac80211/cfg.c                                | 15 +++++++++++++--
 17 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index e32e8c70986d..8847cfc6030e 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2047,7 +2047,7 @@ static void ath10k_mac_vif_ap_csa_count_down(struct ath10k_vif *arvif)
 			ath10k_warn(ar, "failed to update prb tmpl during csa: %d\n",
 				    ret);
 	} else {
-		ieee80211_csa_finish(vif);
+		ieee80211_csa_finish(vif, 0);
 	}
 }
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 88befe92f95d..7ba3ef805fc5 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -3885,7 +3885,7 @@ void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb)
 		 */
 		if (arvif->vif->bss_conf.csa_active &&
 		    ieee80211_beacon_cntdwn_is_complete(arvif->vif)) {
-			ieee80211_csa_finish(arvif->vif);
+			ieee80211_csa_finish(arvif->vif, 0);
 			continue;
 		}
 
diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
index 8a65fa04b48d..3944a4d2e4d2 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.c
+++ b/drivers/net/wireless/ath/ath11k/wmi.c
@@ -8267,7 +8267,7 @@ ath11k_wmi_process_csa_switch_count_event(struct ath11k_base *ab,
 		}
 
 		if (arvif->is_up && arvif->vif->bss_conf.csa_active)
-			ieee80211_csa_finish(arvif->vif);
+			ieee80211_csa_finish(arvif->vif, 0);
 	}
 	rcu_read_unlock();
 }
diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
index 11cc3005c0f9..d505aa8c2225 100644
--- a/drivers/net/wireless/ath/ath12k/wmi.c
+++ b/drivers/net/wireless/ath/ath12k/wmi.c
@@ -6446,7 +6446,7 @@ ath12k_wmi_process_csa_switch_count_event(struct ath12k_base *ab,
 		}
 
 		if (arvif->is_up && arvif->vif->bss_conf.csa_active)
-			ieee80211_csa_finish(arvif->vif);
+			ieee80211_csa_finish(arvif->vif, 0);
 	}
 	rcu_read_unlock();
 }
diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index ee72faac2f1d..4e48407138b2 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -368,7 +368,7 @@ bool ath9k_csa_is_finished(struct ath_softc *sc, struct ieee80211_vif *vif)
 	if (!ieee80211_beacon_cntdwn_is_complete(vif))
 		return false;
 
-	ieee80211_csa_finish(vif);
+	ieee80211_csa_finish(vif, 0);
 	return true;
 }
 
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c b/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c
index 533471e69400..8179d35dc310 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c
@@ -517,7 +517,7 @@ bool ath9k_htc_csa_is_finished(struct ath9k_htc_priv *priv)
 	if (!ieee80211_beacon_cntdwn_is_complete(vif))
 		return false;
 
-	ieee80211_csa_finish(vif);
+	ieee80211_csa_finish(vif, 0);
 
 	priv->csa_vif = NULL;
 	return true;
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
index 581c2d3fc534..57a94ffb12d7 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c
@@ -1486,7 +1486,7 @@ static void iwl_mvm_csa_count_down(struct iwl_mvm *mvm,
 		}
 	} else if (!iwl_mvm_te_scheduled(&mvmvif->time_event_data)) {
 		/* we don't have CSA NoA scheduled yet, switch now */
-		ieee80211_csa_finish(csa_vif);
+		ieee80211_csa_finish(csa_vif, 0);
 		RCU_INIT_POINTER(mvm->csa_vif, NULL);
 	}
 }
@@ -1832,7 +1832,7 @@ void iwl_mvm_channel_switch_start_notif(struct iwl_mvm *mvm,
 				      msecs_to_jiffies(IWL_MVM_CS_UNBLOCK_TX_TIMEOUT *
 						       csa_vif->bss_conf.beacon_int));
 
-		ieee80211_csa_finish(csa_vif);
+		ieee80211_csa_finish(csa_vif, 0);
 
 		rcu_read_unlock();
 
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/time-event.c b/drivers/net/wireless/intel/iwlwifi/mvm/time-event.c
index aceab96bcb97..885d22ca6f2d 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/time-event.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/time-event.c
@@ -168,7 +168,7 @@ static void iwl_mvm_csa_noa_start(struct iwl_mvm *mvm)
 		goto out_unlock;
 	}
 
-	ieee80211_csa_finish(csa_vif);
+	ieee80211_csa_finish(csa_vif, 0);
 
 	rcu_read_unlock();
 
diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
index 8a3a90d1bfac..8bf82755ca4c 100644
--- a/drivers/net/wireless/mediatek/mt76/mac80211.c
+++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
@@ -1614,7 +1614,7 @@ static void
 __mt76_csa_finish(void *priv, u8 *mac, struct ieee80211_vif *vif)
 {
 	if (vif->bss_conf.csa_active && ieee80211_beacon_cntdwn_is_complete(vif))
-		ieee80211_csa_finish(vif);
+		ieee80211_csa_finish(vif, 0);
 }
 
 void mt76_csa_finish(struct mt76_dev *dev)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index ae34d019e588..c807bd8d928d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -353,7 +353,7 @@ static void
 mt7615_mcu_csa_finish(void *priv, u8 *mac, struct ieee80211_vif *vif)
 {
 	if (vif->bss_conf.csa_active)
-		ieee80211_csa_finish(vif);
+		ieee80211_csa_finish(vif, 0);
 }
 
 static void
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index c67c4f6ca2aa..f1522c7f0597 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -228,7 +228,7 @@ mt7915_mcu_csa_finish(void *priv, u8 *mac, struct ieee80211_vif *vif)
 	if (!vif->bss_conf.csa_active || vif->type == NL80211_IFTYPE_STATION)
 		return;
 
-	ieee80211_csa_finish(vif);
+	ieee80211_csa_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 3c729b563edc..0eeaf428179b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
@@ -341,7 +341,7 @@ mt7996_mcu_csa_finish(void *priv, u8 *mac, struct ieee80211_vif *vif)
 	if (!vif->bss_conf.csa_active || vif->type == NL80211_IFTYPE_STATION)
 		return;
 
-	ieee80211_csa_finish(vif);
+	ieee80211_csa_finish(vif, 0);
 }
 
 static void
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 3b954c2fe448..6b01876e23f3 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -5737,7 +5737,7 @@ static void rtl8xxxu_update_beacon_work_callback(struct work_struct *work)
 
 	if (vif->bss_conf.csa_active) {
 		if (ieee80211_beacon_cntdwn_is_complete(vif)) {
-			ieee80211_csa_finish(vif);
+			ieee80211_csa_finish(vif, 0);
 			return;
 		}
 		schedule_delayed_work(&priv->update_beacon_work,
diff --git a/drivers/net/wireless/ti/wlcore/event.c b/drivers/net/wireless/ti/wlcore/event.c
index 1e082d039b82..2499dc908305 100644
--- a/drivers/net/wireless/ti/wlcore/event.c
+++ b/drivers/net/wireless/ti/wlcore/event.c
@@ -233,7 +233,7 @@ void wlcore_event_channel_switch(struct wl1271 *wl,
 			cancel_delayed_work(&wlvif->channel_switch_work);
 		} else {
 			set_bit(WLVIF_FLAG_BEACON_DISABLED, &wlvif->flags);
-			ieee80211_csa_finish(vif);
+			ieee80211_csa_finish(vif, 0);
 		}
 	}
 }
diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c
index 403d892c0468..0a7664c0a672 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
@@ -2285,7 +2285,7 @@ static void mac80211_hwsim_beacon_tx(void *arg, u8 *mac,
 	}
 
 	if (link_conf->csa_active && ieee80211_beacon_cntdwn_is_complete(vif))
-		ieee80211_csa_finish(vif);
+		ieee80211_csa_finish(vif, link_id);
 }
 
 static enum hrtimer_restart
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ad70dea96a46..9ab79bedde3d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5536,12 +5536,13 @@ void ieee80211_beacon_set_cntdwn(struct ieee80211_vif *vif, u8 counter);
 /**
  * ieee80211_csa_finish - notify mac80211 about channel switch
  * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @link_id: valid link_id during MLO or 0 for non-MLO
  *
  * After a channel switch announcement was scheduled and the counter in this
  * announcement hits 1, this function must be called by the driver to
  * notify mac80211 that the channel can be changed.
  */
-void ieee80211_csa_finish(struct ieee80211_vif *vif);
+void ieee80211_csa_finish(struct ieee80211_vif *vif, unsigned int link_id);
 
 /**
  * ieee80211_beacon_cntdwn_is_complete - find out if countdown reached 1
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 4427259154e2..d3c637f12e26 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3543,13 +3543,24 @@ cfg80211_beacon_dup(struct cfg80211_beacon_data *beacon)
 	return new_beacon;
 }
 
-void ieee80211_csa_finish(struct ieee80211_vif *vif)
+void ieee80211_csa_finish(struct ieee80211_vif *vif, unsigned int link_id)
 {
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
 	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_link_data *link_data;
+
+	if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
+		return;
 
 	rcu_read_lock();
 
+	link_data = rcu_dereference(sdata->link[link_id]);
+	if (WARN_ON(!link_data)) {
+		rcu_read_unlock();
+		return;
+	}
+
+	/* TODO: MBSSID with MLO changes */
 	if (vif->mbssid_tx_vif == vif) {
 		/* Trigger ieee80211_csa_finish() on the non-transmitting
 		 * interfaces when channel switch is received on
@@ -3568,7 +3579,7 @@ void ieee80211_csa_finish(struct ieee80211_vif *vif)
 					 &iter->deflink.csa_finalize_work);
 		}
 	}
-	wiphy_work_queue(local->hw.wiphy, &sdata->deflink.csa_finalize_work);
+	wiphy_work_queue(local->hw.wiphy, &link_data->csa_finalize_work);
 
 	rcu_read_unlock();
 }
-- 
2.25.1