On 01/07/2016 03:41 PM, Johannes Berg wrote:

On Thu, 2016-01-07 at 13:52 +0100, Arend van Spriel wrote:

quoted

Agree. However, one of the reasons I added the ext_feature is because
people gave feedback they wanted it for testing purposes so they know
what to expect from the driver. That same argument could used for the
supported selection criteria. I have no strong opinion though.

Yeah, that's true - if we play the "advisory" card then we don't really
need to add it at all... Not sure what to do really. I'd love to
specify it very clearly if we need to at all.

Yes. If we add something like this it should be a specific as possible.

Perhaps for features we can add NL80211_ATTR_BSS_SELECT to the wiphy
information, and within that add each of the supported
ATTR_BSS_SELECT_* as an NLA_FLAG? That way at least we don't need to
define all kinds of new flags in the API.

That sounds like a plan.

quoted

quoted

Right. So realistically, writing this a bit more verbosely, you
have

1) rssi_preference, band_preference(band)
2) rssi_adjust(band, delta), rssi_preference

and perhaps

3) rssi_preference

as the default?

Good point.

quoted

As for 1), you said it was "band, rssi" but it seems you really
meant
the other way around since before you said "band" was a tie-
breaker.

My bad. When using "band_preference()" the selected BSS is in specified
band even though the BSS in other band would be stronger. If there is no
proper BSS in the specified band the preference is obviously
discarded.

So 1) really becomes just
  1) band_preference(band)

without any RSSI at all? Ah, I get it. You have to kind of reword that
in terms of the logic used:

  1) select_highest_rssi_in_band(band), select_highest_rssi()
  2) adjust_rssi_if_in_band(band, delta), select_highest_rssi()
  3) select_highest_rssi()

But if you look at that, then "select_highest_rssi_in_band(band)" is
really the same as "adjust_rssi_if_in_band(band, 1000)" since you can't
have real RSSI that high, which leaves us with just a single primitive
and perhaps a standard value for that "1000"?

True. Although I may have a firmware issue with that as RSSI is probably 
typed as s8 there. At least the adjustment is so can do up to 127 there, 
which would probably be enough as well. I think I prefer an explicit 
primitive though.

quoted

quoted

Perhaps then, the API should just expose the two "primitives"
   * band_preference(band)
   * rssi_adjust(band, delta)?

I guess it makes even more sense as I just worded it.

quoted

quoted

And for use-case number #3:

    * rssi_preference(void)

Yeah although you'd assume that's the default anyway?

That kinda depends on the driver I guess. Currently our driver defaults 
to rssi_adjust(5g, 8) and we restore to that when no BSS_SELECT 
attribute is provided in the .connect() callback.

Regards,
Arend