2024-02-15 16:28:36

by Aditya Kumar Singh

[permalink] [raw]
Subject: [PATCH 0/2] wifi: mac80211/mac80211_hwsim: support MLO CSA test case

Recently MLO CSA is enabled on the stack. Add test infra support in
mac80211_hwsim in order to support basic MLO CSA test case.

Aditya Kumar Singh (2):
wifi: mac80211: check beacon countdown is complete on per link basis
wifi: mac80211_hwsim: add support for switch_vif_chanctx callback
---
drivers/net/wireless/ath/ath10k/mac.c | 2 +-
drivers/net/wireless/ath/ath10k/wmi.c | 2 +-
drivers/net/wireless/ath/ath11k/mac.c | 2 +-
drivers/net/wireless/ath/ath9k/beacon.c | 2 +-
.../net/wireless/ath/ath9k/htc_drv_beacon.c | 2 +-
.../net/wireless/intel/iwlwifi/mvm/mac-ctxt.c | 2 +-
.../wireless/intel/iwlwifi/mvm/time-event.c | 2 +-
drivers/net/wireless/mediatek/mt76/mac80211.c | 4 +--
.../wireless/realtek/rtl8xxxu/rtl8xxxu_core.c | 2 +-
drivers/net/wireless/virtual/mac80211_hwsim.c | 35 +++++++++++++++++--
include/net/mac80211.h | 4 ++-
net/mac80211/tx.c | 14 ++++++--
12 files changed, 58 insertions(+), 15 deletions(-)


base-commit: 42ffccd0a36e099dea3d3272c5d62a0454ded1f0
--
2.25.1



2024-02-15 16:54:03

by Aditya Kumar Singh

[permalink] [raw]
Subject: [PATCH 2/2] wifi: mac80211_hwsim: add support for switch_vif_chanctx callback

Currently switch_vif_chanctx mac80211 callback is not supported for
MLO. Add it.

Signed-off-by: Aditya Kumar Singh <[email protected]>
---
drivers/net/wireless/virtual/mac80211_hwsim.c | 33 ++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/virtual/mac80211_hwsim.c b/drivers/net/wireless/virtual/mac80211_hwsim.c
index 2ea11a86d920..ccc321898d6f 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim.c
@@ -3215,6 +3215,36 @@ static void mac80211_hwsim_unassign_vif_chanctx(struct ieee80211_hw *hw,
}
}

