Currently, during channel switch, deflink (or link_id 0) is always
considered. However, with Multi-Link Operation (MLO), there is a
need to handle link specific data structures based on the actual
operational link_id during channel switch operation.
Hence, add support for the same. Non-MLO based operations will use
link_id as 0 or deflink member as applicable.
While at it, beacon count down now needs to be updated on proper
link_id's beacon, do that as well.
Aditya Kumar Singh (5):
wifi: cfg80211: send link id in channel_switch ops
wifi: mac80211: update beacon counters per link basis
wifi: mac80211: handle set csa/after_csa beacon on per link basis
wifi: mac80211: start and finalize channel switch on link basis
wifi: mac80211: add support to call csa_finish on a link
---
v8: * fixed space error in documentation in 1/5
* minor commit message rephrasing in 3/5 and 5/5.
v7: * fixed proper link id validation in 2/5 and 5/5. (>=)
* added comma in trace print in 1/5
v6: * splitted v5 2/3 into v6 3/5, 4/5 and 5/5
* rephrased commit mesage in 1/5
v5: * fixed compilation issue reported by kernel test robot.
v4: * fixed compilation issue reported by kernel test robot.
* rebased on latest ToT.
* moved link_id arguement into csa_params struct in [1/3]
v3: * splitted [v2 1/2] into [v3 1/3] and [v3 2/3] having simple cfg80211
changes in 1/3 for easy review. Rest in 2/3 [Johannes]
* used wiphy_dereference() instead of sdata_dereference() [Johannes]
v2: * reabsed on ToT
* removed unwanted locking sequence during cancelling CSA work handler
since now locking is moved to wiphy level, that part is uncessary now.
---
drivers/net/wireless/ath/ath10k/mac.c | 4 +-
drivers/net/wireless/ath/ath10k/wmi.c | 2 +-
drivers/net/wireless/ath/ath11k/mac.c | 2 +-
drivers/net/wireless/ath/ath11k/wmi.c | 2 +-
drivers/net/wireless/ath/ath12k/wmi.c | 2 +-
drivers/net/wireless/ath/ath9k/beacon.c | 2 +-
.../net/wireless/ath/ath9k/htc_drv_beacon.c | 2 +-
.../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c | 6 +-
.../wireless/intel/iwlwifi/mvm/time-event.c | 2 +-
drivers/net/wireless/mediatek/mt76/mac80211.c | 2 +-
.../net/wireless/mediatek/mt76/mt7615/mcu.c | 2 +-
.../net/wireless/mediatek/mt76/mt7915/mcu.c | 2 +-
.../net/wireless/mediatek/mt76/mt7996/mcu.c | 2 +-
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
drivers/net/wireless/ti/wlcore/event.c | 2 +-
drivers/net/wireless/virtual/mac80211_hwsim.c | 2 +-
include/net/cfg80211.h | 3 +
include/net/mac80211.h | 7 +-
net/mac80211/cfg.c | 108 +++++++++++-------
net/mac80211/link.c | 2 +
net/mac80211/tx.c | 14 ++-
net/wireless/nl80211.c | 1 +
net/wireless/trace.h | 7 +-
23 files changed, 112 insertions(+), 68 deletions(-)
base-commit: 28b3df1fe6ba2cb439ba109f095aa841fef3a54f
--
2.25.1
Currently, during channel switch, no link id information is passed down.
In order to support channel switch during Multi Link Operation, it is
required to pass link id as well.
Add changes to pass link id in the channel_switch cfg80211_ops.
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
include/net/cfg80211.h | 3 +++
net/wireless/nl80211.c | 1 +
net/wireless/trace.h | 7 +++++--
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 51b9e6fa12f8..60d99336751a 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1531,6 +1531,8 @@ struct cfg80211_ap_update {
* @punct_bitmap: Preamble puncturing bitmap. Each bit represents
* a 20 MHz channel, lowest bit corresponding to the lowest channel.
* Bit set to 1 indicates that the channel is punctured.
+ * @link_id: defines the link on which channel switch is expected during
+ * MLO. 0 in case of non-MLO.
*/
struct cfg80211_csa_settings {
struct cfg80211_chan_def chandef;
@@ -1544,6 +1546,7 @@ struct cfg80211_csa_settings {
bool block_tx;
u8 count;
u16 punct_bitmap;
+ u8 link_id;
};
/**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e4f41f86e295..7f58e11eba58 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -10232,6 +10232,7 @@ static int nl80211_channel_switch(struct sk_buff *skb, struct genl_info *info)
goto free;
}
+ params.link_id = link_id;
err = rdev_channel_switch(rdev, dev, ¶ms);
free:
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 1f374c8a17a5..77972f7b774f 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2324,6 +2324,7 @@ TRACE_EVENT(rdev_channel_switch,
__field(u8, count)
__dynamic_array(u16, bcn_ofs, params->n_counter_offsets_beacon)
__dynamic_array(u16, pres_ofs, params->n_counter_offsets_presp)
+ __field(u8, link_id)
),
TP_fast_assign(
WIPHY_ASSIGN;
@@ -2341,11 +2342,13 @@ TRACE_EVENT(rdev_channel_switch,
memcpy(__get_dynamic_array(pres_ofs),
params->counter_offsets_presp,
params->n_counter_offsets_presp * sizeof(u16));
+ __entry->link_id = params->link_id;
),
TP_printk(WIPHY_PR_FMT ", " NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT
- ", block_tx: %d, count: %u, radar_required: %d",
+ ", block_tx: %d, count: %u, radar_required: %d, link_id: %d",
WIPHY_PR_ARG, NETDEV_PR_ARG, CHAN_DEF_PR_ARG,
- __entry->block_tx, __entry->count, __entry->radar_required)
+ __entry->block_tx, __entry->count, __entry->radar_required,
+ __entry->link_id)
);
TRACE_EVENT(rdev_set_qos_map,
--
2.25.1
Add changes to start a channel switch as well as finalize it on link basis
in order to support CSA with MLO as well.
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
net/mac80211/cfg.c | 69 ++++++++++++++++++++++++++-------------------
net/mac80211/link.c | 2 ++
2 files changed, 42 insertions(+), 29 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a5d510932cbe..4427259154e2 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3629,6 +3629,7 @@ static int __ieee80211_csa_finalize(struct ieee80211_link_data *link_data)
{
struct ieee80211_sub_if_data *sdata = link_data->sdata;
struct ieee80211_local *local = sdata->local;
+ struct ieee80211_bss_conf *link_conf = link_data->conf;
u64 changed = 0;
int err;
@@ -3650,22 +3651,21 @@ static int __ieee80211_csa_finalize(struct ieee80211_link_data *link_data)
if (link_data->reserved_ready)
return 0;
- return ieee80211_link_use_reserved_context(&sdata->deflink);
+ return ieee80211_link_use_reserved_context(link_data);
}
- if (!cfg80211_chandef_identical(&link_data->conf->chandef,
+ if (!cfg80211_chandef_identical(&link_conf->chandef,
&link_data->csa_chandef))
return -EINVAL;
- sdata->vif.bss_conf.csa_active = false;
+ link_conf->csa_active = false;
- err = ieee80211_set_after_csa_beacon(&sdata->deflink, &changed);
+ err = ieee80211_set_after_csa_beacon(link_data, &changed);
if (err)
return err;
- if (sdata->vif.bss_conf.eht_puncturing != sdata->vif.bss_conf.csa_punct_bitmap) {
- sdata->vif.bss_conf.eht_puncturing =
- sdata->vif.bss_conf.csa_punct_bitmap;
+ if (link_conf->eht_puncturing != link_conf->csa_punct_bitmap) {
+ link_conf->eht_puncturing = link_conf->csa_punct_bitmap;
changed |= BSS_CHANGED_EHT_PUNCTURING;
}
@@ -3693,7 +3693,8 @@ static void ieee80211_csa_finalize(struct ieee80211_link_data *link_data)
struct ieee80211_sub_if_data *sdata = link_data->sdata;
if (__ieee80211_csa_finalize(link_data)) {
- sdata_info(sdata, "failed to finalize CSA, disconnecting\n");
+ sdata_info(sdata, "failed to finalize CSA on link %d, disconnecting\n",
+ link_data->link_id);
cfg80211_stop_iface(sdata->local->hw.wiphy, &sdata->wdev,
GFP_KERNEL);
}
@@ -3869,7 +3870,10 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_channel_switch ch_switch;
struct ieee80211_chanctx_conf *conf;
struct ieee80211_chanctx *chanctx;
+ struct ieee80211_bss_conf *link_conf;
+ struct ieee80211_link_data *link_data;
u64 changed = 0;
+ u8 link_id = params->link_id;
int err;
lockdep_assert_wiphy(local->hw.wiphy);
@@ -3880,16 +3884,23 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
if (sdata->wdev.cac_started)
return -EBUSY;
- if (cfg80211_chandef_identical(¶ms->chandef,
- &sdata->vif.bss_conf.chandef))
+ if (WARN_ON(link_id >= IEEE80211_MLD_MAX_NUM_LINKS))
+ return -EINVAL;
+
+ link_data = wiphy_dereference(wiphy, sdata->link[link_id]);
+ if (!link_data)
+ return -ENOLINK;
+
+ link_conf = link_data->conf;
+
+ if (cfg80211_chandef_identical(¶ms->chandef, &link_conf->chandef))
return -EINVAL;
/* don't allow another channel switch if one is already active. */
- if (sdata->vif.bss_conf.csa_active)
+ if (link_conf->csa_active)
return -EBUSY;
- conf = rcu_dereference_protected(sdata->vif.bss_conf.chanctx_conf,
- lockdep_is_held(&local->hw.wiphy->mtx));
+ conf = wiphy_dereference(wiphy, link_conf->chanctx_conf);
if (!conf) {
err = -EBUSY;
goto out;
@@ -3913,7 +3924,7 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
if (err)
goto out;
- err = ieee80211_link_reserve_chanctx(&sdata->deflink, ¶ms->chandef,
+ err = ieee80211_link_reserve_chanctx(link_data, ¶ms->chandef,
chanctx->mode,
params->radar_required);
if (err)
@@ -3922,44 +3933,44 @@ __ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
/* if reservation is invalid then this will fail */
err = ieee80211_check_combinations(sdata, NULL, chanctx->mode, 0);
if (err) {
- ieee80211_link_unreserve_chanctx(&sdata->deflink);
+ ieee80211_link_unreserve_chanctx(link_data);
goto out;
}
/* if there is a color change in progress, abort it */
- if (sdata->vif.bss_conf.color_change_active)
+ if (link_conf->color_change_active)
ieee80211_color_change_abort(sdata);
- err = ieee80211_set_csa_beacon(&sdata->deflink, params, &changed);
+ err = ieee80211_set_csa_beacon(link_data, params, &changed);
if (err) {
- ieee80211_link_unreserve_chanctx(&sdata->deflink);
+ ieee80211_link_unreserve_chanctx(link_data);
goto out;
}
- if (params->punct_bitmap && !sdata->vif.bss_conf.eht_support)
+ if (params->punct_bitmap && link_conf->eht_support)
goto out;
- sdata->deflink.csa_chandef = params->chandef;
- sdata->deflink.csa_block_tx = params->block_tx;
- sdata->vif.bss_conf.csa_active = true;
- sdata->vif.bss_conf.csa_punct_bitmap = params->punct_bitmap;
+ link_data->csa_chandef = params->chandef;
+ link_data->csa_block_tx = params->block_tx;
+ link_conf->csa_active = true;
+ link_conf->csa_punct_bitmap = params->punct_bitmap;
- if (sdata->deflink.csa_block_tx)
+ if (link_data->csa_block_tx)
ieee80211_stop_vif_queues(local, sdata,
IEEE80211_QUEUE_STOP_REASON_CSA);
cfg80211_ch_switch_started_notify(sdata->dev,
- &sdata->deflink.csa_chandef, 0,
+ &link_data->csa_chandef, link_id,
params->count, params->block_tx,
- sdata->vif.bss_conf.csa_punct_bitmap);
+ link_conf->csa_punct_bitmap);
if (changed) {
- ieee80211_link_info_change_notify(sdata, &sdata->deflink,
- changed);
+ ieee80211_link_info_change_notify(sdata, link_data, changed);
+ /* link_id to be passed here? */
drv_channel_switch_beacon(sdata, ¶ms->chandef);
} else {
/* if the beacon didn't change, we can finalize immediately */
- ieee80211_csa_finalize(&sdata->deflink);
+ ieee80211_csa_finalize(link_data);
}
out:
diff --git a/net/mac80211/link.c b/net/mac80211/link.c
index d4f86955afa6..0a1d0ef09d9f 100644
--- a/net/mac80211/link.c
+++ b/net/mac80211/link.c
@@ -73,6 +73,8 @@ void ieee80211_link_stop(struct ieee80211_link_data *link)
ieee80211_mgd_stop_link(link);
cancel_delayed_work_sync(&link->color_collision_detect_work);
+ wiphy_work_cancel(link->sdata->local->hw.wiphy,
+ &link->csa_finalize_work);
ieee80211_link_release_channel(link);
}
--
2.25.1
On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote:
> Add changes to start a channel switch as well as finalize it on link basis
> in order to support CSA with MLO as well.
>
FYI, this had a number of conflicts with my other work - please check
the tree now.
johannes
On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote:
> On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote:
> > Add changes to start a channel switch as well as finalize it on link basis
> > in order to support CSA with MLO as well.
> >
>
> FYI, this had a number of conflicts with my other work - please check
> the tree now.
>
Also here btw, some hostap test with hwsim would be nice - though again
don't know how well hostapd supports all this yet.
johannes
On 2/8/24 19:35, Johannes Berg wrote:
> On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote:
>> On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote:
>>> Add changes to start a channel switch as well as finalize it on link basis
>>> in order to support CSA with MLO as well.
>>>
>>
>> FYI, this had a number of conflicts with my other work - please check
>> the tree now.
>>
>
> Also here btw, some hostap test with hwsim would be nice - though again
> don't know how well hostapd supports all this yet.
>
> johannes
Sure, I will rebase on latest ToT and will add some hwsim test cases (if
supported well in hostapd) as well.
On 2/12/24 12:10, Aditya Kumar Singh wrote:
> On 2/8/24 19:35, Johannes Berg wrote:
>> On Thu, 2024-02-08 at 14:48 +0100, Johannes Berg wrote:
>>> On Tue, 2024-01-30 at 19:39 +0530, Aditya Kumar Singh wrote:
>>>> Add changes to start a channel switch as well as finalize it on link
>>>> basis
>>>> in order to support CSA with MLO as well.
>>>>
>>>
>>> FYI, this had a number of conflicts with my other work - please check
>>> the tree now.
>>>
>>
>> Also here btw, some hostap test with hwsim would be nice - though again
>> don't know how well hostapd supports all this yet.
>>
>> johannes
>
> Sure, I will rebase on latest ToT and will add some hwsim test cases (if
> supported well in hostapd) as well.
>
>
My bad! I see you have applied the changes. Thanks :)
Looks good to me. Let me see on the hwsim test cases and if possible
send it soon for review to Jouni.
On Mon, 2024-02-12 at 12:48 +0530, Aditya Kumar Singh wrote:
> My bad! I see you have applied the changes. Thanks :)
Right, sorry that wasn't clear.
> Looks good to me.
OK, thanks for checking.
> Let me see on the hwsim test cases and if possible
> send it soon for review to Jouni.
So ... I'm looking at the client side, and thinking about that.
According to the spec, multi-link element should be present in beacons
of APs affiliated with the same MLD if one of the (other) links is doing
CSA, and then have also the CSA counters of course, relative to the
target link's TBTT (of course.)
Theoretically, both mac80211 and hostapd today support different beacon
interval on different links, I believe.
This makes the whole thing of including CSA for link A on beacons/probe
responses transmitted on link B very tricky, because you have to know
the timing, etc.
For the CSA counter of a link _itself_ we have counter_offsets_beacon
and counter_offsets_presp (for probe response offload) in struct
cfg80211_csa_settings, and also counter offsets in struct
cfg80211_mgmt_tx_params for sending probe responses.
But ... for the cross-link information in the MLE this gets way more
tricky? Especially if the beacon interval is different - we couldn't
just count down, we'd have to fill in the information when we know where
the frame is transmitted. For probe responses maybe we can afford to not
be perfect, but for beacons it better be right - so we have to calculate
the right counter value(s) for (all of) the switching link(s) according
to the current TSF, TSF offset and TBTT - not all of which we even have
in the driver?
I can see a few ways of implementing this:
a) Punt to firmware and it parses the multi-link element etc. to
find the places to update. But that's awful, it requires parsing
possibly fragmented MLE with fragmented subelements containing the
CSA elements inside ...
b) Punt to firmware and give it a (possibly long) list of offsets K_N
where to put the N'th counter for link K when transmitting the
frame.
c) Require the get_tsf operation and add an operation to retrieve TSF
offset (**), and then calculate the TBTT for each link when
beacon_get is called with the per-link beacon intervals and update
the values correctly if CSA is in progress on any link ... requires
against parsing like in (a) or offset information like in (b), but
at least now it's in software?
For probe responses this could be a bit off I guess, but maybe that
doesn't matter as much - probe responses are not authenticated so a
client probably shouldn't use them for real CSA anyway.
(**) which I guess we need anyway for hostapd to put it into beacons
etc.?
d) Require beacon intervals to be the same, and then just count down?
Still requires the offset information etc., but that's also not
great because if the configuration happens during a TBTT on any of
the links (some before, some after), it'll be very wrong. So not
very inclined to do this one ...
Do you have any plans for any of this?
I'm mostly asking right now because I want a reliable way to test the
work I'm trying to do on the client side though, so I could also live
with some hacks (inject through debugfs?), but having it for real would
be nice.
johannes
On 2/12/24 20:16, Johannes Berg wrote:
> So ... I'm looking at the client side, and thinking about that.
> According to the spec, multi-link element should be present in beacons
> of APs affiliated with the same MLD if one of the (other) links is doing
> CSA, and then have also the CSA counters of course, relative to the
> target link's TBTT (of course.)
>
Yes correct, you are referring to critical update(s) right?
> Theoretically, both mac80211 and hostapd today support different beacon
> interval on different links, I believe.
>
> This makes the whole thing of including CSA for link A on beacons/probe
> responses transmitted on link B very tricky, because you have to know
> the timing, etc.
At least handling for probe response seems to be a bit easier since
there we need not maintain any timer as such (as you have said too - we
need not be perfect there). But beacons, as you said, it is indeed bit
tricky to handle full support considering both links could beacon at
different intervals.
>
> For the CSA counter of a link _itself_ we have counter_offsets_beacon
> and counter_offsets_presp (for probe response offload) in struct
> cfg80211_csa_settings, and also counter offsets in struct
> cfg80211_mgmt_tx_params for sending probe responses.
>
> But ... for the cross-link information in the MLE this gets way more
> tricky? Especially if the beacon interval is different - we couldn't
> just count down, we'd have to fill in the information when we know where
> the frame is transmitted. For probe responses maybe we can afford to not
> be perfect, but for beacons it better be right - so we have to calculate
> the right counter value(s) for (all of) the switching link(s) according
> to the current TSF, TSF offset and TBTT - not all of which we even have
> in the driver?
>
Yes correct :)
> I can see a few ways of implementing this:
>
> a) Punt to firmware and it parses the multi-link element etc. to
> find the places to update. But that's awful, it requires parsing
> possibly fragmented MLE with fragmented subelements containing the
> CSA elements inside ...
>
> b) Punt to firmware and give it a (possibly long) list of offsets K_N
> where to put the N'th counter for link K when transmitting the
> frame.
>
At least for beacons if firmware takes care of it then it will be good.
Firmware could maintain the counter (for the affected link) and (I
assume it will be aware of the partner links) so whenever partner link
transmits a beacon it could add the CSA (or such IEs) in per STA profile
of the reporting link. It could get the contents from the affected link
but actual value of the counter could be filled from the global counter
being maintained. But not sure whether we can force every driver's
firmware to do so?
Let aside actual drivers, adding the test case for mac80211_hw_sim would
also be tricky since there beacon Tx is handled in software only.
> c) Require the get_tsf operation and add an operation to retrieve TSF
> offset (**), and then calculate the TBTT for each link when
> beacon_get is called with the per-link beacon intervals and update
> the values correctly if CSA is in progress on any link ... requires
> against parsing like in (a) or offset information like in (b), but
> at least now it's in software?
> For probe responses this could be a bit off I guess, but maybe that
> doesn't matter as much - probe responses are not authenticated so a
> client probably shouldn't use them for real CSA anyway.
>
> (**) which I guess we need anyway for hostapd to put it into beacons
> etc.?
Yes I'm too anticipating hostapd changes to support the critical update.
>
> d) Require beacon intervals to be the same, and then just count down?
> Still requires the offset information etc., but that's also not
> great because if the configuration happens during a TBTT on any of
> the links (some before, some after), it'll be very wrong. So not
> very inclined to do this one ...
>
Agreed. Can't enforce the intervals to be same.
>
> Do you have any plans for any of this?
I do have some idea (possibly half baked code) for handling critical
updates in probe responses. Beacons, I assume firmware takes care hence
I have changes to mark it in the link_conf so that drivers could read
that and give it to its firmware.
Handling in beacon at kernel level, I need to think of it since anyways
hw_sim test case is good to have :)
Do we have any data right now, how many actual drivers have offloaded
beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)?
If we could see majority are using firmware then we could first add
support for such drivers then we can think about handling it in kernel?
>
> I'm mostly asking right now because I want a reliable way to test the
> work I'm trying to do on the client side though, so I could also live
> with some hacks (inject through debugfs?), but having it for real would
> be nice.
Okay. I'm right now working to add basic test case in hw_sim for MLO CSA
(on first link) as we discussed. There is need of a few mac80211 changes
in order to support for other links. I'm checking those now. Once done
with these, then I'll take up critical updates.
On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote:
> On 2/12/24 20:16, Johannes Berg wrote:
> > So ... I'm looking at the client side, and thinking about that.
> > According to the spec, multi-link element should be present in beacons
> > of APs affiliated with the same MLD if one of the (other) links is doing
> > CSA, and then have also the CSA counters of course, relative to the
> > target link's TBTT (of course.)
>
> Yes correct, you are referring to critical update(s) right?
I was actually thinking the critical update flag is rather not all that
interesting, but the inclusion of the CSA-related elements in a
(partial) per-STA profile in the Multi-Link Element is. Those contain
the countdown fields, so need to be updated - according to the TBTT of
the link doing the CSA, not the link sending it.
> > Theoretically, both mac80211 and hostapd today support different beacon
> > interval on different links, I believe.
> >
> > This makes the whole thing of including CSA for link A on beacons/probe
> > responses transmitted on link B very tricky, because you have to know
> > the timing, etc.
>
> At least handling for probe response seems to be a bit easier since
> there we need not maintain any timer as such (as you have said too - we
> need not be perfect there).
OTOH, if we already need some infrastructure for beacons, we can also
use that infrastructure to fill in probe responses? Might be easier
overall, because otherwise you'd have to start timers in hostapd etc. if
you actually wanted to fill that in hostapd, and that's messy ...
And as a complete aside: link removal _also_ has a countdown, but I'm
not sure that affects cross-link, but if it does we might want a more
general cross-link counter infrastructure? Though that might not mesh
well with how (your) firmware handles it?
> But beacons, as you said, it is indeed bit
> tricky to handle full support considering both links could beacon at
> different intervals.
Indeed.
> > For the CSA counter of a link _itself_ we have counter_offsets_beacon
> > and counter_offsets_presp (for probe response offload) in struct
> > cfg80211_csa_settings, and also counter offsets in struct
> > cfg80211_mgmt_tx_params for sending probe responses.
> >
> > But ... for the cross-link information in the MLE this gets way more
> > tricky? Especially if the beacon interval is different - we couldn't
> > just count down, we'd have to fill in the information when we know where
> > the frame is transmitted. For probe responses maybe we can afford to not
> > be perfect, but for beacons it better be right - so we have to calculate
> > the right counter value(s) for (all of) the switching link(s) according
> > to the current TSF, TSF offset and TBTT - not all of which we even have
> > in the driver?
> >
>
> Yes correct :)
:)
> > I can see a few ways of implementing this:
> >
> > a) Punt to firmware and it parses the multi-link element etc. to
> > find the places to update. But that's awful, it requires parsing
> > possibly fragmented MLE with fragmented subelements containing the
> > CSA elements inside ...
> >
> > b) Punt to firmware and give it a (possibly long) list of offsets K_N
> > where to put the N'th counter for link K when transmitting the
> > frame.
> >
>
> At least for beacons if firmware takes care of it then it will be good.
Note that I'm basically asking you, Mediatek and Realtek how this should
work. We're almost certainly not going to support multi-link AP in our
device any time soon, and we could adjust the firmware to it anyway if
needed.
So while I agree that punting it to firmware would be somewhat nice, we
still have to agree on (a) or (b), because (b) requires more information
from hostapd etc., but (a) requires _substantially_ more (and very
complex) parsing in firmware... If you wanted to support multi-BSSID,
then it's even more complex, which would argue for (b)?
I was trying to keep opinions out of this, but personally, I really
don't like (a), it puts too much complexity in the hands of firmware
engineers ;-)
> Firmware could maintain the counter (for the affected link) and (I
> assume it will be aware of the partner links) so whenever partner link
> transmits a beacon it could add the CSA (or such IEs) in per STA profile
> of the reporting link. It could get the contents from the affected link
> but actual value of the counter could be filled from the global counter
> being maintained. But not sure whether we can force every driver's
> firmware to do so?
I'm not sure I understand that description - how would firmware even
know which elements to take from what partner link etc.? I think hostapd
still has to add the elements, and firmware would just populate the
counter field(s)? But yeah that requires knowing from which partner link
to populate.
If you really wanted the firmware to handle the partner link information
completely it'd not only have to parse the Multi-Link Element on other
links but also _modify_, and know what to do, etc. - I don't think
that'll really work so well.
Think about it some more: On the link doing the CSA you'd have to parse
the beacon template and understand where the CSA-related elements are
(CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them
aside. Then on the partner link(s) parse the Multi-Link Element, see if
there's a partial STA profile already for some reason, merge the CSA-
related elements into that, update the counters; all while first
removing the 2 levels of fragmentation, and then re-doing them ...
I'm not even sure how I'd implement that in hwsim, and there I already
have all the helper functions and pretty much unlimited memory/code
size, so doing it in firmware seems awful?
Not to mention that it would never extend for future stuff, since new
elements wouldn't be copied over, firmware wouldn't necessarily know the
right order to merge them with existing elements in the per-STA profile,
etc.
> Let aside actual drivers, adding the test case for mac80211_hw_sim would
> also be tricky since there beacon Tx is handled in software only.
Yeah but that's easy, especially if we go with (b) approach, even
mac80211 basically has all the necessary information to handle hwsim's
model of beacon transmission, we could add a little bit of additional
help there too (e.g. pass the TBTT to beacon_get).
> > c) Require the get_tsf operation and add an operation to retrieve TSF
> > offset (**), and then calculate the TBTT for each link when
> > beacon_get is called with the per-link beacon intervals and update
> > the values correctly if CSA is in progress on any link ... requires
> > against parsing like in (a) or offset information like in (b), but
> > at least now it's in software?
> > For probe responses this could be a bit off I guess, but maybe that
> > doesn't matter as much - probe responses are not authenticated so a
> > client probably shouldn't use them for real CSA anyway.
> >
> > (**) which I guess we need anyway for hostapd to put it into beacons
> > etc.?
>
> Yes I'm too anticipating hostapd changes to support the critical update.
Oh hostapd will have to make changes here for everything - the (**)
footnote was about how hostapd knows the TSF offset between links?
Unless we're planning to always keep the TSF offset zero?
> > Do you have any plans for any of this?
>
> I do have some idea (possibly half baked code) for handling critical
> updates in probe responses. Beacons, I assume firmware takes care hence
> I have changes to mark it in the link_conf so that drivers could read
> that and give it to its firmware.
> Handling in beacon at kernel level, I need to think of it since anyways
> hw_sim test case is good to have :)
>
> Do we have any data right now, how many actual drivers have offloaded
> beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)?
> If we could see majority are using firmware then we could first add
> support for such drivers then we can think about handling it in kernel?
I agree that we can go with one approach for now, but I think we should
agree on the first model of how firmware does this. I'm not convinced
that your description above where firmware even magically handles
propagating the CSA elements to partner links will really work? But see
above.
> > I'm mostly asking right now because I want a reliable way to test the
> > work I'm trying to do on the client side though, so I could also live
> > with some hacks (inject through debugfs?), but having it for real would
> > be nice.
>
> Okay. I'm right now working to add basic test case in hw_sim for MLO CSA
> (on first link) as we discussed. There is need of a few mac80211 changes
> in order to support for other links. I'm checking those now. Once done
> with these, then I'll take up critical updates.
>
>
>
Uh, sorry, accidentally sent too soon.
> > > I'm mostly asking right now because I want a reliable way to test the
> > > work I'm trying to do on the client side though, so I could also live
> > > with some hacks (inject through debugfs?), but having it for real would
> > > be nice.
> >
> > Okay. I'm right now working to add basic test case in hw_sim for MLO CSA
> > (on first link) as we discussed. There is need of a few mac80211 changes
> > in order to support for other links. I'm checking those now. Once done
> > with these, then I'll take up critical updates.
OK. I also have a few patches that I was working on. Mostly client
related, but maybe these would be relevant for you as well:
https://p.sipsolutions.net/f9e976055a616eda.txt
https://p.sipsolutions.net/054dcb0502247890.txt
Sorry about the format - once I've fully tested them etc. I'll send them
out.
johannes
On 2/13/24 14:12, Johannes Berg wrote:
> On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote:
>> On 2/12/24 20:16, Johannes Berg wrote:
>>> So ... I'm looking at the client side, and thinking about that.
>>> According to the spec, multi-link element should be present in beacons
>>> of APs affiliated with the same MLD if one of the (other) links is doing
>>> CSA, and then have also the CSA counters of course, relative to the
>>> target link's TBTT (of course.)
>>
>> Yes correct, you are referring to critical update(s) right?
>
> I was actually thinking the critical update flag is rather not all that
> interesting, but the inclusion of the CSA-related elements in a
> (partial) per-STA profile in the Multi-Link Element is. Those contain
> the countdown fields, so need to be updated - according to the TBTT of
> the link doing the CSA, not the link sending it.
>
Yes correct, this CSA thing is interesting but there is one more thing -
handling of the BSS param change count (BPCC). Let's say the affected
link undergoes CSA, then in its Basic Multi Link IE, BPCC should be
incremented. But at the same time, in RNR of its partner links, where
this affected link is there, the BPCC should also be incremented.
All though here it is not incremented with every beacon Tx, but still
change in one affects beacon of other partner link(s) at least once.
>>> Theoretically, both mac80211 and hostapd today support different beacon
>>> interval on different links, I believe.
>>>
>>> This makes the whole thing of including CSA for link A on beacons/probe
>>> responses transmitted on link B very tricky, because you have to know
>>> the timing, etc.
>>
>> At least handling for probe response seems to be a bit easier since
>> there we need not maintain any timer as such (as you have said too - we
>> need not be perfect there).
>
> OTOH, if we already need some infrastructure for beacons, we can also
> use that infrastructure to fill in probe responses? Might be easier
> overall, because otherwise you'd have to start timers in hostapd etc. if
> you actually wanted to fill that in hostapd, and that's messy ...
>
Correct!
> And as a complete aside: link removal _also_ has a countdown, but I'm
> not sure that affects cross-link, but if it does we might want a more
> general cross-link counter infrastructure? Though that might not mesh
> well with how (your) firmware handles it?
>
Yes.
>> But beacons, as you said, it is indeed bit
>> tricky to handle full support considering both links could beacon at
>> different intervals.
>
> Indeed.
>
>>> For the CSA counter of a link _itself_ we have counter_offsets_beacon
>>> and counter_offsets_presp (for probe response offload) in struct
>>> cfg80211_csa_settings, and also counter offsets in struct
>>> cfg80211_mgmt_tx_params for sending probe responses.
>>>
>>> But ... for the cross-link information in the MLE this gets way more
>>> tricky? Especially if the beacon interval is different - we couldn't
>>> just count down, we'd have to fill in the information when we know where
>>> the frame is transmitted. For probe responses maybe we can afford to not
>>> be perfect, but for beacons it better be right - so we have to calculate
>>> the right counter value(s) for (all of) the switching link(s) according
>>> to the current TSF, TSF offset and TBTT - not all of which we even have
>>> in the driver?
>>>
>>
>> Yes correct :)
>
> :)
>
>>> I can see a few ways of implementing this:
>>>
>>> a) Punt to firmware and it parses the multi-link element etc. to
>>> find the places to update. But that's awful, it requires parsing
>>> possibly fragmented MLE with fragmented subelements containing the
>>> CSA elements inside ...
>>>
>>> b) Punt to firmware and give it a (possibly long) list of offsets K_N
>>> where to put the N'th counter for link K when transmitting the
>>> frame.
>>>
>>
>> At least for beacons if firmware takes care of it then it will be good.
>
> Note that I'm basically asking you, Mediatek and Realtek how this should
> work. We're almost certainly not going to support multi-link AP in our
> device any time soon, and we could adjust the firmware to it anyway if
> needed.
>
> So while I agree that punting it to firmware would be somewhat nice, we
> still have to agree on (a) or (b), because (b) requires more information
> from hostapd etc., but (a) requires _substantially_ more (and very
> complex) parsing in firmware... If you wanted to support multi-BSSID,
> then it's even more complex, which would argue for (b)?
>
Correct. Both has got its own advantages and disadvantages. Need to
agree on any one so that all can be benefited. Let's hear from other folks.
> I was trying to keep opinions out of this, but personally, I really
> don't like (a), it puts too much complexity in the hands of firmware
> engineers ;-)
>
>> Firmware could maintain the counter (for the affected link) and (I
>> assume it will be aware of the partner links) so whenever partner link
>> transmits a beacon it could add the CSA (or such IEs) in per STA profile
>> of the reporting link. It could get the contents from the affected link
>> but actual value of the counter could be filled from the global counter
>> being maintained. But not sure whether we can force every driver's
>> firmware to do so?
>
> I'm not sure I understand that description - how would firmware even
> know which elements to take from what partner link etc.? I think hostapd
> still has to add the elements, and firmware would just populate the
> counter field(s)? But yeah that requires knowing from which partner link
> to populate.
>
Yeah so underlying assumption/thought was -
1. Firmware is aware of partner links
2. Host driver just needs to update the beacon template of the affected
link and some how indicate to firmware that this is critical update.
3. As per spec, there are around 21-22 changes classified as critical
update and among those only handful of those which has adding the IE in
per STA profile. So firmware could possibly maintain a table to see if
the affected link's beacon template has such IEs which if present needs
to be added in partner links, it could add it as and when partner link
transmits a beacon.
4. Once CSA is finished, the host would update the beacon template which
will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to
per STA profile as well of the reporting links.
But yeah adding to per STA profile when already elements are there and
then first de-fragment and then add and then later fragment would be a
tedious job :)
> If you really wanted the firmware to handle the partner link information
> completely it'd not only have to parse the Multi-Link Element on other
> links but also _modify_, and know what to do, etc. - I don't think
> that'll really work so well.
>
> Think about it some more: On the link doing the CSA you'd have to parse
> the beacon template and understand where the CSA-related elements are
> (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them
> aside. Then on the partner link(s) parse the Multi-Link Element, see if
> there's a partial STA profile already for some reason, merge the CSA-
> related elements into that, update the counters; all while first
> removing the 2 levels of fragmentation, and then re-doing them ...
>
Yeah complex on firmware level. But even if we solve this on kernel
level, existing drivers who have offloaded beacon to firmware from 11ax
or before itself, still need to handle it? It would looks ugly that till
11ax it is offloaded and from 11be it is again put back to kernel?
> I'm not even sure how I'd implement that in hwsim, and there I already
> have all the helper functions and pretty much unlimited memory/code
> size, so doing it in firmware seems awful?
>
Yes.
> Not to mention that it would never extend for future stuff, since new
> elements wouldn't be copied over, firmware wouldn't necessarily know the
> right order to merge them with existing elements in the per-STA profile,
> etc.
>
Yeah so if we go with handle on firmware level, as and when spec
changes, firmware changes are required too.
>
>> Let aside actual drivers, adding the test case for mac80211_hw_sim would
>> also be tricky since there beacon Tx is handled in software only.
>
> Yeah but that's easy, especially if we go with (b) approach, even
> mac80211 basically has all the necessary information to handle hwsim's
> model of beacon transmission, we could add a little bit of additional
> help there too (e.g. pass the TBTT to beacon_get).
>
>>> c) Require the get_tsf operation and add an operation to retrieve TSF
>>> offset (**), and then calculate the TBTT for each link when
>>> beacon_get is called with the per-link beacon intervals and update
>>> the values correctly if CSA is in progress on any link ... requires
>>> against parsing like in (a) or offset information like in (b), but
>>> at least now it's in software?
>>> For probe responses this could be a bit off I guess, but maybe that
>>> doesn't matter as much - probe responses are not authenticated so a
>>> client probably shouldn't use them for real CSA anyway.
>>>
>>> (**) which I guess we need anyway for hostapd to put it into beacons
>>> etc.?
>>
>> Yes I'm too anticipating hostapd changes to support the critical update.
>
> Oh hostapd will have to make changes here for everything - the (**)
> footnote was about how hostapd knows the TSF offset between links?
> Unless we're planning to always keep the TSF offset zero?
>
>>> Do you have any plans for any of this?
>>
>> I do have some idea (possibly half baked code) for handling critical
>> updates in probe responses. Beacons, I assume firmware takes care hence
>> I have changes to mark it in the link_conf so that drivers could read
>> that and give it to its firmware.
>> Handling in beacon at kernel level, I need to think of it since anyways
>> hw_sim test case is good to have :)
>>
>> Do we have any data right now, how many actual drivers have offloaded
>> beacon Tx to its firmware (at least those who are supporting Wi-Fi 7)?
>> If we could see majority are using firmware then we could first add
>> support for such drivers then we can think about handling it in kernel?
>
> I agree that we can go with one approach for now, but I think we should
> agree on the first model of how firmware does this. I'm not convinced
> that your description above where firmware even magically handles
> propagating the CSA elements to partner links will really work? But see
> above.
>
>>> I'm mostly asking right now because I want a reliable way to test the
>>> work I'm trying to do on the client side though, so I could also live
>>> with some hacks (inject through debugfs?), but having it for real would
>>> be nice.
>>
>> Okay. I'm right now working to add basic test case in hw_sim for MLO CSA
>> (on first link) as we discussed. There is need of a few mac80211 changes
>> in order to support for other links. I'm checking those now. Once done
>> with these, then I'll take up critical updates.
>>
>>
>>
On Tue, 2024-02-13 at 15:48 +0530, Aditya Kumar Singh wrote:
> On 2/13/24 14:12, Johannes Berg wrote:
> > On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote:
> > > On 2/12/24 20:16, Johannes Berg wrote:
> > > > So ... I'm looking at the client side, and thinking about that.
> > > > According to the spec, multi-link element should be present in beacons
> > > > of APs affiliated with the same MLD if one of the (other) links is doing
> > > > CSA, and then have also the CSA counters of course, relative to the
> > > > target link's TBTT (of course.)
> > >
> > > Yes correct, you are referring to critical update(s) right?
> >
> > I was actually thinking the critical update flag is rather not all that
> > interesting, but the inclusion of the CSA-related elements in a
> > (partial) per-STA profile in the Multi-Link Element is. Those contain
> > the countdown fields, so need to be updated - according to the TBTT of
> > the link doing the CSA, not the link sending it.
> >
>
> Yes correct, this CSA thing is interesting but there is one more thing -
> handling of the BSS param change count (BPCC). Let's say the affected
> link undergoes CSA, then in its Basic Multi Link IE, BPCC should be
> incremented. But at the same time, in RNR of its partner links, where
> this affected link is there, the BPCC should also be incremented.
> All though here it is not incremented with every beacon Tx, but still
> change in one affects beacon of other partner link(s) at least once.
Yes, I know, but IMHO that's not all that interesting, I'd think hostapd
could handle that.
> > Note that I'm basically asking you, Mediatek and Realtek how this should
> > work. We're almost certainly not going to support multi-link AP in our
> > device any time soon, and we could adjust the firmware to it anyway if
> > needed.
> >
> > So while I agree that punting it to firmware would be somewhat nice, we
> > still have to agree on (a) or (b), because (b) requires more information
> > from hostapd etc., but (a) requires _substantially_ more (and very
> > complex) parsing in firmware... If you wanted to support multi-BSSID,
> > then it's even more complex, which would argue for (b)?
> >
>
> Correct. Both has got its own advantages and disadvantages. Need to
> agree on any one so that all can be benefited. Let's hear from other folks.
Do you _have_ firmware for this already, perhaps?
I mean, I'm happy to be designing this "greenfield", but ... it seems
unlikely at this point?
Also what about ath12k with the multi-device changes, would the driver
have to handle it? Or I guess you have to sync the TSF somehow anyway,
so you could handle it based on TSF/TBTT?
> > I was trying to keep opinions out of this, but personally, I really
> > don't like (a), it puts too much complexity in the hands of firmware
> > engineers ;-)
> >
> > > Firmware could maintain the counter (for the affected link) and (I
> > > assume it will be aware of the partner links) so whenever partner link
> > > transmits a beacon it could add the CSA (or such IEs) in per STA profile
> > > of the reporting link. It could get the contents from the affected link
> > > but actual value of the counter could be filled from the global counter
> > > being maintained. But not sure whether we can force every driver's
> > > firmware to do so?
> >
> > I'm not sure I understand that description - how would firmware even
> > know which elements to take from what partner link etc.? I think hostapd
> > still has to add the elements, and firmware would just populate the
> > counter field(s)? But yeah that requires knowing from which partner link
> > to populate.
> >
>
> Yeah so underlying assumption/thought was -
> 1. Firmware is aware of partner links
Sure.
> 2. Host driver just needs to update the beacon template of the affected
> link and some how indicate to firmware that this is critical update.
Sure.
> 3. As per spec, there are around 21-22 changes classified as critical
> update and among those only handful of those which has adding the IE in
> per STA profile. So firmware could possibly maintain a table to see if
> the affected link's beacon template has such IEs which if present needs
> to be added in partner links, it could add it as and when partner link
> transmits a beacon.
That seems messy for all the reasons I mentioned, since it involves
parsing/defragmenting/merging/fragmenting per-STA profiles etc.
Do you really want to go there?
> 4. Once CSA is finished, the host would update the beacon template which
> will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to
> per STA profile as well of the reporting links.
Yeah I guess that's just a corollary of 3 :)
> But yeah adding to per STA profile when already elements are there and
> then first de-fragment and then add and then later fragment would be a
> tedious job :)
Yeah...
> > If you really wanted the firmware to handle the partner link information
> > completely it'd not only have to parse the Multi-Link Element on other
> > links but also _modify_, and know what to do, etc. - I don't think
> > that'll really work so well.
> >
> > Think about it some more: On the link doing the CSA you'd have to parse
> > the beacon template and understand where the CSA-related elements are
> > (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them
> > aside. Then on the partner link(s) parse the Multi-Link Element, see if
> > there's a partial STA profile already for some reason, merge the CSA-
> > related elements into that, update the counters; all while first
> > removing the 2 levels of fragmentation, and then re-doing them ...
> >
>
> Yeah complex on firmware level. But even if we solve this on kernel
> level, existing drivers who have offloaded beacon to firmware from 11ax
> or before itself, still need to handle it? It would looks ugly that till
> 11ax it is offloaded and from 11be it is again put back to kernel?
Not sure what you mean here - we can still offload beaconing with a
template, just have to fill in the template a bit more (with partner
link CSA counters rather than just own CSA counters)?
I'm basically thinking this:
1. Firmware is aware of partner link CSA counters/TSF offset/TBTT
2. Host driver (hostapd) sets beacon template of affected link to
include CSA elements, and already today we include in the setting the
offset to the countdown fields, so it doesn't matter where those are.
3. Host driver (hostapd) *also* updates beacon templates of partner
links, but instead of saying "here's the CSA counter to update" it says
"here's the CSA counter for your partner link M to set" (OK, so there
potentially are multiple of those, but that's OK)
4. Firmware doesn't need to parse anything, it just needs to go through
the beacon when transmitting it (for any link) and set the CSA counter
fields for the link itself and/or for the partner links according to
what the correct counter is per TBTT rules, TSF offset, beacon interval
etc.
Note that this could easily be implemented even in software for hwsim or
drivers that don't offload beacon template to the device (like ath9k
though obviously that has no multi-link), *iff* you know the TSF offset
and TBTT of the beacon being created.
Currently the CSA counters are just decremented every time you request a
beacon, but the partner link counters would have to be calculated - but
if you know
- the intended TBTT of the beacon to transmit
- the TSF offset to the link undergoing CSA
- the beacon interval of the link undergoing CSA
you can calculate
- the TSF of the link undergoing CSA at the TBTT of the beacon we're
creating,
- the previous TBTT of the link undergoing CSA by the TSF (some
calculation involving TSF modulo the beacon interval)
- therefore the CSA counter of the link undergoing CSA to fill in
In hwsim it's even easier because we only request the beacon at the
TBTT, and the TSF offset is 0 ...
johannes
On 2/13/24 16:25, Johannes Berg wrote:
> On Tue, 2024-02-13 at 15:48 +0530, Aditya Kumar Singh wrote:
>> On 2/13/24 14:12, Johannes Berg wrote:
>>> On Tue, 2024-02-13 at 10:46 +0530, Aditya Kumar Singh wrote:
>>>> On 2/12/24 20:16, Johannes Berg wrote:
>>>>> So ... I'm looking at the client side, and thinking about that.
>>>>> According to the spec, multi-link element should be present in beacons
>>>>> of APs affiliated with the same MLD if one of the (other) links is doing
>>>>> CSA, and then have also the CSA counters of course, relative to the
>>>>> target link's TBTT (of course.)
>>>>
>>>> Yes correct, you are referring to critical update(s) right?
>>>
>>> I was actually thinking the critical update flag is rather not all that
>>> interesting, but the inclusion of the CSA-related elements in a
>>> (partial) per-STA profile in the Multi-Link Element is. Those contain
>>> the countdown fields, so need to be updated - according to the TBTT of
>>> the link doing the CSA, not the link sending it.
>>>
>>
>> Yes correct, this CSA thing is interesting but there is one more thing -
>> handling of the BSS param change count (BPCC). Let's say the affected
>> link undergoes CSA, then in its Basic Multi Link IE, BPCC should be
>> incremented. But at the same time, in RNR of its partner links, where
>> this affected link is there, the BPCC should also be incremented.
>> All though here it is not incremented with every beacon Tx, but still
>> change in one affects beacon of other partner link(s) at least once.
>
> Yes, I know, but IMHO that's not all that interesting, I'd think hostapd
> could handle that.
>
>>> Note that I'm basically asking you, Mediatek and Realtek how this should
>>> work. We're almost certainly not going to support multi-link AP in our
>>> device any time soon, and we could adjust the firmware to it anyway if
>>> needed.
>>>
>>> So while I agree that punting it to firmware would be somewhat nice, we
>>> still have to agree on (a) or (b), because (b) requires more information
>>> from hostapd etc., but (a) requires _substantially_ more (and very
>>> complex) parsing in firmware... If you wanted to support multi-BSSID,
>>> then it's even more complex, which would argue for (b)?
>>>
>>
>> Correct. Both has got its own advantages and disadvantages. Need to
>> agree on any one so that all can be benefited. Let's hear from other folks.
>
> Do you _have_ firmware for this already, perhaps?
>
Yes, private firmware is there. Not yet upstreamed since the
infrastructure in mac80211 and driver is not there. AFAIK it is in
pipeline. Internal testing have so far been good and did not see any
issues. So at least from Qualcomm perspective, we are fine with
offloading (the above we discussed) to firmware.
> I mean, I'm happy to be designing this "greenfield", but ... it seems
> unlikely at this point?
>
> Also what about ath12k with the multi-device changes, would the driver
> have to handle it? Or I guess you have to sync the TSF somehow anyway,
> so you could handle it based on TSF/TBTT?
>
Ath12k firmware also supports beacon offload and hence firmware is
taking care to add the respective IEs in per-STA profile of the
reporting links while host just sends the template for the affected link
via beacon template update and indicating that critical update has happened.
>
>>> I was trying to keep opinions out of this, but personally, I really
>>> don't like (a), it puts too much complexity in the hands of firmware
>>> engineers ;-)
>>>
>>>> Firmware could maintain the counter (for the affected link) and (I
>>>> assume it will be aware of the partner links) so whenever partner link
>>>> transmits a beacon it could add the CSA (or such IEs) in per STA profile
>>>> of the reporting link. It could get the contents from the affected link
>>>> but actual value of the counter could be filled from the global counter
>>>> being maintained. But not sure whether we can force every driver's
>>>> firmware to do so?
>>>
>>> I'm not sure I understand that description - how would firmware even
>>> know which elements to take from what partner link etc.? I think hostapd
>>> still has to add the elements, and firmware would just populate the
>>> counter field(s)? But yeah that requires knowing from which partner link
>>> to populate.
>>>
>>
>> Yeah so underlying assumption/thought was -
>> 1. Firmware is aware of partner links
>
> Sure.
>
>> 2. Host driver just needs to update the beacon template of the affected
>> link and some how indicate to firmware that this is critical update.
>
> Sure.
>
>> 3. As per spec, there are around 21-22 changes classified as critical
>> update and among those only handful of those which has adding the IE in
>> per STA profile. So firmware could possibly maintain a table to see if
>> the affected link's beacon template has such IEs which if present needs
>> to be added in partner links, it could add it as and when partner link
>> transmits a beacon.
>
> That seems messy for all the reasons I mentioned, since it involves
> parsing/defragmenting/merging/fragmenting per-STA profiles etc.
>
> Do you really want to go there?
>
I understand things looks to be messy. But at least internally we have
done in that way and things look simpler (on kernel level). Not sure
whether all firmwares would agree to that.
>> 4. Once CSA is finished, the host would update the beacon template which
>> will not have CSA/ECSA IE... such IEs. Then firmware can stop adding to
>> per STA profile as well of the reporting links.
>
> Yeah I guess that's just a corollary of 3 :)
>
>> But yeah adding to per STA profile when already elements are there and
>> then first de-fragment and then add and then later fragment would be a
>> tedious job :)
>
> Yeah...
>
>>> If you really wanted the firmware to handle the partner link information
>>> completely it'd not only have to parse the Multi-Link Element on other
>>> links but also _modify_, and know what to do, etc. - I don't think
>>> that'll really work so well.
>>>
>>> Think about it some more: On the link doing the CSA you'd have to parse
>>> the beacon template and understand where the CSA-related elements are
>>> (CSA, ECSA, Channel Switch Wrapper, possibly more?), and copy them
>>> aside. Then on the partner link(s) parse the Multi-Link Element, see if
>>> there's a partial STA profile already for some reason, merge the CSA-
>>> related elements into that, update the counters; all while first
>>> removing the 2 levels of fragmentation, and then re-doing them ...
>>>
>>
>> Yeah complex on firmware level. But even if we solve this on kernel
>> level, existing drivers who have offloaded beacon to firmware from 11ax
>> or before itself, still need to handle it? It would looks ugly that till
>> 11ax it is offloaded and from 11be it is again put back to kernel?
>
> Not sure what you mean here - we can still offload beaconing with a
> template, just have to fill in the template a bit more (with partner
> link CSA counters rather than just own CSA counters)?
> > I'm basically thinking this:
>
> 1. Firmware is aware of partner link CSA counters/TSF offset/TBTT
>
> 2. Host driver (hostapd) sets beacon template of affected link to
> include CSA elements, and already today we include in the setting the
> offset to the countdown fields, so it doesn't matter where those are.
>
> 3. Host driver (hostapd) *also* updates beacon templates of partner
> links, but instead of saying "here's the CSA counter to update" it says
> "here's the CSA counter for your partner link M to set" (OK, so there
> potentially are multiple of those, but that's OK)
>
> 4. Firmware doesn't need to parse anything, it just needs to go through
> the beacon when transmitting it (for any link) and set the CSA counter
> fields for the link itself and/or for the partner links according to
> what the correct counter is per TBTT rules, TSF offset, beacon interval
> etc.
>
Hmm.. I see your point. So basically there is tradeoff - complexity of
handling the parsing and all in firmware or complexity of handling those
many offsets in kernel. I believe when hostapd updates the templates for
the partner links, it would have to send offsets so we may need to
extend our NL infrastructure for MLO in order to accommodate all those
list of offsets during beacon template update.
>
> Note that this could easily be implemented even in software for hwsim or
> drivers that don't offload beacon template to the device (like ath9k
> though obviously that has no multi-link), *iff* you know the TSF offset
> and TBTT of the beacon being created.
> Currently the CSA counters are just decremented every time you request a
> beacon, but the partner link counters would have to be calculated - but
> if you know
> - the intended TBTT of the beacon to transmit
> - the TSF offset to the link undergoing CSA
> - the beacon interval of the link undergoing CSA
> you can calculate
> - the TSF of the link undergoing CSA at the TBTT of the beacon we're
> creating,
> - the previous TBTT of the link undergoing CSA by the TSF (some
> calculation involving TSF modulo the beacon interval)
> - therefore the CSA counter of the link undergoing CSA to fill in
>
> In hwsim it's even easier because we only request the beacon at the
> TBTT, and the TSF offset is 0 ...
>
Yep, agree.
On Tue, 2024-02-13 at 18:11 +0530, Aditya Kumar Singh wrote:
> > > Correct. Both has got its own advantages and disadvantages. Need to
> > > agree on any one so that all can be benefited. Let's hear from other folks.
> >
> > Do you _have_ firmware for this already, perhaps?
> >
>
> Yes, private firmware is there. Not yet upstreamed since the
> infrastructure in mac80211 and driver is not there. AFAIK it is in
> pipeline. Internal testing have so far been good and did not see any
> issues. So at least from Qualcomm perspective, we are fine with
> offloading (the above we discussed) to firmware.
Ah, OK.
Note that if we have all the offsets I guess you could still do all the
parsing? Though adding the per-STA profile in cross-link scenarios would
be a difference if hostapd has to do it in the template vs. firmware
copying the elements ...
For the record, I really don't like copying the elements and not sure
I'd want to implement that in mac80211/hwsim...
> > I mean, I'm happy to be designing this "greenfield", but ... it seems
> > unlikely at this point?
> >
> > Also what about ath12k with the multi-device changes, would the driver
> > have to handle it? Or I guess you have to sync the TSF somehow anyway,
> > so you could handle it based on TSF/TBTT?
> >
>
> Ath12k firmware also supports beacon offload and hence firmware is
> taking care to add the respective IEs in per-STA profile of the
> reporting links while host just sends the template for the affected link
> via beacon template update and indicating that critical update has happened.
OK. So I guess we have to support that? Or are there any chances we
could agree on something here overall and then the firmware can be
changed to support another mode?
> > That seems messy for all the reasons I mentioned, since it involves
> > parsing/defragmenting/merging/fragmenting per-STA profiles etc.
> >
> > Do you really want to go there?
> >
>
> I understand things looks to be messy. But at least internally we have
> done in that way and things look simpler (on kernel level). Not sure
> whether all firmwares would agree to that.
And hwsim, but yeah :)
> > Not sure what you mean here - we can still offload beaconing with a
> > template, just have to fill in the template a bit more (with partner
> > link CSA counters rather than just own CSA counters)?
> > > I'm basically thinking this:
> >
> > 1. Firmware is aware of partner link CSA counters/TSF offset/TBTT
> >
> > 2. Host driver (hostapd) sets beacon template of affected link to
> > include CSA elements, and already today we include in the setting the
> > offset to the countdown fields, so it doesn't matter where those are.
> >
> > 3. Host driver (hostapd) *also* updates beacon templates of partner
> > links, but instead of saying "here's the CSA counter to update" it says
> > "here's the CSA counter for your partner link M to set" (OK, so there
> > potentially are multiple of those, but that's OK)
> >
> > 4. Firmware doesn't need to parse anything, it just needs to go through
> > the beacon when transmitting it (for any link) and set the CSA counter
> > fields for the link itself and/or for the partner links according to
> > what the correct counter is per TBTT rules, TSF offset, beacon interval
> > etc.
> >
>
> Hmm.. I see your point. So basically there is tradeoff - complexity of
> handling the parsing and all in firmware or complexity of handling those
> many offsets in kernel. I believe when hostapd updates the templates for
> the partner links, it would have to send offsets so we may need to
> extend our NL infrastructure for MLO in order to accommodate all those
> list of offsets during beacon template update.
Yes, of course. But that doesn't seem _too_ complicated? Even supporting
max # of links (14) and ~3 counters each isn't a huge list... netlink
doesn't care, and drivers have lower link limits anyway.
Maybe I'll do a prototype later? But for now
a) I need to test the client-side code I'm writing in some other way
(element injection or something)
b) let's wait for Realtek to comment also after the Lunar New Year :)
Thanks!
johannes
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Tuesday, February 13, 2024 8:49 PM
> To: Aditya Kumar Singh <[email protected]>
> Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> <[email protected]>; Jouni Malinen <[email protected]>; Ping-Ke Shih <[email protected]>; Ryder Lee
> <[email protected]>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
>
> b) let's wait for Realtek to comment also after the Lunar New Year :)
>
Sorry for the late.
Realtek firmware can update partner links' CSA/ECSA by given offset of
beacon template, which matches the conclusion of this thread, and Realtek vendor
(out of tree) driver has verified the firmware interface. So that will
work to Realtek WiFi 7 chips.
Ping-Ke
>
> Sorry for the late.
>
> Realtek firmware can update partner links' CSA/ECSA by given offset of
> beacon template, which matches the conclusion of this thread, and Realtek vendor
> (out of tree) driver has verified the firmware interface. So that will
> work to Realtek WiFi 7 chips.
Thanks!
I guess that'd also apply to probe responses? Or does it not send those
at all? But we discussed before that maybe we don't have to be perfect
there, so I guess we can find some solution to that.
So we have, so far
Realtek:
- uses offset in beacon template to set partner counter,
- requires host to set CSA/ECSA elements
Qualcomm:
- copies and updates CSA/ECSA elements all by itself
- btw, not sure here about probe responses, does it do that too?
hwsim:
- personally, I'd prefer it works like Realtek, for less complexity
Intel:
- not implemented, probably not going to happen any time soon, but I
think we might prefer the Realtek way too if we ever do this
Mediatek? Broadcom? Anyone else?
I guess then for Qualcomm we'll need to just add an extended feature
flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
update the parter link templates, or so?
Thanks,
johannes
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Wednesday, February 21, 2024 4:10 PM
> To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> <[email protected]>; Felix Fietkau <[email protected]>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
>
> I guess that'd also apply to probe responses? Or does it not send those
> at all? But we discussed before that maybe we don't have to be perfect
> there, so I guess we can find some solution to that.
Unfortunately, Realtek firmware doesn't send probe responses at all. Still
need hostapd to reply those.
Ping-Ke
On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote:
> > -----Original Message-----
> > From: Johannes Berg <[email protected]>
> > Sent: Wednesday, February 21, 2024 4:10 PM
> > To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> > Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> > <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> > <[email protected]>; Felix Fietkau <[email protected]>
> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> >
> > I guess that'd also apply to probe responses? Or does it not send those
> > at all? But we discussed before that maybe we don't have to be perfect
> > there, so I guess we can find some solution to that.
>
> Unfortunately, Realtek firmware doesn't send probe responses at all. Still
> need hostapd to reply those.
>
Right, but filling in the CSA counters?
johannes
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Wednesday, February 21, 2024 4:20 PM
> To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> <[email protected]>; Felix Fietkau <[email protected]>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
>
> On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote:
> > > -----Original Message-----
> > > From: Johannes Berg <[email protected]>
> > > Sent: Wednesday, February 21, 2024 4:10 PM
> > > To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> > > Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> > > <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> > > <[email protected]>; Felix Fietkau <[email protected]>
> > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > >
> > > I guess that'd also apply to probe responses? Or does it not send those
> > > at all? But we discussed before that maybe we don't have to be perfect
> > > there, so I guess we can find some solution to that.
> >
> > Unfortunately, Realtek firmware doesn't send probe responses at all. Still
> > need hostapd to reply those.
> >
>
> Right, but filling in the CSA counters?
>
No, firmware doesn't modify content of probe response frame.
Ping-Ke
On Wed, 2024-02-21 at 08:28 +0000, Ping-Ke Shih wrote:
>
> > -----Original Message-----
> > From: Johannes Berg <[email protected]>
> > Sent: Wednesday, February 21, 2024 4:20 PM
> > To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> > Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> > <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> > <[email protected]>; Felix Fietkau <[email protected]>
> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> >
> > On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote:
> > > > -----Original Message-----
> > > > From: Johannes Berg <[email protected]>
> > > > Sent: Wednesday, February 21, 2024 4:10 PM
> > > > To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> > > > Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> > > > <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> > > > <[email protected]>; Felix Fietkau <[email protected]>
> > > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > > >
> > > > I guess that'd also apply to probe responses? Or does it not send those
> > > > at all? But we discussed before that maybe we don't have to be perfect
> > > > there, so I guess we can find some solution to that.
> > >
> > > Unfortunately, Realtek firmware doesn't send probe responses at all. Still
> > > need hostapd to reply those.
> > >
> >
> > Right, but filling in the CSA counters?
> >
>
> No, firmware doesn't modify content of probe response frame.
>
Can you get that fixed? ;-)
With differing beacon intervals etc., I don't know there's a good way to
keep the counters even with a semblance of correctness, especially if we
don't know when the beacon was transmitted?
Or maybe just fill it in the driver since you probably have some beacon
timing data more easily accessible?
johannes
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Wednesday, February 21, 2024 4:35 PM
> To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> <[email protected]>; Felix Fietkau <[email protected]>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
>
> On Wed, 2024-02-21 at 08:28 +0000, Ping-Ke Shih wrote:
> >
> > > -----Original Message-----
> > > From: Johannes Berg <[email protected]>
> > > Sent: Wednesday, February 21, 2024 4:20 PM
> > > To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> > > Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> > > <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> > > <[email protected]>; Felix Fietkau <[email protected]>
> > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > >
> > > On Wed, 2024-02-21 at 08:19 +0000, Ping-Ke Shih wrote:
> > > > > -----Original Message-----
> > > > > From: Johannes Berg <[email protected]>
> > > > > Sent: Wednesday, February 21, 2024 4:10 PM
> > > > > To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> > > > > Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> > > > > <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> > > > > <[email protected]>; Felix Fietkau <[email protected]>
> > > > > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> > > > >
> > > > > I guess that'd also apply to probe responses? Or does it not send those
> > > > > at all? But we discussed before that maybe we don't have to be perfect
> > > > > there, so I guess we can find some solution to that.
> > > >
> > > > Unfortunately, Realtek firmware doesn't send probe responses at all. Still
> > > > need hostapd to reply those.
> > > >
> > >
> > > Right, but filling in the CSA counters?
> > >
> >
> > No, firmware doesn't modify content of probe response frame.
> >
>
> Can you get that fixed? ;-)
>
> With differing beacon intervals etc., I don't know there's a good way to
> keep the counters even with a semblance of correctness, especially if we
> don't know when the beacon was transmitted?
>
> Or maybe just fill it in the driver since you probably have some beacon
> timing data more easily accessible?
If driver can get CSA or ECSA offset simply, I probably can fill a reasonable
CSA counter (not sure if I can get 100% accurate counter for now), but
seemingly neither ieee80211_tx_control nor ieee80211_tx_info (SKB_CB) doesn't
have these offsets. Any suggestions?
I wonder our out-of-tree driver generates probe response itself. Let's me
check how it does.
Ping-Ke
On Wed, 2024-02-21 at 08:57 +0000, Ping-Ke Shih wrote:
> > >
> > > No, firmware doesn't modify content of probe response frame.
> > >
> >
> > Can you get that fixed? ;-)
> >
> > With differing beacon intervals etc., I don't know there's a good way to
> > keep the counters even with a semblance of correctness, especially if we
> > don't know when the beacon was transmitted?
> >
> > Or maybe just fill it in the driver since you probably have some beacon
> > timing data more easily accessible?
>
> If driver can get CSA or ECSA offset simply, I probably can fill a reasonable
> CSA counter (not sure if I can get 100% accurate counter for now), but
> seemingly neither ieee80211_tx_control nor ieee80211_tx_info (SKB_CB) doesn't
> have these offsets. Any suggestions?
We're not there yet! This thread is debating how we want to handle it.
Although .. you have a point, we have that issue now already, and we
don't pass the CSA offsets in probe responses if hostapd is filling them
in. I guess we also have work to do on this.
> I wonder our out-of-tree driver generates probe response itself. Let's me
> check how it does.
You _could_ also actually just implement probe response "offload" in the
driver, then you get the template from hostapd/mac80211, and that should
come with the offsets to fill in. Might be easier, overall.
johannes
On Wed, 2024-02-21 at 09:17 +0000, Ping-Ke Shih wrote:
>
> > -----Original Message-----
> > From: Johannes Berg <[email protected]>
> > Sent: Wednesday, February 21, 2024 5:00 PM
> > To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> > Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> > <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> > <[email protected]>; Felix Fietkau <[email protected]>
> > Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
> >
> > You _could_ also actually just implement probe response "offload" in the
> > driver, then you get the template from hostapd/mac80211, and that should
> > come with the offsets to fill in. Might be easier, overall.
>
> Agree. That will be easier. What I only concern is hostapd can still work well
> without probe request, which is dropped by driver implementing offload.
That's what happens today, so yeah, that should work?
You need to not answer&drop certain P2P probe requests though. But since
we're talking about this in WiFi7 AP context, you could just make it
different depending on that, I think we get the templates all the time
in the kernel and just ignore them if not handled.
johannes
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Wednesday, February 21, 2024 5:00 PM
> To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> <[email protected]>; Felix Fietkau <[email protected]>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
>
> You _could_ also actually just implement probe response "offload" in the
> driver, then you get the template from hostapd/mac80211, and that should
> come with the offsets to fill in. Might be easier, overall.
Agree. That will be easier. What I only concern is hostapd can still work well
without probe request, which is dropped by driver implementing offload.
Ping-Ke
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Wednesday, February 21, 2024 5:20 PM
> To: Ping-Ke Shih <[email protected]>; Aditya Kumar Singh <[email protected]>
> Cc: [email protected]; Jeff Johnson <[email protected]>; Ilan Peer
> <[email protected]>; Jouni Malinen <[email protected]>; Ryder Lee <[email protected]>; Arend van Spriel
> <[email protected]>; Felix Fietkau <[email protected]>
> Subject: Re: [PATCH v8 4/5] wifi: mac80211: start and finalize channel switch on link basis
>
> You need to not answer&drop certain P2P probe requests though. But since
> we're talking about this in WiFi7 AP context, you could just make it
> different depending on that, I think we get the templates all the time
> in the kernel and just ignore them if not handled.
>
Got it!
> I guess then for Qualcomm we'll need to just add an extended feature
> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
> update the parter link templates, or so?
>
Or then again ... Maybe the other way around, since it works for
Qualcomm now, assuming it all works at all? Doesn't really matter, we
can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted
logic.
johannes
On 2/21/24 15:12, Johannes Berg wrote:
>
>> I guess then for Qualcomm we'll need to just add an extended feature
>> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
>> update the parter link templates, or so?
>>
> Or then again ... Maybe the other way around, since it works for
> Qualcomm now, assuming it all works at all? Doesn't really matter, we
> can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted
> logic.
>
Yeah anything is fine. Basically this flag should be visible till
hostapd (so probably some wiphy level flag), so that it may skip forming
beacon templates of partner links in CSA case. Based on the flag, those
drivers which want it, it should form and send.
On 2/21/24 15:12, Johannes Berg wrote:
>
>> I guess then for Qualcomm we'll need to just add an extended feature
>> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
>> update the parter link templates, or so?
>>
> Or then again ... Maybe the other way around, since it works for
> Qualcomm now, assuming it all works at all? Doesn't really matter, we
> can call it "WANT_PARTNER_LINK_CSA_TEMPLATE" too and have it inverted
> logic.
>
> johannes
Yeah anything is fine. Basically this flag should be visible till
hostapd (so probably some wiphy level flag), so that it may skip forming
beacon templates of partner links in CSA case. Based on the flag, those
drivers which want it, it should form and send.
On 2/21/24 13:39, Johannes Berg wrote:
> Qualcomm:
> - copies and updates CSA/ECSA elements all by itself
> - btw, not sure here about probe responses, does it do that too?
We had a thought about keeping this CSA/ECSA handling at host/kernel
level only. But the major point of concern is _synchronization_ among
firmware of each of the links participating in the MLD.
* Even if we ignore TSF/TBTT synchronization for a moment, how firmware
will know when to transmit the beacon with a particular counter or when
CSA has finished on other link? If rely on host's update then there is
room for further delay and hence errors.
- This is because, counter value on the reported link depends on
the last beacon transmitted by the affected link.
* Host can send the template on all links but how to ensure that first
template is reached on the affected link and then only on the partner
links? Host will queue the command properly but reaping of the command
on n (no of links) independent firmware can not be guaranteed in the
same order in which host has filled. It depends how busy each of host to
firmware path is.
* And then obviously, considering TSF/TBTT will be again complicating
the synchronization part and making it more difficult to manage just via
host.
Hence there is a strong urge to let firmware handle all this for beacons.
As far as how firmware will _magically_ communicate among themselves is
concerned, we have *IPC* in place to achieve that. One link firmware can
talk to other link firmware when required.
Kernel Level
____________________________________________________________________________
-------------- -------------- --------------
| Firmware 1 | <- IPC -> | Firmware 2 | <- IPC -> | Firmware 3 |
| on HW 1 | | on HW 2 | | on HW 3 |
-------------- -------------- --------------
Hence, host just needs to update template of the affected link and
indicate to firmware that it is a critical update. This firmware then
can indicate other link firmware(s) to append CSA/ECSA IE with a given
counter value to its beacon via this IPC. Parsing the IE and
de-fragmenting and fragmenting it again can be done by firmware itself.
(Agree that it is bit complex but when comparing with complexity of
maintaining synchronicity across links, this looks more doable)
Hence we have taken "offloading beacons fully to firmware" approach.
For probe responses, it is handled in host/kernel only. Firmware sends
back the last transmitted count in beacon to host. So we have the last
transmitted count info. Per STA profile generation logic is also there.
So we manage via that.
Hi,
On Wed, 2024-02-21 at 18:22 +0530, Aditya Kumar Singh wrote:
> On 2/21/24 13:39, Johannes Berg wrote:
> > Qualcomm:
> > - copies and updates CSA/ECSA elements all by itself
> > - btw, not sure here about probe responses, does it do that too?
>
> We had a thought about keeping this CSA/ECSA handling at host/kernel
> level only. But the major point of concern is _synchronization_ among
> firmware of each of the links participating in the MLD.
Sure.
> * Even if we ignore TSF/TBTT synchronization for a moment, how firmware
> will know when to transmit the beacon with a particular counter or when
> CSA has finished on other link? If rely on host's update then there is
> room for further delay and hence errors.
> - This is because, counter value on the reported link depends on
> the last beacon transmitted by the affected link.
Sure.
I don't think anyone suggested that the host will put the exact counter
value there, just to have the template.
> * Host can send the template on all links
Right.
> but how to ensure that first
> template is reached on the affected link and then only on the partner
> links? Host will queue the command properly but reaping of the command
> on n (no of links) independent firmware can not be guaranteed in the
> same order in which host has filled. It depends how busy each of host to
> firmware path is.
True, and there's a potential for race conditions there I suppose, but I
suppose in the Intel, Realtek and hwsim case at least we wouldn't have
*different* firmwares running multiple links, but a single one.
In any case, you could solve this even with multiple, by applying the
new template only after you have the CSA'ing link's new beacon template,
if it requires filling in CSA counters, or such.
> * And then obviously, considering TSF/TBTT will be again complicating
> the synchronization part and making it more difficult to manage just via
> host.
Again, not suggesting that it is managed completely by the host, just
the templates.
> Hence there is a strong urge to let firmware handle all this for beacons.
Sure, that's fine, your call :)
> As far as how firmware will _magically_ communicate among themselves is
> concerned, we have *IPC* in place to achieve that. One link firmware can
> talk to other link firmware when required.
:-)
It probably doesn't actually help you make it race-free though, so does
it really matter? But again, it's your call how you want to do it, and
we'll just have to handle it in software appropriately. While I'd prefer
to have _one_ way of doing things, at least so far we've basically seen
one way of having the host involved and ath12k not having the host
involved, so it's all still really simple.
> Kernel Level
> ____________________________________________________________________________
> -------------- -------------- --------------
> > Firmware 1 | <- IPC -> | Firmware 2 | <- IPC -> | Firmware 3 |
> > on HW 1 | | on HW 2 | | on HW 3 |
> -------------- -------------- --------------
>
>
> Hence, host just needs to update template of the affected link and
> indicate to firmware that it is a critical update. This firmware then
> can indicate other link firmware(s) to append CSA/ECSA IE with a given
> counter value to its beacon via this IPC.
Sure. You still have a race, because if you send a message over IPC
saying that the CSA needs to be included and it's just before the TBTT,
chances are the TBTT event will still happen without that. So in a sense
it's similar to the host updating the partner link's beacon template
"too late". You can have a situation where the CSA link's template is
updated just before _its_ TBTT, and the partner link's TBTT is just a
little bit later, but the update is delayed ...
You could probably solve that by making your IPC synchronous but then
you risk your TBTT timings in cases like this.
Arguably, I'm not sure it matters. I'm thinking we'll enforce that the
CSA must be in progress when updating the partner links beacon/probe
response templates referring to it (*), but ... ultimately, I think we
can accept that the partner link updates the CSA one beacon later if
their TBTTs are very close.
(*) and now that I think about it, that might have to immediately come
with a separate template to use _after_ the switch, like CSA does for
the CSA link
> Parsing the IE and
> de-fragmenting and fragmenting it again can be done by firmware itself.
> (Agree that it is bit complex but when comparing with complexity of
> maintaining synchronicity across links, this looks more doable)
That sync maintenance is because of your hardware design though, others
don't necessarily have that because multiple links are handled by the
same NIC, not separate ones.
> Hence we have taken "offloading beacons fully to firmware" approach.
Sure, fair enough.
> For probe responses, it is handled in host/kernel only. Firmware sends
> back the last transmitted count in beacon to host. So we have the last
> transmitted count info. Per STA profile generation logic is also there.
> So we manage via that.
So I think like in Realtek's case I'd probably advocate doing that in
the driver with the offsets given by hostapd/software stack, although
that requires having *two* feature flags, one for beacons and one for
probe responses ... since you lack the "magic" for probe responses.
johannes
On 2/21/24 18:38, Johannes Berg wrote:
> Hi,
>
> On Wed, 2024-02-21 at 18:22 +0530, Aditya Kumar Singh wrote:
>> On 2/21/24 13:39, Johannes Berg wrote:
>>> Qualcomm:
>>> - copies and updates CSA/ECSA elements all by itself
>>> - btw, not sure here about probe responses, does it do that too?
>>
>> We had a thought about keeping this CSA/ECSA handling at host/kernel
>> level only. But the major point of concern is _synchronization_ among
>> firmware of each of the links participating in the MLD.
>
> Sure.
>
>> * Even if we ignore TSF/TBTT synchronization for a moment, how firmware
>> will know when to transmit the beacon with a particular counter or when
>> CSA has finished on other link? If rely on host's update then there is
>> room for further delay and hence errors.
>> - This is because, counter value on the reported link depends on
>> the last beacon transmitted by the affected link.
>
> Sure.
>
> I don't think anyone suggested that the host will put the exact counter
> value there, just to have the template.
>
>> * Host can send the template on all links
>
> Right.
>
>> but how to ensure that first
>> template is reached on the affected link and then only on the partner
>> links? Host will queue the command properly but reaping of the command
>> on n (no of links) independent firmware can not be guaranteed in the
>> same order in which host has filled. It depends how busy each of host to
>> firmware path is.
>
> True, and there's a potential for race conditions there I suppose, but I
> suppose in the Intel, Realtek and hwsim case at least we wouldn't have
> *different* firmwares running multiple links, but a single one.
oh okay, makes sense.
>
> In any case, you could solve this even with multiple, by applying the
> new template only after you have the CSA'ing link's new beacon template,
> if it requires filling in CSA counters, or such.
>
>> * And then obviously, considering TSF/TBTT will be again complicating
>> the synchronization part and making it more difficult to manage just via
>> host.
>
> Again, not suggesting that it is managed completely by the host, just
> the templates.
>
>> Hence there is a strong urge to let firmware handle all this for beacons.
>
> Sure, that's fine, your call :)
>
>> As far as how firmware will _magically_ communicate among themselves is
>> concerned, we have *IPC* in place to achieve that. One link firmware can
>> talk to other link firmware when required.
>
> :-)
>
> It probably doesn't actually help you make it race-free though, so does
> it really matter? But again, it's your call how you want to do it, and
> we'll just have to handle it in software appropriately. While I'd prefer
> to have _one_ way of doing things, at least so far we've basically seen
> one way of having the host involved and ath12k not having the host
> involved, so it's all still really simple.
Yes!
>
>> Kernel Level
>> ____________________________________________________________________________
>> -------------- -------------- --------------
>>> Firmware 1 | <- IPC -> | Firmware 2 | <- IPC -> | Firmware 3 |
>>> on HW 1 | | on HW 2 | | on HW 3 |
>> -------------- -------------- --------------
>>
>>
>> Hence, host just needs to update template of the affected link and
>> indicate to firmware that it is a critical update. This firmware then
>> can indicate other link firmware(s) to append CSA/ECSA IE with a given
>> counter value to its beacon via this IPC.
>
> Sure. You still have a race, because if you send a message over IPC
> saying that the CSA needs to be included and it's just before the TBTT,
> chances are the TBTT event will still happen without that. So in a sense
> it's similar to the host updating the partner link's beacon template
> "too late". You can have a situation where the CSA link's template is
> updated just before _its_ TBTT, and the partner link's TBTT is just a
> little bit later, but the update is delayed ...
>
> You could probably solve that by making your IPC synchronous but then
> you risk your TBTT timings in cases like this.
>
> Arguably, I'm not sure it matters. I'm thinking we'll enforce that the
> CSA must be in progress when updating the partner links beacon/probe
> response templates referring to it (*), but ... ultimately, I think we
> can accept that the partner link updates the CSA one beacon later if
> their TBTTs are very close.
>
>
> (*) and now that I think about it, that might have to immediately come
> with a separate template to use _after_ the switch, like CSA does for
> the CSA link
True. So each link will have to send two templates. beacon_csa and
beacon_after
>
>> Parsing the IE and
>> de-fragmenting and fragmenting it again can be done by firmware itself.
>> (Agree that it is bit complex but when comparing with complexity of
>> maintaining synchronicity across links, this looks more doable)
>
> That sync maintenance is because of your hardware design though, others
> don't necessarily have that because multiple links are handled by the
> same NIC, not separate ones.
>
>> Hence we have taken "offloading beacons fully to firmware" approach.
>
> Sure, fair enough.
>
>> For probe responses, it is handled in host/kernel only. Firmware sends
>> back the last transmitted count in beacon to host. So we have the last
>> transmitted count info. Per STA profile generation logic is also there.
>> So we manage via that.
>
> So I think like in Realtek's case I'd probably advocate doing that in
> the driver with the offsets given by hostapd/software stack, although
> that requires having *two* feature flags, one for beacons and one for
> probe responses ... since you lack the "magic" for probe responses.
>
:) I guess yeah. So this approach we discussed is fine right?
Have some _feature flag_ to indicate whether host would like templates
(with offsets) for reporting links or not. If it does then hostapd can
send those, otherwise it won't. Hosts can further handle as per their need.
On Wed, 2024-02-21 at 09:09 +0100, Johannes Berg wrote:
>
> I guess that'd also apply to probe responses? Or does it not send
> those
> at all? But we discussed before that maybe we don't have to be
> perfect
> there, so I guess we can find some solution to that.
>
> So we have, so far
>
> Realtek:
> - uses offset in beacon template to set partner counter,
> - requires host to set CSA/ECSA elements
>
> Qualcomm:
> - copies and updates CSA/ECSA elements all by itself
> - btw, not sure here about probe responses, does it do that too?
>
> hwsim:
> - personally, I'd prefer it works like Realtek, for less complexity
>
> Intel:
> - not implemented, probably not going to happen any time soon, but I
> think we might prefer the Realtek way too if we ever do this
>
>
> Mediatek? Broadcom? Anyone else?
>
>
> I guess then for Qualcomm we'll need to just add an extended feature
> flag for "FW_HANDLES_PARTNER_LINK_CSA" and then hostapd can just not
> update the parter link templates, or so?
>
> Thanks,
Hi,
Sorry for the late reply.
In summary, MediaTek firmware behaves similarly to Realtek's firmware,
so we agree on the conclusion of this thread.
By the way, this behavior does not apply to probe response in MediaTek
firmware. In other words, MediaTek firmware does not modify the probe
response and it requires hostapd to fill all CSA-related Elements.
I think this could be fixed if hostapd provides CSA offsets when
sending the probe response.
Best,
Michael
> johannes