+ 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
--
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.
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.
[1]
https://elixir.bootlin.com/linux/v5.10.7/source/include/net/cfg80211.h#L3780
>
> 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.
From what I gathered, the set_cqm_rssi_range_config(low, high) API
should configure the driver to send a LOW/HIGH event to cfg80211
whenever the RSSI is outside of the range [low, high]. cfg80211 seems to
take care of how to deal with multiple thresholds then by calling back
into _range_config from cfg80211_cqm_rssi_notify() to readjust the
range. I could be oversimplifying things though and I would be glad to
get some clarification.
Kind regards,
Alvin
>
> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
>