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

[Johannes Berg <johannes@sipsolutions.net>]

> 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?


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?



> +	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?



johannes