Re: [PATCH v4 3/5] cfg80211: Accept multiple RSSI thresholds for CQM

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

Hi,

On 8 February 2017 at 10:58, Johannes Berg <johannes@sipsolutions.net> wrote:
>
>> 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.
>
> I'm not really sure I buy this.
>
> What if I configure a few ranges, and let's say one of the boundaries
> is -50dBm. Now if I sit just on that value of -50dBm and thus my signal
> fluctuates say -48..-52, I'd have to continuously wake up the host.
>
> You try to avoid that here, I think:
>
> +       if (low > (s32) (last - hyst))
> +               low = last - hyst;
> +       if (high < (s32) (last + hyst))
> +               high = last + hyst;
>
> but it's not clear to me that this is effective?
>
> Let's see. last is -52, so low will be -60 and high will be -50.
>
> Let's say hyst is 3 since I chose the ranges so closely. I'm guessing a
> higher hysteresis and larger ranges would actually be better, but let's
> stick to this for the sake of the example.
>
> last-hyst = -55, so low isn't > that, low = -60
> last+hyst = -49, so high = -49
>
> but now it still fluctuates around -50, so if I next hit -48 you do the
> same dance again with -50/-40, getting -51/-40 and never really
> applying a full hysteresis, no?
>
> I'll probably see an intermediate value of -50 at some point, but I'll
> never actually *report* it, so "last" can switch between -48 and -52
> constantly in this scenario, no?

Yes, sounds like this could happen.

>
>
> I think it would make sense to unconditionally apply the hysteresis to
> low/high, i.e. always set
>  low = low - hyst
>  high = high + hyst
>
> so that you get "sticky" ranges once you're in them?

Yes, maybe that's better, I guess I want to avoid just adding a lag /
delay in reporting changes that are not due to measurement error or
temporary.  Could also do something in between, e.g. use "low - hyst"
if signal is close to low, otherwise just "low".

The question is how the current hysteresis parameter is defined, what
is expected of the firmware and how do driver authors decide whether
their firmware/hardware implements the same mechanism as expected by
the kernel.  Would the same thing happen with firmware implementations
if the hysteresis value is 3 and the rssi fluctuates by +/- 2?  Or
would it have to be +/- 3 or +/- 4 before this starts to happen.  (If
this isn't well specified then it makes more sense to try to do it in
one place for consistency.)

>
>
>
>> +     if (!wiphy_ext_feature_isset(&rdev->wiphy,
>> +
>>            NL80211_EXT_FEATURE_CQM_RSSI_LIST))
>> +             return
>> -EOPNOTSUPP;
>
>
> That check should be earlier in the function
> cfg80211_cqm_rssi_update(), no?

Oh looks like it can actually be removed in the current code because
nl80211_set_cqm_rssi already has this check and wdev->cqm_config can't
be set outside that function.

Best regards