+static int mac80211_hwsim_switch_vif_chanctx(struct ieee80211_hw *hw,
+ struct ieee80211_vif_chanctx_switch *vifs,
+ int n_vifs,
+ enum ieee80211_chanctx_switch_mode mode)
+{
+ int i;
+
+ if (n_vifs <= 0)
+ return -EINVAL;
+
+ wiphy_dbg(hw->wiphy,
+ "switch vif channel context mode: %u\n", mode);
+
+ for (i = 0; i < n_vifs; i++) {
+ hwsim_check_chanctx_magic(vifs[i].old_ctx);
+ wiphy_dbg(hw->wiphy,
+ "switch vif channel context: %d MHz/width: %d/cfreqs:%d/%d MHz -> %d MHz/width: %d/cfreqs:%d/%d MHz\n",
+ vifs[i].old_ctx->def.chan->center_freq,
+ vifs[i].old_ctx->def.width,
+ vifs[i].old_ctx->def.center_freq1,
+ vifs[i].old_ctx->def.center_freq2,
+ vifs[i].new_ctx->def.chan->center_freq,
+ vifs[i].new_ctx->def.width,
+ vifs[i].new_ctx->def.center_freq1,
+ vifs[i].new_ctx->def.center_freq2);
+ hwsim_set_chanctx_magic(vifs[i].new_ctx);
+ }
+ return 0;
+}
+
static const char mac80211_hwsim_gstrings_stats[][ETH_GSTRING_LEN] = {
"tx_pkts_nic",
"tx_bytes_nic",
@@ -3940,7 +3970,8 @@ static const struct ieee80211_ops mac80211_hwsim_ops = {
.remove_chanctx = mac80211_hwsim_remove_chanctx, \
.change_chanctx = mac80211_hwsim_change_chanctx, \
.assign_vif_chanctx = mac80211_hwsim_assign_vif_chanctx,\
- .unassign_vif_chanctx = mac80211_hwsim_unassign_vif_chanctx,
+ .unassign_vif_chanctx = mac80211_hwsim_unassign_vif_chanctx, \
+ .switch_vif_chanctx = mac80211_hwsim_switch_vif_chanctx,

static const struct ieee80211_ops mac80211_hwsim_mchan_ops = {
HWSIM_COMMON_OPS
--
2.25.1


2024-02-15 19:52:25

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] wifi: mac80211_hwsim: add support for switch_vif_chanctx callback

On Thu, 2024-02-15 at 21:58 +0530, Aditya Kumar Singh wrote:
> +static int mac80211_hwsim_switch_vif_chanctx(struct ieee80211_hw *hw,
> + struct ieee80211_vif_chanctx_switch *vifs,
> + int n_vifs,
> + enum ieee80211_chanctx_switch_mode mode)
> +{
> + int i;
> +
> + if (n_vifs <= 0)
> + return -EINVAL;
> +
> + wiphy_dbg(hw->wiphy,
> + "switch vif channel context mode: %u\n", mode);
> +
> + for (i = 0; i < n_vifs; i++) {
> + hwsim_check_chanctx_magic(vifs[i].old_ctx);
> + wiphy_dbg(hw->wiphy,
> + "switch vif channel context: %d MHz/width: %d/cfreqs:%d/%d MHz -> %d MHz/width: %d/cfreqs:%d/%d MHz\n",
> + vifs[i].old_ctx->def.chan->center_freq,
> + vifs[i].old_ctx->def.width,
> + vifs[i].old_ctx->def.center_freq1,
> + vifs[i].old_ctx->def.center_freq2,
> + vifs[i].new_ctx->def.chan->center_freq,
> + vifs[i].new_ctx->def.width,
> + vifs[i].new_ctx->def.center_freq1,
> + vifs[i].new_ctx->def.center_freq2);
> + hwsim_set_chanctx_magic(vifs[i].new_ctx);


hwsim_set_chanctx_magic() is only partially correct, I think, this
depends on the mode? For CHANCTX_SWMODE_REASSIGN_VIF the chanctx should
already exist as well, so should also be hwsim_check_chanctx_magic() in
that case?

johannes


2024-02-16 03:16:02

by Aditya Kumar Singh

[permalink] [raw]
Subject: Re: [PATCH 2/2] wifi: mac80211_hwsim: add support for switch_vif_chanctx callback

On 2/16/24 01:20, Johannes Berg wrote:
> On Thu, 2024-02-15 at 21:58 +0530, Aditya Kumar Singh wrote:
>> +static int mac80211_hwsim_switch_vif_chanctx(struct ieee80211_hw *hw,
>> + struct ieee80211_vif_chanctx_switch *vifs,
>> + int n_vifs,
>> + enum ieee80211_chanctx_switch_mode mode)
>> +{
>> + int i;
>> +
>> + if (n_vifs <= 0)
>> + return -EINVAL;
>> +
>> + wiphy_dbg(hw->wiphy,
>> + "switch vif channel context mode: %u\n", mode);
>> +
>> + for (i = 0; i < n_vifs; i++) {
>> + hwsim_check_chanctx_magic(vifs[i].old_ctx);
>> + wiphy_dbg(hw->wiphy,
>> + "switch vif channel context: %d MHz/width: %d/cfreqs:%d/%d MHz -> %d MHz/width: %d/cfreqs:%d/%d MHz\n",
>> + vifs[i].old_ctx->def.chan->center_freq,
>> + vifs[i].old_ctx->def.width,
>> + vifs[i].old_ctx->def.center_freq1,
>> + vifs[i].old_ctx->def.center_freq2,
>> + vifs[i].new_ctx->def.chan->center_freq,
>> + vifs[i].new_ctx->def.width,
>> + vifs[i].new_ctx->def.center_freq1,
>> + vifs[i].new_ctx->def.center_freq2);
>> + hwsim_set_chanctx_magic(vifs[i].new_ctx);
>
>
> hwsim_set_chanctx_magic() is only partially correct, I think, this
> depends on the mode? For CHANCTX_SWMODE_REASSIGN_VIF the chanctx should
> already exist as well, so should also be hwsim_check_chanctx_magic() in
> that case?
>

Oh yeah missed the mode dependency here. Thanks for pointing it out. So
it should be something like -

...

/* old already exist so check magic */
hwsim_check_chanctx_magic(vifs[i].old_ctx);

if (mode == CHANCTX_SWMODE_REASSIGN_VIF) {
/* Reassign then new would also exist, just interface is
* switching
*/
hwsim_check_chanctx_magic(vifs[i].new_ctx);
} else {
/* SWAP_* then new context does not exist hence set magic.
* Also old will no longer exist so clear the magic
*/
hwsim_set_chanctx_magic(vifs[i].new_ctx);
hwsim_clear_chanctx_magic(vifs[i].old_ctx);
}

...

One thing, in patch should I keep those comments or those are noisy?
Seems noisy to me (if at least mode documentation is referred then
things are clear already)?

2024-02-16 07:13:06

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/2] wifi: mac80211_hwsim: add support for switch_vif_chanctx callback

On Fri, 2024-02-16 at 08:45 +0530, Aditya Kumar Singh wrote:
>
> > > + for (i = 0; i < n_vifs; i++) {
> > > + hwsim_check_chanctx_magic(vifs[i].old_ctx);
> > > + wiphy_dbg(hw->wiphy,
> > > + "switch vif channel context: %d MHz/width: %d/cfreqs:%d/%d MHz -> %d MHz/width: %d/cfreqs:%d/%d MHz\n",
> > > + vifs[i].old_ctx->def.chan->center_freq,
> > > + vifs[i].old_ctx->def.width,
> > > + vifs[i].old_ctx->def.center_freq1,
> > > + vifs[i].old_ctx->def.center_freq2,
> > > + vifs[i].new_ctx->def.chan->center_freq,
> > > + vifs[i].new_ctx->def.width,
> > > + vifs[i].new_ctx->def.center_freq1,
> > > + vifs[i].new_ctx->def.center_freq2);
> > > + hwsim_set_chanctx_magic(vifs[i].new_ctx);
> >
> >
> > hwsim_set_chanctx_magic() is only partially correct, I think, this
> > depends on the mode? For CHANCTX_SWMODE_REASSIGN_VIF the chanctx should
> > already exist as well, so should also be hwsim_check_chanctx_magic() in
> > that case?
> >
>
> Oh yeah missed the mode dependency here. Thanks for pointing it out. So
> it should be something like -
>
> ...
>
> /* old already exist so check magic */
> hwsim_check_chanctx_magic(vifs[i].old_ctx);
>
> if (mode == CHANCTX_SWMODE_REASSIGN_VIF) {
> /* Reassign then new would also exist, just interface is
> * switching
> */
> hwsim_check_chanctx_magic(vifs[i].new_ctx);
> } else {
> /* SWAP_* then new context does not exist hence set magic.
> * Also old will no longer exist so clear the magic
> */
> hwsim_set_chanctx_magic(vifs[i].new_ctx);
> hwsim_clear_chanctx_magic(vifs[i].old_ctx);
> }
>
> ...
>
> One thing, in patch should I keep those comments or those are noisy?
> Seems noisy to me (if at least mode documentation is referred then
> things are clear already)?
>

I'm not sure I care all that much, but I'd say even with the reference
to the modes, it's fairly easy to figure out at least by looking at the
mode docs?

I'd probably go for a switch () statement on the modes and even WARN on
invalid mode, while at it? hwsim is a test vehicle, after all.

No strong opinions on either (comments and switch) though.

johannes

2024-02-16 14:22:27

by Aditya Kumar Singh

[permalink] [raw]
Subject: Re: [PATCH 2/2] wifi: mac80211_hwsim: add support for switch_vif_chanctx callback

On 2/16/24 12:42, Johannes Berg wrote:
> On Fri, 2024-02-16 at 08:45 +0530, Aditya Kumar Singh wrote:
>>
>>>> + for (i = 0; i < n_vifs; i++) {
>>>> + hwsim_check_chanctx_magic(vifs[i].old_ctx);
>>>> + wiphy_dbg(hw->wiphy,
>>>> + "switch vif channel context: %d MHz/width: %d/cfreqs:%d/%d MHz -> %d MHz/width: %d/cfreqs:%d/%d MHz\n",
>>>> + vifs[i].old_ctx->def.chan->center_freq,
>>>> + vifs[i].old_ctx->def.width,
>>>> + vifs[i].old_ctx->def.center_freq1,
>>>> + vifs[i].old_ctx->def.center_freq2,
>>>> + vifs[i].new_ctx->def.chan->center_freq,
>>>> + vifs[i].new_ctx->def.width,
>>>> + vifs[i].new_ctx->def.center_freq1,
>>>> + vifs[i].new_ctx->def.center_freq2);
>>>> + hwsim_set_chanctx_magic(vifs[i].new_ctx);
>>>
>>>
>>> hwsim_set_chanctx_magic() is only partially correct, I think, this
>>> depends on the mode? For CHANCTX_SWMODE_REASSIGN_VIF the chanctx should
>>> already exist as well, so should also be hwsim_check_chanctx_magic() in
>>> that case?
>>>
>>
>> Oh yeah missed the mode dependency here. Thanks for pointing it out. So
>> it should be something like -
>>
>> ...
>>
>> /* old already exist so check magic */
>> hwsim_check_chanctx_magic(vifs[i].old_ctx);
>>
>> if (mode == CHANCTX_SWMODE_REASSIGN_VIF) {
>> /* Reassign then new would also exist, just interface is
>> * switching
>> */
>> hwsim_check_chanctx_magic(vifs[i].new_ctx);
>> } else {
>> /* SWAP_* then new context does not exist hence set magic.
>> * Also old will no longer exist so clear the magic
>> */
>> hwsim_set_chanctx_magic(vifs[i].new_ctx);
>> hwsim_clear_chanctx_magic(vifs[i].old_ctx);
>> }
>>
>> ...
>>
>> One thing, in patch should I keep those comments or those are noisy?
>> Seems noisy to me (if at least mode documentation is referred then
>> things are clear already)?
>>
>
> I'm not sure I care all that much, but I'd say even with the reference
> to the modes, it's fairly easy to figure out at least by looking at the
> mode docs?
>

Yes correct. I will drop inline comments. Not required tbh.

> I'd probably go for a switch () statement on the modes and even WARN on
> invalid mode, while at it? hwsim is a test vehicle, after all.
>

Yes, good point :). Thanks will incorporate and send v2 soon.