netdev - Re: [PATCH 0/3] mwifiex: disable ps_mode by default for stability

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20201220214333.rxtkt72niq7adfin@pali>
Date:   Sun, 20 Dec 2020 22:43:33 +0100
From:   Pali Rohár <pali@...nel.org>
To:     Tsuchiya Yuto <kitakar@...il.com>
Cc:     Amitkumar Karwar <amitkarwar@...il.com>,
        Ganapathi Bhat <ganapathi.bhat@....com>,
        Xinming Hu <huxinming820@...il.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Maximilian Luz <luzmaximilian@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...el.com>, verdre@...d.nl
Subject: Re: [PATCH 0/3] mwifiex: disable ps_mode by default for stability

Hello!

Please CC me in future for mwifiex discussion :-)

On Wednesday 28 October 2020 23:24:30 Tsuchiya Yuto wrote:
> Hello all,
> 
> On Microsoft Surface devices (PCIe-88W8897), we are observing stability
> issues when ps_mode (IEEE power_save) is enabled, then eventually causes
> firmware crash. Especially on 5GHz APs, the connection is completely
> unstable and almost unusable.
> 
> I think the most desirable change is to fix the ps_mode itself. But is
> seems to be hard work [1], I'm afraid we have to go this way.
> 
> Therefore, the first patch of this series disables the ps_mode by default
> instead of enabling it on driver init. I'm not sure if explicitly
> disabling it is really required or not. I don't have access to the details
> of this chip. Let me know if it's enough to just remove the code that
> enables ps_mode.
> 
> The Second patch adds a new module parameter named "allow_ps_mode". Since
> other wifi drivers just disable power_save by default by module parameter
> like this, I also added this.
> 
> The third patch adds a message when ps_mode will be changed. Useful when
> diagnosing connection issues.

There are more issues with power save API and implementation in mwifiex.

See my email for more details:
https://lore.kernel.org/linux-wireless/20200609111544.v7u5ort3yk4s7coy@pali/T/#u

These patches would just break power save API and reporting status to
userspace even more due to WIPHY_FLAG_PS_ON_BY_DEFAULT and
CONFIG_CFG80211_DEFAULT_PS options.

I would suggest to first fix issues mentioned in my email and then start
providing a way how to blacklist or whitelist power save feature
depending on firmware or card/chip version.

>From my experience I know that e.g. 88W8997 cards have lot of bugs in
their firmware and I'm not aware that bugs are going to be fixed... So
we really need workarounds, like disabling power save mode to have cards
usable.

> Thanks,
> Tsuchiya Yuto
> 
> [1] https://bugzilla.kernel.org/show_bug.cgi?id=109681
> 
> Tsuchiya Yuto (3):
>   mwifiex: disable ps_mode explicitly by default instead
>   mwifiex: add allow_ps_mode module parameter
>   mwifiex: print message when changing ps_mode
> 
>  .../net/wireless/marvell/mwifiex/cfg80211.c   | 23 +++++++++++++++++++
>  .../net/wireless/marvell/mwifiex/sta_cmd.c    | 11 ++++++---
>  2 files changed, 31 insertions(+), 3 deletions(-)
> 
> -- 
> 2.29.1
>