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

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

[Thomas Pedersen]

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

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

[Thomas Pedersen]

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 <thomas@adapt-ip.com>
---
 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

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

[Thomas Pedersen]

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

Signed-off-by: Thomas Pedersen <thomas@adapt-ip.com>
---
 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

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

[Thomas Pedersen]

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 <thomas@adapt-ip.com>
---
 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

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

[Johannes Berg]

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

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

[Thomas Pedersen]

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

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

[Johannes Berg]

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

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

[Johannes Berg]

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

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

[Thomas Pedersen]

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

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

[Johannes Berg]

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

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

[Thomas Pedersen]

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

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

[Johannes Berg]

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

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

[Johannes Berg]

On Wed, 2020-01-15 at 23:53 -0800, thomas@adapt-ip.com 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

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

[Thomas Pedersen]

On 1/15/20 11:54 PM, Johannes Berg wrote:
> On Wed, 2020-01-15 at 23:53 -0800, thomas@adapt-ip.com 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/20200114055940.18502-2-thomas@adapt-ip.com/T/#u

-- 
-- thomas

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

[Johannes Berg]

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, thomas@adapt-ip.com 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/20200114055940.18502-2-thomas@adapt-ip.com/T/#u

Right.

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

johannes