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

[Andrew Zaborowski <andrew.zaborowski@intel.com>]

On Fri, 15 Jan 2021 at 15:12, Arend Van Spriel
<arend.vanspriel@broadcom.com> 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.

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