2019-11-19 06:08:04

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

This patchset tries to make mac80211 power save testable with the hostap
hwsim tests, and fixes a bug in STA power save.

Basic tests for AP and STA power save will be submitted to hostap
separately.

Thomas Pedersen (3):
mac80211_hwsim: add power save support
mac80211: expose HW conf flags through debugfs
mac80211: consider QoS Null frames for STA_NULLFUNC_ACKED

drivers/net/wireless/mac80211_hwsim.c | 8 ++++++++
net/mac80211/debugfs.c | 3 +++
net/mac80211/status.c | 3 ++-
3 files changed, 13 insertions(+), 1 deletion(-)

--

v2:
- got Johannes' email wrong
- weird duplicate emails

2.20.1


2019-11-19 06:08:55

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH v2 2/3] mac80211: expose HW conf flags through debugfs

This is useful during testing to eg. check the currently
configured HW power save state.

Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/debugfs.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/net/mac80211/debugfs.c b/net/mac80211/debugfs.c
index 568b3b276931..5c52429038c3 100644
--- a/net/mac80211/debugfs.c
+++ b/net/mac80211/debugfs.c
@@ -59,6 +59,8 @@ static const struct file_operations name## _ops = { \
debugfs_create_file(#name, mode, phyd, local, &name## _ops);


+DEBUGFS_READONLY_FILE(hw_conf, "%x",
+ local->hw.conf.flags);
DEBUGFS_READONLY_FILE(user_power, "%d",
local->user_power_level);
DEBUGFS_READONLY_FILE(power, "%d",
@@ -433,6 +435,7 @@ void debugfs_hw_add(struct ieee80211_local *local)
DEBUGFS_ADD(hwflags);
DEBUGFS_ADD(user_power);
DEBUGFS_ADD(power);
+ DEBUGFS_ADD(hw_conf);
DEBUGFS_ADD_MODE(force_tx_status, 0600);

if (local->ops->wake_tx_queue)
--
2.20.1

2019-11-19 06:10:24

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH v2 1/3] mac80211_hwsim: add power save support

Advertise the correct flags to mac80211 to indicate PS
trigger frames and frame buffering should be handled by
mac80211. This means mac80211_hwsim will now also have to
release buffered multicast frames after a (DTIM) beacon.

Signed-off-by: Thomas Pedersen <[email protected]>
---
drivers/net/wireless/mac80211_hwsim.c | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c
index 03738107fd10..219c23571919 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -1595,6 +1595,11 @@ static void mac80211_hwsim_beacon_tx(void *arg, u8 *mac,
mac80211_hwsim_tx_frame(hw, skb,
rcu_dereference(vif->chanctx_conf)->def.chan);

+ while ((skb = ieee80211_get_buffered_bc(hw, vif)) != NULL) {
+ mac80211_hwsim_tx_frame(hw, skb,
+ rcu_dereference(vif->chanctx_conf)->def.chan);
+ }
+
if (vif->csa_active && ieee80211_csa_is_complete(vif))
ieee80211_csa_finish(vif);
}
@@ -2925,6 +2930,9 @@ static int mac80211_hwsim_new_radio(struct genl_info *info,
ieee80211_hw_set(hw, MFP_CAPABLE);
ieee80211_hw_set(hw, SIGNAL_DBM);
ieee80211_hw_set(hw, SUPPORTS_PS);
+ ieee80211_hw_set(hw, REPORTS_TX_ACK_STATUS);
+ ieee80211_hw_set(hw, HOST_BROADCAST_PS_BUFFERING);
+ ieee80211_hw_set(hw, PS_NULLFUNC_STACK);
ieee80211_hw_set(hw, TDLS_WIDER_BW);
if (rctbl)
ieee80211_hw_set(hw, SUPPORTS_RC_TABLE);
--
2.20.1

2019-11-19 06:10:24

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH v2 3/3] mac80211: consider QoS Null frames for STA_NULLFUNC_ACKED

Commit 7b6ddeaf27ec ("mac80211: use QoS NDP for AP probing")
let STAs send QoS Null frames as PS triggers if the AP was
a QoS STA. However, the mac80211 PS stack relies on an
interface flag IEEE80211_STA_NULLFUNC_ACKED for
determining trigger frame ACK, which was not being set for
acked non-QoS Null frames. The effect is an inability to
trigger hardware sleep via IEEE80211_CONF_PS since the QoS
Null frame was seemingly never acked.

This bug only applies to drivers which set both
IEEE80211_HW_REPORTS_TX_ACK_STATUS and
IEEE80211_HW_PS_NULLFUNC_STACK.

Detect the acked QoS Null frame to restore STA power save.

Fixes: 7b6ddeaf27ec ("mac80211: use QoS NDP for AP probing")
Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/status.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index ab8ba5835ca0..5a3d645fe1bc 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -1030,7 +1030,8 @@ static void __ieee80211_tx_status(struct ieee80211_hw *hw,
I802_DEBUG_INC(local->dot11FailedCount);
}

- if (ieee80211_is_nullfunc(fc) && ieee80211_has_pm(fc) &&
+ if ((ieee80211_is_nullfunc(fc) || ieee80211_is_qos_nullfunc(fc)) &&
+ ieee80211_has_pm(fc) &&
ieee80211_hw_check(&local->hw, REPORTS_TX_ACK_STATUS) &&
!(info->flags & IEEE80211_TX_CTL_INJECTED) &&
local->ps_sdata && !(local->scanning)) {
--
2.20.1

2019-11-22 12:39:08

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

Hi,

On Mon, 2019-11-18 at 21:35 -0800, Thomas Pedersen wrote:
> This patchset tries to make mac80211 power save testable with the hostap
> hwsim tests, and fixes a bug in STA power save.
>
> Basic tests for AP and STA power save will be submitted to hostap
> separately.

Cool :)

I was going to apply all of these, but then it turned out that the first
one causes a bunch of hwsim tests to fail?

It seems quite possible that this is just a mac80211 bug somewhere, but
I'd want to look at this more closely, so I've dropped it back to "Under
Review" in patchwork again.

Any thoughts?

johannes

2019-11-22 18:02:11

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

On 11/22/19 4:38 AM, Johannes Berg wrote:
> Hi,
>
> On Mon, 2019-11-18 at 21:35 -0800, Thomas Pedersen wrote:
>> This patchset tries to make mac80211 power save testable with the hostap
>> hwsim tests, and fixes a bug in STA power save.
>>
>> Basic tests for AP and STA power save will be submitted to hostap
>> separately.
>
> Cool :)
>
> I was going to apply all of these, but then it turned out that the first
> one causes a bunch of hwsim tests to fail?

OK. If you point me toward which tests started to fail I can take a
look.

> It seems quite possible that this is just a mac80211 bug somewhere, but
> I'd want to look at this more closely, so I've dropped it back to "Under
> Review" in patchwork again.
>
> Any thoughts?
>
> johannes
>


--
-- thomas

2019-11-22 18:09:36

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

On Fri, 2019-11-22 at 19:04 +0100, Johannes Berg wrote:
>
> Various. Jouni just said mostly concurrency ones, e.g.
>
> p2p_cli_invite
> ap_cipher_tkip_countermeasures_ap_mixed_mode
> discovery_while_go_p2p_dev
> radius_macacl_unreachable

Ah, still had my list open:

discovery_while_go radius_macacl_unreachable
discovery_while_go_p2p_dev bgscan_learn_beacon_loss
wifi_display_parsing ap_vlan_iface_cleanup_multibss
ap_vlan_iface_cleanup_multibss_per_sta_vif
autogo_2cli
bgscan_simple_beacon_loss
rrm_lci_req_timeout
rrm_ftm_range_req_timeout
p2p_cli_invite

johannes

2019-11-22 18:09:37

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

On Fri, 2019-11-22 at 10:01 -0800, Thomas Pedersen wrote:
> On 11/22/19 4:38 AM, Johannes Berg wrote:
> > Hi,
> >
> > On Mon, 2019-11-18 at 21:35 -0800, Thomas Pedersen wrote:
> > > This patchset tries to make mac80211 power save testable with the hostap
> > > hwsim tests, and fixes a bug in STA power save.
> > >
> > > Basic tests for AP and STA power save will be submitted to hostap
> > > separately.
> >
> > Cool :)
> >
> > I was going to apply all of these, but then it turned out that the first
> > one causes a bunch of hwsim tests to fail?
>
> OK. If you point me toward which tests started to fail I can take a
> look.

