linux-kernel - Re: [PATCH v2] brcmfmac: add support for CQM RSSI notifications

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Jan 2021 10:03:18 +0000
From:   Alvin Šipraga <ALSI@...g-olufsen.dk>
To:     Arend Van Spriel <arend.vanspriel@...adcom.com>,
        Arend van Spriel <aspriel@...il.com>,
        Franky Lin <franky.lin@...adcom.com>,
        Hante Meuleman <hante.meuleman@...adcom.com>,
        Chi-hsien Lin <chi-hsien.lin@...ineon.com>,
        Wright Feng <wright.feng@...ineon.com>,
        Chung-hsien Hsu <chung-hsien.hsu@...ineon.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        Andrew Zaborowski <andrew.zaborowski@...el.com>
CC:     "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "brcm80211-dev-list.pdl@...adcom.com" 
        <brcm80211-dev-list.pdl@...adcom.com>,
        "SHA-cyfmac-dev-list@...ineon.com" <SHA-cyfmac-dev-list@...ineon.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] brcmfmac: add support for CQM RSSI notifications

Hi,

On 1/19/21 9:30 AM, Arend Van Spriel wrote:
> On 1/15/2021 3:57 PM, Alvin Šipraga wrote:
>> Hi Arend,
>>
>> On 1/15/21 3:10 PM, Arend Van Spriel wrote:
>>> + Johannes
>>> - netdevs
>>>
>>> On 1/14/2021 5:36 PM, 'Alvin Šipraga' via BRCM80211-DEV-LIST,PDL wrote:
>>>> Add support for CQM RSSI measurement reporting and advertise the
>>>> NL80211_EXT_FEATURE_CQM_RSSI_LIST feature. This enables a userspace
>>>> supplicant such as iwd to be notified of changes in the RSSI for 
>>>> roaming
>>>> and signal monitoring purposes.
>>>
>>> The more I am looking into this API the less I understand it or at least
>>> it raises a couple of questions. Looking into nl80211_set_cqm_rssi() [1]
>>> two behaviors are supported: 1) driver is provisioned with a threshold
>>> and hysteresis, or 2) driver is provisioned with high and low 
>>> threshold. >
>>> The second behavior is used when the driver advertises
>>> NL80211_EXT_FEATURE_CQM_RSSI_LIST *and* user-space provides more than
>>> one RSSI threshold. In both cases the same driver callback is being used
>>> so I wonder what is expected from the driver. Seems to me the driver
>>> would need to be able to distinguish between the two behavioral
>>> scenarios. As there is no obvious way I assume the driver should behave
>>> the same for both cases, but again it is unclear to me what that
>>> expected/required behavior is.
>>
>> It will only provision the driver according to behaviour (1) if 0 or 1
>> thresholds are being set AND the driver implements
>> set_cqm_rssi_config(). But it says in the documentation for the
>> set_cqm_rssi_range_config() callback[1] that it supersedes
>> set_cqm_rssi_config() (or at least that there is no point in
>> implementing _config if range_config is implemented). In that case, and
>> if just one threshold is supplied (with a hysteresis), then a suitable
>> range is computed by cfg80211_cqm_rssi_update() and provided to
>> set_cqm_rssi_range_config(). I guess the implication here is that the
>> two behaviours are functionally equivalent. I'm not sure I can argue for
>> or against that because I don't really know what the semantics of the
>> original API were supposed to be, but it seems reasonable.
>>
>> As a starting point - and since the firmware behaviour is very close
>> already - I implemented only set_cqm_rssi_range(). I have been testing
>> with iwd, which by default sets just a single threshold and hysteresis,
>> and the driver was sending notifications as would be expected.
> 
> OK. I overlooked that there were two different callbacks involved. So I 
> will review the patch with that knowledge. What wifi chip did you test 
> this with and more importantly which firmware version?

All testing was done with a PCIe Cypress CYW88359 (Murata Type 1VA).

I tested with two firmwares:

1. A custom firmware from Cypress with some vendor-specific features:
    version 9.40.98.19 (r727154 CY) FWID 01-1ff1c30

2. The latest public firmware release from Murata[1] for this chip:
    version 9.40.130 (r724855 CY) FWID 01-9ae2cd6d

Thanks for the review.

[1] https://github.com/murata-wireless/cyw-fmac-fw

Kind regards,
Alvin