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

On Fri, 15 Jan 2021 at 15:12, Arend Van Spriel
[off-list ref] wrote:>

+ Johannes
- netdevs

On 1/14/2021 5:36 PM, 'Alvin Šipraga' via BRCM80211-DEV-LIST,PDL wrote:

quoted

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.

Right.

The second behavior is used when the driver advertises
NL80211_EXT_FEATURE_CQM_RSSI_LIST *and* user-space provides more than
one RSSI threshold.

Or, when the driver doesn't implement set_cqm_rssi_config (the old
method).  So the driver can stop supporting set_cqm_rssi_config when
it implements set_cqm_rssi_range_config.

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.

When the driver wants to implement both methods it may be because the
hardware has a better method of handling the hysteresis than what is
implemented nl80211.c.  In that case, yes, it'd need to remember which
method was used and the parameters.  If set_cqm_rssi_range_config was
used the driver is expected to report when a momentary rssi
measurement goes out of the configured range.

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.

That may be a good reason to keep supporting both methods.

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.

Right, with one threshold the behaviour is unchanged.

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.

I guess the answer is that if the driver implements both methods it is
expected to report threshold crossings according to the method that
was called last.

Best regards