Various. Jouni just said mostly concurrency ones, e.g.

p2p_cli_invite
ap_cipher_tkip_countermeasures_ap_mixed_mode
discovery_while_go_p2p_dev
radius_macacl_unreachable

but I saw quite a few more failing as well, though not always entirely
reproducibly.

johannes

2020-01-14 06:02:24

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

Hi Johannes,

On 11/22/19 10:08 AM, Johannes Berg wrote:
> On Fri, 2019-11-22 at 19:04 +0100, Johannes Berg wrote:
>>
>> Various. Jouni just said mostly concurrency ones, e.g.
>>
>> p2p_cli_invite
>> ap_cipher_tkip_countermeasures_ap_mixed_mode
>> discovery_while_go_p2p_dev
>> radius_macacl_unreachable
>
> Ah, still had my list open:
>
> discovery_while_go radius_macacl_unreachable
> discovery_while_go_p2p_dev bgscan_learn_beacon_loss
> wifi_display_parsing ap_vlan_iface_cleanup_multibss
> ap_vlan_iface_cleanup_multibss_per_sta_vif
> autogo_2cli
> bgscan_simple_beacon_loss
> rrm_lci_req_timeout
> rrm_ftm_range_req_timeout
> p2p_cli_invite

Is there a list of known passing hwsim tests somewhere?

