Currently whenever sta_flush() function is called, it flushes all stations
connected to the given interface. However in case of MLO, all the links
would be using the same interface and hence at certain cases flushing all
stations is not desireable.
There is a need to flush the stations based on link ID. This series aims
to add support for the same.
Currently two cases are handled -
1. During NL80211_CMD_DEL_STATION command handling. If this is called
without any mac address, all stations present on that interfaces are
flushed. More details in the patch [1/3]
2. During stopping link AP via ieee80211_stop_ap(). Again here, all
stations are flushed. More details in the patch [3/3]
Aditya Kumar Singh (3):
wifi: cfg80211: add support for link id attribute in
NL80211_CMD_DEL_STATION
wifi: mac80211: add link id argument for sta_flush() function
wifi: mac80211: remove only own link stations during stop_ap
---
include/net/cfg80211.h | 3 +++
include/uapi/linux/nl80211.h | 3 ++-
net/mac80211/cfg.c | 4 ++--
net/mac80211/ibss.c | 4 ++--
net/mac80211/iface.c | 2 +-
net/mac80211/mesh.c | 2 +-
net/mac80211/mlme.c | 2 +-
net/mac80211/ocb.c | 2 +-
net/mac80211/sta_info.c | 20 +++++++++++++-------
net/mac80211/sta_info.h | 14 +++++++++++---
net/wireless/nl80211.c | 18 +++++++++++++++++-
net/wireless/trace.h | 7 +++++--
12 files changed, 59 insertions(+), 22 deletions(-)
base-commit: acf868ff60b1cd1f2e597f0b15aee2ff43f9fcd3
--
2.25.1
Currently whenever NL80211_CMD_DEL_STATION command is called without any
MAC address, all stations present on that interface are flushed.
However with MLO there is a need to flush the stations from a particular
link in the interface, and not from all the links associated with the MLD
interface.
For example - 2 GHz and 5 GHz are part of an AP MLD. When 2 GHz BSS is
brought up, it sends flush command on the interface (MLD). Then eventually
5 GHZ links comes up and that also sends the command on the same interface.
Now by the time 5 GHz link comes up, if any station gets connected to 2 GHz
link, it would be flushed while 5 GHz link is started which is wrong.
Hence, add an option to pass link ID as well in the command so that if link
ID is passed, station using that passed link ID alone would be deleted
and others will not be removed.
A subsequent patch would add logic to delete only the station using the
passed link ID.
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
include/net/cfg80211.h | 3 +++
include/uapi/linux/nl80211.h | 3 ++-
net/wireless/nl80211.c | 18 +++++++++++++++++-
net/wireless/trace.h | 7 +++++--
4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index cf79656ce09c..2e194638717d 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1766,11 +1766,14 @@ struct station_parameters {
* @subtype: Management frame subtype to use for indicating removal
* (10 = Disassociation, 12 = Deauthentication)
* @reason_code: Reason code for the Disassociation/Deauthentication frame
+ * @link_id: Link ID on which station should be connected at least in order
+ * to delete its entry. Valid only during MLO.
*/
struct station_del_parameters {
const u8 *mac;
u8 subtype;
u16 reason_code;
+ int link_id;
};
/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 1ccdcae24372..4197d7097591 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -438,7 +438,8 @@
* %NL80211_ATTR_REASON_CODE can optionally be used to specify which type
* of disconnection indication should be sent to the station
* (Deauthentication or Disassociation frame and reason code for that
- * frame).
+ * frame). %NL80211_ATTR_MLO_LINK_ID can be used optionally to remove
+ * stations connected and using at least that link.
*
* @NL80211_CMD_GET_MPATH: Get mesh path attributes for mesh path to
* destination %NL80211_ATTR_MAC on the interface identified by
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 60877b532993..a99537828978 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7627,14 +7627,16 @@ static int nl80211_del_station(struct sk_buff *skb, struct genl_info *info)
{
struct cfg80211_registered_device *rdev = info->user_ptr[0];
struct net_device *dev = info->user_ptr[1];
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
struct station_del_parameters params;
+ int link_id = nl80211_link_id_or_invalid(info->attrs);
memset(¶ms, 0, sizeof(params));
if (info->attrs[NL80211_ATTR_MAC])
params.mac = nla_data(info->attrs[NL80211_ATTR_MAC]);
- switch (dev->ieee80211_ptr->iftype) {
+ switch (wdev->iftype) {
case NL80211_IFTYPE_AP:
case NL80211_IFTYPE_AP_VLAN:
case NL80211_IFTYPE_MESH_POINT:
@@ -7675,6 +7677,17 @@ static int nl80211_del_station(struct sk_buff *skb, struct genl_info *info)
params.reason_code = WLAN_REASON_PREV_AUTH_NOT_VALID;
}
+ /* Link ID not expected in case of non-ML operation */
+ if (!wdev->valid_links && link_id != -1)
+ return -EINVAL;
+
+ /* If given, a valid link ID should be passed during MLO */
+ if (wdev->valid_links && link_id >= 0 &&
+ !(wdev->valid_links & BIT(link_id)))
+ return -EINVAL;
+
+ params.link_id = link_id;
+
return rdev_del_station(rdev, dev, ¶ms);
}
@@ -16827,6 +16840,9 @@ static const struct genl_small_ops nl80211_small_ops[] = {
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = nl80211_del_station,
.flags = GENL_UNS_ADMIN_PERM,
+ /* cannot use NL80211_FLAG_MLO_VALID_LINK_ID, depends on
+ * MAC address
+ */
.internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
},
{
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 1f374c8a17a5..838107186b91 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -859,6 +859,7 @@ DECLARE_EVENT_CLASS(station_del,
MAC_ENTRY(sta_mac)
__field(u8, subtype)
__field(u16, reason_code)
+ __field(int, link_id)
),
TP_fast_assign(
WIPHY_ASSIGN;
@@ -866,11 +867,13 @@ DECLARE_EVENT_CLASS(station_del,
MAC_ASSIGN(sta_mac, params->mac);
__entry->subtype = params->subtype;
__entry->reason_code = params->reason_code;
+ __entry->link_id = params->link_id;
),
TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", station mac: %pM"
- ", subtype: %u, reason_code: %u",
+ ", subtype: %u, reason_code: %u, link_id: %d",
WIPHY_PR_ARG, NETDEV_PR_ARG, __entry->sta_mac,
- __entry->subtype, __entry->reason_code)
+ __entry->subtype, __entry->reason_code,
+ __entry->link_id)
);
DEFINE_EVENT(station_del, rdev_del_station,
--
2.25.1
Whenever sta_flush() function is invoked, all sta present in that interface
is flushed. However, there is a requirement in case of MLO that only flush
those sta who are using a given link id. This is not possible currently.
Hence add the support for the same.
Passing link id is done currently via cfg80211 op - del_station.
A subsequent patch would also use this during stopping AP.
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
net/mac80211/cfg.c | 4 ++--
net/mac80211/ibss.c | 4 ++--
net/mac80211/iface.c | 2 +-
net/mac80211/mesh.c | 2 +-
net/mac80211/mlme.c | 2 +-
net/mac80211/ocb.c | 2 +-
net/mac80211/sta_info.c | 20 +++++++++++++-------
net/mac80211/sta_info.h | 14 +++++++++++---
8 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 489dd97f5172..26a8ea553401 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1618,7 +1618,7 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
link_conf->ema_ap = false;
link_conf->bssid_indicator = 0;
- __sta_info_flush(sdata, true);
+ __sta_info_flush(sdata, true, -1);
ieee80211_free_keys(sdata, true);
link_conf->enable_beacon = false;
@@ -2093,7 +2093,7 @@ static int ieee80211_del_station(struct wiphy *wiphy, struct net_device *dev,
if (params->mac)
return sta_info_destroy_addr_bss(sdata, params->mac);
- sta_info_flush(sdata);
+ sta_info_flush(sdata, params->link_id);
return 0;
}
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 8f2b445a5ec3..e4c55760f90c 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -237,7 +237,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
drv_reset_tsf(local, sdata);
if (!ether_addr_equal(ifibss->bssid, bssid))
- sta_info_flush(sdata);
+ sta_info_flush(sdata, -1);
/* if merging, indicate to driver that we leave the old IBSS */
if (sdata->vif.cfg.ibss_joined) {
@@ -682,7 +682,7 @@ static void ieee80211_ibss_disconnect(struct ieee80211_sub_if_data *sdata)
ifibss->state = IEEE80211_IBSS_MLME_SEARCH;
- sta_info_flush(sdata);
+ sta_info_flush(sdata, -1);
spin_lock_bh(&ifibss->incomplete_lock);
while (!list_empty(&ifibss->incomplete_stations)) {
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index e4e7c0b38cb6..97728bb51f9b 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -511,7 +511,7 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_do
* would have removed them, but in other modes there shouldn't
* be any stations.
*/
- flushed = sta_info_flush(sdata);
+ flushed = sta_info_flush(sdata, -1);
WARN_ON_ONCE(sdata->vif.type != NL80211_IFTYPE_AP_VLAN && flushed > 0);
/* don't count this interface for allmulti while it is down */
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index fccbcde3359a..16b5af8ee859 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1234,7 +1234,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
netif_carrier_off(sdata->dev);
/* flush STAs and mpaths on this iface */
- sta_info_flush(sdata);
+ sta_info_flush(sdata, -1);
ieee80211_free_keys(sdata, true);
mesh_path_flush_by_iface(sdata);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 073105deb424..b3f2de3b1669 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2991,7 +2991,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
sdata->vif.cfg.ssid_len = 0;
/* remove AP and TDLS peers */
- sta_info_flush(sdata);
+ sta_info_flush(sdata, -1);
/* finally reset all BSS / config parameters */
if (!ieee80211_vif_is_mld(&sdata->vif))
diff --git a/net/mac80211/ocb.c b/net/mac80211/ocb.c
index 449af4e1cca4..7d06166f63bb 100644
--- a/net/mac80211/ocb.c
+++ b/net/mac80211/ocb.c
@@ -207,7 +207,7 @@ int ieee80211_ocb_leave(struct ieee80211_sub_if_data *sdata)
lockdep_assert_wiphy(sdata->local->hw.wiphy);
ifocb->joined = false;
- sta_info_flush(sdata);
+ sta_info_flush(sdata, -1);
spin_lock_bh(&ifocb->incomplete_lock);
while (!list_empty(&ifocb->incomplete_stations)) {
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index bf1adcd96b41..6c9aa8db3c62 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1561,7 +1561,8 @@ void sta_info_stop(struct ieee80211_local *local)
}
-int __sta_info_flush(struct ieee80211_sub_if_data *sdata, bool vlans)
+int __sta_info_flush(struct ieee80211_sub_if_data *sdata, bool vlans,
+ int link_id)
{
struct ieee80211_local *local = sdata->local;
struct sta_info *sta, *tmp;
@@ -1575,12 +1576,17 @@ int __sta_info_flush(struct ieee80211_sub_if_data *sdata, bool vlans)
WARN_ON(vlans && !sdata->bss);
list_for_each_entry_safe(sta, tmp, &local->sta_list, list) {
- if (sdata == sta->sdata ||
- (vlans && sdata->bss == sta->sdata->bss)) {
- if (!WARN_ON(__sta_info_destroy_part1(sta)))
- list_add(&sta->free_list, &free_list);
- ret++;
- }
+ if (sdata != sta->sdata &&
+ (!vlans || sdata->bss != sta->sdata->bss))
+ continue;
+
+ if (link_id >= 0 && !(sta->sta.valid_links & BIT(link_id)))
+ continue;
+
+ if (!WARN_ON(__sta_info_destroy_part1(sta)))
+ list_add(&sta->free_list, &free_list);
+
+ ret++;
}
if (!list_empty(&free_list)) {
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 5ef1554f991f..f03731a5bbee 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -886,8 +886,12 @@ void sta_info_stop(struct ieee80211_local *local);
*
* @sdata: sdata to remove all stations from
* @vlans: if the given interface is an AP interface, also flush VLANs
+ * @link_id: if given (>=0), all those STA entries using @link_id only
+ * will be removed. If -1 is passed, all STA entries will be
+ * removed.
*/
-int __sta_info_flush(struct ieee80211_sub_if_data *sdata, bool vlans);
+int __sta_info_flush(struct ieee80211_sub_if_data *sdata, bool vlans,
+ int link_id);
/**
* sta_info_flush - flush matching STA entries from the STA table
@@ -895,10 +899,14 @@ int __sta_info_flush(struct ieee80211_sub_if_data *sdata, bool vlans);
* Returns the number of removed STA entries.
*
* @sdata: sdata to remove all stations from
+ * @link_id: if given (>=0), all those STA entries using @link_id only
+ * will be removed. If -1 is passed, all STA entries will be
+ * removed.
*/
-static inline int sta_info_flush(struct ieee80211_sub_if_data *sdata)
+static inline int sta_info_flush(struct ieee80211_sub_if_data *sdata,
+ int link_id)
{
- return __sta_info_flush(sdata, false);
+ return __sta_info_flush(sdata, false, link_id);
}
void sta_set_rate_info_tx(struct sta_info *sta,
--
2.25.1
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 (3):
wifi: cfg80211: send link id in channel_switch ops
wifi: mac80211: add support for AP channel switch with MLO
wifi: mac80211: update beacon counters per link basis
---
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 | 110 +++++++++++-------
net/mac80211/tx.c | 14 ++-
net/wireless/nl80211.c | 1 +
net/wireless/trace.h | 7 +-
22 files changed, 112 insertions(+), 68 deletions(-)
base-commit: acf868ff60b1cd1f2e597f0b15aee2ff43f9fcd3
--
2.25.1
Currently, during channel switch, no link id information is passed
due to which channel switch is carried on deflink always. 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 <[email protected]>
---
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 cf79656ce09c..098a4f10fdfd 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 is 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 60877b532993..7022e34d0a93 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10277,6 +10277,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, ¶ms);
free:
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 1f374c8a17a5..2af58f5fbf51 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
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
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
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 +-
.../net/wireless/ath/ath9k/htc_drv_beacon.c | 2 +-
.../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c | 4 +-
.../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/mac80211.h | 3 +-
net/mac80211/cfg.c | 110 +++++++++++-------
17 files changed, 86 insertions(+), 59 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 090bcf148d0c..07c5c51ff7b2 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 c4f96125cf33..3b6819f75430 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 218fdf1ed530..b2f9771ae713 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 a84340c2075f..a410ecdca6d1 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 d400fe2e8668..850053ed2366 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5482,12 +5482,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 489dd97f5172..a5d1c7b043db 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1588,6 +1588,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
link->csa_block_tx = false;
}
+ wiphy_work_cancel(wiphy, &link->csa_finalize_work);
+
ieee80211_free_next_beacon(link);
/* turn off carrier for this interface and dependent VLANs */
@@ -3540,13 +3542,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
@@ -3565,7 +3578,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();
}
@@ -3583,20 +3596,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;
@@ -3625,6 +3639,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;
@@ -3646,22 +3661,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, &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;
}
@@ -3689,7 +3703,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);
}
@@ -3714,18 +3729,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(¶ms->beacon_after);
- if (!sdata->deflink.u.ap.next_beacon)
+ if (!link_data->u.ap.next_beacon)
return -ENOMEM;
/*
@@ -3751,7 +3767,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;
}
@@ -3761,11 +3777,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,
¶ms->beacon_csa, &csa,
NULL, changed);
if (err < 0) {
- ieee80211_free_next_beacon(&sdata->deflink);
+ ieee80211_free_next_beacon(link_data);
return err;
}
@@ -3864,7 +3880,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);
@@ -3875,16 +3894,23 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
if (sdata->wdev.cac_started)
return -EBUSY;
- if (cfg80211_chandef_identical(¶ms->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(¶ms->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;
@@ -3908,7 +3934,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
if (err)
goto out;
- err = ieee80211_link_reserve_chanctx(&sdata->deflink, ¶ms->chandef,
+ err = ieee80211_link_reserve_chanctx(link_data, ¶ms->chandef,
chanctx->mode,
params->radar_required);
if (err)
@@ -3917,44 +3943,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, 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, ¶ms->chandef);
} else {
/* if the beacon didn't change, we can finalize immediately */
- ieee80211_csa_finalize(&sdata->deflink);
+ ieee80211_csa_finalize(link_data);
}
out:
--
2.25.1
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 <[email protected]>
---
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 07c5c51ff7b2..8847cfc6030e 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 db241589424d..74e114140343 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 3b6819f75430..57a94ffb12d7 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 850053ed2366..d8e2b5efbba9 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5455,6 +5455,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
@@ -5464,7 +5465,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 314998fdb1a5..aab3fd9895fe 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
Kindly ignore this series. Got clubbed with other series. Will send as
separate one.
On 1/25/24 18:28, Aditya Kumar Singh wrote:
> Currently whenever sta_flush() function is called, it flushes all stations
> connected to the given interface. However in case of MLO, all the links
> would be using the same interface and hence at certain cases flushing all
> stations is not desireable.
>
> There is a need to flush the stations based on link ID. This series aims
> to add support for the same.
>
> Currently two cases are handled -
> 1. During NL80211_CMD_DEL_STATION command handling. If this is called
> without any mac address, all stations present on that interfaces are
> flushed. More details in the patch [1/3]
>
> 2. During stopping link AP via ieee80211_stop_ap(). Again here, all
> stations are flushed. More details in the patch [3/3]
Currently, whenever AP link is brought down via ieee80211_stop_ap()
function, all stations connected to the sdata are flushed. However, in case
of MLO there is a requirement to flush only stations connected to that link
and not all.
For instance - Consider 2 GHz and 5 GHz are AP MLD. Now due to some reason
5 GHz link of this AP is going down (link removal or any other case). All
stations connected, even legacy stations connected to 2 GHz link AP would
also be flushed. Flushing of other link stations is wrong.
Fix this issue by passing self link ID to sta_flush() function. This would
then only remove the stations which are still using the passed link
ID as their link sta. Other stations will not be affected.
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
net/mac80211/cfg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 26a8ea553401..968ceadd88a7 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1618,7 +1618,7 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev,
link_conf->ema_ap = false;
link_conf->bssid_indicator = 0;
- __sta_info_flush(sdata, true, -1);
+ __sta_info_flush(sdata, true, link_id);
ieee80211_free_keys(sdata, true);
link_conf->enable_beacon = false;
--
2.25.1
On Thu, 2024-01-25 at 18:28 +0530, Aditya Kumar Singh wrote:
> Currently whenever NL80211_CMD_DEL_STATION command is called without any
> MAC address, all stations present on that interface are flushed.
True.
> However with MLO there is a need to flush the stations from a particular
> link in the interface, and not from all the links associated with the MLD
> interface.
Fair enough, I can get behind that.
Edit: reading the code - I think I misunderstand that ... you're
actually trying to remove all MLDs ("STATION") that have an active link
on this link? So then maybe disregard all the below, and just write a
better commit message?
But I'll leave the below because I'm not really sure what you're trying
to do here.
> For example - 2 GHz and 5 GHz are part of an AP MLD. When 2 GHz BSS is
> brought up, it sends flush command on the interface (MLD). Then eventually
> 5 GHZ links comes up and that also sends the command on the same interface.
> Now by the time 5 GHz link comes up, if any station gets connected to 2 GHz
> link, it would be flushed while 5 GHz link is started which is wrong.
Right. Though in this case - after bringup - you wouldn't really have to
flush anyway, so it could just not do that, I guess? Feels a bit like a
broken flow which is a bad justification, but I do understand there's
justification for this.
> Hence, add an option to pass link ID as well in the command so that if link
> ID is passed, station using that passed link ID alone would be deleted
> and others will not be removed.
So first: Do you want some feature flag that indicates this? Or will we
just eat the cost of kicking out everyone (without even sending deauth
though, I think?) when running on older kernels?
Secondly: why is this part of NL80211_CMD_DEL_STATION? I'm not convinced
that makes sense. I actually kind of get why you're doing that - it's
easier to retrofit into the existing hostapd, but I don't necessarily
think that the hostap design (problems?) should influence this too much.
IOW, it would feel much more appropriate to have this as part of
NL80211_CMD_REMOVE_LINK_STA? After all, when going to MLD then "STATION"
now represents a "peer MLD", and "LINK_STA" now represents an affiliated
STA. And flushing all affiliated STAs is what you want.
So I think it should be NL80211_CMD_REMOVE_LINK_STA without a
NL80211_ATTR_MLD_ADDR.
> A subsequent patch would add logic to delete only the station using the
> passed link ID.
Not sure I'd say that here - I mean, (1) yeah obviously, otherwise we
won't apply this patch? and (2) it's not related to cfg80211.
> case NL80211_IFTYPE_MESH_POINT:
> @@ -7675,6 +7677,17 @@ static int nl80211_del_station(struct sk_buff *skb, struct genl_info *info)
> params.reason_code = WLAN_REASON_PREV_AUTH_NOT_VALID;
> }
>
> + /* Link ID not expected in case of non-ML operation */
> + if (!wdev->valid_links && link_id != -1)
> + return -EINVAL;
> +
> + /* If given, a valid link ID should be passed during MLO */
> + if (wdev->valid_links && link_id >= 0 &&
> + !(wdev->valid_links & BIT(link_id)))
> + return -EINVAL;
Maybe refactor this with the NL80211_FLAG_MLO_VALID_LINK_ID checks?
> @@ -16827,6 +16840,9 @@ static const struct genl_small_ops nl80211_small_ops[] = {
> .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> .doit = nl80211_del_station,
> .flags = GENL_UNS_ADMIN_PERM,
> + /* cannot use NL80211_FLAG_MLO_VALID_LINK_ID, depends on
> + * MAC address
> + */
> .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
Hmm? How does NL80211_FLAG_MLO_VALID_LINK_ID depend on the MAC address?!
It ... doesn't?
johannes
On Thu, 2024-01-25 at 18:28 +0530, Aditya Kumar Singh wrote:
> Whenever sta_flush() function is invoked, all sta present in that interface
STAs
> is flushed. However, there is a requirement in case of MLO that only flush
are flushed; The sentence starting here could be reworded.
> those sta who are using a given link id.
who -> which/that? They're not people :)
> This is not possible currently.
reorder the words a bit here?
> Hence add the support for the same.
I think (and hope :) ) you can improve the commit messages ... Maybe say
"Add support for this by implementing the new cfg80211 API ..."
> Passing link id is done currently via cfg80211 op - del_station.
The mechanics of this aren't really all _that_ interesting unless
they're really complicated - we can all read the code if we care about
that part.
johannes
On 1/26/24 14:36, Johannes Berg wrote:
> On Thu, 2024-01-25 at 18:28 +0530, Aditya Kumar Singh wrote:
>> Currently whenever NL80211_CMD_DEL_STATION command is called without any
>> MAC address, all stations present on that interface are flushed.
>
> True.
>
>> However with MLO there is a need to flush the stations from a particular
>> link in the interface, and not from all the links associated with the MLD
>> interface.
>
> Fair enough, I can get behind that.
>
> Edit: reading the code - I think I misunderstand that ... you're
> actually trying to remove all MLDs ("STATION") that have an active link
> on this link?
Yes correct. The station might not be MLD station. It could be a legacy
station (non EHT) as well.
> So then maybe disregard all the below, and just write a
> better commit message?
> But I'll leave the below because I'm not really sure what you're trying
> to do here.
>
Sure.
>
>> For example - 2 GHz and 5 GHz are part of an AP MLD. When 2 GHz BSS is
>> brought up, it sends flush command on the interface (MLD). Then eventually
>> 5 GHZ links comes up and that also sends the command on the same interface.
>> Now by the time 5 GHz link comes up, if any station gets connected to 2 GHz
>> link, it would be flushed while 5 GHz link is started which is wrong.
>
> Right. Though in this case - after bringup - you wouldn't really have to
> flush anyway, so it could just not do that, I guess? Feels a bit like a
> broken flow which is a bad justification, but I do understand there's
> justification for this.
>
Correct, for the first bring up not required but one use case I see is -
the hostapd interface was disabled for some reason. While going down, it
would have cleared the stations on the kernel but what if for some
reason kernel did not clear the station entries and there are some stale
entries present? So at next bring up (during enable) it would send the
command without any MAC address to flush all stale entries (probably as
a safety so that kernel and hostapd would now be on par).
>> Hence, add an option to pass link ID as well in the command so that if link
>> ID is passed, station using that passed link ID alone would be deleted
>> and others will not be removed.
>
> So first: Do you want some feature flag that indicates this? Or will we
> just eat the cost of kicking out everyone (without even sending deauth
> though, I think?) when running on older kernels?
>
If what I said above was the actual intention, then kicking out everyone
without even sending deauth makes sense? Yes? If yes then we don't need
a feature flag.
>
> Secondly: why is this part of NL80211_CMD_DEL_STATION? I'm not convinced
> that makes sense. I actually kind of get why you're doing that - it's
> easier to retrofit into the existing hostapd, but I don't necessarily
> think that the hostap design (problems?) should influence this too much.
>
> IOW, it would feel much more appropriate to have this as part of
> NL80211_CMD_REMOVE_LINK_STA? After all, when going to MLD then "STATION"
> now represents a "peer MLD", and "LINK_STA" now represents an affiliated
> STA. And flushing all affiliated STAs is what you want.
>
> So I think it should be NL80211_CMD_REMOVE_LINK_STA without a
> NL80211_ATTR_MLD_ADDR.
>
At least as per the current way of NL80211_CMD_REMOVE_LINK_STA
implementation, it did not made any sense to delete all link STAs if
MLD_ADDR is not passed. So probably the command should be called as many
times as there are active links in the STA?
Still I feel that NL80211_CMD_DEL_STATION is the proper place to put
this? Without the current change also, it used to flush all STAs
whenever MAC address is not passed. With MLO, now we need to flush STAs
only if it is using the given link ID. So that link STAs from other
affiliated links of AP would not be flushed.
Scenario I'm targeting is this -
Pre-MLO
----------------------------
sdata -> 2 GHz AP interface
sta_lists ->
1. sta -> connected 2 GHz AP sdata
2. sta -> connected 2 GHz AP sdata
After NL80211_CMD_DEL_STATION is given without any MAC address,
sta_lists ->
No entry(ies)
With MLO
-----------------------------
sdata ->
link_data -> 2 GHz AP link (link ID 0)
link_data -> 5 GHz AP link (link ID 1)
link_data -> 6 GHz AP link (link ID 2)
sta_lists ->
1. sta -> connected AP MLD sdata
link_sta 0 -> connected to 2 GHz link
2. sta -> connected AP MLD sdata
link_sta 1 -> connected to 5 GHz link
3. sta -> connected AP MLD sdata
link_sta 2 -> connected to 6 GHz link
4. sta -> connected AP MLD sdata
link_sta 0 -> connected to 2 GHz link
link_sta 1 -> connected to 5 GHz link
link_sta 2 -> connected to 6 GHz link
Assume 5 GHz goes down and it gives NL80211_CMD_DEL_STATION without any
MAC address,
sta_lists ->
No entry(ies)
This is not desirable since 5 GHz link went down, why 2/6 GHz STA also
got flushed.
Hence with the proposed change, only sta #2 and #4 would be flushed
since only these two are using passed link ID (which would be 1).
Hence after the command,
sta_lists ->
1. sta -> connected AP MLD sdata
link_sta 0 -> connected to 2 GHz link
3. sta -> connected AP MLD sdata
link_sta 2 -> connected to 6 GHz link
Now, if ML re-config support is present, then hostapd (or the user space
controller for that matters), could first issue
NL80211_CMD_REMOVE_LINK_STA for the MLD STA (#4) and remove link sta
with ID 1 from it. So that when NL80211_CMD_DEL_STATION comes, it would
not remove the 2/6 GHz link STA as well from the MLD STA and hence flush
the whole entry.
The above change is not there yet in hostapd, so for the time being,
whole MLD STA would be flushed.
>> A subsequent patch would add logic to delete only the station using the
>> passed link ID.
>
> Not sure I'd say that here - I mean, (1) yeah obviously, otherwise we
> won't apply this patch? and (2) it's not related to cfg80211.
Sure got it.
>
>> case NL80211_IFTYPE_MESH_POINT:
>> @@ -7675,6 +7677,17 @@ static int nl80211_del_station(struct sk_buff *skb, struct genl_info *info)
>> params.reason_code = WLAN_REASON_PREV_AUTH_NOT_VALID;
>> }
>>
>> + /* Link ID not expected in case of non-ML operation */
>> + if (!wdev->valid_links && link_id != -1)
>> + return -EINVAL;
>> +
>> + /* If given, a valid link ID should be passed during MLO */
>> + if (wdev->valid_links && link_id >= 0 &&
>> + !(wdev->valid_links & BIT(link_id)))
>> + return -EINVAL;
>
> Maybe refactor this with the NL80211_FLAG_MLO_VALID_LINK_ID checks?
See comment below -
>
>> @@ -16827,6 +16840,9 @@ static const struct genl_small_ops nl80211_small_ops[] = {
>> .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
>> .doit = nl80211_del_station,
>> .flags = GENL_UNS_ADMIN_PERM,
>> + /* cannot use NL80211_FLAG_MLO_VALID_LINK_ID, depends on
>> + * MAC address
>> + */
>> .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
>
> Hmm? How does NL80211_FLAG_MLO_VALID_LINK_ID depend on the MAC address?!
> It ... doesn't?
>
I mean intention was that if MAC addresses is passed then no need of
link ID. That is why did not add the valid link flag since it would
expect the link ID even when MAC address is passed.
On 1/26/24 14:41, Johannes Berg wrote:
> On Thu, 2024-01-25 at 18:28 +0530, Aditya Kumar Singh wrote:
>> Whenever sta_flush() function is invoked, all sta present in that interface
>
> STAs
>
>> is flushed. However, there is a requirement in case of MLO that only flush
>
> are flushed; The sentence starting here could be reworded.
>
>> those sta who are using a given link id.
>
> who -> which/that? They're not people :)
>
>> This is not possible currently.
>
> reorder the words a bit here?
>
>> Hence add the support for the same.
>
> I think (and hope :) ) you can improve the commit messages ... Maybe say
> "Add support for this by implementing the new cfg80211 API ..."
>
>> Passing link id is done currently via cfg80211 op - del_station.
>
> The mechanics of this aren't really all _that_ interesting unless
> they're really complicated - we can all read the code if we care about
> that part.
>
> johannes
Sure got it. Thanks for all suggestion. Will address them in next version.
On Sat, 2024-01-27 at 11:14 +0530, Aditya Kumar Singh wrote:
> On 1/26/24 14:36, Johannes Berg wrote:
> > On Thu, 2024-01-25 at 18:28 +0530, Aditya Kumar Singh wrote:
> > > Currently whenever NL80211_CMD_DEL_STATION command is called without any
> > > MAC address, all stations present on that interface are flushed.
> >
> > True.
> >
> > > However with MLO there is a need to flush the stations from a particular
> > > link in the interface, and not from all the links associated with the MLD
> > > interface.
> >
> > Fair enough, I can get behind that.
> >
> > Edit: reading the code - I think I misunderstand that ... you're
> > actually trying to remove all MLDs ("STATION") that have an active link
> > on this link?
>
> Yes correct. The station might not be MLD station. It could be a legacy
> station (non EHT) as well.
We pretty much treat that the same though as an MLD station with a
single link, with some caveats of translations, no?
> Correct, for the first bring up not required but one use case I see is -
> the hostapd interface was disabled for some reason. While going down, it
> would have cleared the stations on the kernel but what if for some
> reason kernel did not clear the station entries and there are some stale
> entries present? So at next bring up (during enable) it would send the
> command without any MAC address to flush all stale entries (probably as
> a safety so that kernel and hostapd would now be on par).
I don't think this really makes much sense. The kernel can't keep track
of those stations properly if they're there, and anyway that'd be a
(pretty massive!) kernel bug?
Anyway, I think there probably _is_ justification for this (link
removal?), I'm just not sure this bringup flow really is a good
justification.
> > > Hence, add an option to pass link ID as well in the command so that if link
> > > ID is passed, station using that passed link ID alone would be deleted
> > > and others will not be removed.
> >
> > So first: Do you want some feature flag that indicates this? Or will we
> > just eat the cost of kicking out everyone (without even sending deauth
> > though, I think?) when running on older kernels?
> >
>
> If what I said above was the actual intention, then kicking out everyone
> without even sending deauth makes sense? Yes? If yes then we don't need
> a feature flag.
Does it though? Even if you're talking about init, you could have init
of one link much delayed for CSA, for example, with stations already
connected on the other(s).
> > Secondly: why is this part of NL80211_CMD_DEL_STATION? I'm not convinced
> > that makes sense. I actually kind of get why you're doing that - it's
> > easier to retrofit into the existing hostapd, but I don't necessarily
> > think that the hostap design (problems?) should influence this too much.
> >
> > IOW, it would feel much more appropriate to have this as part of
> > NL80211_CMD_REMOVE_LINK_STA? After all, when going to MLD then "STATION"
> > now represents a "peer MLD", and "LINK_STA" now represents an affiliated
> > STA. And flushing all affiliated STAs is what you want.
> >
> > So I think it should be NL80211_CMD_REMOVE_LINK_STA without a
> > NL80211_ATTR_MLD_ADDR.
> >
>
> At least as per the current way of NL80211_CMD_REMOVE_LINK_STA
> implementation, it did not made any sense to delete all link STAs if
> MLD_ADDR is not passed. So probably the command should be called as many
> times as there are active links in the STA?
Not sure I understand this, we're doing a kind of flush here, so you
could (conceptually) say "flush all link STAs on link 5", no? And
obviously stations that have no link left after this need to be removed
completely.
Note this raises an interesting point in mac80211, in that there's one
link ('deflink', the link the STA used to assoc) that cannot be removed
from an MLD station even.
But again this comes down to what you actually _want_, I think, so I'll
keep reading for now.
> Still I feel that NL80211_CMD_DEL_STATION is the proper place to put
> this? Without the current change also, it used to flush all STAs
> whenever MAC address is not passed. With MLO, now we need to flush STAs
> only if it is using the given link ID. So that link STAs from other
> affiliated links of AP would not be flushed.
Right so this is coming to the point where I wasn't sure earlier what
you actually meant, and I'm still not entirely positive I've understood
it. Let me read on ...
> Scenario I'm targeting is this -
>
> Pre-MLO
> ----------------------------
>
> sdata -> 2 GHz AP interface
> sta_lists ->
> 1. sta -> connected 2 GHz AP sdata
> 2. sta -> connected 2 GHz AP sdata
>
> After NL80211_CMD_DEL_STATION is given without any MAC address,
>
> sta_lists ->
> No entry(ies)
Right.
> With MLO
> -----------------------------
> sdata ->
> link_data -> 2 GHz AP link (link ID 0)
> link_data -> 5 GHz AP link (link ID 1)
> link_data -> 6 GHz AP link (link ID 2)
> sta_lists ->
> 1. sta -> connected AP MLD sdata
> link_sta 0 -> connected to 2 GHz link
> 2. sta -> connected AP MLD sdata
> link_sta 1 -> connected to 5 GHz link
> 3. sta -> connected AP MLD sdata
> link_sta 2 -> connected to 6 GHz link
> 4. sta -> connected AP MLD sdata
> link_sta 0 -> connected to 2 GHz link
> link_sta 1 -> connected to 5 GHz link
> link_sta 2 -> connected to 6 GHz link
>
> Assume 5 GHz goes down and it gives NL80211_CMD_DEL_STATION without any
> MAC address,
>
> sta_lists ->
> No entry(ies)
>
> This is not desirable since 5 GHz link went down, why 2/6 GHz STA also
> got flushed.
>
> Hence with the proposed change, only sta #2 and #4 would be flushed
> since only these two are using passed link ID (which would be 1).
> Hence after the command,
>
> sta_lists ->
> 1. sta -> connected AP MLD sdata
> link_sta 0 -> connected to 2 GHz link
> 3. sta -> connected AP MLD sdata
> link_sta 2 -> connected to 6 GHz link
Right, OK.
So you _are_ indeed wanting to remove all MLDs *entirely*, if they use a
specific link.
Agree that in this case, NL80211_CMD_DEL_STATION with link ID makes
sense as implemented, but probably need to clarify a little bit overall
that this is the operation, seeing how I was confused about whether you
want to remove only the link STAs on on those links, or the entire MLD
stations.
(and yeah, our terminology here is confusing and doesn't help either,
but that's because we didn't rename STATION to MLD or something
everywhere)
> Now, if ML re-config support is present, then hostapd (or the user space
> controller for that matters), could first issue
> NL80211_CMD_REMOVE_LINK_STA for the MLD STA (#4) and remove link sta
> with ID 1 from it. So that when NL80211_CMD_DEL_STATION comes, it would
> not remove the 2/6 GHz link STA as well from the MLD STA and hence flush
> the whole entry.
Right, OK!
But see above - that NL80211_CMD_REMOVE_LINK_STA as described here may
or may not be possible today in mac80211.
> The above change is not there yet in hostapd, so for the time being,
> whole MLD STA would be flushed.
OK.
> > > @@ -16827,6 +16840,9 @@ static const struct genl_small_ops nl80211_small_ops[] = {
> > > .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
> > > .doit = nl80211_del_station,
> > > .flags = GENL_UNS_ADMIN_PERM,
> > > + /* cannot use NL80211_FLAG_MLO_VALID_LINK_ID, depends on
> > > + * MAC address
> > > + */
> > > .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
> >
> > Hmm? How does NL80211_FLAG_MLO_VALID_LINK_ID depend on the MAC address?!
> > It ... doesn't?
> >
> I mean intention was that if MAC addresses is passed then no need of
> link ID. That is why did not add the valid link flag since it would
> expect the link ID even when MAC address is passed.
>
Ah, OK, that makes sense.
Maybe rephrase that comment? I was also thinking of just refactoring the
logic into a helper function, but that may be difficult, not sure. It
just looked similar.
johannes
On 1/30/24 16:16, Johannes Berg wrote:
> On Sat, 2024-01-27 at 11:14 +0530, Aditya Kumar Singh wrote:
>> On 1/26/24 14:36, Johannes Berg wrote:
>>> On Thu, 2024-01-25 at 18:28 +0530, Aditya Kumar Singh wrote:
>>>> Currently whenever NL80211_CMD_DEL_STATION command is called without any
>>>> MAC address, all stations present on that interface are flushed.
>>>
>>> True.
>>>
>>>> However with MLO there is a need to flush the stations from a particular
>>>> link in the interface, and not from all the links associated with the MLD
>>>> interface.
>>>
>>> Fair enough, I can get behind that.
>>>
>>> Edit: reading the code - I think I misunderstand that ... you're
>>> actually trying to remove all MLDs ("STATION") that have an active link
>>> on this link?
>>
>> Yes correct. The station might not be MLD station. It could be a legacy
>> station (non EHT) as well.
>
> We pretty much treat that the same though as an MLD station with a
> single link, with some caveats of translations, no?
>
Yes correct!
>> Correct, for the first bring up not required but one use case I see is -
>> the hostapd interface was disabled for some reason. While going down, it
>> would have cleared the stations on the kernel but what if for some
>> reason kernel did not clear the station entries and there are some stale
>> entries present? So at next bring up (during enable) it would send the
>> command without any MAC address to flush all stale entries (probably as
>> a safety so that kernel and hostapd would now be on par).
>
> I don't think this really makes much sense. The kernel can't keep track
> of those stations properly if they're there, and anyway that'd be a
> (pretty massive!) kernel bug?
>
Yeah :). Even I haven't seen kernel not removing the entries while the
interface was going down. Was just thinking out loud here. Tbh, even I
don't know the exact reason it was written in that way. Was guessing.
> Anyway, I think there probably _is_ justification for this (link
> removal?), I'm just not sure this bringup flow really is a good
> justification.
>
Yes we have.
>>>> Hence, add an option to pass link ID as well in the command so that if link
>>>> ID is passed, station using that passed link ID alone would be deleted
>>>> and others will not be removed.
>>>
>>> So first: Do you want some feature flag that indicates this? Or will we
>>> just eat the cost of kicking out everyone (without even sending deauth
>>> though, I think?) when running on older kernels?
>>>
>>
>> If what I said above was the actual intention, then kicking out everyone
>> without even sending deauth makes sense? Yes? If yes then we don't need
>> a feature flag.
>
> Does it though? Even if you're talking about init, you could have init
> of one link much delayed for CSA, for example, with stations already
> connected on the other(s).
>
hmm.. correct.
>>> Secondly: why is this part of NL80211_CMD_DEL_STATION? I'm not convinced
>>> that makes sense. I actually kind of get why you're doing that - it's
>>> easier to retrofit into the existing hostapd, but I don't necessarily
>>> think that the hostap design (problems?) should influence this too much.
>>>
>>> IOW, it would feel much more appropriate to have this as part of
>>> NL80211_CMD_REMOVE_LINK_STA? After all, when going to MLD then "STATION"
>>> now represents a "peer MLD", and "LINK_STA" now represents an affiliated
>>> STA. And flushing all affiliated STAs is what you want.
>>>
>>> So I think it should be NL80211_CMD_REMOVE_LINK_STA without a
>>> NL80211_ATTR_MLD_ADDR.
>>>
>>
>> At least as per the current way of NL80211_CMD_REMOVE_LINK_STA
>> implementation, it did not made any sense to delete all link STAs if
>> MLD_ADDR is not passed. So probably the command should be called as many
>> times as there are active links in the STA?
>
> Not sure I understand this, we're doing a kind of flush here, so you
> could (conceptually) say "flush all link STAs on link 5", no? And
> obviously stations that have no link left after this need to be removed
> completely.
>
> Note this raises an interesting point in mac80211, in that there's one
> link ('deflink', the link the STA used to assoc) that cannot be removed
> from an MLD station even.
>
Good point :) I did not think about this. Let me think again how to
handle this case. Thanks.
> But again this comes down to what you actually _want_, I think, so I'll
> keep reading for now.
>
>> Still I feel that NL80211_CMD_DEL_STATION is the proper place to put
>> this? Without the current change also, it used to flush all STAs
>> whenever MAC address is not passed. With MLO, now we need to flush STAs
>> only if it is using the given link ID. So that link STAs from other
>> affiliated links of AP would not be flushed.
>
>
> Right so this is coming to the point where I wasn't sure earlier what
> you actually meant, and I'm still not entirely positive I've understood
> it. Let me read on ...
>
>> Scenario I'm targeting is this -
>>
>> Pre-MLO
>> ----------------------------
>>
>> sdata -> 2 GHz AP interface
>> sta_lists ->
>> 1. sta -> connected 2 GHz AP sdata
>> 2. sta -> connected 2 GHz AP sdata
>>
>> After NL80211_CMD_DEL_STATION is given without any MAC address,
>>
>> sta_lists ->
>> No entry(ies)
>
> Right.
>
>> With MLO
>> -----------------------------
>> sdata ->
>> link_data -> 2 GHz AP link (link ID 0)
>> link_data -> 5 GHz AP link (link ID 1)
>> link_data -> 6 GHz AP link (link ID 2)
>> sta_lists ->
>> 1. sta -> connected AP MLD sdata
>> link_sta 0 -> connected to 2 GHz link
>> 2. sta -> connected AP MLD sdata
>> link_sta 1 -> connected to 5 GHz link
>> 3. sta -> connected AP MLD sdata
>> link_sta 2 -> connected to 6 GHz link
>> 4. sta -> connected AP MLD sdata
>> link_sta 0 -> connected to 2 GHz link
>> link_sta 1 -> connected to 5 GHz link
>> link_sta 2 -> connected to 6 GHz link
>>
>> Assume 5 GHz goes down and it gives NL80211_CMD_DEL_STATION without any
>> MAC address,
>>
>> sta_lists ->
>> No entry(ies)
>>
>> This is not desirable since 5 GHz link went down, why 2/6 GHz STA also
>> got flushed.
>>
>> Hence with the proposed change, only sta #2 and #4 would be flushed
>> since only these two are using passed link ID (which would be 1).
>> Hence after the command,
>>
>> sta_lists ->
>> 1. sta -> connected AP MLD sdata
>> link_sta 0 -> connected to 2 GHz link
>> 3. sta -> connected AP MLD sdata
>> link_sta 2 -> connected to 6 GHz link
>
> Right, OK.
>
> So you _are_ indeed wanting to remove all MLDs *entirely*, if they use a
> specific link.
>
Yes correct.
> Agree that in this case, NL80211_CMD_DEL_STATION with link ID makes
> sense as implemented, but probably need to clarify a little bit overall
> that this is the operation, seeing how I was confused about whether you
> want to remove only the link STAs on on those links, or the entire MLD
> stations.
>
okay sure, I will try to explain in the commit message as well as in
code if needed.
> (and yeah, our terminology here is confusing and doesn't help either,
> but that's because we didn't rename STATION to MLD or something
> everywhere)
>
>> Now, if ML re-config support is present, then hostapd (or the user space
>> controller for that matters), could first issue
>> NL80211_CMD_REMOVE_LINK_STA for the MLD STA (#4) and remove link sta
>> with ID 1 from it. So that when NL80211_CMD_DEL_STATION comes, it would
>> not remove the 2/6 GHz link STA as well from the MLD STA and hence flush
>> the whole entry.
>
> Right, OK!
> But see above - that NL80211_CMD_REMOVE_LINK_STA as described here may
> or may not be possible today in mac80211.
Sure will check that.
>
>> The above change is not there yet in hostapd, so for the time being,
>> whole MLD STA would be flushed.
>
> OK.
>
>>>> @@ -16827,6 +16840,9 @@ static const struct genl_small_ops nl80211_small_ops[] = {
>>>> .validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
>>>> .doit = nl80211_del_station,
>>>> .flags = GENL_UNS_ADMIN_PERM,
>>>> + /* cannot use NL80211_FLAG_MLO_VALID_LINK_ID, depends on
>>>> + * MAC address
>>>> + */
>>>> .internal_flags = IFLAGS(NL80211_FLAG_NEED_NETDEV_UP),
>>>
>>> Hmm? How does NL80211_FLAG_MLO_VALID_LINK_ID depend on the MAC address?!
>>> It ... doesn't?
>>>
>> I mean intention was that if MAC addresses is passed then no need of
>> link ID. That is why did not add the valid link flag since it would
>> expect the link ID even when MAC address is passed.
>>
>
> Ah, OK, that makes sense.
>
> Maybe rephrase that comment? I was also thinking of just refactoring the
> logic into a helper function, but that may be difficult, not sure. It
> just looked similar.
>
Sure, I will see what I can do here. Thanks for your inputs. Will send
v2 soon :)
On Tue, 2024-01-30 at 16:41 +0530, Aditya Kumar Singh wrote:
> Yeah :). Even I haven't seen kernel not removing the entries while the
> interface was going down. Was just thinking out loud here. Tbh, even I
> don't know the exact reason it was written in that way. Was guessing.
It's probably just old ... :)
> > Note this raises an interesting point in mac80211, in that there's one
> > link ('deflink', the link the STA used to assoc) that cannot be removed
> > from an MLD station even.
> >
>
> Good point :) I did not think about this. Let me think again how to
> handle this case. Thanks.
You don't actually have to now though - you're _not_ removing just
links, you're removing the whole station with all links, as long as it
has a link with the relevant link ID.
johannes
On 1/30/24 17:24, Johannes Berg wrote:
> On Tue, 2024-01-30 at 16:41 +0530, Aditya Kumar Singh wrote:
>
>> Yeah :). Even I haven't seen kernel not removing the entries while the
>> interface was going down. Was just thinking out loud here. Tbh, even I
>> don't know the exact reason it was written in that way. Was guessing.
>
> It's probably just old ... :)
>
>>> Note this raises an interesting point in mac80211, in that there's one
>>> link ('deflink', the link the STA used to assoc) that cannot be removed
>>> from an MLD station even.
>>>
>>
>> Good point :) I did not think about this. Let me think again how to
>> handle this case. Thanks.
>
> You don't actually have to now though - you're _not_ removing just
> links, you're removing the whole station with all links, as long as it
> has a link with the relevant link ID.
>
Yeah that is true. Not for this change. But the other one I'm working on
is the link removal area. So there I need to be care full.