Message-ID: <2adec5d6-fbc9-680c-01d6-25f83327bf21@broadcom.com>
Date:   Fri, 15 Jan 2021 15:10:57 +0100
From:   Arend Van Spriel <arend.vanspriel@...adcom.com>
To:     Alvin Šipraga <ALSI@...g-olufsen.dk>,
        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

+ 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.

With behavior 2) some processing is done in cfg80211 itself by 
cfg80211_cqm_rssi_update() which is called from nl80211_set_cqm_rssi() 
upon NL80211_CMD_SET_CQM and cfg80211_cqm_rssi_notify() called by 
driver. If I look at that it matches pretty close what our firmware is 
doing. The difference is that our firmware avoids RSSI oscillation with 
a time constraint between RSSI events whereas cfg80211 uses the hysteresis.

So before moving forward, I hope Johannes can chime in and clarify 
things. Added the commit message introducing the extended feature below. 
It mentions backward compatibility, but it only considers the extended 
feature setting when user-space provides more than one threshold. 
However, when the drivers set the extended feature is expects (low, 
high) and (threshold, hysteresis) if not. So it seems the extended 
feature should have precedence over the number of thresholds provided by 
user-space.

Regards,
Arend

[1] 
https://elixir.bootlin.com/linux/v5.10.7/source/net/wireless/nl80211.c#L11479

---8<-----------------------------------------------------------------
commit 4a4b8169501b18c3450ac735a7e277b24886a651
Author: Andrew Zaborowski <andrew.zaborowski@...el.com>
Date:   Fri Feb 10 10:02:31 2017 +0100

     cfg80211: Accept multiple RSSI thresholds for CQM

     Change the SET CQM command's RSSI threshold attribute to accept any
     number of thresholds as a sorted array.  The API should be backwards
     compatible so that if one s32 threshold value is passed, the old
     mechanism is enabled.  The netlink event generated is the same in both
     cases.

     cfg80211 handles an arbitrary number of RSSI thresholds but drivers 
have
     to provide a method (set_cqm_rssi_range_config) that configures a range
     set by a high and a low value.  Drivers have to call back when the RSSI
     goes out of that range and there's no additional event for each 
time the
     range is reconfigured as there was with the current one-threshold API.

     This method doesn't have a hysteresis parameter because there's no
     benefit to the cfg80211 code from having the hysteresis be handled by
     hardware/driver in terms of the number of wakeups.  At the same time
     it would likely be less consistent between drivers if offloaded or
     done in the drivers.

     Signed-off-by: Andrew Zaborowski <andrew.zaborowski@...el.com>
     Signed-off-by: Johannes Berg <johannes.berg@...el.com>

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4176 bytes)

Powered by blists - more mailing lists