| Message ID | 20240215162811.506065-2-quic_adisi@quicinc.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: mac80211/mac80211_hwsim: support MLO CSA test case | expand |
> -bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif) > +bool ieee80211_beacon_cntdwn_is_complete(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 *beacon_data; > size_t beacon_data_len; > @@ -5106,9 +5108,17 @@ bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif) > if (!ieee80211_sdata_running(sdata)) > return false; > > + 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 out; > Maybe that should be a warning too? Not sure I see any case where the driver can/should call it with a link that's not even there? Oh ... and maybe it should check if the link is active? We had the sdata_running() check before, but that doesn't mean much for MLO? Though then again we have the check below anyway: > if (vif->type == NL80211_IFTYPE_AP) { > - beacon = rcu_dereference(sdata->deflink.u.ap.beacon); > + beacon = rcu_dereference(link->u.ap.beacon); > if (WARN_ON(!beacon || !beacon->tail)) > goto out; > So that will just be NULL if it's not active... so I guess that's fine. johannes
On 2/16/24 01:18, Johannes Berg wrote: > >> -bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif) >> +bool ieee80211_beacon_cntdwn_is_complete(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 *beacon_data; >> size_t beacon_data_len; >> @@ -5106,9 +5108,17 @@ bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif) >> if (!ieee80211_sdata_running(sdata)) >> return false; >> >> + 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 out; >> > > Maybe that should be a warning too? Not sure I see any case where the > driver can/should call it with a link that's not even there? > Yes even I don't see any case like that. > Oh ... and maybe it should check if the link is active? We had the > sdata_running() check before, but that doesn't mean much for MLO? > Correct. But at least in this function I don't see much use of checking if link is active. > Though then again we have the check below anyway: > >> if (vif->type == NL80211_IFTYPE_AP) { >> - beacon = rcu_dereference(sdata->deflink.u.ap.beacon); >> + beacon = rcu_dereference(link->u.ap.beacon); >> if (WARN_ON(!beacon || !beacon->tail)) >> goto out; >> > > So that will just be NULL if it's not active... so I guess that's fine. > Yep. That's the intention here. Ultimately beacon would be NULL if link is not active so eventually a WARN_ON() will trigger.
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 41053219ee95..e322b528baaf 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -2034,7 +2034,7 @@ static void ath10k_mac_vif_ap_csa_count_down(struct ath10k_vif *arvif) if (!arvif->is_up) return; - if (!ieee80211_beacon_cntdwn_is_complete(vif)) { + if (!ieee80211_beacon_cntdwn_is_complete(vif, 0)) { ieee80211_beacon_update_cntdwn(vif, 0); ret = ath10k_mac_setup_bcn_tmpl(arvif); diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c index ddf15717d504..2e9661f4bea8 100644 --- a/drivers/net/wireless/ath/ath10k/wmi.c +++ b/drivers/net/wireless/ath/ath10k/wmi.c @@ -3884,7 +3884,7 @@ void ath10k_wmi_event_host_swba(struct ath10k *ar, struct sk_buff *skb) * actual channel switch is done */ if (arvif->vif->bss_conf.csa_active && - ieee80211_beacon_cntdwn_is_complete(arvif->vif)) { + ieee80211_beacon_cntdwn_is_complete(arvif->vif, 0)) { ieee80211_csa_finish(arvif->vif, 0); continue; } diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index f7cab50bdfd1..a1e2729582e8 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -1577,7 +1577,7 @@ void ath11k_mac_bcn_tx_event(struct ath11k_vif *arvif) return; if (vif->bss_conf.color_change_active && - ieee80211_beacon_cntdwn_is_complete(vif)) { + ieee80211_beacon_cntdwn_is_complete(vif, 0)) { arvif->bcca_zero_sent = true; ieee80211_color_change_finish(vif); return; diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c index 4e48407138b2..b399a7926ef5 100644 --- a/drivers/net/wireless/ath/ath9k/beacon.c +++ b/drivers/net/wireless/ath/ath9k/beacon.c @@ -365,7 +365,7 @@ bool ath9k_csa_is_finished(struct ath_softc *sc, struct ieee80211_vif *vif) if (!vif || !vif->bss_conf.csa_active) return false; - if (!ieee80211_beacon_cntdwn_is_complete(vif)) + if (!ieee80211_beacon_cntdwn_is_complete(vif, 0)) return false; ieee80211_csa_finish(vif, 0); diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c b/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c index 8179d35dc310..547634f82183 100644 --- a/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c +++ b/drivers/net/wireless/ath/ath9k/htc_drv_beacon.c @@ -514,7 +514,7 @@ bool ath9k_htc_csa_is_finished(struct ath9k_htc_priv *priv) if (!vif || !vif->bss_conf.csa_active) return false; - if (!ieee80211_beacon_cntdwn_is_complete(vif)) + if (!ieee80211_beacon_cntdwn_is_complete(vif, 0)) return false; ieee80211_csa_finish(vif, 0); diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c index cc592e288188..123fe9bba982 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac-ctxt.c @@ -1466,7 +1466,7 @@ static void iwl_mvm_csa_count_down(struct iwl_mvm *mvm, mvmvif->csa_countdown = true; - if (!ieee80211_beacon_cntdwn_is_complete(csa_vif)) { + if (!ieee80211_beacon_cntdwn_is_complete(csa_vif, 0)) { int c = ieee80211_beacon_update_cntdwn(csa_vif, 0); iwl_mvm_mac_ctxt_beacon_changed(mvm, csa_vif, diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/time-event.c b/drivers/net/wireless/intel/iwlwifi/mvm/time-event.c index 89b1c7a87660..a59d264a11c5 100644 --- a/drivers/net/wireless/intel/iwlwifi/mvm/time-event.c +++ b/drivers/net/wireless/intel/iwlwifi/mvm/time-event.c @@ -156,7 +156,7 @@ static void iwl_mvm_csa_noa_start(struct iwl_mvm *mvm) * So we just do nothing here and the switch * will be performed on the last TBTT. */ - if (!ieee80211_beacon_cntdwn_is_complete(csa_vif)) { + if (!ieee80211_beacon_cntdwn_is_complete(csa_vif, 0)) { IWL_WARN(mvm, "CSA NOA started too early\n"); goto out_unlock; } diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c index 8bf82755ca4c..758e380fdf1d 100644 --- a/drivers/net/wireless/mediatek/mt76/mac80211.c +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c @@ -1613,7 +1613,7 @@ EXPORT_SYMBOL_GPL(mt76_get_sar_power); 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)) + if (vif->bss_conf.csa_active && ieee80211_beacon_cntdwn_is_complete(vif, 0)) ieee80211_csa_finish(vif, 0); } @@ -1638,7 +1638,7 @@ __mt76_csa_check(void *priv, u8 *mac, struct ieee80211_vif *vif) if (!vif->bss_conf.csa_active) return; - dev->csa_complete |= ieee80211_beacon_cntdwn_is_complete(vif); + dev->csa_complete |= ieee80211_beacon_cntdwn_is_complete(vif, 0); } void mt76_csa_check(struct mt76_dev *dev) diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c index 66bf92c164c3..db5041d23938 100644 --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c @@ -5739,7 +5739,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)) { + if (ieee80211_beacon_cntdwn_is_complete(vif, 0)) { ieee80211_csa_finish(vif, 0); return; } diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c index 0554946ed30e..2ea11a86d920 100644 --- a/drivers/net/wireless/virtual/mac80211_hwsim.c +++ b/drivers/net/wireless/virtual/mac80211_hwsim.c @@ -2305,7 +2305,7 @@ static void mac80211_hwsim_beacon_tx(void *arg, u8 *mac, rcu_dereference(link_conf->chanctx_conf)->def.chan); } - if (link_conf->csa_active && ieee80211_beacon_cntdwn_is_complete(vif)) + if (link_conf->csa_active && ieee80211_beacon_cntdwn_is_complete(vif, link_id)) ieee80211_csa_finish(vif, link_id); } diff --git a/include/net/mac80211.h b/include/net/mac80211.h index fc223761e3af..25c892ea9eb3 100644 --- a/include/net/mac80211.h +++ b/include/net/mac80211.h @@ -5566,10 +5566,12 @@ void ieee80211_csa_finish(struct ieee80211_vif *vif, unsigned int link_id); /** * ieee80211_beacon_cntdwn_is_complete - find out if countdown reached 1 * @vif: &struct ieee80211_vif pointer from the add_interface callback. + * @link_id: valid link_id during MLO or 0 for non-MLO * * This function returns whether the countdown reached zero. */ -bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif); +bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif, + unsigned int link_id); /** * ieee80211_color_change_finish - notify mac80211 about color change diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index f4be4af568be..6bf223e6cd1a 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -5095,9 +5095,11 @@ void ieee80211_beacon_set_cntdwn(struct ieee80211_vif *vif, u8 counter) } EXPORT_SYMBOL(ieee80211_beacon_set_cntdwn); -bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif) +bool ieee80211_beacon_cntdwn_is_complete(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 *beacon_data; size_t beacon_data_len; @@ -5106,9 +5108,17 @@ bool ieee80211_beacon_cntdwn_is_complete(struct ieee80211_vif *vif) if (!ieee80211_sdata_running(sdata)) return false; + 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 out; + if (vif->type == NL80211_IFTYPE_AP) { - beacon = rcu_dereference(sdata->deflink.u.ap.beacon); + beacon = rcu_dereference(link->u.ap.beacon); if (WARN_ON(!beacon || !beacon->tail)) goto out; beacon_data = beacon->tail;
Currently, function to check if beacon countdown is complete uses deflink to fetch the beacon and check the counter. However, with MLO, there is a need to check 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. 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/mac.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 | 2 +- .../net/wireless/intel/iwlwifi/mvm/time-event.c | 2 +- drivers/net/wireless/mediatek/mt76/mac80211.c | 4 ++-- .../net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +- drivers/net/wireless/virtual/mac80211_hwsim.c | 2 +- include/net/mac80211.h | 4 +++- net/mac80211/tx.c | 14 ++++++++++++-- 12 files changed, 26 insertions(+), 14 deletions(-)