Alright, here's another set of MLO patches :-)
I'm preparing everything here for link switching (which is also in
wireless-next/mld-wip branch already), along with some fixes to
avoid VLANs/4-addr and various other fixes that came up during
this work. Not all of them are related, e.g. the SW scan stop just
came up due to the new hwsim checks.
johannes
From: Johannes Berg <[email protected]>
With the link activation handling in mac80211, skip
any inactive links on TX.
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index a75420d92816..f17de806a35d 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1735,6 +1735,9 @@ mac80211_hwsim_select_tx_link(struct mac80211_hwsim_data *data,
/* round-robin the available link IDs */
link_id = (sp->last_link + i + 1) % ARRAY_SIZE(vif->link_conf);
+ if (!(vif->active_links & BIT(link_id)))
+ continue;
+
*link_sta = rcu_dereference(sta->link[link_id]);
if (!*link_sta)
continue;
@@ -1743,6 +1746,10 @@ mac80211_hwsim_select_tx_link(struct mac80211_hwsim_data *data,
if (WARN_ON_ONCE(!bss_conf))
continue;
+ /* can happen while switching links */
+ if (!rcu_access_pointer(bss_conf->chanctx_conf))
+ continue;
+
sp->last_link = link_id;
return bss_conf;
}
--
2.37.2
From: Johannes Berg <[email protected]>
Track the powersave bit on frames where we can look up
the STA by link addresses and set the links active or
inactive accordingly, and use this information to TX
only on links that are actually active in the peer.
Note that this doesn't implement powersave fully so
if no link is active things will not work right.
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 41 +++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index f17de806a35d..b5bc707d8e4e 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -229,6 +229,7 @@ static inline void hwsim_clear_magic(struct ieee80211_vif *vif)
struct hwsim_sta_priv {
u32 magic;
unsigned int last_link;
+ u16 active_links_rx;
};
#define HWSIM_STA_MAGIC 0x6d537749
@@ -1567,6 +1568,29 @@ static void mac80211_hwsim_rx(struct mac80211_hwsim_data *data,
struct ieee80211_rx_status *rx_status,
struct sk_buff *skb)
{
+ struct ieee80211_hdr *hdr = (void *)skb->data;
+
+ if (!ieee80211_has_morefrags(hdr->frame_control) &&
+ !is_multicast_ether_addr(hdr->addr1) &&
+ (ieee80211_is_mgmt(hdr->frame_control) ||
+ ieee80211_is_data(hdr->frame_control))) {
+ struct ieee80211_sta *sta;
+ unsigned int link_id;
+
+ rcu_read_lock();
+ sta = ieee80211_find_sta_by_link_addrs(data->hw, hdr->addr2,
+ hdr->addr1, &link_id);
+ if (sta) {
+ struct hwsim_sta_priv *sp = (void *)sta->drv_priv;
+
+ if (ieee80211_has_pm(hdr->frame_control))
+ sp->active_links_rx &= ~BIT(link_id);
+ else
+ sp->active_links_rx |= BIT(link_id);
+ }
+ rcu_read_unlock();
+ }
+
memcpy(IEEE80211_SKB_RXCB(skb), rx_status, sizeof(*rx_status));
mac80211_hwsim_add_vendor_rtap(skb);
@@ -1738,6 +1762,9 @@ mac80211_hwsim_select_tx_link(struct mac80211_hwsim_data *data,
if (!(vif->active_links & BIT(link_id)))
continue;
+ if (!(sp->active_links_rx & BIT(link_id)))
+ continue;
+
*link_sta = rcu_dereference(sta->link[link_id]);
if (!*link_sta)
continue;
@@ -2412,10 +2439,19 @@ static int mac80211_hwsim_sta_add(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *sta)
{
+ struct hwsim_sta_priv *sp = (void *)sta->drv_priv;
+
hwsim_check_magic(vif);
hwsim_set_sta_magic(sta);
mac80211_hwsim_sta_rc_update(hw, vif, sta, 0);
+ if (sta->valid_links) {
+ WARN(hweight16(sta->valid_links) > 1,
+ "expect to add STA with single link, have 0x%x\n",
+ sta->valid_links);
+ sp->active_links_rx = sta->valid_links;
+ }
+
return 0;
}
@@ -3037,8 +3073,13 @@ static int mac80211_hwsim_change_sta_links(struct ieee80211_hw *hw,
struct ieee80211_sta *sta,
u16 old_links, u16 new_links)
{
+ struct hwsim_sta_priv *sp = (void *)sta->drv_priv;
+
hwsim_check_sta_magic(sta);
+ if (vif->type == NL80211_IFTYPE_STATION)
+ sp->active_links_rx = new_links;
+
return 0;
}
--
2.37.2
From: Johannes Berg <[email protected]>
Just as an additional check that mac80211 isn't doing
anything strange, add a check of the STA magic (which
gets assigned when the station is added, and cleared
when the station is removed).
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 87176b205fc1..48b1c39c1c05 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3021,6 +3021,8 @@ static int mac80211_hwsim_change_sta_links(struct ieee80211_hw *hw,
struct ieee80211_sta *sta,
u16 old_links, u16 new_links)
{
+ hwsim_check_sta_magic(sta);
+
return 0;
}
--
2.37.2
From: Johannes Berg <[email protected]>
In order to let the driver select active links and properly
make multi-link connections, as a first step isolate the
driver from inactive links, and set the active links to be
only the association link for client-side interfaces. For
AP side nothing changes since APs always have to have all
their links active.
To simplify things, update the for_each_sta_active_link()
API to include the appropriate vif pointer.
This also implies not allocating a chanctx for an inactive
link, which requires a few more changes.
Since we now no longer try to program multiple links to the
driver, remove the check in the MLME code.
Signed-off-by: Johannes Berg <[email protected]>
---
include/net/mac80211.h | 30 +++----
net/mac80211/chan.c | 6 ++
net/mac80211/driver-ops.c | 172 ++++++++++++++++++++++++++++++++++++++
net/mac80211/driver-ops.h | 165 ++++++------------------------------
net/mac80211/key.c | 8 ++
net/mac80211/link.c | 66 ++++++++++++---
net/mac80211/mlme.c | 25 ++----
net/mac80211/util.c | 2 +-
8 files changed, 286 insertions(+), 188 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d4e1d73d88cc..20a2f25a38fa 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1799,6 +1799,9 @@ struct ieee80211_vif_cfg {
* @link_conf: in case of MLD, the per-link BSS configuration,
* indexed by link ID
* @valid_links: bitmap of valid links, or 0 for non-MLO.
+ * @active_links: The bitmap of active links, or 0 for non-MLO.
+ * The driver shouldn't change this directly, but use the
+ * API calls meant for that purpose.
* @addr: address of this interface
* @p2p: indicates whether this AP or STA interface is a p2p
* interface, i.e. a GO or p2p-sta respectively
@@ -1834,7 +1837,7 @@ struct ieee80211_vif {
struct ieee80211_vif_cfg cfg;
struct ieee80211_bss_conf bss_conf;
struct ieee80211_bss_conf __rcu *link_conf[IEEE80211_MLD_MAX_NUM_LINKS];
- u16 valid_links;
+ u16 valid_links, active_links;
u8 addr[ETH_ALEN] __aligned(2);
bool p2p;
@@ -1861,12 +1864,11 @@ struct ieee80211_vif {
u8 drv_priv[] __aligned(sizeof(void *));
};
-/* FIXME: for now loop over all the available links; later will be changed
- * to loop only over the active links.
- */
-#define for_each_vif_active_link(vif, link, link_id) \
- for (link_id = 0; link_id < ARRAY_SIZE((vif)->link_conf); link_id++) \
- if ((link = rcu_dereference((vif)->link_conf[link_id])))
+#define for_each_vif_active_link(vif, link, link_id) \
+ for (link_id = 0; link_id < ARRAY_SIZE((vif)->link_conf); link_id++) \
+ if ((!(vif)->active_links || \
+ (vif)->active_links & BIT(link_id)) && \
+ (link = rcu_dereference((vif)->link_conf[link_id])))
static inline bool ieee80211_vif_is_mesh(struct ieee80211_vif *vif)
{
@@ -2264,13 +2266,13 @@ struct ieee80211_sta {
u8 drv_priv[] __aligned(sizeof(void *));
};
-/* FIXME: need to loop only over links which are active and check the actual
- * lock
- */
-#define for_each_sta_active_link(sta, link_sta, link_id) \
- for (link_id = 0; link_id < ARRAY_SIZE((sta)->link); link_id++) \
- if (((link_sta) = rcu_dereference_protected((sta)->link[link_id],\
- 1))) \
+/* FIXME: check the locking correctly */
+#define for_each_sta_active_link(vif, sta, link_sta, link_id) \
+ for (link_id = 0; link_id < ARRAY_SIZE((sta)->link); link_id++) \
+ if ((!(vif)->active_links || \
+ (vif)->active_links & BIT(link_id)) && \
+ ((link_sta) = rcu_dereference_protected((sta)->link[link_id],\
+ 1)))
/**
* enum sta_notify_cmd - sta notify command
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index f247daa41563..e72cf0749d49 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -1799,6 +1799,12 @@ int ieee80211_link_use_channel(struct ieee80211_link_data *link,
lockdep_assert_held(&local->mtx);
+ if (sdata->vif.active_links &&
+ !(sdata->vif.active_links & BIT(link->link_id))) {
+ ieee80211_link_update_chandef(link, chandef);
+ return 0;
+ }
+
mutex_lock(&local->chanctx_mtx);
ret = cfg80211_chandef_dfs_required(local->hw.wiphy,
diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index 9b61dc7889c2..5392ffa18270 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -192,6 +192,10 @@ int drv_conf_tx(struct ieee80211_local *local,
if (!check_sdata_in_driver(sdata))
return -EIO;
+ if (sdata->vif.active_links &&
+ !(sdata->vif.active_links & BIT(link->link_id)))
+ return 0;
+
if (params->cw_min == 0 || params->cw_min > params->cw_max) {
/*
* If we can't configure hardware anyway, don't warn. We may
@@ -272,6 +276,60 @@ void drv_reset_tsf(struct ieee80211_local *local,
trace_drv_return_void(local);
}
+int drv_assign_vif_chanctx(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_bss_conf *link_conf,
+ struct ieee80211_chanctx *ctx)
+{
+ int ret = 0;
+
+ drv_verify_link_exists(sdata, link_conf);
+ if (!check_sdata_in_driver(sdata))
+ return -EIO;
+
+ if (sdata->vif.active_links &&
+ !(sdata->vif.active_links & BIT(link_conf->link_id)))
+ return 0;
+
+ trace_drv_assign_vif_chanctx(local, sdata, link_conf, ctx);
+ if (local->ops->assign_vif_chanctx) {
+ WARN_ON_ONCE(!ctx->driver_present);
+ ret = local->ops->assign_vif_chanctx(&local->hw,
+ &sdata->vif,
+ link_conf,
+ &ctx->conf);
+ }
+ trace_drv_return_int(local, ret);
+
+ return ret;
+}
+
+void drv_unassign_vif_chanctx(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_bss_conf *link_conf,
+ struct ieee80211_chanctx *ctx)
+{
+ might_sleep();
+
+ drv_verify_link_exists(sdata, link_conf);
+ if (!check_sdata_in_driver(sdata))
+ return;
+
+ if (sdata->vif.active_links &&
+ !(sdata->vif.active_links & BIT(link_conf->link_id)))
+ return;
+
+ trace_drv_unassign_vif_chanctx(local, sdata, link_conf, ctx);
+ if (local->ops->unassign_vif_chanctx) {
+ WARN_ON_ONCE(!ctx->driver_present);
+ local->ops->unassign_vif_chanctx(&local->hw,
+ &sdata->vif,
+ link_conf,
+ &ctx->conf);
+ }
+ trace_drv_return_void(local);
+}
+
int drv_switch_vif_chanctx(struct ieee80211_local *local,
struct ieee80211_vif_chanctx_switch *vifs,
int n_vifs, enum ieee80211_chanctx_switch_mode mode)
@@ -346,3 +404,117 @@ int drv_ampdu_action(struct ieee80211_local *local,
return ret;
}
+
+void drv_link_info_changed(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_bss_conf *info,
+ int link_id, u64 changed)
+{
+ might_sleep();
+
+ if (WARN_ON_ONCE(changed & (BSS_CHANGED_BEACON |
+ BSS_CHANGED_BEACON_ENABLED) &&
+ sdata->vif.type != NL80211_IFTYPE_AP &&
+ sdata->vif.type != NL80211_IFTYPE_ADHOC &&
+ sdata->vif.type != NL80211_IFTYPE_MESH_POINT &&
+ sdata->vif.type != NL80211_IFTYPE_OCB))
+ return;
+
+ if (WARN_ON_ONCE(sdata->vif.type == NL80211_IFTYPE_P2P_DEVICE ||
+ sdata->vif.type == NL80211_IFTYPE_NAN ||
+ (sdata->vif.type == NL80211_IFTYPE_MONITOR &&
+ !sdata->vif.bss_conf.mu_mimo_owner &&
+ !(changed & BSS_CHANGED_TXPOWER))))
+ return;
+
+ if (!check_sdata_in_driver(sdata))
+ return;
+
+ if (sdata->vif.active_links &&
+ !(sdata->vif.active_links & BIT(link_id)))
+ return;
+
+ trace_drv_link_info_changed(local, sdata, info, changed);
+ if (local->ops->link_info_changed)
+ local->ops->link_info_changed(&local->hw, &sdata->vif,
+ info, changed);
+ else if (local->ops->bss_info_changed)
+ local->ops->bss_info_changed(&local->hw, &sdata->vif,
+ info, changed);
+ trace_drv_return_void(local);
+}
+
+int drv_set_key(struct ieee80211_local *local,
+ enum set_key_cmd cmd,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_sta *sta,
+ struct ieee80211_key_conf *key)
+{
+ int ret;
+
+ might_sleep();
+
+ sdata = get_bss_sdata(sdata);
+ if (!check_sdata_in_driver(sdata))
+ return -EIO;
+
+ if (WARN_ON(key->link_id >= 0 && sdata->vif.active_links &&
+ !(sdata->vif.active_links & BIT(key->link_id))))
+ return -ENOLINK;
+
+ trace_drv_set_key(local, cmd, sdata, sta, key);
+ ret = local->ops->set_key(&local->hw, cmd, &sdata->vif, sta, key);
+ trace_drv_return_int(local, ret);
+ return ret;
+}
+
+int drv_change_vif_links(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ u16 old_links, u16 new_links,
+ struct ieee80211_bss_conf *old[IEEE80211_MLD_MAX_NUM_LINKS])
+{
+ int ret = -EOPNOTSUPP;
+
+ might_sleep();
+
+ if (!check_sdata_in_driver(sdata))
+ return -EIO;
+
+ if (old_links == new_links)
+ return 0;
+
+ trace_drv_change_vif_links(local, sdata, old_links, new_links);
+ if (local->ops->change_vif_links)
+ ret = local->ops->change_vif_links(&local->hw, &sdata->vif,
+ old_links, new_links, old);
+ trace_drv_return_int(local, ret);
+
+ return ret;
+}
+
+int drv_change_sta_links(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_sta *sta,
+ u16 old_links, u16 new_links)
+{
+ int ret = -EOPNOTSUPP;
+
+ might_sleep();
+
+ if (!check_sdata_in_driver(sdata))
+ return -EIO;
+
+ old_links &= sdata->vif.active_links;
+ new_links &= sdata->vif.active_links;
+
+ if (old_links == new_links)
+ return 0;
+
+ trace_drv_change_sta_links(local, sdata, sta, old_links, new_links);
+ if (local->ops->change_sta_links)
+ ret = local->ops->change_sta_links(&local->hw, &sdata->vif, sta,
+ old_links, new_links);
+ trace_drv_return_int(local, ret);
+
+ return ret;
+}
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index 482f5c97a72b..81e40b0a3b16 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -165,40 +165,10 @@ static inline void drv_vif_cfg_changed(struct ieee80211_local *local,
trace_drv_return_void(local);
}
-static inline void drv_link_info_changed(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- struct ieee80211_bss_conf *info,
- int link_id, u64 changed)
-{
- might_sleep();
-
- if (WARN_ON_ONCE(changed & (BSS_CHANGED_BEACON |
- BSS_CHANGED_BEACON_ENABLED) &&
- sdata->vif.type != NL80211_IFTYPE_AP &&
- sdata->vif.type != NL80211_IFTYPE_ADHOC &&
- sdata->vif.type != NL80211_IFTYPE_MESH_POINT &&
- sdata->vif.type != NL80211_IFTYPE_OCB))
- return;
-
- if (WARN_ON_ONCE(sdata->vif.type == NL80211_IFTYPE_P2P_DEVICE ||
- sdata->vif.type == NL80211_IFTYPE_NAN ||
- (sdata->vif.type == NL80211_IFTYPE_MONITOR &&
- !sdata->vif.bss_conf.mu_mimo_owner &&
- !(changed & BSS_CHANGED_TXPOWER))))
- return;
-
- if (!check_sdata_in_driver(sdata))
- return;
-
- trace_drv_link_info_changed(local, sdata, info, changed);
- if (local->ops->link_info_changed)
- local->ops->link_info_changed(&local->hw, &sdata->vif,
- info, changed);
- else if (local->ops->bss_info_changed)
- local->ops->bss_info_changed(&local->hw, &sdata->vif,
- info, changed);
- trace_drv_return_void(local);
-}
+void drv_link_info_changed(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_bss_conf *info,
+ int link_id, u64 changed);
static inline u64 drv_prepare_multicast(struct ieee80211_local *local,
struct netdev_hw_addr_list *mc_list)
@@ -256,25 +226,11 @@ static inline int drv_set_tim(struct ieee80211_local *local,
return ret;
}
-static inline int drv_set_key(struct ieee80211_local *local,
- enum set_key_cmd cmd,
- struct ieee80211_sub_if_data *sdata,
- struct ieee80211_sta *sta,
- struct ieee80211_key_conf *key)
-{
- int ret;
-
- might_sleep();
-
- sdata = get_bss_sdata(sdata);
- if (!check_sdata_in_driver(sdata))
- return -EIO;
-
- trace_drv_set_key(local, cmd, sdata, sta, key);
- ret = local->ops->set_key(&local->hw, cmd, &sdata->vif, sta, key);
- trace_drv_return_int(local, ret);
- return ret;
-}
+int drv_set_key(struct ieee80211_local *local,
+ enum set_key_cmd cmd,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_sta *sta,
+ struct ieee80211_key_conf *key);
static inline void drv_update_tkip_key(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata,
@@ -945,52 +901,14 @@ static inline void drv_verify_link_exists(struct ieee80211_sub_if_data *sdata,
sdata_assert_lock(sdata);
}
-static inline int drv_assign_vif_chanctx(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- struct ieee80211_bss_conf *link_conf,
- struct ieee80211_chanctx *ctx)
-{
- int ret = 0;
-
- drv_verify_link_exists(sdata, link_conf);
- if (!check_sdata_in_driver(sdata))
- return -EIO;
-
- trace_drv_assign_vif_chanctx(local, sdata, link_conf, ctx);
- if (local->ops->assign_vif_chanctx) {
- WARN_ON_ONCE(!ctx->driver_present);
- ret = local->ops->assign_vif_chanctx(&local->hw,
- &sdata->vif,
- link_conf,
- &ctx->conf);
- }
- trace_drv_return_int(local, ret);
-
- return ret;
-}
-
-static inline void drv_unassign_vif_chanctx(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- struct ieee80211_bss_conf *link_conf,
- struct ieee80211_chanctx *ctx)
-{
- might_sleep();
-
- drv_verify_link_exists(sdata, link_conf);
- if (!check_sdata_in_driver(sdata))
- return;
-
- trace_drv_unassign_vif_chanctx(local, sdata, link_conf, ctx);
- if (local->ops->unassign_vif_chanctx) {
- WARN_ON_ONCE(!ctx->driver_present);
- local->ops->unassign_vif_chanctx(&local->hw,
- &sdata->vif,
- link_conf,
- &ctx->conf);
- }
- trace_drv_return_void(local);
-}
-
+int drv_assign_vif_chanctx(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_bss_conf *link_conf,
+ struct ieee80211_chanctx *ctx);
+void drv_unassign_vif_chanctx(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_bss_conf *link_conf,
+ struct ieee80211_chanctx *ctx);
int drv_switch_vif_chanctx(struct ieee80211_local *local,
struct ieee80211_vif_chanctx_switch *vifs,
int n_vifs, enum ieee80211_chanctx_switch_mode mode);
@@ -1552,46 +1470,13 @@ static inline int drv_net_fill_forward_path(struct ieee80211_local *local,
return ret;
}
-static inline int drv_change_vif_links(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- u16 old_links, u16 new_links,
- struct ieee80211_bss_conf *old[IEEE80211_MLD_MAX_NUM_LINKS])
-{
- int ret = -EOPNOTSUPP;
-
- might_sleep();
-
- if (!check_sdata_in_driver(sdata))
- return -EIO;
-
- trace_drv_change_vif_links(local, sdata, old_links, new_links);
- if (local->ops->change_vif_links)
- ret = local->ops->change_vif_links(&local->hw, &sdata->vif,
- old_links, new_links, old);
- trace_drv_return_int(local, ret);
-
- return ret;
-}
-
-static inline int drv_change_sta_links(struct ieee80211_local *local,
- struct ieee80211_sub_if_data *sdata,
- struct ieee80211_sta *sta,
- u16 old_links, u16 new_links)
-{
- int ret = -EOPNOTSUPP;
-
- might_sleep();
-
- if (!check_sdata_in_driver(sdata))
- return -EIO;
-
- trace_drv_change_sta_links(local, sdata, sta, old_links, new_links);
- if (local->ops->change_sta_links)
- ret = local->ops->change_sta_links(&local->hw, &sdata->vif, sta,
- old_links, new_links);
- trace_drv_return_int(local, ret);
-
- return ret;
-}
+int drv_change_vif_links(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ u16 old_links, u16 new_links,
+ struct ieee80211_bss_conf *old[IEEE80211_MLD_MAX_NUM_LINKS]);
+int drv_change_sta_links(struct ieee80211_local *local,
+ struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_sta *sta,
+ u16 old_links, u16 new_links);
#endif /* __MAC80211_DRIVER_OPS */
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index d89ec93b243b..f6f0f65fb255 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -177,6 +177,10 @@ static int ieee80211_key_enable_hw_accel(struct ieee80211_key *key)
}
}
+ if (key->conf.link_id >= 0 && sdata->vif.active_links &&
+ !(sdata->vif.active_links & BIT(key->conf.link_id)))
+ return 0;
+
ret = drv_set_key(key->local, SET_KEY, sdata,
sta ? &sta->sta : NULL, &key->conf);
@@ -246,6 +250,10 @@ static void ieee80211_key_disable_hw_accel(struct ieee80211_key *key)
sta = key->sta;
sdata = key->sdata;
+ if (key->conf.link_id >= 0 && sdata->vif.active_links &&
+ !(sdata->vif.active_links & BIT(key->conf.link_id)))
+ return;
+
if (!(key->conf.flags & (IEEE80211_KEY_FLAG_GENERATE_MMIC |
IEEE80211_KEY_FLAG_PUT_MIC_SPACE |
IEEE80211_KEY_FLAG_RESERVE_TAILROOM)))
diff --git a/net/mac80211/link.c b/net/mac80211/link.c
index 096f313c2a6e..8df348a5edce 100644
--- a/net/mac80211/link.c
+++ b/net/mac80211/link.c
@@ -73,28 +73,37 @@ struct link_container {
struct ieee80211_bss_conf conf;
};
-static void ieee80211_free_links(struct ieee80211_sub_if_data *sdata,
- struct link_container **links)
+static void ieee80211_tear_down_links(struct ieee80211_sub_if_data *sdata,
+ struct link_container **links, u16 mask)
{
+ struct ieee80211_link_data *link;
LIST_HEAD(keys);
unsigned int link_id;
for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
- if (!links[link_id])
+ if (!(mask & BIT(link_id)))
continue;
- ieee80211_remove_link_keys(&links[link_id]->data, &keys);
+ link = &links[link_id]->data;
+ if (link_id == 0 && !link)
+ link = &sdata->deflink;
+ if (WARN_ON(!link))
+ continue;
+ ieee80211_remove_link_keys(link, &keys);
+ ieee80211_link_stop(link);
}
synchronize_rcu();
ieee80211_free_key_list(sdata->local, &keys);
+}
- for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
- if (!links[link_id])
- continue;
- ieee80211_link_stop(&links[link_id]->data);
+static void ieee80211_free_links(struct ieee80211_sub_if_data *sdata,
+ struct link_container **links)
+{
+ unsigned int link_id;
+
+ for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++)
kfree(links[link_id]);
- }
}
static int ieee80211_check_dup_link_addrs(struct ieee80211_sub_if_data *sdata)
@@ -123,11 +132,38 @@ static int ieee80211_check_dup_link_addrs(struct ieee80211_sub_if_data *sdata)
return 0;
}
+static void ieee80211_set_vif_links_bitmaps(struct ieee80211_sub_if_data *sdata,
+ u16 links)
+{
+ sdata->vif.valid_links = links;
+
+ if (!links) {
+ sdata->vif.active_links = 0;
+ return;
+ }
+
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_AP:
+ /* in an AP all links are always active */
+ sdata->vif.active_links = links;
+ break;
+ case NL80211_IFTYPE_STATION:
+ if (sdata->vif.active_links)
+ break;
+ WARN_ON(hweight16(links) > 1);
+ sdata->vif.active_links = links;
+ break;
+ default:
+ WARN_ON(1);
+ }
+}
+
static int ieee80211_vif_update_links(struct ieee80211_sub_if_data *sdata,
struct link_container **to_free,
u16 new_links)
{
u16 old_links = sdata->vif.valid_links;
+ u16 old_active = sdata->vif.active_links;
unsigned long add = new_links & ~old_links;
unsigned long rem = old_links & ~new_links;
unsigned int link_id;
@@ -195,13 +231,17 @@ static int ieee80211_vif_update_links(struct ieee80211_sub_if_data *sdata,
ieee80211_link_init(sdata, -1, &sdata->deflink,
&sdata->vif.bss_conf);
- sdata->vif.valid_links = new_links;
-
ret = ieee80211_check_dup_link_addrs(sdata);
if (!ret) {
+ /* for keys we will not be able to undo this */
+ ieee80211_tear_down_links(sdata, to_free, rem);
+
+ ieee80211_set_vif_links_bitmaps(sdata, new_links);
+
/* tell the driver */
ret = drv_change_vif_links(sdata->local, sdata,
- old_links, new_links,
+ old_links & old_active,
+ new_links & sdata->vif.active_links,
old);
}
@@ -209,7 +249,7 @@ static int ieee80211_vif_update_links(struct ieee80211_sub_if_data *sdata,
/* restore config */
memcpy(sdata->link, old_data, sizeof(old_data));
memcpy(sdata->vif.link_conf, old, sizeof(old));
- sdata->vif.valid_links = old_links;
+ ieee80211_set_vif_links_bitmaps(sdata, old_links);
/* and free (only) the newly allocated links */
memset(to_free, 0, sizeof(links));
goto free;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 4c8bbf0bd25b..30edac8724d5 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -4043,11 +4043,11 @@ static bool ieee80211_assoc_config_link(struct ieee80211_link_data *link,
goto out;
}
- sband = ieee80211_get_link_sband(link);
- if (!sband) {
+ if (WARN_ON(!link->conf->chandef.chan)) {
ret = false;
goto out;
}
+ sband = local->hw.wiphy->bands[link->conf->chandef.chan->band];
if (!(link->u.mgd.conn_flags & IEEE80211_CONN_DISABLE_HE) &&
(!elems->he_cap || !elems->he_operation)) {
@@ -4871,8 +4871,10 @@ static bool ieee80211_assoc_success(struct ieee80211_sub_if_data *sdata,
err = ieee80211_prep_channel(sdata, link,
assoc_data->link[link_id].bss,
&link->u.mgd.conn_flags);
- if (err)
+ if (err) {
+ link_info(link, "prep_channel failed\n");
goto out_err;
+ }
}
err = ieee80211_mgd_setup_link_sta(link, sta, link_sta,
@@ -6876,23 +6878,6 @@ int ieee80211_mgd_assoc(struct ieee80211_sub_if_data *sdata,
for (i = 0; i < IEEE80211_MLD_MAX_NUM_LINKS; i++)
size += req->links[i].elems_len;
- if (req->ap_mld_addr) {
- for (i = 0; i < IEEE80211_MLD_MAX_NUM_LINKS; i++) {
- if (!req->links[i].bss)
- continue;
- if (i == assoc_link_id)
- continue;
- /*
- * For now, support only a single link in MLO, we
- * don't have the necessary parsing of the multi-
- * link element in the association response, etc.
- */
- sdata_info(sdata,
- "refusing MLO association with >1 links\n");
- return -EINVAL;
- }
- }
-
assoc_data = kzalloc(size, GFP_KERNEL);
if (!assoc_data)
return -ENOMEM;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 8b4c6b7abafa..0eed18189491 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2902,7 +2902,7 @@ void ieee80211_recalc_min_chandef(struct ieee80211_sub_if_data *sdata,
*/
rcu_read_unlock();
- if (WARN_ON_ONCE(!chanctx_conf))
+ if (!chanctx_conf)
goto unlock;
chanctx = container_of(chanctx_conf, struct ieee80211_chanctx,
--
2.37.2
From: Johannes Berg <[email protected]>
Currently, other paths calling drv_sta_state() hold the mutex
and therefore drivers can assume that, and look at links with
that protection. Fix that for the reconfig path as well; to
do it more easily use ieee80211_reconfig_stations() for the
AP/AP_VLAN station reconfig as well.
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/util.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 0eed18189491..0ea5d50091dc 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2530,7 +2530,6 @@ int ieee80211_reconfig(struct ieee80211_local *local)
if (link)
ieee80211_assign_chanctx(local, sdata, link);
}
- sdata_unlock(sdata);
switch (sdata->vif.type) {
case NL80211_IFTYPE_AP_VLAN:
@@ -2549,6 +2548,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
&sdata->deflink.tx_conf[i]);
break;
}
+ sdata_unlock(sdata);
/* common change flags for all interface types */
changed = BSS_CHANGED_ERP_CTS_PROT |
@@ -2657,23 +2657,21 @@ int ieee80211_reconfig(struct ieee80211_local *local)
}
/* APs are now beaconing, add back stations */
- mutex_lock(&local->sta_mtx);
- list_for_each_entry(sta, &local->sta_list, list) {
- enum ieee80211_sta_state state;
-
- if (!sta->uploaded)
- continue;
-
- if (sta->sdata->vif.type != NL80211_IFTYPE_AP &&
- sta->sdata->vif.type != NL80211_IFTYPE_AP_VLAN)
+ list_for_each_entry(sdata, &local->interfaces, list) {
+ if (!ieee80211_sdata_running(sdata))
continue;
- for (state = IEEE80211_STA_NOTEXIST;
- state < sta->sta_state; state++)
- WARN_ON(drv_sta_state(local, sta->sdata, sta, state,
- state + 1));
+ sdata_lock(sdata);
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_AP_VLAN:
+ case NL80211_IFTYPE_AP:
+ ieee80211_reconfig_stations(sdata);
+ break;
+ default:
+ break;
+ }
+ sdata_unlock(sdata);
}
- mutex_unlock(&local->sta_mtx);
/* add back keys */
list_for_each_entry(sdata, &local->interfaces, list)
--
2.37.2
From: Johannes Berg <[email protected]>
In hwsim we now track when we receive NDP (or other frames
carrying a PM bit) to see if a link should be active, but
then of course we also need to transmit NDPs to at least
set a link to inactive when we deacivate it. Implement that
as well as sending an NDP when we activate a link, which
allows receiving frames before transmitting any.
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 28 +++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index b5bc707d8e4e..18339a56316d 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2966,6 +2966,18 @@ static int mac80211_hwsim_assign_vif_chanctx(struct ieee80211_hw *hw,
hwsim_check_magic(vif);
hwsim_check_chanctx_magic(ctx);
+ /* if we activate a link while already associated wake it up */
+ if (vif->type == NL80211_IFTYPE_STATION && vif->cfg.assoc) {
+ struct sk_buff *skb;
+
+ skb = ieee80211_nullfunc_get(hw, vif, link_conf->link_id, true);
+ if (skb) {
+ local_bh_disable();
+ mac80211_hwsim_tx_frame(hw, skb, ctx->def.chan);
+ local_bh_enable();
+ }
+ }
+
return 0;
}
@@ -2976,6 +2988,22 @@ static void mac80211_hwsim_unassign_vif_chanctx(struct ieee80211_hw *hw,
{
hwsim_check_magic(vif);
hwsim_check_chanctx_magic(ctx);
+
+ /* if we deactivate a link while associated suspend it first */
+ if (vif->type == NL80211_IFTYPE_STATION && vif->cfg.assoc) {
+ struct sk_buff *skb;
+
+ skb = ieee80211_nullfunc_get(hw, vif, link_conf->link_id, true);
+ if (skb) {
+ struct ieee80211_hdr *hdr = (void *)skb->data;
+
+ hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_PM);
+
+ local_bh_disable();
+ mac80211_hwsim_tx_frame(hw, skb, ctx->def.chan);
+ local_bh_enable();
+ }
+ }
}
static const char mac80211_hwsim_gstrings_stats[][ETH_GSTRING_LEN] = {
--
2.37.2
From: Johannes Berg <[email protected]>
Add an MLD address attribute to BSS entries that the interface
is currently associated with to help userspace figure out what's
going on.
Signed-off-by: Johannes Berg <[email protected]>
---
include/uapi/linux/nl80211.h | 2 ++
net/wireless/nl80211.c | 6 ++++--
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 573db20403dc..ac31fbd84402 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4959,6 +4959,7 @@ enum nl80211_bss_scan_width {
* using the nesting index as the antenna number.
* @NL80211_BSS_FREQUENCY_OFFSET: frequency offset in KHz
* @NL80211_BSS_MLO_LINK_ID: MLO link ID of the BSS (u8).
+ * @NL80211_BSS_MLD_ADDR: MLD address of this BSS if connected to it.
* @__NL80211_BSS_AFTER_LAST: internal
* @NL80211_BSS_MAX: highest BSS attribute
*/
@@ -4985,6 +4986,7 @@ enum nl80211_bss {
NL80211_BSS_CHAIN_SIGNAL,
NL80211_BSS_FREQUENCY_OFFSET,
NL80211_BSS_MLO_LINK_ID,
+ NL80211_BSS_MLD_ADDR,
/* keep last */
__NL80211_BSS_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index dad88d231d56..8ab4c907e284 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10182,8 +10182,10 @@ static int nl80211_send_bss(struct sk_buff *msg, struct netlink_callback *cb,
(nla_put_u32(msg, NL80211_BSS_STATUS,
NL80211_BSS_STATUS_ASSOCIATED) ||
(wdev->valid_links &&
- nla_put_u8(msg, NL80211_BSS_MLO_LINK_ID,
- link_id))))
+ (nla_put_u8(msg, NL80211_BSS_MLO_LINK_ID,
+ link_id) ||
+ nla_put(msg, NL80211_BSS_MLD_ADDR, ETH_ALEN,
+ wdev->u.client.connected_addr)))))
goto nla_put_failure;
}
break;
--
2.37.2
From: Johannes Berg <[email protected]>
Add a new API function ieee80211_find_sta_by_link_addrs()
that looks up the STA and link ID based on interface and
station link addresses.
We're going to use it for mac80211-hwsim to track on the
AP side which links are active.
Signed-off-by: Johannes Berg <[email protected]>
---
include/net/mac80211.h | 16 ++++++++++++++++
net/mac80211/sta_info.c | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 20a2f25a38fa..954cc029a9f9 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5986,6 +5986,22 @@ struct ieee80211_sta *ieee80211_find_sta_by_ifaddr(struct ieee80211_hw *hw,
const u8 *addr,
const u8 *localaddr);
+/**
+ * ieee80211_find_sta_by_link_addrs - find STA by link addresses
+ * @hw: pointer as obtained from ieee80211_alloc_hw()
+ * @addr: remote station's link address
+ * @localaddr: local link address, use %NULL for any (but avoid that)
+ * @link_id: pointer to obtain the link ID if the STA is found,
+ * may be %NULL if the link ID is not needed
+ *
+ * Obtain the STA by link address, must use RCU protection.
+ */
+struct ieee80211_sta *
+ieee80211_find_sta_by_link_addrs(struct ieee80211_hw *hw,
+ const u8 *addr,
+ const u8 *localaddr,
+ unsigned int *link_id);
+
/**
* ieee80211_sta_block_awake - block station from waking up
* @hw: the hardware
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 815ab0cee503..1749c21e735c 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -274,6 +274,43 @@ link_sta_info_get_bss(struct ieee80211_sub_if_data *sdata, const u8 *addr)
return NULL;
}
+struct ieee80211_sta *
+ieee80211_find_sta_by_link_addrs(struct ieee80211_hw *hw,
+ const u8 *addr,
+ const u8 *localaddr,
+ unsigned int *link_id)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+ struct link_sta_info *link_sta;
+ struct rhlist_head *tmp;
+
+ for_each_link_sta_info(local, addr, link_sta, tmp) {
+ struct sta_info *sta = link_sta->sta;
+ struct ieee80211_link_data *link;
+ u8 _link_id = link_sta->link_id;
+
+ if (!localaddr) {
+ if (link_id)
+ *link_id = _link_id;
+ return &sta->sta;
+ }
+
+ link = rcu_dereference(sta->sdata->link[_link_id]);
+ if (!link)
+ continue;
+
+ if (memcmp(link->conf->addr, localaddr, ETH_ALEN))
+ continue;
+
+ if (link_id)
+ *link_id = _link_id;
+ return &sta->sta;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(ieee80211_find_sta_by_link_addrs);
+
struct sta_info *sta_info_get_by_addrs(struct ieee80211_local *local,
const u8 *sta_addr, const u8 *vif_addr)
{
--
2.37.2
From: Benjamin Berg <[email protected]>
The SMPS power save mode needs to be per-link rather than being shared
for all links. As such, move it into struct ieee80211_link_sta.
Signed-off-by: Benjamin Berg <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 6 +++---
drivers/net/wireless/ath/ath11k/mac.c | 6 +++---
drivers/net/wireless/ath/ath9k/xmit.c | 2 +-
drivers/net/wireless/intel/iwlegacy/4965-rs.c | 2 +-
drivers/net/wireless/intel/iwlegacy/common.c | 8 ++++----
drivers/net/wireless/intel/iwlwifi/dvm/rs.c | 6 +++---
drivers/net/wireless/intel/iwlwifi/dvm/sta.c | 10 +++++-----
drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c | 6 +++---
drivers/net/wireless/intel/iwlwifi/mvm/rs.c | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt7603/main.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt76x02_mac.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt7915/mcu.c | 4 ++--
drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 2 +-
include/net/mac80211.h | 4 ++--
net/mac80211/he.c | 4 ++--
net/mac80211/ht.c | 7 ++++---
net/mac80211/rc80211_minstrel_ht.c | 6 +++---
net/mac80211/rx.c | 4 ++--
net/mac80211/sta_info.c | 3 ++-
21 files changed, 46 insertions(+), 44 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 23381a9db6ae..e086db920a82 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -8520,7 +8520,7 @@ static void ath10k_sta_rc_update(struct ieee80211_hw *hw,
"mac sta rc update for %pM changed %08x bw %d nss %d smps %d\n",
sta->addr, changed, sta->deflink.bandwidth,
sta->deflink.rx_nss,
- sta->smps_mode);
+ sta->deflink.smps_mode);
if (changed & IEEE80211_RC_BW_CHANGED) {
bw = WMI_PEER_CHWIDTH_20MHZ;
@@ -8554,7 +8554,7 @@ static void ath10k_sta_rc_update(struct ieee80211_hw *hw,
if (changed & IEEE80211_RC_SMPS_CHANGED) {
smps = WMI_PEER_SMPS_PS_NONE;
- switch (sta->smps_mode) {
+ switch (sta->deflink.smps_mode) {
case IEEE80211_SMPS_AUTOMATIC:
case IEEE80211_SMPS_OFF:
smps = WMI_PEER_SMPS_PS_NONE;
@@ -8567,7 +8567,7 @@ static void ath10k_sta_rc_update(struct ieee80211_hw *hw,
break;
case IEEE80211_SMPS_NUM_MODES:
ath10k_warn(ar, "Invalid smps %d in sta rc update for %pM\n",
- sta->smps_mode, sta->addr);
+ sta->deflink.smps_mode, sta->addr);
smps = WMI_PEER_SMPS_PS_NONE;
break;
}
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 7e91e347c9ff..f5bf53d6af7e 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -4701,7 +4701,7 @@ static void ath11k_mac_op_sta_rc_update(struct ieee80211_hw *hw,
"mac sta rc update for %pM changed %08x bw %d nss %d smps %d\n",
sta->addr, changed, sta->deflink.bandwidth,
sta->deflink.rx_nss,
- sta->smps_mode);
+ sta->deflink.smps_mode);
spin_lock_bh(&ar->data_lock);
@@ -4737,7 +4737,7 @@ static void ath11k_mac_op_sta_rc_update(struct ieee80211_hw *hw,
if (changed & IEEE80211_RC_SMPS_CHANGED) {
smps = WMI_PEER_SMPS_PS_NONE;
- switch (sta->smps_mode) {
+ switch (sta->deflink.smps_mode) {
case IEEE80211_SMPS_AUTOMATIC:
case IEEE80211_SMPS_OFF:
smps = WMI_PEER_SMPS_PS_NONE;
@@ -4750,7 +4750,7 @@ static void ath11k_mac_op_sta_rc_update(struct ieee80211_hw *hw,
break;
default:
ath11k_warn(ar->ab, "Invalid smps %d in sta rc update for %pM\n",
- sta->smps_mode, sta->addr);
+ sta->deflink.smps_mode, sta->addr);
smps = WMI_PEER_SMPS_PS_NONE;
break;
}
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index ba16a7f3e23d..ba271a10d4ab 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2160,7 +2160,7 @@ static void setup_frame_info(struct ieee80211_hw *hw,
fi->keyix = an->ps_key;
else
fi->keyix = ATH9K_TXKEYIX_INVALID;
- fi->dyn_smps = sta && sta->smps_mode == IEEE80211_SMPS_DYNAMIC;
+ fi->dyn_smps = sta && sta->deflink.smps_mode == IEEE80211_SMPS_DYNAMIC;
fi->keytype = keytype;
fi->framelen = framelen;
fi->tx_power = txpower;
diff --git a/drivers/net/wireless/intel/iwlegacy/4965-rs.c b/drivers/net/wireless/intel/iwlegacy/4965-rs.c
index c62f299b9e0a..290e2c966ecb 100644
--- a/drivers/net/wireless/intel/iwlegacy/4965-rs.c
+++ b/drivers/net/wireless/intel/iwlegacy/4965-rs.c
@@ -1167,7 +1167,7 @@ il4965_rs_switch_to_mimo2(struct il_priv *il, struct il_lq_sta *lq_sta,
if (!conf_is_ht(conf) || !sta->deflink.ht_cap.ht_supported)
return -1;
- if (sta->smps_mode == IEEE80211_SMPS_STATIC)
+ if (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC)
return -1;
/* Need both Tx chains/antennas to support MIMO */
diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
index 04d27a26260b..341c17fe2af4 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.c
+++ b/drivers/net/wireless/intel/iwlegacy/common.c
@@ -1870,15 +1870,15 @@ il_set_ht_add_station(struct il_priv *il, u8 idx, struct ieee80211_sta *sta)
goto done;
D_ASSOC("spatial multiplexing power save mode: %s\n",
- (sta->smps_mode == IEEE80211_SMPS_STATIC) ? "static" :
- (sta->smps_mode == IEEE80211_SMPS_DYNAMIC) ? "dynamic" :
+ (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC) ? "static" :
+ (sta->deflink.smps_mode == IEEE80211_SMPS_DYNAMIC) ? "dynamic" :
"disabled");
sta_flags = il->stations[idx].sta.station_flags;
sta_flags &= ~(STA_FLG_RTS_MIMO_PROT_MSK | STA_FLG_MIMO_DIS_MSK);
- switch (sta->smps_mode) {
+ switch (sta->deflink.smps_mode) {
case IEEE80211_SMPS_STATIC:
sta_flags |= STA_FLG_MIMO_DIS_MSK;
break;
@@ -1888,7 +1888,7 @@ il_set_ht_add_station(struct il_priv *il, u8 idx, struct ieee80211_sta *sta)
case IEEE80211_SMPS_OFF:
break;
default:
- IL_WARN("Invalid MIMO PS mode %d\n", sta->smps_mode);
+ IL_WARN("Invalid MIMO PS mode %d\n", sta->deflink.smps_mode);
break;
}
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/rs.c b/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
index baffa1cbe8fc..687c906a9d72 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/rs.c
@@ -2,7 +2,7 @@
/******************************************************************************
*
* Copyright(c) 2005 - 2014 Intel Corporation. All rights reserved.
- * Copyright (C) 2019 - 2020 Intel Corporation
+ * Copyright (C) 2019 - 2020, 2022 Intel Corporation
*****************************************************************************/
#include <linux/kernel.h>
#include <linux/skbuff.h>
@@ -1242,7 +1242,7 @@ static int rs_switch_to_mimo2(struct iwl_priv *priv,
if (!conf_is_ht(conf) || !sta->deflink.ht_cap.ht_supported)
return -1;
- if (sta->smps_mode == IEEE80211_SMPS_STATIC)
+ if (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC)
return -1;
/* Need both Tx chains/antennas to support MIMO */
@@ -1297,7 +1297,7 @@ static int rs_switch_to_mimo3(struct iwl_priv *priv,
if (!conf_is_ht(conf) || !sta->deflink.ht_cap.ht_supported)
return -1;
- if (sta->smps_mode == IEEE80211_SMPS_STATIC)
+ if (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC)
return -1;
/* Need both Tx chains/antennas to support MIMO */
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/sta.c b/drivers/net/wireless/intel/iwlwifi/dvm/sta.c
index 476068c0abb7..cef43cf80620 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/sta.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/******************************************************************************
*
- * Copyright(c) 2003 - 2014 Intel Corporation. All rights reserved.
+ * Copyright(c) 2003 - 2014, 2022 Intel Corporation. All rights reserved.
*
* Portions of this file are derived from the ipw3945 project, as well
* as portions of the ieee80211 subsystem header files.
@@ -161,12 +161,12 @@ static void iwl_sta_calc_ht_flags(struct iwl_priv *priv,
IWL_DEBUG_INFO(priv, "STA %pM SM PS mode: %s\n",
sta->addr,
- (sta->smps_mode == IEEE80211_SMPS_STATIC) ?
+ (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC) ?
"static" :
- (sta->smps_mode == IEEE80211_SMPS_DYNAMIC) ?
+ (sta->deflink.smps_mode == IEEE80211_SMPS_DYNAMIC) ?
"dynamic" : "disabled");
- switch (sta->smps_mode) {
+ switch (sta->deflink.smps_mode) {
case IEEE80211_SMPS_STATIC:
*flags |= STA_FLG_MIMO_DIS_MSK;
break;
@@ -176,7 +176,7 @@ static void iwl_sta_calc_ht_flags(struct iwl_priv *priv,
case IEEE80211_SMPS_OFF:
break;
default:
- IWL_WARN(priv, "Invalid MIMO PS mode %d\n", sta->smps_mode);
+ IWL_WARN(priv, "Invalid MIMO PS mode %d\n", sta->deflink.smps_mode);
break;
}
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c
index d8c3d7ff4f44..752d44d96163 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs-fw.c
@@ -143,7 +143,7 @@ rs_fw_vht_set_enabled_rates(const struct ieee80211_sta *sta,
};
/* the station support only a single receive chain */
- if (sta->smps_mode == IEEE80211_SMPS_STATIC)
+ if (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC)
max_nss = 1;
for (i = 0; i < max_nss && i < IWL_TLC_NSS_MAX; i++) {
@@ -205,7 +205,7 @@ rs_fw_he_set_enabled_rates(const struct ieee80211_sta *sta,
u8 nss = sta->deflink.rx_nss;
/* the station support only a single receive chain */
- if (sta->smps_mode == IEEE80211_SMPS_STATIC)
+ if (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC)
nss = 1;
for (i = 0; i < nss && i < IWL_TLC_NSS_MAX; i++) {
@@ -270,7 +270,7 @@ static void rs_fw_set_supp_rates(struct ieee80211_sta *sta,
cpu_to_le16(ht_cap->mcs.rx_mask[0]);
/* the station support only a single receive chain */
- if (sta->smps_mode == IEEE80211_SMPS_STATIC)
+ if (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC)
cmd->ht_rates[IWL_TLC_NSS_2][IWL_TLC_MCS_PER_BW_80] =
0;
else
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
index a79043f30775..814a5e8f3666 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/rs.c
@@ -138,7 +138,7 @@ static bool rs_mimo_allow(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
if (!sta->deflink.ht_cap.ht_supported)
return false;
- if (sta->smps_mode == IEEE80211_SMPS_STATIC)
+ if (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC)
return false;
if (num_of_ant(iwl_mvm_get_valid_tx_ant(mvm)) < 2)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index ff0d3b3df140..cc92706b3d16 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -116,7 +116,7 @@ int iwl_mvm_sta_send_to_fw(struct iwl_mvm *mvm, struct ieee80211_sta *sta,
break;
}
- switch (sta->smps_mode) {
+ switch (sta->deflink.smps_mode) {
case IEEE80211_SMPS_AUTOMATIC:
case IEEE80211_SMPS_NUM_MODES:
WARN_ON(1);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/main.c b/drivers/net/wireless/mediatek/mt76/mt7603/main.c
index 051715ed90dd..ca50feb0b3a9 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/main.c
@@ -658,7 +658,7 @@ mt7603_sta_rate_tbl_update(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
mt7603_wtbl_set_rates(dev, msta, NULL, msta->rates);
msta->rate_probe = false;
mt7603_wtbl_set_smps(dev, msta,
- sta->smps_mode == IEEE80211_SMPS_DYNAMIC);
+ sta->deflink.smps_mode == IEEE80211_SMPS_DYNAMIC);
spin_unlock_bh(&dev->mt76.lock);
}
diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
index 9b17bd97ec09..5c4ca93bcf91 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.c
@@ -896,7 +896,7 @@ void mt76_connac_mcu_wtbl_smps_tlv(struct sk_buff *skb,
tlv = mt76_connac_mcu_add_nested_tlv(skb, WTBL_SMPS, sizeof(*smps),
wtbl_tlv, sta_wtbl);
smps = (struct wtbl_smps *)tlv;
- smps->smps = (sta->smps_mode == IEEE80211_SMPS_DYNAMIC);
+ smps->smps = (sta->deflink.smps_mode == IEEE80211_SMPS_DYNAMIC);
}
EXPORT_SYMBOL_GPL(mt76_connac_mcu_wtbl_smps_tlv);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
index de30cf5e2d2f..93d96739f802 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mac.c
@@ -404,7 +404,7 @@ void mt76x02_mac_write_txwi(struct mt76x02_dev *dev, struct mt76x02_txwi *txwi,
txwi->rate |= cpu_to_le16(MT_RXWI_RATE_LDPC);
if ((info->flags & IEEE80211_TX_CTL_STBC) && nss == 1)
txwi->rate |= cpu_to_le16(MT_RXWI_RATE_STBC);
- if (nss > 1 && sta && sta->smps_mode == IEEE80211_SMPS_DYNAMIC)
+ if (nss > 1 && sta && sta->deflink.smps_mode == IEEE80211_SMPS_DYNAMIC)
txwi_flags |= MT_TXWI_FLAGS_MMPS;
if (!(info->flags & IEEE80211_TX_CTL_NO_ACK))
txwi->ack_ctl |= MT_TXWI_ACK_CTL_REQ;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index f83067961945..1081b893f319 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -1304,7 +1304,7 @@ int mt7915_mcu_set_fixed_rate_ctrl(struct mt7915_dev *dev,
ra->phy = *phy;
break;
case RATE_PARAM_MMPS_UPDATE:
- ra->mmps_mode = mt7915_mcu_get_mmps_mode(sta->smps_mode);
+ ra->mmps_mode = mt7915_mcu_get_mmps_mode(sta->deflink.smps_mode);
break;
default:
break;
@@ -1459,7 +1459,7 @@ mt7915_mcu_sta_rate_ctrl_tlv(struct sk_buff *skb, struct mt7915_dev *dev,
ra->channel = chandef->chan->hw_value;
ra->bw = sta->deflink.bandwidth;
ra->phy.bw = sta->deflink.bandwidth;
- ra->mmps_mode = mt7915_mcu_get_mmps_mode(sta->smps_mode);
+ ra->mmps_mode = mt7915_mcu_get_mmps_mode(sta->deflink.smps_mode);
if (supp_rate) {
supp_rate &= mask->control[band].legacy;
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index 4d06038afd83..98df0aef8168 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -318,7 +318,7 @@ static void rt2x00queue_create_tx_descriptor_ht(struct rt2x00_dev *rt2x00dev,
* when using more then one tx stream (>MCS7).
*/
if (sta && txdesc->u.ht.mcs > 7 &&
- sta->smps_mode == IEEE80211_SMPS_DYNAMIC)
+ sta->deflink.smps_mode == IEEE80211_SMPS_DYNAMIC)
__set_bit(ENTRY_TXD_HT_MIMO_PS, &txdesc->flags);
} else {
txdesc->u.ht.mcs = rt2x00_get_rate_mcs(hwrate->mcs);
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ffd0ebbff294..d4e1d73d88cc 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2136,6 +2136,7 @@ struct ieee80211_sta_txpwr {
* in ieee80211_sta. For MLO Link STA this addr can be same or different
* from addr in ieee80211_sta (representing MLD STA addr)
* @link_id: the link ID for this link STA (0 for deflink)
+ * @smps_mode: current SMPS mode (off, static or dynamic)
* @supp_rates: Bitmap of supported rates
* @ht_cap: HT capabilities of this STA; restricted to our own capabilities
* @vht_cap: VHT capabilities of this STA; restricted to our own capabilities
@@ -2153,6 +2154,7 @@ struct ieee80211_sta_txpwr {
struct ieee80211_link_sta {
u8 addr[ETH_ALEN];
u8 link_id;
+ enum ieee80211_smps_mode smps_mode;
u32 supp_rates[NUM_NL80211_BANDS];
struct ieee80211_sta_ht_cap ht_cap;
@@ -2191,7 +2193,6 @@ struct ieee80211_link_sta {
* if wme is supported. The bits order is like in
* IEEE80211_WMM_IE_STA_QOSINFO_AC_*.
* @max_sp: max Service Period. Only valid if wme is supported.
- * @smps_mode: current SMPS mode (off, static or dynamic)
* @rates: rate control selection table
* @tdls: indicates whether the STA is a TDLS peer
* @tdls_initiator: indicates the STA is an initiator of the TDLS link. Only
@@ -2226,7 +2227,6 @@ struct ieee80211_sta {
bool wme;
u8 uapsd_queues;
u8 max_sp;
- enum ieee80211_smps_mode smps_mode;
struct ieee80211_sta_rates __rcu *rates;
bool tdls;
bool tdls_initiator;
diff --git a/net/mac80211/he.c b/net/mac80211/he.c
index d9228fd3f77a..e73899fd4bbb 100644
--- a/net/mac80211/he.c
+++ b/net/mac80211/he.c
@@ -31,9 +31,9 @@ ieee80211_update_from_he_6ghz_capa(const struct ieee80211_he_6ghz_capa *he_6ghz_
break;
}
- sta->sta.smps_mode = smps_mode;
+ link_sta->pub->smps_mode = smps_mode;
} else {
- sta->sta.smps_mode = IEEE80211_SMPS_OFF;
+ link_sta->pub->smps_mode = IEEE80211_SMPS_OFF;
}
switch (le16_get_bits(he_6ghz_capa->capa,
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c
index 8c24817cd497..12a233ba5031 100644
--- a/net/mac80211/ht.c
+++ b/net/mac80211/ht.c
@@ -299,12 +299,13 @@ bool ieee80211_ht_cap_ie_to_sta_ht_cap(struct ieee80211_sub_if_data *sdata,
break;
}
- if (smps_mode != sta->sta.smps_mode)
+ if (smps_mode != link_sta->pub->smps_mode)
changed = true;
- sta->sta.smps_mode = smps_mode;
+ link_sta->pub->smps_mode = smps_mode;
} else {
- sta->sta.smps_mode = IEEE80211_SMPS_OFF;
+ link_sta->pub->smps_mode = IEEE80211_SMPS_OFF;
}
+
return changed;
}
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 5f27e6746762..8c41a545a8b7 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (C) 2010-2013 Felix Fietkau <[email protected]>
- * Copyright (C) 2019-2021 Intel Corporation
+ * Copyright (C) 2019-2022 Intel Corporation
*/
#include <linux/netdevice.h>
#include <linux/types.h>
@@ -1478,7 +1478,7 @@ minstrel_ht_set_rate(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
* - for fallback rates, to increase chances of getting through
*/
if (offset > 0 ||
- (mi->sta->smps_mode == IEEE80211_SMPS_DYNAMIC &&
+ (mi->sta->deflink.smps_mode == IEEE80211_SMPS_DYNAMIC &&
group->streams > 1)) {
ratetbl->rate[offset].count = ratetbl->rate[offset].count_rts;
flags |= IEEE80211_TX_RC_USE_RTS_CTS;
@@ -1779,7 +1779,7 @@ minstrel_ht_update_caps(void *priv, struct ieee80211_supported_band *sband,
nss = minstrel_mcs_groups[i].streams;
/* Mark MCS > 7 as unsupported if STA is in static SMPS mode */
- if (sta->smps_mode == IEEE80211_SMPS_STATIC && nss > 1)
+ if (sta->deflink.smps_mode == IEEE80211_SMPS_STATIC && nss > 1)
continue;
/* HT rate */
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 38622446ea8a..af5b8f62164a 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3405,9 +3405,9 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
}
/* if no change do nothing */
- if (rx->sta->sta.smps_mode == smps_mode)
+ if (rx->link_sta->pub->smps_mode == smps_mode)
goto handled;
- rx->sta->sta.smps_mode = smps_mode;
+ rx->link_sta->pub->smps_mode = smps_mode;
sta_opmode.smps_mode =
ieee80211_smps_mode_to_smps_mode(smps_mode);
sta_opmode.changed = STA_OPMODE_SMPS_MODE_CHANGED;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 4944d929def6..815ab0cee503 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -475,6 +475,8 @@ static void sta_info_add_link(struct sta_info *sta,
link_sta->link_id = link_id;
rcu_assign_pointer(sta->link[link_id], link_info);
rcu_assign_pointer(sta->sta.link[link_id], link_sta);
+
+ link_sta->smps_mode = IEEE80211_SMPS_OFF;
}
static struct sta_info *
@@ -628,7 +630,6 @@ __sta_info_alloc(struct ieee80211_sub_if_data *sdata,
}
}
- sta->sta.smps_mode = IEEE80211_SMPS_OFF;
sta->sta.max_rc_amsdu_len = IEEE80211_MAX_MPDU_LEN_HT_BA;
sta->cparams.ce_threshold = CODEL_DISABLED_THRESHOLD;
--
2.37.2
From: Johannes Berg <[email protected]>
Add a link_id parameter to ieee80211_nullfunc_get() to be
able to obtain a correctly addressed frame.
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/ath/ath9k/channel.c | 2 +-
drivers/net/wireless/realtek/rtw88/fw.c | 4 +--
drivers/net/wireless/st/cw1200/sta.c | 4 +--
drivers/net/wireless/ti/wl1251/main.c | 2 +-
drivers/net/wireless/ti/wlcore/cmd.c | 4 +--
include/net/mac80211.h | 5 ++-
net/mac80211/mlme.c | 5 +--
net/mac80211/tx.c | 43 +++++++++++++++---------
8 files changed, 43 insertions(+), 26 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
index 6cf087522157..571062f2e82a 100644
--- a/drivers/net/wireless/ath/ath9k/channel.c
+++ b/drivers/net/wireless/ath/ath9k/channel.c
@@ -1113,7 +1113,7 @@ ath_chanctx_send_vif_ps_frame(struct ath_softc *sc, struct ath_vif *avp,
if (!avp->assoc)
return false;
- skb = ieee80211_nullfunc_get(sc->hw, vif, false);
+ skb = ieee80211_nullfunc_get(sc->hw, vif, -1, false);
if (!skb)
return false;
diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index babba68a7132..7f6fdebae203 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -1082,10 +1082,10 @@ static struct sk_buff *rtw_get_rsvd_page_skb(struct ieee80211_hw *hw,
skb_new = ieee80211_proberesp_get(hw, vif);
break;
case RSVD_NULL:
- skb_new = ieee80211_nullfunc_get(hw, vif, false);
+ skb_new = ieee80211_nullfunc_get(hw, vif, -1, false);
break;
case RSVD_QOS_NULL:
- skb_new = ieee80211_nullfunc_get(hw, vif, true);
+ skb_new = ieee80211_nullfunc_get(hw, vif, -1, true);
break;
case RSVD_LPS_PG_DPK:
skb_new = rtw_lps_pg_dpk_get(hw);
diff --git a/drivers/net/wireless/st/cw1200/sta.c b/drivers/net/wireless/st/cw1200/sta.c
index 26d3614519b1..8ef1d06b9bbd 100644
--- a/drivers/net/wireless/st/cw1200/sta.c
+++ b/drivers/net/wireless/st/cw1200/sta.c
@@ -195,7 +195,7 @@ void __cw1200_cqm_bssloss_sm(struct cw1200_common *priv,
priv->bss_loss_state++;
- skb = ieee80211_nullfunc_get(priv->hw, priv->vif, false);
+ skb = ieee80211_nullfunc_get(priv->hw, priv->vif, -1, false);
WARN_ON(!skb);
if (skb)
cw1200_tx(priv->hw, NULL, skb);
@@ -2263,7 +2263,7 @@ static int cw1200_upload_null(struct cw1200_common *priv)
.rate = 0xFF,
};
- frame.skb = ieee80211_nullfunc_get(priv->hw, priv->vif, false);
+ frame.skb = ieee80211_nullfunc_get(priv->hw, priv->vif,-1, false);
if (!frame.skb)
return -ENOMEM;
diff --git a/drivers/net/wireless/ti/wl1251/main.c b/drivers/net/wireless/ti/wl1251/main.c
index 9144ef5538a8..289371689a8d 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -546,7 +546,7 @@ static int wl1251_build_null_data(struct wl1251 *wl)
size = sizeof(struct wl12xx_null_data_template);
ptr = NULL;
} else {
- skb = ieee80211_nullfunc_get(wl->hw, wl->vif, false);
+ skb = ieee80211_nullfunc_get(wl->hw, wl->vif, -1, false);
if (!skb)
goto out;
size = skb->len;
diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index 138edd28b0de..a939fd89a7f5 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -1065,7 +1065,7 @@ int wl12xx_cmd_build_null_data(struct wl1271 *wl, struct wl12xx_vif *wlvif)
} else {
skb = ieee80211_nullfunc_get(wl->hw,
wl12xx_wlvif_to_vif(wlvif),
- false);
+ -1, false);
if (!skb)
goto out;
size = skb->len;
@@ -1092,7 +1092,7 @@ int wl12xx_cmd_build_klv_null_data(struct wl1271 *wl,
struct sk_buff *skb = NULL;
int ret = -ENOMEM;
- skb = ieee80211_nullfunc_get(wl->hw, vif, false);
+ skb = ieee80211_nullfunc_get(wl->hw, vif,-1, false);
if (!skb)
goto out;
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 954cc029a9f9..bfa6a1625c5c 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5298,6 +5298,9 @@ struct sk_buff *ieee80211_pspoll_get(struct ieee80211_hw *hw,
* ieee80211_nullfunc_get - retrieve a nullfunc template
* @hw: pointer obtained from ieee80211_alloc_hw().
* @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @link_id: If the vif is an MLD, get a frame with the link addresses
+ * for the given link ID. For a link_id < 0 you get a frame with
+ * MLD addresses, however useful that might be.
* @qos_ok: QoS NDP is acceptable to the caller, this should be set
* if at all possible
*
@@ -5315,7 +5318,7 @@ struct sk_buff *ieee80211_pspoll_get(struct ieee80211_hw *hw,
*/
struct sk_buff *ieee80211_nullfunc_get(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
- bool qos_ok);
+ int link_id, bool qos_ok);
/**
* ieee80211_probereq_get - retrieve a Probe Request template
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index bbc62815b067..bf883f2e6de3 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1546,8 +1546,9 @@ void ieee80211_send_nullfunc(struct ieee80211_local *local,
struct ieee80211_hdr_3addr *nullfunc;
struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
- skb = ieee80211_nullfunc_get(&local->hw, &sdata->vif,
- !ieee80211_hw_check(&local->hw, DOESNT_SUPPORT_QOS_NDP));
+ skb = ieee80211_nullfunc_get(&local->hw, &sdata->vif, -1,
+ !ieee80211_hw_check(&local->hw,
+ DOESNT_SUPPORT_QOS_NDP));
if (!skb)
return;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 51d564c8742b..30293e49e444 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -5469,33 +5469,39 @@ EXPORT_SYMBOL(ieee80211_pspoll_get);
struct sk_buff *ieee80211_nullfunc_get(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
- bool qos_ok)
+ int link_id, bool qos_ok)
{
+ struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+ struct ieee80211_local *local = sdata->local;
+ struct ieee80211_link_data *link = NULL;
struct ieee80211_hdr_3addr *nullfunc;
- struct ieee80211_sub_if_data *sdata;
- struct ieee80211_local *local;
struct sk_buff *skb;
bool qos = false;
if (WARN_ON(vif->type != NL80211_IFTYPE_STATION))
return NULL;
- sdata = vif_to_sdata(vif);
- local = sdata->local;
+ skb = dev_alloc_skb(local->hw.extra_tx_headroom +
+ sizeof(*nullfunc) + 2);
+ if (!skb)
+ return NULL;
+ rcu_read_lock();
if (qos_ok) {
struct sta_info *sta;
- rcu_read_lock();
- sta = sta_info_get(sdata, sdata->deflink.u.mgd.bssid);
+ sta = sta_info_get(sdata, vif->cfg.ap_addr);
qos = sta && sta->sta.wme;
- rcu_read_unlock();
}
- skb = dev_alloc_skb(local->hw.extra_tx_headroom +
- sizeof(*nullfunc) + 2);
- if (!skb)
- return NULL;
+ if (link_id >= 0) {
+ link = rcu_dereference(sdata->link[link_id]);
+ if (WARN_ON_ONCE(!link)) {
+ rcu_read_unlock();
+ kfree_skb(skb);
+ return NULL;
+ }
+ }
skb_reserve(skb, local->hw.extra_tx_headroom);
@@ -5516,9 +5522,16 @@ struct sk_buff *ieee80211_nullfunc_get(struct ieee80211_hw *hw,
skb_put_data(skb, &qoshdr, sizeof(qoshdr));
}
- memcpy(nullfunc->addr1, sdata->deflink.u.mgd.bssid, ETH_ALEN);
- memcpy(nullfunc->addr2, vif->addr, ETH_ALEN);
- memcpy(nullfunc->addr3, sdata->deflink.u.mgd.bssid, ETH_ALEN);
+ if (link) {
+ memcpy(nullfunc->addr1, link->conf->bssid, ETH_ALEN);
+ memcpy(nullfunc->addr2, link->conf->addr, ETH_ALEN);
+ memcpy(nullfunc->addr3, link->conf->bssid, ETH_ALEN);
+ } else {
+ memcpy(nullfunc->addr1, vif->cfg.ap_addr, ETH_ALEN);
+ memcpy(nullfunc->addr2, vif->addr, ETH_ALEN);
+ memcpy(nullfunc->addr3, vif->cfg.ap_addr, ETH_ALEN);
+ }
+ rcu_read_unlock();
return skb;
}
--
2.37.2
From: Johannes Berg <[email protected]>
We probably should've done that originally, we already have
about 300 lines of code there, and will add more. Move all
the link code we wrote to a new file.
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/Makefile | 1 +
net/mac80211/ieee80211_i.h | 14 +-
net/mac80211/iface.c | 251 -----------------------------------
net/mac80211/link.c | 262 +++++++++++++++++++++++++++++++++++++
4 files changed, 274 insertions(+), 254 deletions(-)
create mode 100644 net/mac80211/link.c
diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile
index af1df3a6bd55..b8de44da1fb8 100644
--- a/net/mac80211/Makefile
+++ b/net/mac80211/Makefile
@@ -16,6 +16,7 @@ mac80211-y := \
s1g.o \
ibss.o \
iface.o \
+ link.o \
rate.o \
michael.o \
tkip.o \
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e902feecfad4..b41c49338cd3 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1930,9 +1930,6 @@ void ieee80211_sdata_stop(struct ieee80211_sub_if_data *sdata);
int ieee80211_add_virtual_monitor(struct ieee80211_local *local);
void ieee80211_del_virtual_monitor(struct ieee80211_local *local);
-int ieee80211_vif_set_links(struct ieee80211_sub_if_data *sdata,
- u16 new_links);
-
bool __ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata);
void ieee80211_recalc_txpower(struct ieee80211_sub_if_data *sdata,
bool update_bss);
@@ -1943,6 +1940,17 @@ static inline bool ieee80211_sdata_running(struct ieee80211_sub_if_data *sdata)
return test_bit(SDATA_STATE_RUNNING, &sdata->state);
}
+/* link handling */
+void ieee80211_link_setup(struct ieee80211_link_data *link);
+void ieee80211_link_init(struct ieee80211_sub_if_data *sdata,
+ int link_id,
+ struct ieee80211_link_data *link,
+ struct ieee80211_bss_conf *link_conf);
+void ieee80211_link_stop(struct ieee80211_link_data *link);
+int ieee80211_vif_set_links(struct ieee80211_sub_if_data *sdata,
+ u16 new_links);
+void ieee80211_vif_clear_links(struct ieee80211_sub_if_data *sdata);
+
/* tx handling */
void ieee80211_clear_tx_pending(struct ieee80211_local *local);
void ieee80211_tx_pending(struct tasklet_struct *t);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 14505278073a..bf1c44d53c30 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -368,257 +368,6 @@ static int ieee80211_open(struct net_device *dev)
return err;
}
-static void ieee80211_link_setup(struct ieee80211_link_data *link)
-{
- if (link->sdata->vif.type == NL80211_IFTYPE_STATION)
- ieee80211_mgd_setup_link(link);
-}
-
-static void ieee80211_link_init(struct ieee80211_sub_if_data *sdata,
- int link_id,
- struct ieee80211_link_data *link,
- struct ieee80211_bss_conf *link_conf)
-{
- bool deflink = link_id < 0;
-
- if (link_id < 0)
- link_id = 0;
-
- rcu_assign_pointer(sdata->vif.link_conf[link_id], link_conf);
- rcu_assign_pointer(sdata->link[link_id], link);
-
- link->sdata = sdata;
- link->link_id = link_id;
- link->conf = link_conf;
- link_conf->link_id = link_id;
-
- INIT_WORK(&link->csa_finalize_work,
- ieee80211_csa_finalize_work);
- INIT_WORK(&link->color_change_finalize_work,
- ieee80211_color_change_finalize_work);
- INIT_LIST_HEAD(&link->assigned_chanctx_list);
- INIT_LIST_HEAD(&link->reserved_chanctx_list);
- INIT_DELAYED_WORK(&link->dfs_cac_timer_work,
- ieee80211_dfs_cac_timer_work);
-
- if (!deflink) {
- switch (sdata->vif.type) {
- case NL80211_IFTYPE_AP:
- ether_addr_copy(link_conf->addr,
- sdata->wdev.links[link_id].addr);
- link_conf->bssid = link_conf->addr;
- WARN_ON(!(sdata->wdev.valid_links & BIT(link_id)));
- break;
- case NL80211_IFTYPE_STATION:
- /* station sets the bssid in ieee80211_mgd_setup_link */
- break;
- default:
- WARN_ON(1);
- }
- }
-}
-
-static void ieee80211_link_stop(struct ieee80211_link_data *link)
-{
- if (link->sdata->vif.type == NL80211_IFTYPE_STATION)
- ieee80211_mgd_stop_link(link);
-
- ieee80211_link_release_channel(link);
-}
-
-struct link_container {
- struct ieee80211_link_data data;
- struct ieee80211_bss_conf conf;
-};
-
-static void ieee80211_free_links(struct ieee80211_sub_if_data *sdata,
- struct link_container **links)
-{
- LIST_HEAD(keys);
- unsigned int link_id;
-
- for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
- if (!links[link_id])
- continue;
- ieee80211_remove_link_keys(&links[link_id]->data, &keys);
- }
-
- synchronize_rcu();
-
- ieee80211_free_key_list(sdata->local, &keys);
-
- for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
- if (!links[link_id])
- continue;
- ieee80211_link_stop(&links[link_id]->data);
- kfree(links[link_id]);
- }
-}
-
-static int ieee80211_check_dup_link_addrs(struct ieee80211_sub_if_data *sdata)
-{
- unsigned int i, j;
-
- for (i = 0; i < IEEE80211_MLD_MAX_NUM_LINKS; i++) {
- struct ieee80211_link_data *link1;
-
- link1 = sdata_dereference(sdata->link[i], sdata);
- if (!link1)
- continue;
- for (j = i + 1; j < IEEE80211_MLD_MAX_NUM_LINKS; j++) {
- struct ieee80211_link_data *link2;
-
- link2 = sdata_dereference(sdata->link[j], sdata);
- if (!link2)
- continue;
-
- if (ether_addr_equal(link1->conf->addr,
- link2->conf->addr))
- return -EALREADY;
- }
- }
-
- return 0;
-}
-
-static int ieee80211_vif_update_links(struct ieee80211_sub_if_data *sdata,
- struct link_container **to_free,
- u16 new_links)
-{
- u16 old_links = sdata->vif.valid_links;
- unsigned long add = new_links & ~old_links;
- unsigned long rem = old_links & ~new_links;
- unsigned int link_id;
- int ret;
- struct link_container *links[IEEE80211_MLD_MAX_NUM_LINKS] = {}, *link;
- struct ieee80211_bss_conf *old[IEEE80211_MLD_MAX_NUM_LINKS];
- struct ieee80211_link_data *old_data[IEEE80211_MLD_MAX_NUM_LINKS];
- bool use_deflink = old_links == 0; /* set for error case */
-
- sdata_assert_lock(sdata);
-
- memset(to_free, 0, sizeof(links));
-
- if (old_links == new_links)
- return 0;
-
- /* if there were no old links, need to clear the pointers to deflink */
- if (!old_links)
- rem |= BIT(0);
-
- /* allocate new link structures first */
- for_each_set_bit(link_id, &add, IEEE80211_MLD_MAX_NUM_LINKS) {
- link = kzalloc(sizeof(*link), GFP_KERNEL);
- if (!link) {
- ret = -ENOMEM;
- goto free;
- }
- links[link_id] = link;
- }
-
- /* keep track of the old pointers for the driver */
- BUILD_BUG_ON(sizeof(old) != sizeof(sdata->vif.link_conf));
- memcpy(old, sdata->vif.link_conf, sizeof(old));
- /* and for us in error cases */
- BUILD_BUG_ON(sizeof(old_data) != sizeof(sdata->link));
- memcpy(old_data, sdata->link, sizeof(old_data));
-
- /* grab old links to free later */
- for_each_set_bit(link_id, &rem, IEEE80211_MLD_MAX_NUM_LINKS) {
- if (rcu_access_pointer(sdata->link[link_id]) != &sdata->deflink) {
- /*
- * we must have allocated the data through this path so
- * we know we can free both at the same time
- */
- to_free[link_id] = container_of(rcu_access_pointer(sdata->link[link_id]),
- typeof(*links[link_id]),
- data);
- }
-
- RCU_INIT_POINTER(sdata->link[link_id], NULL);
- RCU_INIT_POINTER(sdata->vif.link_conf[link_id], NULL);
- }
-
- /* link them into data structures */
- for_each_set_bit(link_id, &add, IEEE80211_MLD_MAX_NUM_LINKS) {
- WARN_ON(!use_deflink &&
- rcu_access_pointer(sdata->link[link_id]) == &sdata->deflink);
-
- link = links[link_id];
- ieee80211_link_init(sdata, link_id, &link->data, &link->conf);
- ieee80211_link_setup(&link->data);
- }
-
- if (new_links == 0)
- ieee80211_link_init(sdata, -1, &sdata->deflink,
- &sdata->vif.bss_conf);
-
- sdata->vif.valid_links = new_links;
-
- ret = ieee80211_check_dup_link_addrs(sdata);
- if (!ret) {
- /* tell the driver */
- ret = drv_change_vif_links(sdata->local, sdata,
- old_links, new_links,
- old);
- }
-
- if (ret) {
- /* restore config */
- memcpy(sdata->link, old_data, sizeof(old_data));
- memcpy(sdata->vif.link_conf, old, sizeof(old));
- sdata->vif.valid_links = old_links;
- /* and free (only) the newly allocated links */
- memset(to_free, 0, sizeof(links));
- goto free;
- }
-
- /* use deflink/bss_conf again if and only if there are no more links */
- use_deflink = new_links == 0;
-
- goto deinit;
-free:
- /* if we failed during allocation, only free all */
- for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
- kfree(links[link_id]);
- links[link_id] = NULL;
- }
-deinit:
- if (use_deflink)
- ieee80211_link_init(sdata, -1, &sdata->deflink,
- &sdata->vif.bss_conf);
- return ret;
-}
-
-int ieee80211_vif_set_links(struct ieee80211_sub_if_data *sdata,
- u16 new_links)
-{
- struct link_container *links[IEEE80211_MLD_MAX_NUM_LINKS];
- int ret;
-
- ret = ieee80211_vif_update_links(sdata, links, new_links);
- ieee80211_free_links(sdata, links);
-
- return ret;
-}
-
-static void ieee80211_vif_clear_links(struct ieee80211_sub_if_data *sdata)
-{
- struct link_container *links[IEEE80211_MLD_MAX_NUM_LINKS];
-
- /*
- * The locking here is different because when we free links
- * in the station case we need to be able to cancel_work_sync()
- * something that also takes the lock.
- */
-
- sdata_lock(sdata);
- ieee80211_vif_update_links(sdata, links, 0);
- sdata_unlock(sdata);
-
- ieee80211_free_links(sdata, links);
-}
-
static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, bool going_down)
{
struct ieee80211_local *local = sdata->local;
diff --git a/net/mac80211/link.c b/net/mac80211/link.c
new file mode 100644
index 000000000000..096f313c2a6e
--- /dev/null
+++ b/net/mac80211/link.c
@@ -0,0 +1,262 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MLO link handling
+ *
+ * Copyright (C) 2022 Intel Corporation
+ */
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <net/mac80211.h>
+#include "ieee80211_i.h"
+#include "driver-ops.h"
+
+void ieee80211_link_setup(struct ieee80211_link_data *link)
+{
+ if (link->sdata->vif.type == NL80211_IFTYPE_STATION)
+ ieee80211_mgd_setup_link(link);
+}
+
+void ieee80211_link_init(struct ieee80211_sub_if_data *sdata,
+ int link_id,
+ struct ieee80211_link_data *link,
+ struct ieee80211_bss_conf *link_conf)
+{
+ bool deflink = link_id < 0;
+
+ if (link_id < 0)
+ link_id = 0;
+
+ rcu_assign_pointer(sdata->vif.link_conf[link_id], link_conf);
+ rcu_assign_pointer(sdata->link[link_id], link);
+
+ link->sdata = sdata;
+ link->link_id = link_id;
+ link->conf = link_conf;
+ link_conf->link_id = link_id;
+
+ INIT_WORK(&link->csa_finalize_work,
+ ieee80211_csa_finalize_work);
+ INIT_WORK(&link->color_change_finalize_work,
+ ieee80211_color_change_finalize_work);
+ INIT_LIST_HEAD(&link->assigned_chanctx_list);
+ INIT_LIST_HEAD(&link->reserved_chanctx_list);
+ INIT_DELAYED_WORK(&link->dfs_cac_timer_work,
+ ieee80211_dfs_cac_timer_work);
+
+ if (!deflink) {
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_AP:
+ ether_addr_copy(link_conf->addr,
+ sdata->wdev.links[link_id].addr);
+ link_conf->bssid = link_conf->addr;
+ WARN_ON(!(sdata->wdev.valid_links & BIT(link_id)));
+ break;
+ case NL80211_IFTYPE_STATION:
+ /* station sets the bssid in ieee80211_mgd_setup_link */
+ break;
+ default:
+ WARN_ON(1);
+ }
+ }
+}
+
+void ieee80211_link_stop(struct ieee80211_link_data *link)
+{
+ if (link->sdata->vif.type == NL80211_IFTYPE_STATION)
+ ieee80211_mgd_stop_link(link);
+
+ ieee80211_link_release_channel(link);
+}
+
+struct link_container {
+ struct ieee80211_link_data data;
+ struct ieee80211_bss_conf conf;
+};
+
+static void ieee80211_free_links(struct ieee80211_sub_if_data *sdata,
+ struct link_container **links)
+{
+ LIST_HEAD(keys);
+ unsigned int link_id;
+
+ for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
+ if (!links[link_id])
+ continue;
+ ieee80211_remove_link_keys(&links[link_id]->data, &keys);
+ }
+
+ synchronize_rcu();
+
+ ieee80211_free_key_list(sdata->local, &keys);
+
+ for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
+ if (!links[link_id])
+ continue;
+ ieee80211_link_stop(&links[link_id]->data);
+ kfree(links[link_id]);
+ }
+}
+
+static int ieee80211_check_dup_link_addrs(struct ieee80211_sub_if_data *sdata)
+{
+ unsigned int i, j;
+
+ for (i = 0; i < IEEE80211_MLD_MAX_NUM_LINKS; i++) {
+ struct ieee80211_link_data *link1;
+
+ link1 = sdata_dereference(sdata->link[i], sdata);
+ if (!link1)
+ continue;
+ for (j = i + 1; j < IEEE80211_MLD_MAX_NUM_LINKS; j++) {
+ struct ieee80211_link_data *link2;
+
+ link2 = sdata_dereference(sdata->link[j], sdata);
+ if (!link2)
+ continue;
+
+ if (ether_addr_equal(link1->conf->addr,
+ link2->conf->addr))
+ return -EALREADY;
+ }
+ }
+
+ return 0;
+}
+
+static int ieee80211_vif_update_links(struct ieee80211_sub_if_data *sdata,
+ struct link_container **to_free,
+ u16 new_links)
+{
+ u16 old_links = sdata->vif.valid_links;
+ unsigned long add = new_links & ~old_links;
+ unsigned long rem = old_links & ~new_links;
+ unsigned int link_id;
+ int ret;
+ struct link_container *links[IEEE80211_MLD_MAX_NUM_LINKS] = {}, *link;
+ struct ieee80211_bss_conf *old[IEEE80211_MLD_MAX_NUM_LINKS];
+ struct ieee80211_link_data *old_data[IEEE80211_MLD_MAX_NUM_LINKS];
+ bool use_deflink = old_links == 0; /* set for error case */
+
+ sdata_assert_lock(sdata);
+
+ memset(to_free, 0, sizeof(links));
+
+ if (old_links == new_links)
+ return 0;
+
+ /* if there were no old links, need to clear the pointers to deflink */
+ if (!old_links)
+ rem |= BIT(0);
+
+ /* allocate new link structures first */
+ for_each_set_bit(link_id, &add, IEEE80211_MLD_MAX_NUM_LINKS) {
+ link = kzalloc(sizeof(*link), GFP_KERNEL);
+ if (!link) {
+ ret = -ENOMEM;
+ goto free;
+ }
+ links[link_id] = link;
+ }
+
+ /* keep track of the old pointers for the driver */
+ BUILD_BUG_ON(sizeof(old) != sizeof(sdata->vif.link_conf));
+ memcpy(old, sdata->vif.link_conf, sizeof(old));
+ /* and for us in error cases */
+ BUILD_BUG_ON(sizeof(old_data) != sizeof(sdata->link));
+ memcpy(old_data, sdata->link, sizeof(old_data));
+
+ /* grab old links to free later */
+ for_each_set_bit(link_id, &rem, IEEE80211_MLD_MAX_NUM_LINKS) {
+ if (rcu_access_pointer(sdata->link[link_id]) != &sdata->deflink) {
+ /*
+ * we must have allocated the data through this path so
+ * we know we can free both at the same time
+ */
+ to_free[link_id] = container_of(rcu_access_pointer(sdata->link[link_id]),
+ typeof(*links[link_id]),
+ data);
+ }
+
+ RCU_INIT_POINTER(sdata->link[link_id], NULL);
+ RCU_INIT_POINTER(sdata->vif.link_conf[link_id], NULL);
+ }
+
+ /* link them into data structures */
+ for_each_set_bit(link_id, &add, IEEE80211_MLD_MAX_NUM_LINKS) {
+ WARN_ON(!use_deflink &&
+ rcu_access_pointer(sdata->link[link_id]) == &sdata->deflink);
+
+ link = links[link_id];
+ ieee80211_link_init(sdata, link_id, &link->data, &link->conf);
+ ieee80211_link_setup(&link->data);
+ }
+
+ if (new_links == 0)
+ ieee80211_link_init(sdata, -1, &sdata->deflink,
+ &sdata->vif.bss_conf);
+
+ sdata->vif.valid_links = new_links;
+
+ ret = ieee80211_check_dup_link_addrs(sdata);
+ if (!ret) {
+ /* tell the driver */
+ ret = drv_change_vif_links(sdata->local, sdata,
+ old_links, new_links,
+ old);
+ }
+
+ if (ret) {
+ /* restore config */
+ memcpy(sdata->link, old_data, sizeof(old_data));
+ memcpy(sdata->vif.link_conf, old, sizeof(old));
+ sdata->vif.valid_links = old_links;
+ /* and free (only) the newly allocated links */
+ memset(to_free, 0, sizeof(links));
+ goto free;
+ }
+
+ /* use deflink/bss_conf again if and only if there are no more links */
+ use_deflink = new_links == 0;
+
+ goto deinit;
+free:
+ /* if we failed during allocation, only free all */
+ for (link_id = 0; link_id < IEEE80211_MLD_MAX_NUM_LINKS; link_id++) {
+ kfree(links[link_id]);
+ links[link_id] = NULL;
+ }
+deinit:
+ if (use_deflink)
+ ieee80211_link_init(sdata, -1, &sdata->deflink,
+ &sdata->vif.bss_conf);
+ return ret;
+}
+
+int ieee80211_vif_set_links(struct ieee80211_sub_if_data *sdata,
+ u16 new_links)
+{
+ struct link_container *links[IEEE80211_MLD_MAX_NUM_LINKS];
+ int ret;
+
+ ret = ieee80211_vif_update_links(sdata, links, new_links);
+ ieee80211_free_links(sdata, links);
+
+ return ret;
+}
+
+void ieee80211_vif_clear_links(struct ieee80211_sub_if_data *sdata)
+{
+ struct link_container *links[IEEE80211_MLD_MAX_NUM_LINKS];
+
+ /*
+ * The locking here is different because when we free links
+ * in the station case we need to be able to cancel_work_sync()
+ * something that also takes the lock.
+ */
+
+ sdata_lock(sdata);
+ ieee80211_vif_update_links(sdata, links, 0);
+ sdata_unlock(sdata);
+
+ ieee80211_free_links(sdata, links);
+}
--
2.37.2
From: Benjamin Berg <[email protected]>
Use rx->link_sta everywhere instead of accessing the default link.
Signed-off-by: Benjamin Berg <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/rx.c | 69 ++++++++++++++++++++++++-----------------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 511c809e2c6b..38622446ea8a 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1452,7 +1452,7 @@ ieee80211_rx_h_check_dup(struct ieee80211_rx_data *rx)
if (unlikely(ieee80211_has_retry(hdr->frame_control) &&
rx->sta->last_seq_ctrl[rx->seqno_idx] == hdr->seq_ctrl)) {
I802_DEBUG_INC(rx->local->dot11FrameDuplicateCount);
- rx->sta->deflink.rx_stats.num_duplicates++;
+ rx->link_sta->rx_stats.num_duplicates++;
return RX_DROP_UNUSABLE;
} else if (!(status->flag & RX_FLAG_AMSDU_MORE)) {
rx->sta->last_seq_ctrl[rx->seqno_idx] = hdr->seq_ctrl;
@@ -1731,12 +1731,13 @@ static ieee80211_rx_result debug_noinline
ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
{
struct sta_info *sta = rx->sta;
+ struct link_sta_info *link_sta = rx->link_sta;
struct sk_buff *skb = rx->skb;
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
int i;
- if (!sta)
+ if (!sta || !link_sta)
return RX_CONTINUE;
/*
@@ -1752,47 +1753,47 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
NL80211_IFTYPE_ADHOC);
if (ether_addr_equal(bssid, rx->sdata->u.ibss.bssid) &&
test_sta_flag(sta, WLAN_STA_AUTHORIZED)) {
- sta->deflink.rx_stats.last_rx = jiffies;
+ link_sta->rx_stats.last_rx = jiffies;
if (ieee80211_is_data(hdr->frame_control) &&
!is_multicast_ether_addr(hdr->addr1))
- sta->deflink.rx_stats.last_rate =
+ link_sta->rx_stats.last_rate =
sta_stats_encode_rate(status);
}
} else if (rx->sdata->vif.type == NL80211_IFTYPE_OCB) {
- sta->deflink.rx_stats.last_rx = jiffies;
+ link_sta->rx_stats.last_rx = jiffies;
} else if (!ieee80211_is_s1g_beacon(hdr->frame_control) &&
!is_multicast_ether_addr(hdr->addr1)) {
/*
* Mesh beacons will update last_rx when if they are found to
* match the current local configuration when processed.
*/
- sta->deflink.rx_stats.last_rx = jiffies;
+ link_sta->rx_stats.last_rx = jiffies;
if (ieee80211_is_data(hdr->frame_control))
- sta->deflink.rx_stats.last_rate = sta_stats_encode_rate(status);
+ link_sta->rx_stats.last_rate = sta_stats_encode_rate(status);
}
- sta->deflink.rx_stats.fragments++;
+ link_sta->rx_stats.fragments++;
- u64_stats_update_begin(&rx->sta->deflink.rx_stats.syncp);
- sta->deflink.rx_stats.bytes += rx->skb->len;
- u64_stats_update_end(&rx->sta->deflink.rx_stats.syncp);
+ u64_stats_update_begin(&link_sta->rx_stats.syncp);
+ link_sta->rx_stats.bytes += rx->skb->len;
+ u64_stats_update_end(&link_sta->rx_stats.syncp);
if (!(status->flag & RX_FLAG_NO_SIGNAL_VAL)) {
- sta->deflink.rx_stats.last_signal = status->signal;
- ewma_signal_add(&sta->deflink.rx_stats_avg.signal,
+ link_sta->rx_stats.last_signal = status->signal;
+ ewma_signal_add(&link_sta->rx_stats_avg.signal,
-status->signal);
}
if (status->chains) {
- sta->deflink.rx_stats.chains = status->chains;
+ link_sta->rx_stats.chains = status->chains;
for (i = 0; i < ARRAY_SIZE(status->chain_signal); i++) {
int signal = status->chain_signal[i];
if (!(status->chains & BIT(i)))
continue;
- sta->deflink.rx_stats.chain_signal_last[i] = signal;
- ewma_signal_add(&sta->deflink.rx_stats_avg.chain_signal[i],
+ link_sta->rx_stats.chain_signal_last[i] = signal;
+ ewma_signal_add(&link_sta->rx_stats_avg.chain_signal[i],
-signal);
}
}
@@ -1853,7 +1854,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
* Update counter and free packet here to avoid
* counting this as a dropped packed.
*/
- sta->deflink.rx_stats.packets++;
+ link_sta->rx_stats.packets++;
dev_kfree_skb(rx->skb);
return RX_QUEUED;
}
@@ -2389,7 +2390,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
out:
ieee80211_led_rx(rx->local);
if (rx->sta)
- rx->sta->deflink.rx_stats.packets++;
+ rx->link_sta->rx_stats.packets++;
return RX_CONTINUE;
}
@@ -2665,9 +2666,9 @@ ieee80211_deliver_skb(struct ieee80211_rx_data *rx)
* for non-QoS-data frames. Here we know it's a data
* frame, so count MSDUs.
*/
- u64_stats_update_begin(&rx->sta->deflink.rx_stats.syncp);
- rx->sta->deflink.rx_stats.msdu[rx->seqno_idx]++;
- u64_stats_update_end(&rx->sta->deflink.rx_stats.syncp);
+ u64_stats_update_begin(&rx->link_sta->rx_stats.syncp);
+ rx->link_sta->rx_stats.msdu[rx->seqno_idx]++;
+ u64_stats_update_end(&rx->link_sta->rx_stats.syncp);
}
if ((sdata->vif.type == NL80211_IFTYPE_AP ||
@@ -3364,7 +3365,7 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
switch (mgmt->u.action.category) {
case WLAN_CATEGORY_HT:
/* reject HT action frames from stations not supporting HT */
- if (!rx->sta->sta.deflink.ht_cap.ht_supported)
+ if (!rx->link_sta->pub->ht_cap.ht_supported)
goto invalid;
if (sdata->vif.type != NL80211_IFTYPE_STATION &&
@@ -3428,26 +3429,26 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
struct sta_opmode_info sta_opmode = {};
/* If it doesn't support 40 MHz it can't change ... */
- if (!(rx->sta->sta.deflink.ht_cap.cap &
+ if (!(rx->link_sta->pub->ht_cap.cap &
IEEE80211_HT_CAP_SUP_WIDTH_20_40))
goto handled;
if (chanwidth == IEEE80211_HT_CHANWIDTH_20MHZ)
max_bw = IEEE80211_STA_RX_BW_20;
else
- max_bw = ieee80211_sta_cap_rx_bw(&rx->sta->deflink);
+ max_bw = ieee80211_sta_cap_rx_bw(rx->link_sta);
/* set cur_max_bandwidth and recalc sta bw */
- rx->sta->deflink.cur_max_bandwidth = max_bw;
- new_bw = ieee80211_sta_cur_vht_bw(&rx->sta->deflink);
+ rx->link_sta->cur_max_bandwidth = max_bw;
+ new_bw = ieee80211_sta_cur_vht_bw(rx->link_sta);
- if (rx->sta->sta.deflink.bandwidth == new_bw)
+ if (rx->link_sta->pub->bandwidth == new_bw)
goto handled;
- rx->sta->sta.deflink.bandwidth = new_bw;
+ rx->link_sta->pub->bandwidth = new_bw;
sband = rx->local->hw.wiphy->bands[status->band];
sta_opmode.bw =
- ieee80211_sta_rx_bw_to_chan_width(&rx->sta->deflink);
+ ieee80211_sta_rx_bw_to_chan_width(rx->link_sta);
sta_opmode.changed = STA_OPMODE_MAX_BW_CHANGED;
rate_control_rate_update(local, sband, rx->sta, 0,
@@ -3641,7 +3642,7 @@ ieee80211_rx_h_action(struct ieee80211_rx_data *rx)
handled:
if (rx->sta)
- rx->sta->deflink.rx_stats.packets++;
+ rx->link_sta->rx_stats.packets++;
dev_kfree_skb(rx->skb);
return RX_QUEUED;
@@ -3685,7 +3686,7 @@ ieee80211_rx_h_userspace_mgmt(struct ieee80211_rx_data *rx)
if (cfg80211_rx_mgmt_ext(&rx->sdata->wdev, &info)) {
if (rx->sta)
- rx->sta->deflink.rx_stats.packets++;
+ rx->link_sta->rx_stats.packets++;
dev_kfree_skb(rx->skb);
return RX_QUEUED;
}
@@ -3723,7 +3724,7 @@ ieee80211_rx_h_action_post_userspace(struct ieee80211_rx_data *rx)
handled:
if (rx->sta)
- rx->sta->deflink.rx_stats.packets++;
+ rx->link_sta->rx_stats.packets++;
dev_kfree_skb(rx->skb);
return RX_QUEUED;
}
@@ -3943,7 +3944,7 @@ static void ieee80211_rx_handlers_result(struct ieee80211_rx_data *rx,
case RX_DROP_MONITOR:
I802_DEBUG_INC(rx->sdata->local->rx_handlers_drop);
if (rx->sta)
- rx->sta->deflink.rx_stats.dropped++;
+ rx->link_sta->rx_stats.dropped++;
fallthrough;
case RX_CONTINUE: {
struct ieee80211_rate *rate = NULL;
@@ -3962,7 +3963,7 @@ static void ieee80211_rx_handlers_result(struct ieee80211_rx_data *rx,
case RX_DROP_UNUSABLE:
I802_DEBUG_INC(rx->sdata->local->rx_handlers_drop);
if (rx->sta)
- rx->sta->deflink.rx_stats.dropped++;
+ rx->link_sta->rx_stats.dropped++;
dev_kfree_skb(rx->skb);
break;
case RX_QUEUED:
--
2.37.2
From: Johannes Berg <[email protected]>
In netlink RX, now that we can actually have multiple
channel contexts for MLO, things don't work well as we
only keep a single pointer, and then on link switching
we might NULL it, and hit the return if the channel is
NULL.
However, we already use mac80211_hwsim_tx_iter() which
deals with all this, so remove the test and adjust the
remaining code a bit.
This then means we no longer use the chanctx pointer,
so remove it as well.
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 26 ++++----------------------
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 18339a56316d..f464d3507fe9 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -653,7 +653,6 @@ struct mac80211_hwsim_data {
u32 ciphers[ARRAY_SIZE(hwsim_ciphers)];
struct mac_address addresses[2];
- struct ieee80211_chanctx_conf *chanctx;
int channels, idx;
bool use_chanctx;
bool destroy_on_close;
@@ -2913,11 +2912,6 @@ static int mac80211_hwsim_croc(struct ieee80211_hw *hw,
static int mac80211_hwsim_add_chanctx(struct ieee80211_hw *hw,
struct ieee80211_chanctx_conf *ctx)
{
- struct mac80211_hwsim_data *hwsim = hw->priv;
-
- mutex_lock(&hwsim->mutex);
- hwsim->chanctx = ctx;
- mutex_unlock(&hwsim->mutex);
hwsim_set_chanctx_magic(ctx);
wiphy_dbg(hw->wiphy,
"add channel context control: %d MHz/width: %d/cfreqs:%d/%d MHz\n",
@@ -2929,11 +2923,6 @@ static int mac80211_hwsim_add_chanctx(struct ieee80211_hw *hw,
static void mac80211_hwsim_remove_chanctx(struct ieee80211_hw *hw,
struct ieee80211_chanctx_conf *ctx)
{
- struct mac80211_hwsim_data *hwsim = hw->priv;
-
- mutex_lock(&hwsim->mutex);
- hwsim->chanctx = NULL;
- mutex_unlock(&hwsim->mutex);
wiphy_dbg(hw->wiphy,
"remove channel context control: %d MHz/width: %d/cfreqs:%d/%d MHz\n",
ctx->def.chan->center_freq, ctx->def.width,
@@ -2946,11 +2935,6 @@ static void mac80211_hwsim_change_chanctx(struct ieee80211_hw *hw,
struct ieee80211_chanctx_conf *ctx,
u32 changed)
{
- struct mac80211_hwsim_data *hwsim = hw->priv;
-
- mutex_lock(&hwsim->mutex);
- hwsim->chanctx = ctx;
- mutex_unlock(&hwsim->mutex);
hwsim_check_chanctx_magic(ctx);
wiphy_dbg(hw->wiphy,
"change channel context control: %d MHz/width: %d/cfreqs:%d/%d MHz\n",
@@ -4354,7 +4338,6 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
hw->wiphy->max_remain_on_channel_duration = 1000;
data->if_combination.radar_detect_widths = 0;
data->if_combination.num_different_channels = data->channels;
- data->chanctx = NULL;
} else {
data->if_combination.num_different_channels = 1;
data->if_combination.radar_detect_widths =
@@ -4929,13 +4912,9 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
if (data2->use_chanctx) {
if (data2->tmp_chan)
channel = data2->tmp_chan;
- else if (data2->chanctx)
- channel = data2->chanctx->def.chan;
} else {
channel = data2->channel;
}
- if (!channel)
- goto out;
if (!hwsim_virtio_enabled) {
if (hwsim_net_get_netgroup(genl_info_net(info)) !=
@@ -4966,6 +4945,7 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
rx_status.freq);
if (!iter_data.channel)
goto out;
+ rx_status.band = iter_data.channel->band;
mutex_lock(&data2->mutex);
if (!hwsim_chans_compat(iter_data.channel, channel)) {
@@ -4978,11 +4958,13 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2,
}
}
mutex_unlock(&data2->mutex);
+ } else if (!channel) {
+ goto out;
} else {
rx_status.freq = channel->center_freq;
+ rx_status.band = channel->band;
}
- rx_status.band = channel->band;
rx_status.rate_idx = nla_get_u32(info->attrs[HWSIM_ATTR_RX_RATE]);
rx_status.signal = nla_get_u32(info->attrs[HWSIM_ATTR_SIGNAL]);
--
2.37.2
On 9/2/2022 10:12 PM, Johannes Berg wrote:
> Alright, here's another set of MLO patches :-)
>
> I'm preparing everything here for link switching (which is also in
> wireless-next/mld-wip branch already), along with some fixes to
> avoid VLANs/4-addr and various other fixes that came up during
> this work. Not all of them are related, e.g. the SW scan stop just
> came up due to the new hwsim checks.
>
> johannes
>
Hi Johannes,
The 3 commit below changed to use MLD address for send
authentication/assoc request for station and
changed to use MLD address of rx management packet include
authentication/assoc response received by station from AP.
Does it has any description about the MLD adress in authentication/assoc
request/assoc response in IEEE P802.11be™/D2.0 or other sepcification?
wifi: mac80211: mlme: transmit assoc frame with address translation
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=4ca04ed36478e21b037fc379a7e6f52d0e6d8d52
wifi: mac80211: support MLO authentication/association with one link
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=81151ce462e533551f3284bfdb8e0f461c9220e6
wifi: mac80211: do link->MLD address translation on RX
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=42fb9148c078004d07b4c39bd7b1086b6165780c
>
Hi,
> The 3 commit below
> wifi: mac80211: mlme: transmit assoc frame with address translation
> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=4ca04ed36478e21b037fc379a7e6f52d0e6d8d52
>
> wifi: mac80211: support MLO authentication/association with one link
> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=81151ce462e533551f3284bfdb8e0f461c9220e6
>
> wifi: mac80211: do link->MLD address translation on RX
> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=42fb9148c078004d07b4c39bd7b1086b6165780c
(Just noting that they're not part of this patchset, they were there
before)
> changed to use MLD address for send
> authentication/assoc request for station and
> changed to use MLD address of rx management packet include
> authentication/assoc response received by station from AP.
Yes.
> Does it has any description about the MLD adress in authentication/assoc
> request/assoc response in IEEE P802.11be™/D2.0 or other sepcification?
Not _really_. I don't think the spec ever really talks about this since
it simply doesn't (need to) care what you do inside your software stack.
However, I believe there are (or will be) cases where even for
management frames we will not want to make a determination which link to
use to transmit - since they're "addressed to the MLD" (see D2.0 35.3.14
Multi-link device individually addressed Management frame delivery).
Note also that for protected management frames, the MLD addresses become
part of the AAD.
Today, auth/assoc frames cannot be encrypted (though that may quite
possibly change for assoc frames in the future), and for them also only
a single link can be selected.
However, I thought that from a software POV it would still be better if
as many MLD-addressed frames actually carried MLD addresses in the
software stack as possible, to unify things with the encryption
requirements etc.
The only exception to this is the first received authentication frame on
the AP which cannot be translated in the stack/driver/firmware since we
don't have a station entry for the new station yet, so hostapd has to be
prepared to handle that very first frame with link addresses.
johannes
On 9/6/2022 3:28 PM, Johannes Berg wrote:
> Hi,
>
>> The 3 commit below
>> wifi: mac80211: mlme: transmit assoc frame with address translation
>> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=4ca04ed36478e21b037fc379a7e6f52d0e6d8d52
>>
>> wifi: mac80211: support MLO authentication/association with one link
>> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=81151ce462e533551f3284bfdb8e0f461c9220e6
>>
>> wifi: mac80211: do link->MLD address translation on RX
>> https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=42fb9148c078004d07b4c39bd7b1086b6165780c
>
> (Just noting that they're not part of this patchset, they were there
> before)
>
>> changed to use MLD address for send
>> authentication/assoc request for station and
>> changed to use MLD address of rx management packet include
>> authentication/assoc response received by station from AP.
> Yes.
>
>> Does it has any description about the MLD adress in authentication/assoc
>> request/assoc response in IEEE P802.11be™/D2.0 or other sepcification?
> Not _really_. I don't think the spec ever really talks about this since
> it simply doesn't (need to) care what you do inside your software stack.
>
> However, I believe there are (or will be) cases where even for
> management frames we will not want to make a determination which link to
> use to transmit - since they're "addressed to the MLD" (see D2.0 35.3.14
> Multi-link device individually addressed Management frame delivery).
>
> Note also that for protected management frames, the MLD addresses become
> part of the AAD.
>
> Today, auth/assoc frames cannot be encrypted (though that may quite
> possibly change for assoc frames in the future), and for them also only
> a single link can be selected.
>
> However, I thought that from a software POV it would still be better if
> as many MLD-addressed frames actually carried MLD addresses in the
> software stack as possible, to unify things with the encryption
> requirements etc.
>
> The only exception to this is the first received authentication frame on
> the AP which cannot be translated in the stack/driver/firmware since we
> don't have a station entry for the new station yet, so hostapd has to be
> prepared to handle that very first frame with link addresses.
>
> johannes
Thanks.
Now I hit an issue is: wpa_supplicant reject the authentication from AP
while connecting.
because the addr of authentication is replaced the link
bssid(00:03:7f:12:99:99) with MLD address(aa:03:7f:12:99:99) in
mac80211's ieee80211_prepare_and_rx_handle().
wls1: SME: Ignore authentication with unexpected peer aa:03:7f:12:99:99
log:
wls1: SME: Trying to authenticate with 00:03:7f:12:99:99
EAPOL: External notification - portValid=0
wls1: State: SCANNING -> AUTHENTICATING
Not configuring frame filtering - BSS 00:00:00:00:00:00 is not a Hotspot
2.0 network
wls1: Determining shared radio frequencies (max len 1)
wls1: Shared frequencies (len=0): completed iteration
nl80211: Authenticate (ifindex=3)
* bssid=00:03:7f:12:99:99
* freq=5180
* SSID=GONGWEN-WKK-MLO
* IEs - hexdump(len=0): [NULL]
* Auth Type 0
* mlo link id=2
* mld address=aa:03:7f:12:99:99
nl80211: Authentication request send successfully
RTM_NEWLINK: ifi_index=3 ifname=wls1 wext ifi_family=0 ifi_flags=0x1003
([UP])
nl80211: Event message available
nl80211: Drv Event 19 (NL80211_CMD_NEW_STATION) received for wls1
nl80211: New station aa:03:7f:12:99:99
nl80211: Event message available
nl80211: Drv Event 37 (NL80211_CMD_AUTHENTICATE) received for wls1
nl80211: MLME event 37 (NL80211_CMD_AUTHENTICATE) on
wls1(00:03:7f:37:12:16) A1=00:03:7f:37:12:16 A2=aa:03:7f:12:99:99
nl80211: MLME event frame - hexdump(len=42): b0 00 f0 00 00 03 7f 37 12
16 aa 03 7f 12 99 99 aa 03 7f 12 99 99 e0 b8 00 00 02 00 00 00 ff 0a 6b
00 00 07 aa 03 7f 12 99 99
nl80211: Authenticate event
wls1: Event AUTH (10) received
wls1: SME: Ignore authentication with unexpected peer aa:03:7f:12:99:99
On Tue, 2022-09-06 at 15:58 +0800, Wen Gong wrote:
>
> Now I hit an issue is: wpa_supplicant reject the authentication from AP
> while connecting.
> because the addr of authentication is replaced the link
> bssid(00:03:7f:12:99:99) with MLD address(aa:03:7f:12:99:99) in
> mac80211's ieee80211_prepare_and_rx_handle().
> wls1: SME: Ignore authentication with unexpected peer aa:03:7f:12:99:99
>
Well, OK, you obviously are adjusting the supplicant to work with MLO
(otherwise you wouldn't get an MLO connection in the first place), so
yeah, this is part of the adjustments needed.
Ilan/Andrei have all of this working, maybe we can share the patches
even before rebase etc.
johannes
On 9/6/2022 4:03 PM, Johannes Berg wrote:
> On Tue, 2022-09-06 at 15:58 +0800, Wen Gong wrote:
>> Now I hit an issue is: wpa_supplicant reject the authentication from AP
>> while connecting.
>> because the addr of authentication is replaced the link
>> bssid(00:03:7f:12:99:99) with MLD address(aa:03:7f:12:99:99) in
>> mac80211's ieee80211_prepare_and_rx_handle().
>> wls1: SME: Ignore authentication with unexpected peer aa:03:7f:12:99:99
>>
> Well, OK, you obviously are adjusting the supplicant to work with MLO
> (otherwise you wouldn't get an MLO connection in the first place), so
> yeah, this is part of the adjustments needed.
>
> Ilan/Andrei have all of this working, maybe we can share the patches
> even before rebase etc.
>
> johannes
Thanks.
It is good to share me the wpa_supplicant patches ASAP.
And I have another question:
When mac80211 use the MLD addr in authentication/assoc request,
finally, it should be replaced with one link's address in air port, right?
It means the MLD addr will never exist in mac80211 header of packet in
the air port, right?
Hi Ilan Peer, Andrei Otcheretianski, Johannes,
Could you share me the patches of wpa_supplicant/hostapd for MLO?
It is not need rebased to latest git://w1.fi/hostap.git, I only need to
test it now.
I see you have many patches in the [email protected] with below
links, but not found any patches for MLO.
https://patchwork.ozlabs.org/project/hostap/list/?series=&submitter=25407&state=*&q=&archive=&delegate=
https://patchwork.ozlabs.org/project/hostap/list/?series=&submitter=62065&state=*&q=&archive=&delegate=
On 9/6/2022 4:03 PM, Johannes Berg wrote:
> On Tue, 2022-09-06 at 15:58 +0800, Wen Gong wrote:
>> Now I hit an issue is: wpa_supplicant reject the authentication from AP
>> while connecting.
>> because the addr of authentication is replaced the link
>> bssid(00:03:7f:12:99:99) with MLD address(aa:03:7f:12:99:99) in
>> mac80211's ieee80211_prepare_and_rx_handle().
>> wls1: SME: Ignore authentication with unexpected peer aa:03:7f:12:99:99
>>
> Well, OK, you obviously are adjusting the supplicant to work with MLO
> (otherwise you wouldn't get an MLO connection in the first place), so
> yeah, this is part of the adjustments needed.
>
> Ilan/Andrei have all of this working, maybe we can share the patches
> even before rebase etc.
>
> johannes
On 9/2/2022 10:12 PM, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> In order to let the driver select active links and properly
> make multi-link connections, as a first step isolate the
> driver from inactive links, and set the active links to be
> only the association link for client-side interfaces. For
> AP side nothing changes since APs always have to have all
> their links active.
>
> To simplify things, update the for_each_sta_active_link()
> API to include the appropriate vif pointer.
>
> This also implies not allocating a chanctx for an inactive
> link, which requires a few more changes.
>
> Since we now no longer try to program multiple links to the
> driver, remove the check in the MLME code.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> include/net/mac80211.h | 30 +++----
> net/mac80211/chan.c | 6 ++
> net/mac80211/driver-ops.c | 172 ++++++++++++++++++++++++++++++++++++++
> net/mac80211/driver-ops.h | 165 ++++++------------------------------
> net/mac80211/key.c | 8 ++
> net/mac80211/link.c | 66 ++++++++++++---
> net/mac80211/mlme.c | 25 ++----
> net/mac80211/util.c | 2 +-
> 8 files changed, 286 insertions(+), 188 deletions(-)
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index d4e1d73d88cc..20a2f25a38fa 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1799,6 +1799,9 @@ struct ieee80211_vif_cfg {
> * @link_conf: in case of MLD, the per-link BSS configuration,
> * indexed by link ID
> * @valid_links: bitmap of valid links, or 0 for non-MLO.
> + * @active_links: The bitmap of active links, or 0 for non-MLO.
> + * The driver shouldn't change this directly, but use the
> + * API calls meant for that purpose.
> * @addr: address of this interface
> * @p2p: indicates whether this AP or STA interface is a p2p
> * interface, i.e. a GO or p2p-sta respectively
> @@ -1834,7 +1837,7 @@ struct ieee80211_vif {
> struct ieee80211_vif_cfg cfg;
> struct ieee80211_bss_conf bss_conf;
> struct ieee80211_bss_conf __rcu *link_conf[IEEE80211_MLD_MAX_NUM_LINKS];
> - u16 valid_links;
> + u16 valid_links, active_links;
> u8 addr[ETH_ALEN] __aligned(2);
> bool p2p;
>
...
> @@ -123,11 +132,38 @@ static int ieee80211_check_dup_link_addrs(struct ieee80211_sub_if_data *sdata)
> return 0;
> }
>
> +static void ieee80211_set_vif_links_bitmaps(struct ieee80211_sub_if_data *sdata,
> + u16 links)
> +{
> + sdata->vif.valid_links = links;
> +
> + if (!links) {
> + sdata->vif.active_links = 0;
> + return;
> + }
> +
> + switch (sdata->vif.type) {
> + case NL80211_IFTYPE_AP:
> + /* in an AP all links are always active */
> + sdata->vif.active_links = links;
> + break;
> + case NL80211_IFTYPE_STATION:
> + if (sdata->vif.active_links)
> + break;
> + WARN_ON(hweight16(links) > 1);
> + sdata->vif.active_links = links;
> + break;
> + default:
> + WARN_ON(1);
> + }
> +}
> +
Now I found it only active the primay link(the link for
authentication/assoc request) in my station MLO test,
change_vif_links of struct ieee80211_ops *ops of driver will only be
called one time for the primary link.
it means only one link for MLO.
I plan to revert this patch in my local test now.
Will you implement muti-links later?
> ...
On Thu, 2022-09-08 at 23:23 +0800, Wen Gong wrote:
>
> Now I found it only active the primay link(the link for
> authentication/assoc request) in my station MLO test,
Yes, that's intentional. It gives the driver choice about which links to
activate; first of all because we don't have interface/link combinations
stuff yet (waiting for your side on that), and secondly because we might
very well (want to) negotiate more links than we can concurrently have
active, e.g. a NIC that can have two active might still want to
negotiate four and switch dynamically.
> change_vif_links of struct ieee80211_ops *ops of driver will only be
> called one time for the primary link.
Correct.
> it means only one link for MLO.
Right.
> I plan to revert this patch in my local test now.
>
> Will you implement muti-links later?
Yes. I have patches pending to add API that the driver can call to pick
the active links (as a bitmap).
I'll send it out when I can, likely tomorrow.
johannes
>
On Thu, 2022-09-08 at 23:51 +0800, Wen Gong wrote:
>
> Another thing is what is the local MLD addr and local primary link(send
> authentication/assoc requset) addr relation?
> I think they are same address for station, right?
No, they aren't, and shouldn't be.
> And the others local link address is random generated by eth_random_addr
> in ieee80211_mgd_assoc() , right?
Yes, at least for now all the link addresses are randomly generated.
johannes
On 9/8/2022 11:36 PM, Johannes Berg wrote:
> On Thu, 2022-09-08 at 23:23 +0800, Wen Gong wrote:
>> Now I found it only active the primay link(the link for
>> authentication/assoc request) in my station MLO test,
> Yes, that's intentional. It gives the driver choice about which links to
> activate; first of all because we don't have interface/link combinations
> stuff yet (waiting for your side on that), and secondly because we might
> very well (want to) negotiate more links than we can concurrently have
> active, e.g. a NIC that can have two active might still want to
> negotiate four and switch dynamically.
>
>> change_vif_links of struct ieee80211_ops *ops of driver will only be
>> called one time for the primary link.
> Correct.
>
>> it means only one link for MLO.
> Right.
>
>> I plan to revert this patch in my local test now.
>>
>> Will you implement muti-links later?
> Yes. I have patches pending to add API that the driver can call to pick
> the active links (as a bitmap).
>
> I'll send it out when I can, likely tomorrow.
>
> johannes
Thanks.
Another thing is what is the local MLD addr and local primary link(send
authentication/assoc requset) addr relation?
I think they are same address for station, right?
And the others local link address is random generated by eth_random_addr
in ieee80211_mgd_assoc() , right?
On 9/8/2022 11:52 PM, Johannes Berg wrote:
> On Thu, 2022-09-08 at 23:51 +0800, Wen Gong wrote:
>> Another thing is what is the local MLD addr and local primary link(send
>> authentication/assoc requset) addr relation?
>> I think they are same address for station, right?
> No, they aren't, and shouldn't be.
IEEE P802.11be™/D2.0
35.3.3 Multi-link device addressing
An MLD has an MLD MAC address that singly identifies the MLD.
Each STA affiliated with an MLD shall have a different MAC address.
NOTE 1—The MLD MAC address of an MLD might be the same as the MAC
address of one affiliated STA or different
from the MAC address of any affiliated STA.
This means the MLD address can be same with one link.
I suggest to set primary link local addr same with MLD address for station.
reason is:
When station up, one link interface of driver will be created with the
addr of struct ieee80211_vif,
it is used for scan and non-MLO connection.
If station start to do MLO connection now, then random local link addr
will be generated by below call stack.
for the 1st link. This lead driver must change the link interface local
address to this random addr.
After disconnect MLO connection, driver also need to change the link
interface local address back to
addr of struct ieee80211_vif. It increased the complexity and driver
need to sync the link interface
if this is a scan running at this moment.
ieee80211_mgd_auth()
->ieee80211_prep_connection()
->ieee80211_vif_set_links()
->ieee80211_vif_update_links()
->ieee80211_link_setup()
->ieee80211_mgd_setup_link()
eth_random_addr(link->conf->addr);//sdata->u.mgd.assoc_data is null at
this point
>> And the others local link address is random generated by eth_random_addr
>> in ieee80211_mgd_assoc() , right?
> Yes, at least for now all the link addresses are randomly generated.
>
> johannes
>
Hi,
> > No, they aren't, and shouldn't be.
> IEEE P802.11be™/D2.0
> 35.3.3 Multi-link device addressing
> An MLD has an MLD MAC address that singly identifies the MLD.
> Each STA affiliated with an MLD shall have a different MAC address.
> NOTE 1—The MLD MAC address of an MLD might be the same as the MAC
> address of one affiliated STA or different
> from the MAC address of any affiliated STA.
Right. I was over-simplifying, that was basically the "tl;dr" version of
my statement, without the longer one ;-)
> This means the MLD address can be same with one link.
True.
> I suggest to set primary link local addr same with MLD address for station.
I wouldn't suggest that, but YMMV.
> reason is:
> When station up, one link interface of driver will be created with the
> addr of struct ieee80211_vif,
> it is used for scan and non-MLO connection.
> If station start to do MLO connection now, then random local link addr
> will be generated by below call stack.
> for the 1st link. This lead driver must change the link interface local
> address to this random addr.
Well, that depends how you treat "address of an interface", no? I don't
think there's really any need to "install" a MAC address to the NIC
until you even start any kind of operation.
True, if you cannot scan using the MLD address while you also have a
different link address, you might be in trouble - but I find this
unrealistic because you would want to be able to scan on any part of the
hardware that is doing any of the links?
In any case, changing this makes the receive logic a bit different. You
would have to ensure that your driver does indeed indicate the link a
frame was received on, I think? Also, ieee80211_rx_for_interface() might
have to change, something like the below maybe?
If we just change the first link's address to the same as the MLD
address without any changes then the code without the changes below
would overwrite the link ID because it can find the link STA address,
even if the device already did address translation. Of course this is
only relevant if it does address translation w/o indicating the link,
which it shouldn't ... hence the patch.
In any case, I expect this will end up being some kind of driver policy,
so I can imagine that we could make a relatively simple patch with a new
method to let drivers set the link address that gets used. It cannot be
changing the link address when it's added to the driver since this patch
that this thread is based on means the driver doesn't get to know about
the links until it's far too late (and even before this patch, the links
were only created after assoc, when the link addresses were already sent
to the AP)
johannes
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index ac2bad57933f..648b2de8dd3e 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1482,7 +1482,8 @@ enum mac80211_rx_encoding {
* @ampdu_delimiter_crc: A-MPDU delimiter CRC
* @zero_length_psdu_type: radiotap type of the 0-length PSDU
* @link_valid: if the link which is identified by @link_id is valid. This flag
- * is set only when connection is MLO.
+ * is set only when connection is MLO. Note that setting this also implies
+ * address translation was done.
* @link_id: id of the link used to receive the packet. This is used along with
* @link_valid.
*/
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index a57811372027..963de5d880d7 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -4946,22 +4946,24 @@ static void __ieee80211_rx_handle_8023(struct ieee80211_hw *hw,
static bool ieee80211_rx_for_interface(struct ieee80211_rx_data *rx,
struct sk_buff *skb, bool consume)
{
- struct link_sta_info *link_sta;
+ struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_hdr *hdr = (void *)skb->data;
+ struct link_sta_info *link_sta = NULL;
/*
- * Look up link station first, in case there's a
- * chance that they might have a link address that
- * is identical to the MLD address, that way we'll
- * have the link information if needed.
+ * Unless the driver did addr translation and provided the link
+ * ID, look up link station first. Note that if we get a frame
+ * without link ID in the status and the device happens to use
+ * identical addresses for one of the links and the MLD, then
+ * we cannot identify whether it was translated already or not.
*/
- link_sta = link_sta_info_get_bss(rx->sdata, hdr->addr2);
+ if (!status->link_valid)
+ link_sta = link_sta_info_get_bss(rx->sdata, hdr->addr2);
+
if (link_sta) {
rx->sta = link_sta->sta;
rx->link_id = link_sta->link_id;
} else {
- struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
-
rx->sta = sta_info_get_bss(rx->sdata, hdr->addr2);
if (rx->sta) {
if (status->link_valid &&
On 9/9/2022 3:28 PM, Johannes Berg wrote:
> Hi,
>
>>> No, they aren't, and shouldn't be.
>> IEEE P802.11be™/D2.0
>> 35.3.3 Multi-link device addressing
>> An MLD has an MLD MAC address that singly identifies the MLD.
>> Each STA affiliated with an MLD shall have a different MAC address.
>> NOTE 1—The MLD MAC address of an MLD might be the same as the MAC
>> address of one affiliated STA or different
>> from the MAC address of any affiliated STA.
> Right. I was over-simplifying, that was basically the "tl;dr" version of
> my statement, without the longer one ;-)
>
>> This means the MLD address can be same with one link.
> True.
>
>> I suggest to set primary link local addr same with MLD address for station.
> I wouldn't suggest that, but YMMV.
>
>> reason is:
>> When station up, one link interface of driver will be created with the
>> addr of struct ieee80211_vif,
>> it is used for scan and non-MLO connection.
>> If station start to do MLO connection now, then random local link addr
>> will be generated by below call stack.
>> for the 1st link. This lead driver must change the link interface local
>> address to this random addr.
> Well, that depends how you treat "address of an interface", no? I don't
> think there's really any need to "install" a MAC address to the NIC
> until you even start any kind of operation.
>
> True, if you cannot scan using the MLD address while you also have a
> different link address, you might be in trouble - but I find this
> unrealistic because you would want to be able to scan on any part of the
> hardware that is doing any of the links?
Scan probe request needs the local address, so we must fill one address
to it.
And we use the same local address to scan for 2.4 GHz/5 GHz/6 GHz band.
>
>
> In any case, changing this makes the receive logic a bit different. You
> would have to ensure that your driver does indeed indicate the link a
> frame was received on, I think? Also, ieee80211_rx_for_interface() might
> have to change, something like the below maybe?
I looked the ieee80211_rx_for_interface(), it is to find struct
link_sta_info with the source
address of an rx frame. For station, the hdr->addr2 means the address of
the AP, so
the the change of mac80211/wireless will not effect the
ieee80211_rx_for_interface().
Because it is the MLD/link address of the AP(maybe it is same addr for
MLD/one link) when we use as station.
>
> If we just change the first link's address to the same as the MLD
> address without any changes then the code without the changes below
> would overwrite the link ID because it can find the link STA address,
> even if the device already did address translation. Of course this is
> only relevant if it does address translation w/o indicating the link,
> which it shouldn't ... hence the patch.
>
> In any case, I expect this will end up being some kind of driver policy,
> so I can imagine that we could make a relatively simple patch with a new
> method to let drivers set the link address that gets used. It cannot be
> changing the link address when it's added to the driver since this patch
> that this thread is based on means the driver doesn't get to know about
> the links until it's far too late (and even before this patch, the links
> were only created after assoc, when the link addresses were already sent
> to the AP)
>
> johannes
>
>
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index ac2bad57933f..648b2de8dd3e 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1482,7 +1482,8 @@ enum mac80211_rx_encoding {
> * @ampdu_delimiter_crc: A-MPDU delimiter CRC
> * @zero_length_psdu_type: radiotap type of the 0-length PSDU
> * @link_valid: if the link which is identified by @link_id is valid. This flag
> - * is set only when connection is MLO.
> + * is set only when connection is MLO. Note that setting this also implies
> + * address translation was done.
> * @link_id: id of the link used to receive the packet. This is used along with
> * @link_valid.
> */
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index a57811372027..963de5d880d7 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -4946,22 +4946,24 @@ static void __ieee80211_rx_handle_8023(struct ieee80211_hw *hw,
> static bool ieee80211_rx_for_interface(struct ieee80211_rx_data *rx,
> struct sk_buff *skb, bool consume)
> {
> - struct link_sta_info *link_sta;
> + struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
> struct ieee80211_hdr *hdr = (void *)skb->data;
> + struct link_sta_info *link_sta = NULL;
>
> /*
> - * Look up link station first, in case there's a
> - * chance that they might have a link address that
> - * is identical to the MLD address, that way we'll
> - * have the link information if needed.
> + * Unless the driver did addr translation and provided the link
> + * ID, look up link station first. Note that if we get a frame
> + * without link ID in the status and the device happens to use
> + * identical addresses for one of the links and the MLD, then
> + * we cannot identify whether it was translated already or not.
> */
> - link_sta = link_sta_info_get_bss(rx->sdata, hdr->addr2);
> + if (!status->link_valid)
> + link_sta = link_sta_info_get_bss(rx->sdata, hdr->addr2);
> +
> if (link_sta) {
> rx->sta = link_sta->sta;
> rx->link_id = link_sta->link_id;
> } else {
> - struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
> -
> rx->sta = sta_info_get_bss(rx->sdata, hdr->addr2);
> if (rx->sta) {
> if (status->link_valid &&
Thanks.
Below patch has said driver should report link id if addr translated.
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=ea9d807b56428d65cf43030cbd7ae5a580077147
wifi: mac80211: add link information in ieee80211_rx_status
In MLO, when the address translation from link to MLD is done
in fw/hw, it is necessary to be able to have some information
on the link on which the frame has been received. Extend the
rx API to include link_id and a valid flag in ieee80211_rx_status.
Also make chanes to mac80211 rx APIs to make use of the reported
link_id after sanity checks.
On 9/9/2022 3:28 PM, Johannes Berg wrote:
> Hi,
>
>>> No, they aren't, and shouldn't be.
>> IEEE P802.11be™/D2.0
>> 35.3.3 Multi-link device addressing
>> An MLD has an MLD MAC address that singly identifies the MLD.
>> Each STA affiliated with an MLD shall have a different MAC address.
>> NOTE 1—The MLD MAC address of an MLD might be the same as the MAC
>> address of one affiliated STA or different
>> from the MAC address of any affiliated STA.
> Right. I was over-simplifying, that was basically the "tl;dr" version of
> my statement, without the longer one ;-)
>
>> This means the MLD address can be same with one link.
> True.
>
>> I suggest to set primary link local addr same with MLD address for station.
> I wouldn't suggest that, but YMMV.
>
>> reason is:
>> When station up, one link interface of driver will be created with the
>> addr of struct ieee80211_vif,
>> it is used for scan and non-MLO connection.
>> If station start to do MLO connection now, then random local link addr
>> will be generated by below call stack.
>> for the 1st link. This lead driver must change the link interface local
>> address to this random addr.
> Well, that depends how you treat "address of an interface", no? I don't
> think there's really any need to "install" a MAC address to the NIC
> until you even start any kind of operation.
>
> True, if you cannot scan using the MLD address while you also have a
> different link address, you might be in trouble - but I find this
> unrealistic because you would want to be able to scan on any part of the
> hardware that is doing any of the links?
Scan probe request needs the local address, so we must fill one address
to it.
And we use the same local address to scan for 2.4 GHz/5 GHz/6 GHz band.
>
>
> In any case, changing this makes the receive logic a bit different. You
> would have to ensure that your driver does indeed indicate the link a
> frame was received on, I think? Also, ieee80211_rx_for_interface() might
> have to change, something like the below maybe?
I looked the ieee80211_rx_for_interface(), it is to find struct
link_sta_info with the source
address of an rx frame. For station, the hdr->addr2 means the address of
the AP, so
the the change of mac80211/wireless will not effect the
ieee80211_rx_for_interface().
Because it is the MLD/link address of the AP(maybe it is same addr for
MLD/one link) when we as station.
> If we just change the first link's address to the same as the MLD
> address without any changes then the code without the changes below
> would overwrite the link ID because it can find the link STA address,
> even if the device already did address translation. Of course this is
> only relevant if it does address translation w/o indicating the link,
> which it shouldn't ... hence the patch.
>
> In any case, I expect this will end up being some kind of driver policy,
> so I can imagine that we could make a relatively simple patch with a new
> method to let drivers set the link address that gets used. It cannot be
> changing the link address when it's added to the driver since this patch
> that this thread is based on means the driver doesn't get to know about
> the links until it's far too late (and even before this patch, the links
> were only created after assoc, when the link addresses were already sent
> to the AP)
Thanks for the incoming new method to let drivers set the link address.
It is better to let driver to fill all the links' address in load phase.
And then it never change again. And one of the address array is always
used for primary link.
>
> johannes
>
>
>
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index ac2bad57933f..648b2de8dd3e 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -1482,7 +1482,8 @@ enum mac80211_rx_encoding {
> * @ampdu_delimiter_crc: A-MPDU delimiter CRC
> * @zero_length_psdu_type: radiotap type of the 0-length PSDU
> * @link_valid: if the link which is identified by @link_id is valid. This flag
> - * is set only when connection is MLO.
> + * is set only when connection is MLO. Note that setting this also implies
> + * address translation was done.
> * @link_id: id of the link used to receive the packet. This is used along with
> * @link_valid.
> */
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index a57811372027..963de5d880d7 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -4946,22 +4946,24 @@ static void __ieee80211_rx_handle_8023(struct ieee80211_hw *hw,
> static bool ieee80211_rx_for_interface(struct ieee80211_rx_data *rx,
> struct sk_buff *skb, bool consume)
> {
> - struct link_sta_info *link_sta;
> + struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
> struct ieee80211_hdr *hdr = (void *)skb->data;
> + struct link_sta_info *link_sta = NULL;
>
> /*
> - * Look up link station first, in case there's a
> - * chance that they might have a link address that
> - * is identical to the MLD address, that way we'll
> - * have the link information if needed.
> + * Unless the driver did addr translation and provided the link
> + * ID, look up link station first. Note that if we get a frame
> + * without link ID in the status and the device happens to use
> + * identical addresses for one of the links and the MLD, then
> + * we cannot identify whether it was translated already or not.
> */
> - link_sta = link_sta_info_get_bss(rx->sdata, hdr->addr2);
> + if (!status->link_valid)
> + link_sta = link_sta_info_get_bss(rx->sdata, hdr->addr2);
> +
> if (link_sta) {
> rx->sta = link_sta->sta;
> rx->link_id = link_sta->link_id;
> } else {
> - struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
> -
> rx->sta = sta_info_get_bss(rx->sdata, hdr->addr2);
> if (rx->sta) {
> if (status->link_valid &&
Thanks.
Below patch has said driver should report link id if addr translated.
https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git/commit/?h=mld&id=ea9d807b56428d65cf43030cbd7ae5a580077147
wifi: mac80211: add link information in ieee80211_rx_status
In MLO, when the address translation from link to MLD is done
in fw/hw, it is necessary to be able to have some information
on the link on which the frame has been received. Extend the
rx API to include link_id and a valid flag in ieee80211_rx_status.
Also make chanes to mac80211 rx APIs to make use of the reported
link_id after sanity checks.
> > Well, OK, you obviously are adjusting the supplicant to work with MLO
> > (otherwise you wouldn't get an MLO connection in the first place), so
> > yeah, this is part of the adjustments needed.
> >
> > Ilan/Andrei have all of this working, maybe we can share the patches
> > even before rebase etc.
Hi,
Our implementation is based on our internal tree, so it will take some time to cleanup and port it for upstream.
Hopefully I will have some time to work on it this and next week and maybe we will be able to share something initial.
Andrei
> >
> > johannes
>
> Thanks.
>
> It is good to share me the wpa_supplicant patches ASAP.
>
> And I have another question:
>
> When mac80211 use the MLD addr in authentication/assoc request,
>
> finally, it should be replaced with one link's address in air port, right?
>
> It means the MLD addr will never exist in mac80211 header of packet in the
> air port, right?
On 9/12/2022 9:17 PM, Otcheretianski, Andrei wrote:
>>> Well, OK, you obviously are adjusting the supplicant to work with MLO
>>> (otherwise you wouldn't get an MLO connection in the first place), so
>>> yeah, this is part of the adjustments needed.
>>>
>>> Ilan/Andrei have all of this working, maybe we can share the patches
>>> even before rebase etc.
> Hi,
> Our implementation is based on our internal tree, so it will take some time to cleanup and port it for upstream.
> Hopefully I will have some time to work on it this and next week and maybe we will be able to share something initial.
>
> Andrei
Thanks, waiting for your patches.
Does your patches of supplicant only support single link or support
muti-link of MLO?
>>> johannes
>> Thanks.
>>
>> It is good to share me the wpa_supplicant patches ASAP.
>>
>> And I have another question:
>>
>> When mac80211 use the MLD addr in authentication/assoc request,
>>
>> finally, it should be replaced with one link's address in air port, right?
>>
>> It means the MLD addr will never exist in mac80211 header of packet in the
>> air port, right?
On 9/12/2022 9:17 PM, Otcheretianski, Andrei wrote:
>>> Well, OK, you obviously are adjusting the supplicant to work with MLO
>>> (otherwise you wouldn't get an MLO connection in the first place), so
>>> yeah, this is part of the adjustments needed.
>>>
>>> Ilan/Andrei have all of this working, maybe we can share the patches
>>> even before rebase etc.
> Hi,
> Our implementation is based on our internal tree, so it will take some time to cleanup and port it for upstream.
> Hopefully I will have some time to work on it this and next week and maybe we will be able to share something initial.
May I get your patches?
>
> Andrei
>>> johannes
>> Thanks.
>>
>> It is good to share me the wpa_supplicant patches ASAP.
>>
>> And I have another question:
>>
>> When mac80211 use the MLD addr in authentication/assoc request,
>>
>> finally, it should be replaced with one link's address in air port, right?
>>
>> It means the MLD addr will never exist in mac80211 header of packet in the
>> air port, right?
Hi johannes,
May I know some more info/status about the "incoming new method to let
drivers set the link address"?
On 9/9/2022 4:58 PM, Wen Gong wrote:
> On 9/9/2022 3:28 PM, Johannes Berg wrote:
>
...
>> If we just change the first link's address to the same as the MLD
>> address without any changes then the code without the changes below
>> would overwrite the link ID because it can find the link STA address,
>> even if the device already did address translation. Of course this is
>> only relevant if it does address translation w/o indicating the link,
>> which it shouldn't ... hence the patch.
>>
>> In any case, I expect this will end up being some kind of driver policy,
>> so I can imagine that we could make a relatively simple patch with a new
>> method to let drivers set the link address that gets used. It cannot be
>> changing the link address when it's added to the driver since this patch
>> that this thread is based on means the driver doesn't get to know about
>> the links until it's far too late (and even before this patch, the links
>> were only created after assoc, when the link addresses were already sent
>> to the AP)
> Thanks for the incoming new method to let drivers set the link address.
> It is better to let driver to fill all the links' address in load phase.
> And then it never change again. And one of the address array is always
> used for primary link.
>
...
Hi,
Sorry - still catching up from PF related matters ...
> May I know some more info/status about the "incoming new method to let
> drivers set the link address"?
>
I wasn't actually planning to work on that myself, FWIW.
johannes
Hi Ilan/Andrei,
Will you send your patches of wpa_supplicant to upstream?????
On 9/28/2022 11:12 PM, Wen Gong wrote:
> On 9/12/2022 9:17 PM, Otcheretianski, Andrei wrote:
>>>> Well, OK, you obviously are adjusting the supplicant to work with MLO
>>>> (otherwise you wouldn't get an MLO connection in the first place), so
>>>> yeah, this is part of the adjustments needed.
>>>>
>>>> Ilan/Andrei have all of this working, maybe we can share the patches
>>>> even before rebase etc.
>> Hi,
>> Our implementation is based on our internal tree, so it will take
>> some time to cleanup and port it for upstream.
>> Hopefully I will have some time to work on it this and next week and
>> maybe we will be able to share something initial.
> May I get your patches?
>>
>> Andrei
>>>> johannes
>>> Thanks.
>>>
>>> It is good to share me the wpa_supplicant patches ASAP.
>>>
>>> And I have another question:
>>>
>>> When mac80211 use the MLD addr in authentication/assoc request,
>>>
>>> finally, it should be replaced with one link's address in air port,
>>> right?
>>>
>>> It means the MLD addr will never exist in mac80211 header of packet
>>> in the
>>> air port, right?
On 9/28/2022 11:28 PM, Johannes Berg wrote:
...
>
>> May I know some more info/status about the "incoming new method to let
>> drivers set the link address"?
>>
> I wasn't actually planning to work on that myself, FWIW.
>
> johannes
OK. So has some body will work for that now?????
On Tue, 2022-10-11 at 12:07 +0800, Wen Gong wrote:
> On 9/28/2022 11:28 PM, Johannes Berg wrote:
> ...
> >
> > > May I know some more info/status about the "incoming new method to let
> > > drivers set the link address"?
> > >
> > I wasn't actually planning to work on that myself, FWIW.
> >
> > johannes
>
> OK. So has some body will work for that now?????
>
Yes, I don't personally have a need for anything other than what we have
right now.
Btw, I also merged pretty much all the things into wireless-next now, I
think only maybe some debugfs updates are still not upstream, and a few
minor bugfixes perhaps.
johannes
Hi Ilan/Andrei,
Will you send your patches of wpa_supplicant for MLO to upstream?????
On 10/11/2022 10:28 AM, Wen Gong wrote:
> Hi Ilan/Andrei,
>
> Will you send your patches of wpa_supplicant to upstream?????
>
> On 9/28/2022 11:12 PM, Wen Gong wrote:
>> On 9/12/2022 9:17 PM, Otcheretianski, Andrei wrote:
>>>>> Well, OK, you obviously are adjusting the supplicant to work with MLO
>>>>> (otherwise you wouldn't get an MLO connection in the first place), so
>>>>> yeah, this is part of the adjustments needed.
>>>>>
>>>>> Ilan/Andrei have all of this working, maybe we can share the patches
>>>>> even before rebase etc.
>>> Hi,
>>> Our implementation is based on our internal tree, so it will take
>>> some time to cleanup and port it for upstream.
>>> Hopefully I will have some time to work on it this and next week and
>>> maybe we will be able to share something initial.
>> May I get your patches?
>>>
>>> Andrei
>>>>> johannes
>>>> Thanks.
>>>>
>>>> It is good to share me the wpa_supplicant patches ASAP.
>>>>
>>>> And I have another question:
>>>>
>>>> When mac80211 use the MLD addr in authentication/assoc request,
>>>>
>>>> finally, it should be replaced with one link's address in air port,
>>>> right?
>>>>
>>>> It means the MLD addr will never exist in mac80211 header of packet
>>>> in the
>>>> air port, right?
> Hi Ilan/Andrei,
>
> Will you send your patches of wpa_supplicant for MLO to upstream?????
Hi,
Yeah, sorry, still WIP. We've been on a long holidays period until yesterday.
Andrei
>
> On 10/11/2022 10:28 AM, Wen Gong wrote:
> > Hi Ilan/Andrei,
> >
> > Will you send your patches of wpa_supplicant to upstream?????
> >
> > On 9/28/2022 11:12 PM, Wen Gong wrote:
> >> On 9/12/2022 9:17 PM, Otcheretianski, Andrei wrote:
> >>>>> Well, OK, you obviously are adjusting the supplicant to work with
> >>>>> MLO (otherwise you wouldn't get an MLO connection in the first
> >>>>> place), so yeah, this is part of the adjustments needed.
> >>>>>
> >>>>> Ilan/Andrei have all of this working, maybe we can share the
> >>>>> patches even before rebase etc.
> >>> Hi,
> >>> Our implementation is based on our internal tree, so it will take
> >>> some time to cleanup and port it for upstream.
> >>> Hopefully I will have some time to work on it this and next week and
> >>> maybe we will be able to share something initial.
> >> May I get your patches?
> >>>
> >>> Andrei
> >>>>> johannes
> >>>> Thanks.
> >>>>
> >>>> It is good to share me the wpa_supplicant patches ASAP.
> >>>>
> >>>> And I have another question:
> >>>>
> >>>> When mac80211 use the MLD addr in authentication/assoc request,
> >>>>
> >>>> finally, it should be replaced with one link's address in air port,
> >>>> right?
> >>>>
> >>>> It means the MLD addr will never exist in mac80211 header of packet
> >>>> in the air port, right?
Hi Andrei,
Is below all your patches for MLO in wpa_suppplicant for station mode?
[00/13] MLD STA: Add SME MLO support
https://patchwork.ozlabs.org/project/hostap/list/?series=329909&state=*
On 10/19/2022 6:04 PM, Wen Gong wrote:
> Hi Ilan/Andrei,
>
> Will you send your patches of wpa_supplicant for MLO to upstream?????
>
> On 10/11/2022 10:28 AM, Wen Gong wrote:
>> Hi Ilan/Andrei,
>>
>> Will you send your patches of wpa_supplicant to upstream?????
>>
>> On 9/28/2022 11:12 PM, Wen Gong wrote:
>>> On 9/12/2022 9:17 PM, Otcheretianski, Andrei wrote:
>>>>>> Well, OK, you obviously are adjusting the supplicant to work with
>>>>>> MLO
>>>>>> (otherwise you wouldn't get an MLO connection in the first
>>>>>> place), so
>>>>>> yeah, this is part of the adjustments needed.
>>>>>>
>>>>>> Ilan/Andrei have all of this working, maybe we can share the patches
>>>>>> even before rebase etc.
>>>> Hi,
>>>> Our implementation is based on our internal tree, so it will take
>>>> some time to cleanup and port it for upstream.
>>>> Hopefully I will have some time to work on it this and next week
>>>> and maybe we will be able to share something initial.
>>> May I get your patches?
>>>>
>>>> Andrei
>>>>>> johannes
>>>>> Thanks.
>>>>>
>>>>> It is good to share me the wpa_supplicant patches ASAP.
>>>>>
>>>>> And I have another question:
>>>>>
>>>>> When mac80211 use the MLD addr in authentication/assoc request,
>>>>>
>>>>> finally, it should be replaced with one link's address in air
>>>>> port, right?
>>>>>
>>>>> It means the MLD addr will never exist in mac80211 header of
>>>>> packet in the
>>>>> air port, right?
> Hi Andrei,
>
> Is below all your patches for MLO in wpa_suppplicant for station mode?
Hi Wen,
We have few more for station side, like SAE/PMKSA support and some additional configs - but this is mostly what I have.
SAE support patch is somewhat similar to patches sent by Veerendranath.
We have more stuff for AP side (mostly for testing purposes), hwsim tests etc.. I'm starting to clean this up and will send it as well.
Andrei
On 11/28/2022 10:05 PM, Otcheretianski, Andrei wrote:
>> Hi Andrei,
>>
>> Is below all your patches for MLO in wpa_suppplicant for station mode?
> Hi Wen,
> We have few more for station side, like SAE/PMKSA support and some additional configs - but this is mostly what I have.
> SAE support patch is somewhat similar to patches sent by Veerendranath.
> We have more stuff for AP side (mostly for testing purposes), hwsim tests etc.. I'm starting to clean this up and will send it as well.
>
> Andrei
Thanks Andrei, did you tested your station with single link or 2 link or
more link?
> Thanks Andrei, did you tested your station with single link or 2 link or more
> link?
Tested with 1 and 2 links, but should also work with more links as well
Andrei
On 11/29/2022 2:59 PM, Otcheretianski, Andrei wrote:
>> Thanks Andrei, did you tested your station with single link or 2 link or more
>> link?
> Tested with 1 and 2 links, but should also work with more links as well
>
> Andrei
Thanks a lot!
On 10/11/2022 3:26 PM, Johannes Berg wrote:
> On Tue, 2022-10-11 at 12:07 +0800, Wen Gong wrote:
>> On 9/28/2022 11:28 PM, Johannes Berg wrote:
>> ...
>>>> May I know some more info/status about the "incoming new method to let
>>>> drivers set the link address"?
>>>>
>>> I wasn't actually planning to work on that myself, FWIW.
>>>
>>> johannes
>> OK. So has some body will work for that now?????
>>
> Yes, I don't personally have a need for anything other than what we have
> right now.
May I add method to let low-drivers set the primay link address like below?
I add a field in struct wiphy_iftype_ext_capab, if it is valid, then it
will be used as
local primary/assoc link addr in function ieee80211_mgd_setup_link() for
station.
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5079,6 +5079,7 @@ struct wiphy_iftype_ext_capab {
u8 extended_capabilities_len;
u16 eml_capabilities;
u16 mld_capa_and_ops;
+ struct mac_address assoc_link_addr;
};
...
On 9/8/2022 11:36 PM, Johannes Berg wrote:
> On Thu, 2022-09-08 at 23:23 +0800, Wen Gong wrote:
>> Now I found it only active the primay link(the link for
>> authentication/assoc request) in my station MLO test,
> Yes, that's intentional. It gives the driver choice about which links to
> activate; first of all because we don't have interface/link combinations
> stuff yet (waiting for your side on that), and secondly because we might
> very well (want to) negotiate more links than we can concurrently have
> active, e.g. a NIC that can have two active might still want to
> negotiate four and switch dynamically.
>
>> change_vif_links of struct ieee80211_ops *ops of driver will only be
>> called one time for the primary link.
> Correct.
>
>> it means only one link for MLO.
> Right.
>
>> I plan to revert this patch in my local test now.
>>
>> Will you implement muti-links later?
> Yes. I have patches pending to add API that the driver can call to pick
> the active links (as a bitmap).
>
> I'll send it out when I can, likely tomorrow.
>
> johannes
The patches should be "wifi: mac80211: implement link switching" for
ieee80211_set_active_links().
May I also add a field such as "u16 active_links_count" in struct
wiphy_iftype_ext_capab,
and add logic in function ieee80211_set_vif_links_bitmaps() for station
like this ?:
if (active_links_count && hweight16(links) <= active_links_count)
then sdata->vif.active_links = links;
On Tue, 2023-04-04 at 10:54 +0800, Wen Gong wrote:
> On 10/11/2022 3:26 PM, Johannes Berg wrote:
> > On Tue, 2022-10-11 at 12:07 +0800, Wen Gong wrote:
> > > On 9/28/2022 11:28 PM, Johannes Berg wrote:
> > > ...
> > > > > May I know some more info/status about the "incoming new method to let
> > > > > drivers set the link address"?
> > > > >
> > > > I wasn't actually planning to work on that myself, FWIW.
> > > >
> > > > johannes
> > > OK. So has some body will work for that now?????
> > >
> > Yes, I don't personally have a need for anything other than what we have
> > right now.
>
> May I add method to let low-drivers set the primay link address like below?
> I add a field in struct wiphy_iftype_ext_capab, if it is valid, then it
> will be used as
>
> local primary/assoc link addr in function ieee80211_mgd_setup_link() for
> station.
>
I don't really think that it makes sense to push this to cfg80211 when
we only need it in mac80211?
johannes
On Tue, 2023-04-04 at 11:28 +0800, Wen Gong wrote:
>
> May I also add a field such as "u16 active_links_count" in struct
> wiphy_iftype_ext_capab,
> and add logic in function ieee80211_set_vif_links_bitmaps() for station
> like this ?:
> if (active_links_count && hweight16(links) <= active_links_count)
> then sdata->vif.active_links = links;
>
Also here, not sure it makes sense in cfg80211 level?
Though I'm not sure what the idea here is at all - you can refuse to
link switch etc, what would you use this for?
Then again, we haven't really designed out all the link selection stuff,
do we want wpa_s to do it, driver to do it, etc.? Hence debugfs. So
depending on what end up doing there, we will obviously need to
advertise some level of link-concurrency to userspace.
johannes
On 4/11/2023 3:32 PM, Johannes Berg wrote:
> On Tue, 2023-04-04 at 10:54 +0800, Wen Gong wrote:
>> On 10/11/2022 3:26 PM, Johannes Berg wrote:
>>> On Tue, 2022-10-11 at 12:07 +0800, Wen Gong wrote:
>>>> On 9/28/2022 11:28 PM, Johannes Berg wrote:
>>>> ...
>>>>>> May I know some more info/status about the "incoming new method to let
>>>>>> drivers set the link address"?
>>>>>>
>>>>> I wasn't actually planning to work on that myself, FWIW.
>>>>>
>>>>> johannes
>>>> OK. So has some body will work for that now?????
>>>>
>>> Yes, I don't personally have a need for anything other than what we have
>>> right now.
>> May I add method to let low-drivers set the primay link address like below?
>> I add a field in struct wiphy_iftype_ext_capab, if it is valid, then it
>> will be used as
>>
>> local primary/assoc link addr in function ieee80211_mgd_setup_link() for
>> station.
>>
> I don't really think that it makes sense to push this to cfg80211 when
> we only need it in mac80211?
>
> johannes
OK. So I will try to put this in mac80211 layer, is it OK?
On 4/11/2023 3:38 PM, Johannes Berg wrote:
> On Tue, 2023-04-04 at 11:28 +0800, Wen Gong wrote:
>> May I also add a field such as "u16 active_links_count" in struct
>> wiphy_iftype_ext_capab,
>> and add logic in function ieee80211_set_vif_links_bitmaps() for station
>> like this ?:
>> if (active_links_count && hweight16(links) <= active_links_count)
>> then sdata->vif.active_links = links;
>>
> Also here, not sure it makes sense in cfg80211 level?
>
> Though I'm not sure what the idea here is at all - you can refuse to
> link switch etc, what would you use this for?
If I use ieee80211_set_active_links(),
then I need add BSS_CHANGED_ASSOC and key for 2nd link in lower-driver.
I would like to active all links while assoc,
then BSS_CHANGED_ASSOC and key will auto set for the 2nd link to
lower-driver from mac80211.
>
> Then again, we haven't really designed out all the link selection stuff,
> do we want wpa_s to do it, driver to do it, etc.? Hence debugfs. So
> depending on what end up doing there, we will obviously need to
> advertise some level of link-concurrency to userspace.
>
> johannes
On Mon, 2023-04-17 at 22:07 +0800, Wen Gong wrote:
>
> OK. So I will try to put this in mac80211 layer, is it OK?
>
I guess? I'm still not really sure why you even want it, but hey, that's
up to you in a way. I really didn't like the suggestion with
wiphy_iftype_ext_capab (or any other capability for that matter), it
feels like it should be more dynamic, like maybe a new "add link"
callback or something? At least then you can't blame mac80211 for when
it breaks when you have two 5 GHz links ...
johannes
On Mon, 2023-04-17 at 22:13 +0800, Wen Gong wrote:
> On 4/11/2023 3:38 PM, Johannes Berg wrote:
> > On Tue, 2023-04-04 at 11:28 +0800, Wen Gong wrote:
> > > May I also add a field such as "u16 active_links_count" in struct
> > > wiphy_iftype_ext_capab,
> > > and add logic in function ieee80211_set_vif_links_bitmaps() for station
> > > like this ?:
> > > if (active_links_count && hweight16(links) <= active_links_count)
> > > then sdata->vif.active_links = links;
> > >
> > Also here, not sure it makes sense in cfg80211 level?
> >
> > Though I'm not sure what the idea here is at all - you can refuse to
> > link switch etc, what would you use this for?
> If I use ieee80211_set_active_links(),
> then I need add BSS_CHANGED_ASSOC and key for 2nd link in lower-driver.
>
> I would like to active all links while assoc,
> then BSS_CHANGED_ASSOC and key will auto set for the 2nd link to
> lower-driver from mac80211.
I'm not convinced that makes sense. You're going to have to be able to
deal with changing links after association _anyway_, unless you plan on
breaking the entire connection once any of the links is getting out of
range or something?
So anyway you're going to have to be able to this for new links anyway?
I mean doing key management when link switching, and "association" (in
quotes, because as a term doesn't even make sense since this state is on
the MLD level, not the link level)...
So not sure I get it?
johannes
On 4/18/2023 4:15 PM, Johannes Berg wrote:
> On Mon, 2023-04-17 at 22:07 +0800, Wen Gong wrote:
>> OK. So I will try to put this in mac80211 layer, is it OK?
>>
> I guess? I'm still not really sure why you even want it, but hey, that's
> up to you in a way. I really didn't like the suggestion with
> wiphy_iftype_ext_capab (or any other capability for that matter), it
> feels like it should be more dynamic, like maybe a new "add link"
> callback or something? At least then you can't blame mac80211 for when
> it breaks when you have two 5 GHz links ...
ok, so I would like to add callback such as
"add_link(struct ieee80211_hw *hw, struct ieee80211_vif vif, struct
ieee80211_bss_conf *link_conf, unsigned int link_id)"
in struct ieee80211_ops, and mac80211 call it in
ieee80211_mgd_setup_link()/ieee80211_vif_update_links,
then lower-drvier could dynamic set the local addr of assoc
link_conf(also for 2nd link_conf), is it OK?
>
> johannes
On Tue, 2023-04-18 at 16:59 +0800, Wen Gong wrote:
> On 4/18/2023 4:15 PM, Johannes Berg wrote:
> > On Mon, 2023-04-17 at 22:07 +0800, Wen Gong wrote:
> > > OK. So I will try to put this in mac80211 layer, is it OK?
> > >
> > I guess? I'm still not really sure why you even want it, but hey, that's
> > up to you in a way. I really didn't like the suggestion with
> > wiphy_iftype_ext_capab (or any other capability for that matter), it
> > feels like it should be more dynamic, like maybe a new "add link"
> > callback or something? At least then you can't blame mac80211 for when
> > it breaks when you have two 5 GHz links ...
>
> ok, so I would like to add callback such as
>
> "add_link(struct ieee80211_hw *hw, struct ieee80211_vif vif, struct
> ieee80211_bss_conf *link_conf, unsigned int link_id)"
>
> in struct ieee80211_ops, and mac80211 call it in
> ieee80211_mgd_setup_link()/ieee80211_vif_update_links,
>
> then lower-drvier could dynamic set the local addr of assoc
> link_conf(also for 2nd link_conf), is it OK?
>
Seems OK, but I'm not sure that _works_?
After all, we first set the addresses in assoc_data, when we don't have
a link_conf yet, no? Just what we were discussing in the other thread
about the leak.
johannes
On 4/18/2023 5:11 PM, Johannes Berg wrote:
> On Tue, 2023-04-18 at 16:59 +0800, Wen Gong wrote:
>> On 4/18/2023 4:15 PM, Johannes Berg wrote:
>>> On Mon, 2023-04-17 at 22:07 +0800, Wen Gong wrote:
>>>> OK. So I will try to put this in mac80211 layer, is it OK?
>>>>
>>> I guess? I'm still not really sure why you even want it, but hey, that's
>>> up to you in a way. I really didn't like the suggestion with
>>> wiphy_iftype_ext_capab (or any other capability for that matter), it
>>> feels like it should be more dynamic, like maybe a new "add link"
>>> callback or something? At least then you can't blame mac80211 for when
>>> it breaks when you have two 5 GHz links ...
>> ok, so I would like to add callback such as
>>
>> "add_link(struct ieee80211_hw *hw, struct ieee80211_vif vif, struct
>> ieee80211_bss_conf *link_conf, unsigned int link_id)"
>>
>> in struct ieee80211_ops, and mac80211 call it in
>> ieee80211_mgd_setup_link()/ieee80211_vif_update_links,
>>
>> then lower-drvier could dynamic set the local addr of assoc
>> link_conf(also for 2nd link_conf), is it OK?
>>
> Seems OK, but I'm not sure that _works_?
>
> After all, we first set the addresses in assoc_data, when we don't have
> a link_conf yet, no? Just what we were discussing in the other thread
> about the leak.
>
> johannes
It should work, I will test it later.
For the 1st assoc link, the data->u.mgd.assoc_data is empty in
ieee80211_mgd_setup_link(),
because ieee80211_mgd_setup_link() is called from nl80211_authenticate()
for the 1st assoc link.
So ieee80211_mgd_setup_link() use eth_random_addr() for the 1st assoc link.
For the 2nd link, ieee80211_mgd_setup_link() is called from
nl80211_associate(),
the sdata->u.mgd.assoc_data is NOT empty,
and the sdata->u.mgd.assoc_data->link[link_id].addr is valid,
it is addr by eth_random_addr(assoc_data->link[i].addr) in
ieee80211_mgd_assoc().
On 4/18/2023 4:18 PM, Johannes Berg wrote:
> On Mon, 2023-04-17 at 22:13 +0800, Wen Gong wrote:
>> On 4/11/2023 3:38 PM, Johannes Berg wrote:
>>> On Tue, 2023-04-04 at 11:28 +0800, Wen Gong wrote:
>>>> May I also add a field such as "u16 active_links_count" in struct
>>>> wiphy_iftype_ext_capab,
>>>> and add logic in function ieee80211_set_vif_links_bitmaps() for station
>>>> like this ?:
>>>> if (active_links_count && hweight16(links) <= active_links_count)
>>>> then sdata->vif.active_links = links;
>>>>
>>> Also here, not sure it makes sense in cfg80211 level?
>>>
>>> Though I'm not sure what the idea here is at all - you can refuse to
>>> link switch etc, what would you use this for?
>> If I use ieee80211_set_active_links(),
>> then I need add BSS_CHANGED_ASSOC and key for 2nd link in lower-driver.
>>
>> I would like to active all links while assoc,
>> then BSS_CHANGED_ASSOC and key will auto set for the 2nd link to
>> lower-driver from mac80211.
> I'm not convinced that makes sense. You're going to have to be able to
> deal with changing links after association _anyway_, unless you plan on
> breaking the entire connection once any of the links is getting out of
> range or something?
>
> So anyway you're going to have to be able to this for new links anyway?
> I mean doing key management when link switching, and "association" (in
> quotes, because as a term doesn't even make sense since this state is on
> the MLD level, not the link level)...
>
> So not sure I get it?
>
> johannes
Yes, you are right.
Now lower driver I used do not store the key and do not trigger
BSS_CHANGED_ASSOC for new links after assoc.
So my suggestion is a way to active all links while assoc, this way is
simple for lower driver I used.
Also ieee80211_set_active_links() is another way to active all links
after assoc.
On Tue, 2023-04-18 at 17:22 +0800, Wen Gong wrote:
>
> It should work, I will test it later.
>
> For the 1st assoc link, the data->u.mgd.assoc_data is empty in
> ieee80211_mgd_setup_link(),
Yeah for the first link it should work.
> because ieee80211_mgd_setup_link() is called from nl80211_authenticate()
> for the 1st assoc link.
>
> So ieee80211_mgd_setup_link() use eth_random_addr() for the 1st assoc link.
Right.
> For the 2nd link, ieee80211_mgd_setup_link() is called from
> nl80211_associate()
I don't think so, it should only be called from
ieee80211_assoc_success()?
> the sdata->u.mgd.assoc_data is NOT empty,
>
> and the sdata->u.mgd.assoc_data->link[link_id].addr is valid,
>
> it is addr by eth_random_addr(assoc_data->link[i].addr) in
> ieee80211_mgd_assoc().
>
Exactly, so we've already decided on the address long before we actually
add the link data structure, so your callback would be much too late.
We'd need to have it called from ieee80211_mgd_assoc() already?
johannes
On Tue, 2023-04-18 at 17:27 +0800, Wen Gong wrote:
>
> Now lower driver I used do not store the key
>
Sure, that's fine.
> and do not trigger
> BSS_CHANGED_ASSOC for new links after assoc.
I think you need to think hard about this ... whatever BSS_CHANGED_ASSOC
causes is likely no longer correct in MLO. Again, the assoc state
*itself* is only changed once, when the whole MLD associated.
> So my suggestion is a way to active all links while assoc, this way is
> simple for lower driver I used.
Sure, and we do that.
But that's not what you're asking - you're asking to re-do some *MLD*
state when a new link is added, and I'm saying that it doesn't make
sense to "add" (again) a key to the MLD that was already added, nor
calling a vif (MLD!) level method saying the MLD changed state to
associated (again).
I really think you should solve this in the driver, that doesn't mean
you have to _store_ he key, you can use one of the iteration functions
as well.
> Also ieee80211_set_active_links() is another way to active all links
> after assoc.
>
Sure.
johannes
On 4/18/2023 5:31 PM, Johannes Berg wrote:
> On Tue, 2023-04-18 at 17:22 +0800, Wen Gong wrote:
>> It should work, I will test it later.
>>
>> For the 1st assoc link, the data->u.mgd.assoc_data is empty in
>> ieee80211_mgd_setup_link(),
> Yeah for the first link it should work.
>
>> because ieee80211_mgd_setup_link() is called from nl80211_authenticate()
>> for the 1st assoc link.
>>
>> So ieee80211_mgd_setup_link() use eth_random_addr() for the 1st assoc link.
> Right.
>
>> For the 2nd link, ieee80211_mgd_setup_link() is called from
>> nl80211_associate()
> I don't think so, it should only be called from
> ieee80211_assoc_success()?
Yes, I checked again, you are right. It is not from nl80211_associate().
>> the sdata->u.mgd.assoc_data is NOT empty,
>>
>> and the sdata->u.mgd.assoc_data->link[link_id].addr is valid,
>>
>> it is addr by eth_random_addr(assoc_data->link[i].addr) in
>> ieee80211_mgd_assoc().
>>
> Exactly, so we've already decided on the address long before we actually
> add the link data structure, so your callback would be much too late.
> We'd need to have it called from ieee80211_mgd_assoc() already?
For the 2nd link, is it OK for me to use the random addr which is set in
ieee80211_mgd_assoc().
I only need to set the 1st assoc link in low driver.
> johannes
On Tue, 2023-04-18 at 17:37 +0800, Wen Gong wrote:
> > > the sdata->u.mgd.assoc_data is NOT empty,
> > >
> > > and the sdata->u.mgd.assoc_data->link[link_id].addr is valid,
> > >
> > > it is addr by eth_random_addr(assoc_data->link[i].addr) in
> > > ieee80211_mgd_assoc().
> > >
> > Exactly, so we've already decided on the address long before we actually
> > add the link data structure, so your callback would be much too late.
> > We'd need to have it called from ieee80211_mgd_assoc() already?
>
> For the 2nd link, is it OK for me to use the random addr which is set in
> ieee80211_mgd_assoc().
>
> I only need to set the 1st assoc link in low driver.
>
Ah. But does it make sense to restrict the API for that? I mean, if you
just change the prototype a little bit and call it without the link
conf, you can easily solve this problem too, no?
Then your driver just has to call eth_radnom_addr() when it's "don't
care", but that's OK?
johannes
On 4/18/2023 5:38 PM, Johannes Berg wrote:
> On Tue, 2023-04-18 at 17:37 +0800, Wen Gong wrote:
>
>
>>>> the sdata->u.mgd.assoc_data is NOT empty,
>>>>
>>>> and the sdata->u.mgd.assoc_data->link[link_id].addr is valid,
>>>>
>>>> it is addr by eth_random_addr(assoc_data->link[i].addr) in
>>>> ieee80211_mgd_assoc().
>>>>
>>> Exactly, so we've already decided on the address long before we actually
>>> add the link data structure, so your callback would be much too late.
>>> We'd need to have it called from ieee80211_mgd_assoc() already?
>> For the 2nd link, is it OK for me to use the random addr which is set in
>> ieee80211_mgd_assoc().
>>
>> I only need to set the 1st assoc link in low driver.
>>
> Ah. But does it make sense to restrict the API for that? I mean, if you
> just change the prototype a little bit and call it without the link
> conf, you can easily solve this problem too, no?
Sorry, I am not sure how to solve this problem by remove the link conf
in prototype.
>
> Then your driver just has to call eth_radnom_addr() when it's "don't
> care", but that's OK?
Yes, it is also OK for me to call eth_radnom_addr()for 2nd link.
>
> johannes
On 4/18/2023 5:34 PM, Johannes Berg wrote:
> On Tue, 2023-04-18 at 17:27 +0800, Wen Gong wrote:
>> Now lower driver I used do not store the key
>>
> Sure, that's fine.
>
>> and do not trigger
>> BSS_CHANGED_ASSOC for new links after assoc.
> I think you need to think hard about this ... whatever BSS_CHANGED_ASSOC
> causes is likely no longer correct in MLO. Again, the assoc state
> *itself* is only changed once, when the whole MLD associated.
>
>> So my suggestion is a way to active all links while assoc, this way is
>> simple for lower driver I used.
> Sure, and we do that.
>
> But that's not what you're asking - you're asking to re-do some *MLD*
> state when a new link is added, and I'm saying that it doesn't make
> sense to "add" (again) a key to the MLD that was already added, nor
> calling a vif (MLD!) level method saying the MLD changed state to
> associated (again).
My purpose it to:
add logic in function ieee80211_set_vif_links_bitmaps() for station,
and to active all link as same as AP type.
>
> I really think you should solve this in the driver, that doesn't mean
> you have to _store_ he key, you can use one of the iteration functions
> as well.
Ok, I will try to find the iteration functions.
>
>> Also ieee80211_set_active_links() is another way to active all links
>> after assoc.
>>
> Sure.
>
> johannes
On Tue, 2023-04-18 at 17:44 +0800, Wen Gong wrote:
> On 4/18/2023 5:38 PM, Johannes Berg wrote:
> > On Tue, 2023-04-18 at 17:37 +0800, Wen Gong wrote:
> >
> >
> > > > > the sdata->u.mgd.assoc_data is NOT empty,
> > > > >
> > > > > and the sdata->u.mgd.assoc_data->link[link_id].addr is valid,
> > > > >
> > > > > it is addr by eth_random_addr(assoc_data->link[i].addr) in
> > > > > ieee80211_mgd_assoc().
> > > > >
> > > > Exactly, so we've already decided on the address long before we actually
> > > > add the link data structure, so your callback would be much too late.
> > > > We'd need to have it called from ieee80211_mgd_assoc() already?
> > > For the 2nd link, is it OK for me to use the random addr which is set in
> > > ieee80211_mgd_assoc().
> > >
> > > I only need to set the 1st assoc link in low driver.
> > >
> > Ah. But does it make sense to restrict the API for that? I mean, if you
> > just change the prototype a little bit and call it without the link
> > conf, you can easily solve this problem too, no?
> Sorry, I am not sure how to solve this problem by remove the link conf
> in prototype.
Why, then you can have an output parameter for the address, and call it
in mac80211 wherever it calls eth_random_addr() today, no?
johannes
>
(removing HTML)
On Tue, 2023-04-18 at 18:42 +0800, Wen Gong wrote:
>
> > Why, then you can have an output parameter for the address, and call
> > it
> > in mac80211 wherever it calls eth_random_addr() today, no?
> >
> OK. Got it. So it is like this, right?
> add_link(struct ieee80211_hw *hw, struct ieee80211_vif vif, u8
> *link_local_addr, unsigned int link_id)"
Seems like that could work, yeah.
johannes
On 4/11/2023 3:38 PM, Johannes Berg wrote:
> On Tue, 2023-04-04 at 11:28 +0800, Wen Gong wrote:
>> May I also add a field such as "u16 active_links_count" in struct
>> wiphy_iftype_ext_capab,
>> and add logic in function ieee80211_set_vif_links_bitmaps() for station
>> like this ?:
>> if (active_links_count && hweight16(links) <= active_links_count)
>> then sdata->vif.active_links = links;
>>
> Also here, not sure it makes sense in cfg80211 level?
>
> Though I'm not sure what the idea here is at all - you can refuse to
> link switch etc, what would you use this for?
>
> Then again, we haven't really designed out all the link selection stuff,
> do we want wpa_s to do it, driver to do it, etc.? Hence debugfs. So
> depending on what end up doing there, we will obviously need to
> advertise some level of link-concurrency to userspace.
So will you plan to do something to let wpa_s/userspace app
active/deactive links?
Or you already have implemented that?
>
> johannes
On Wed, 2023-05-10 at 19:06 +0800, Wen Gong wrote:
> On 4/11/2023 3:38 PM, Johannes Berg wrote:
> > On Tue, 2023-04-04 at 11:28 +0800, Wen Gong wrote:
> > > May I also add a field such as "u16 active_links_count" in struct
> > > wiphy_iftype_ext_capab,
> > > and add logic in function ieee80211_set_vif_links_bitmaps() for station
> > > like this ?:
> > > if (active_links_count && hweight16(links) <= active_links_count)
> > > then sdata->vif.active_links = links;
> > >
> > Also here, not sure it makes sense in cfg80211 level?
> >
> > Though I'm not sure what the idea here is at all - you can refuse to
> > link switch etc, what would you use this for?
> >
> > Then again, we haven't really designed out all the link selection stuff,
> > do we want wpa_s to do it, driver to do it, etc.? Hence debugfs. So
> > depending on what end up doing there, we will obviously need to
> > advertise some level of link-concurrency to userspace.
>
> So will you plan to do something to let wpa_s/userspace app
> active/deactive links?
>
> Or you already have implemented that?
>
No plans right now, and honestly not sure what the right thing even is.
johannes
On 5/10/2023 7:24 PM, Johannes Berg wrote:
> On Wed, 2023-05-10 at 19:06 +0800, Wen Gong wrote:
>> On 4/11/2023 3:38 PM, Johannes Berg wrote:
>>> On Tue, 2023-04-04 at 11:28 +0800, Wen Gong wrote:
>>>> May I also add a field such as "u16 active_links_count" in struct
>>>> wiphy_iftype_ext_capab,
>>>> and add logic in function ieee80211_set_vif_links_bitmaps() for station
>>>> like this ?:
>>>> if (active_links_count && hweight16(links) <= active_links_count)
>>>> then sdata->vif.active_links = links;
>>>>
>>> Also here, not sure it makes sense in cfg80211 level?
>>>
>>> Though I'm not sure what the idea here is at all - you can refuse to
>>> link switch etc, what would you use this for?
>>>
>>> Then again, we haven't really designed out all the link selection stuff,
>>> do we want wpa_s to do it, driver to do it, etc.? Hence debugfs. So
>>> depending on what end up doing there, we will obviously need to
>>> advertise some level of link-concurrency to userspace.
>> So will you plan to do something to let wpa_s/userspace app
>> active/deactive links?
>>
>> Or you already have implemented that?
>>
> No plans right now, and honestly not sure what the right thing even is.
OK. For "advertise some level of link-concurrency to userspace", do you
have any plan/idea?
>
> johannes
On Wed, 2023-05-10 at 20:25 +0800, Wen Gong wrote:
> On 5/10/2023 7:24 PM, Johannes Berg wrote:
> > On Wed, 2023-05-10 at 19:06 +0800, Wen Gong wrote:
> > > On 4/11/2023 3:38 PM, Johannes Berg wrote:
> > > > On Tue, 2023-04-04 at 11:28 +0800, Wen Gong wrote:
> > > > > May I also add a field such as "u16 active_links_count" in struct
> > > > > wiphy_iftype_ext_capab,
> > > > > and add logic in function ieee80211_set_vif_links_bitmaps() for station
> > > > > like this ?:
> > > > > if (active_links_count && hweight16(links) <= active_links_count)
> > > > > then sdata->vif.active_links = links;
> > > > >
> > > > Also here, not sure it makes sense in cfg80211 level?
> > > >
> > > > Though I'm not sure what the idea here is at all - you can refuse to
> > > > link switch etc, what would you use this for?
> > > >
> > > > Then again, we haven't really designed out all the link selection stuff,
> > > > do we want wpa_s to do it, driver to do it, etc.? Hence debugfs. So
> > > > depending on what end up doing there, we will obviously need to
> > > > advertise some level of link-concurrency to userspace.
> > > So will you plan to do something to let wpa_s/userspace app
> > > active/deactive links?
> > >
> > > Or you already have implemented that?
> > >
> > No plans right now, and honestly not sure what the right thing even is.
> OK. For "advertise some level of link-concurrency to userspace", do you
> have any plan/idea?
No, not really.
johannes
>
On 4/18/2023 5:34 PM, Johannes Berg wrote:
> On Tue, 2023-04-18 at 17:27 +0800, Wen Gong wrote:
>> Now lower driver I used do not store the key
>>
> Sure, that's fine.
>
>> and do not trigger
>> BSS_CHANGED_ASSOC for new links after assoc.
> I think you need to think hard about this ... whatever BSS_CHANGED_ASSOC
> causes is likely no longer correct in MLO. Again, the assoc state
> *itself* is only changed once, when the whole MLD associated.
>
>> So my suggestion is a way to active all links while assoc, this way is
>> simple for lower driver I used.
> Sure, and we do that.
>
> But that's not what you're asking - you're asking to re-do some *MLD*
> state when a new link is added, and I'm saying that it doesn't make
> sense to "add" (again) a key to the MLD that was already added, nor
> calling a vif (MLD!) level method saying the MLD changed state to
> associated (again).
>
> I really think you should solve this in the driver, that doesn't mean
> you have to _store_ he key, you can use one of the iteration functions
> as well.
>
>> Also ieee80211_set_active_links() is another way to active all links
>> after assoc.
>>
> Sure.
Hi Johannes,
May I add a new ops in struct ieee80211_ops? like this:
u16 active_links(struct ieee80211_hw *hw, struct ieee80211_vif vif, u16
new_links)"
then ieee80211_set_vif_links_bitmaps() call the ops to get the links for
station and set the sdata->vif.active_links with the return value from
lower driver,
it means lower driver will dynamic select the links count at this moment.
If lower driver not register ops active_links, then keep current logic.
>
> johannes
On 4/18/2023 5:34 PM, Johannes Berg wrote:
> On Tue, 2023-04-18 at 17:27 +0800, Wen Gong wrote:
>> Now lower driver I used do not store the key
>>
> Sure, that's fine.
>
>> and do not trigger
>> BSS_CHANGED_ASSOC for new links after assoc.
> I think you need to think hard about this ... whatever BSS_CHANGED_ASSOC
> causes is likely no longer correct in MLO. Again, the assoc state
> *itself* is only changed once, when the whole MLD associated.
>
>> So my suggestion is a way to active all links while assoc, this way is
>> simple for lower driver I used.
> Sure, and we do that.
>
> But that's not what you're asking - you're asking to re-do some *MLD*
> state when a new link is added, and I'm saying that it doesn't make
> sense to "add" (again) a key to the MLD that was already added, nor
> calling a vif (MLD!) level method saying the MLD changed state to
> associated (again).
>
> I really think you should solve this in the driver, that doesn't mean
> you have to _store_ he key, you can use one of the iteration functions
> as well.
>
>> Also ieee80211_set_active_links() is another way to active all links
>> after assoc.
>>
> Sure.
May I add a new ops in struct ieee80211_ops? like this:
u16 active_links(struct ieee80211_hw *hw, struct ieee80211_vif vif, u16
new_links)"
then ieee80211_set_vif_links_bitmaps() call the ops to get the links for
station and set the sdata->vif.active_links with the return value from
lower driver,
it means lower driver will dynamic select the links count at this moment.
If lower driver not register ops active_links, then keep current logic.
> johannes
On Wed, 2023-05-24 at 15:41 +0800, Wen Gong wrote:
>
> May I add a new ops in struct ieee80211_ops? like this:
>
> u16 active_links(struct ieee80211_hw *hw, struct ieee80211_vif vif, u16
> new_links)"
>
> then ieee80211_set_vif_links_bitmaps() call the ops to get the links for
> station and set the sdata->vif.active_links with the return value from
> lower driver,
> it means lower driver will dynamic select the links count at this moment.
>
> If lower driver not register ops active_links, then keep current logic.
>
I guess you can can send patches for whatever you want :)
But I have no idea what you're trying to do? Why would you need to have
a callback?
Was this for link selection in the driver? We should have a patch
somewhere that adds a BSS_CHANGE flag for when the valid links change,
so the driver can select others.
johannes
On 6/15/2023 2:32 AM, Johannes Berg wrote:
> On Wed, 2023-05-24 at 15:41 +0800, Wen Gong wrote:
>> May I add a new ops in struct ieee80211_ops? like this:
>>
>> u16 active_links(struct ieee80211_hw *hw, struct ieee80211_vif vif, u16
>> new_links)"
>>
>> then ieee80211_set_vif_links_bitmaps() call the ops to get the links for
>> station and set the sdata->vif.active_links with the return value from
>> lower driver,
>> it means lower driver will dynamic select the links count at this moment.
>>
>> If lower driver not register ops active_links, then keep current logic.
>>
> I guess you can can send patches for whatever you want :)
>
> But I have no idea what you're trying to do? Why would you need to have
> a callback?
Currently driver could use ieee80211_set_active_links_async() to active
links after connection completed.
But I would like to allow driver to select active links in a early time,
it will be more convenient for driver.
>
> Was this for link selection in the driver? We should have a patch
> somewhere that adds a BSS_CHANGE flag for when the valid links change,
> so the driver can select others.
>
> johannes
Yes, it is for link selection in driver at a early time before
connection completed.
Could you tell detail about how the BSS_CHANGE flag works?????
On Thu, 2023-06-15 at 10:26 +0800, Wen Gong wrote:
> On 6/15/2023 2:32 AM, Johannes Berg wrote:
> > On Wed, 2023-05-24 at 15:41 +0800, Wen Gong wrote:
> > > May I add a new ops in struct ieee80211_ops? like this:
> > >
> > > u16 active_links(struct ieee80211_hw *hw, struct ieee80211_vif vif, u16
> > > new_links)"
> > >
> > > then ieee80211_set_vif_links_bitmaps() call the ops to get the links for
> > > station and set the sdata->vif.active_links with the return value from
> > > lower driver,
> > > it means lower driver will dynamic select the links count at this moment.
> > >
> > > If lower driver not register ops active_links, then keep current logic.
> > >
> > I guess you can can send patches for whatever you want :)
> >
> > But I have no idea what you're trying to do? Why would you need to have
> > a callback?
>
> Currently driver could use ieee80211_set_active_links_async() to active
> links after connection completed.
Right.
> But I would like to allow driver to select active links in a early time,
> it will be more convenient for driver.
How so? All you have to do is look for the connection becoming
authorized (e.g. sta state for the AP moving to authorized) and then
selecting the links you want. We've already been working on that, it's
really easy?
On the flip-side, it would be highly inconvenient for mac80211 to try to
enable more links *during* the association process, and actually it's
not even allowed by spec until the 4-way-HS finishes. So the earliest
possible time is pretty much when you can just do it in the driver as I
just described.
> > Was this for link selection in the driver? We should have a patch
> > somewhere that adds a BSS_CHANGE flag for when the valid links change,
> > so the driver can select others.
> >
> > johannes
>
> Yes, it is for link selection in driver at a early time before
> connection completed.
This is not really allowed ... At least not without also finding ways to
really transmit the 802.1X and 4-way-HS only on the right link, etc.
> Could you tell detail about how the BSS_CHANGE flag works?????
The work isn't complete yet, but basically it just calls the callback
whenever the valid_links changed, say by link-reconfiguration.
johannes
On 6/15/2023 3:56 PM, Johannes Berg wrote:
> On Thu, 2023-06-15 at 10:26 +0800, Wen Gong wrote:
>> On 6/15/2023 2:32 AM, Johannes Berg wrote:
>>> On Wed, 2023-05-24 at 15:41 +0800, Wen Gong wrote:
>>>> May I add a new ops in struct ieee80211_ops? like this:
>>>>
>>>> u16 active_links(struct ieee80211_hw *hw, struct ieee80211_vif vif, u16
>>>> new_links)"
>>>>
>>>> then ieee80211_set_vif_links_bitmaps() call the ops to get the links for
>>>> station and set the sdata->vif.active_links with the return value from
>>>> lower driver,
>>>> it means lower driver will dynamic select the links count at this moment.
>>>>
>>>> If lower driver not register ops active_links, then keep current logic.
>>>>
>>> I guess you can can send patches for whatever you want :)
>>>
>>> But I have no idea what you're trying to do? Why would you need to have
>>> a callback?
>> Currently driver could use ieee80211_set_active_links_async() to active
>> links after connection completed.
> Right.
>
>> But I would like to allow driver to select active links in a early time,
>> it will be more convenient for driver.
> How so? All you have to do is look for the connection becoming
> authorized (e.g. sta state for the AP moving to authorized) and then
> selecting the links you want. We've already been working on that, it's
> really easy?
It is more complex for ath12k drivers.
>
> On the flip-side, it would be highly inconvenient for mac80211 to try to
> enable more links *during* the association process, and actually it's
> not even allowed by spec until the 4-way-HS finishes. So the earliest
> possible time is pretty much when you can just do it in the driver as I
> just described.
>
>>> Was this for link selection in the driver? We should have a patch
>>> somewhere that adds a BSS_CHANGE flag for when the valid links change,
>>> so the driver can select others.
>>>
>>> johannes
>> Yes, it is for link selection in driver at a early time before
>> connection completed.
> This is not really allowed ... At least not without also finding ways to
> really transmit the 802.1X and 4-way-HS only on the right link, etc.
Yes, I also found this in section "2.7.6.1 General" of IEEE P802.11be:
"For MLO, if RSNA has not been established, each message of the 4-way
handshake shall be sent on
the same link used by the latest exchange of successful (Re)Association
Request/Response frames."
For ath12k drivers, only the primary link(the link used by the latest
exchange of successful (Re)Association
Request/Response frames) is active before key installed, so the 802.1X
and 4-way-HS will always sent on
the primary link, so it will meet the rule above of section "2.7.6.1
General".
It means: lower driver will ensure the rule when it implmented the
active_links callback such as ath12k.
>> Could you tell detail about how the BSS_CHANGE flag works?????
> The work isn't complete yet, but basically it just calls the callback
> whenever the valid_links changed, say by link-reconfiguration.
>
> johannes
On 6/21/2023 3:55 PM, Wen Gong wrote:
> On 6/15/2023 3:56 PM, Johannes Berg wrote:
>> On Thu, 2023-06-15 at 10:26 +0800, Wen Gong wrote:
>>> On 6/15/2023 2:32 AM, Johannes Berg wrote:
>>>> On Wed, 2023-05-24 at 15:41 +0800, Wen Gong wrote:
>>>>> May I add a new ops in struct ieee80211_ops? like this:
>>>>>
>>>>> u16 active_links(struct ieee80211_hw *hw, struct ieee80211_vif
>>>>> vif, u16
>>>>> new_links)"
>>>>>
>>>>> then ieee80211_set_vif_links_bitmaps() call the ops to get the
>>>>> links for
>>>>> station and set the sdata->vif.active_links with the return value
>>>>> from
>>>>> lower driver,
>>>>> it means lower driver will dynamic select the links count at this
>>>>> moment.
>>>>>
>>>>> If lower driver not register ops active_links, then keep current
>>>>> logic.
>>>>>
>>>> I guess you can can send patches for whatever you want :)
>>>>
>>>> But I have no idea what you're trying to do? Why would you need to
>>>> have
>>>> a callback?
>>> Currently driver could use ieee80211_set_active_links_async() to active
>>> links after connection completed.
>> Right.
>>
>>> But I would like to allow driver to select active links in a early
>>> time,
>>> it will be more convenient for driver.
>> How so? All you have to do is look for the connection becoming
>> authorized (e.g. sta state for the AP moving to authorized) and then
>> selecting the links you want. We've already been working on that, it's
>> really easy?
> It is more complex for ath12k drivers.
>>
>> On the flip-side, it would be highly inconvenient for mac80211 to try to
>> enable more links *during* the association process, and actually it's
>> not even allowed by spec until the 4-way-HS finishes. So the earliest
>> possible time is pretty much when you can just do it in the driver as I
>> just described.
>>
>>>> Was this for link selection in the driver? We should have a patch
>>>> somewhere that adds a BSS_CHANGE flag for when the valid links change,
>>>> so the driver can select others.
>>>>
>>>> johannes
>>> Yes, it is for link selection in driver at a early time before
>>> connection completed.
>> This is not really allowed ... At least not without also finding ways to
>> really transmit the 802.1X and 4-way-HS only on the right link, etc.
> Yes, I also found this in section "2.7.6.1 General" of IEEE P802.11be:
> "For MLO, if RSNA has not been established, each message of the 4-way
> handshake shall be sent on
> the same link used by the latest exchange of successful
> (Re)Association Request/Response frames."
>
> For ath12k drivers, only the primary link(the link used by the latest
> exchange of successful (Re)Association
> Request/Response frames) is active before key installed, so the 802.1X
> and 4-way-HS will always sent on
> the primary link, so it will meet the rule above of section "2.7.6.1
> General".
>
> It means: lower driver will ensure the rule when it implmented the
> active_links callback such as ath12k.
Johannes, do you have more comments for my answer above?????
>>> Could you tell detail about how the BSS_CHANGE flag works?????
>> The work isn't complete yet, but basically it just calls the callback
>> whenever the valid_links changed, say by link-reconfiguration.
>>
>> johannes
On 6/15/2023 3:56 PM, Johannes Berg wrote:
> On Thu, 2023-06-15 at 10:26 +0800, Wen Gong wrote:
>> On 6/15/2023 2:32 AM, Johannes Berg wrote:
>>> On Wed, 2023-05-24 at 15:41 +0800, Wen Gong wrote:
>>>
...
>> Could you tell detail about how the BSS_CHANGE flag works?????
> The work isn't complete yet, but basically it just calls the callback
> whenever the valid_links changed, say by link-reconfiguration.
>
> johannes
I guess the link-reconfiguration you said is for station, it means
station will do corresponding
link-reconfiguration after receive link reconfiguration indication(e.g.
Reconfiguration
Multi-Link element) from MLO AP, right?
I guess you will add enum BSS_CHANGED_xxx(e.g.
BSS_CHANGED_LINK_RECONFIG), and call
vif_cfg_changed of struct ieee80211_ops for link-reconfiguration, right?
And do you will implement both remove link and add link of station?
For add link, it should calculate the new key of the new link("35.3.6.4
ML reconfiguration
to the ML setup" of IEEE P802.11be™/D3.2).