Maybe a subset of suites / tests you like to see passing as a smoke test?

Or do you run through the 3300+ tests and expect them all to pass?

Thanks,

--
-- thomas

2020-01-14 08:02:30

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

Hi Thomas,

> Is there a list of known passing hwsim tests somewhere?
>
> Maybe a subset of suites / tests you like to see passing as a smoke test?

Not really.

> Or do you run through the 3300+ tests and expect them all to pass?

Pretty much, yes. It takes <10 minutes on my quad-core desktop to run
using the UML time-travel mode against recent kernels.

I don't always get 100% pass on the first round, but the handful that
fails sometimes I expect to pass when running just those manually with
vm-run.sh.

johannes

2020-01-16 05:40:30

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

On 1/14/20 12:00 AM, Johannes Berg wrote:
> Hi Thomas,
>
>> Is there a list of known passing hwsim tests somewhere?
>>
>> Maybe a subset of suites / tests you like to see passing as a smoke test?
>
> Not really.
>
>> Or do you run through the 3300+ tests and expect them all to pass?
>
> Pretty much, yes. It takes <10 minutes on my quad-core desktop to run
> using the UML time-travel mode against recent kernels.
>
> I don't always get 100% pass on the first round, but the handful that
> fails sometimes I expect to pass when running just those manually with
> vm-run.sh.

Thanks, I'll give UML time-travel mode a try.


--
-- thomas

2020-01-16 07:42:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

