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 <andrew.zaborowski@xxxxxxxxx> > 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@xxxxxxxxx> > Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx> >