On Wed, 2020-01-15 at 21:23 -0800, Thomas Pedersen wrote:
> On 1/14/20 12:00 AM, Johannes Berg wrote:
> > Hi Thomas,
> >
> > > Is there a list of known passing hwsim tests somewhere?
> > >
> > > Maybe a subset of suites / tests you like to see passing as a smoke test?
> >
> > Not really.
> >
> > > Or do you run through the 3300+ tests and expect them all to pass?
> >
> > Pretty much, yes. It takes <10 minutes on my quad-core desktop to run
> > using the UML time-travel mode against recent kernels.
> >
> > I don't always get 100% pass on the first round, but the handful that
> > fails sometimes I expect to pass when running just those manually with
> > vm-run.sh.
>
> Thanks, I'll give UML time-travel mode a try.

FWIW, I applied your patches yesterday. There was one issue remaining
uncovered by them, but it wasn't actually related to your patches, just
related to the test sending too many frames. I sent a workaround to the
hostap list and also the "mac80211: use more bits for ack_frame_id" to
the kernel to fix that.

johannes

2020-01-16 08:08:58

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

On Wed, 2020-01-15 at 23:53 -0800, [email protected] wrote:
>
> > FWIW, I applied your patches yesterday. There was one issue remaining
> > uncovered by them, but it wasn't actually related to your patches, just
> > related to the test sending too many frames. I sent a workaround to the
> > hostap list and also the "mac80211: use more bits for ack_frame_id" to
> > the kernel to fix that.
> >
>
> But in the v3, no frames should actually be buffered unless power save
> is explicitly enabled on the STA side. Or is this some rrm specific
> behavior?

I didn't check whether or not powersave got enabled by default now, I
sort of assumed it did? But that isn't really bad, almost all real
devices will have it enabled by default too.

johannes

2020-01-16 17:54:42

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

On 1/15/20 11:54 PM, Johannes Berg wrote:
> On Wed, 2020-01-15 at 23:53 -0800, [email protected] wrote:
>>
>>> FWIW, I applied your patches yesterday. There was one issue remaining
>>> uncovered by them, but it wasn't actually related to your patches, just
>>> related to the test sending too many frames. I sent a workaround to the
>>> hostap list and also the "mac80211: use more bits for ack_frame_id" to
>>> the kernel to fix that.
>>>
>>
>> But in the v3, no frames should actually be buffered unless power save
>> is explicitly enabled on the STA side. Or is this some rrm specific
>> behavior?
>
> I didn't check whether or not powersave got enabled by default now, I
> sort of assumed it did? But that isn't really bad, almost all real
> devices will have it enabled by default too.

In v2 powersave was enabled by default, but that broke some test
assumptions (and p2p stuff I don't really understand), so v3 disables it
by default: https://lore.kernel.org/linux-wireless/[email protected]/T/#u

--
-- thomas

2020-01-16 17:57:21

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] fix a STA PS bug and add PS support to mac80211_hwsim

On Thu, 2020-01-16 at 09:54 -0800, Thomas Pedersen wrote:
> On 1/15/20 11:54 PM, Johannes Berg wrote:
> > On Wed, 2020-01-15 at 23:53 -0800, [email protected] wrote:
> > > > FWIW, I applied your patches yesterday. There was one issue remaining
> > > > uncovered by them, but it wasn't actually related to your patches, just
> > > > related to the test sending too many frames. I sent a workaround to the
> > > > hostap list and also the "mac80211: use more bits for ack_frame_id" to
> > > > the kernel to fix that.
> > > >
> > >
> > > But in the v3, no frames should actually be buffered unless power save
> > > is explicitly enabled on the STA side. Or is this some rrm specific
> > > behavior?
> >
> > I didn't check whether or not powersave got enabled by default now, I
> > sort of assumed it did? But that isn't really bad, almost all real
> > devices will have it enabled by default too.
>
> In v2 powersave was enabled by default, but that broke some test
> assumptions (and p2p stuff I don't really understand), so v3 disables it
> by default: https://lore.kernel.org/linux-wireless/[email protected]/T/#u

Right.

Not sure. I bisected the failure to the *next* patch actually, before
actually debugging it ...

johannes