Re: [RFC 1/2] nl80211: add extended feature for BSS selection support

[Arend van Spriel <arend@broadcom.com>]

On 01/05/2016 05:31 PM, Johannes Berg wrote:
> On Tue, 2016-01-05 at 10:50 +0100, Arend van Spriel wrote:
>
>>>> +struct cfg80211_bss_selection {
>>>> +> 	> bool present;
>>>> +> 	> enum nl80211_band pref_band;
>>>> +> 	> u8 rssi_adjust;
>>>> +> 	> bool ignore_rssi;
>>>> +};
>>>
>>> Hm. Isn't it possible to specify *some* parameters of these? Or at
>>> least, in the future (if we extend this), it would be?
>>>
>>> Seems that 'present' might want to be a bitmap or so? Or perhaps be
>>> done by using invalid values by default (e.g. NUM_BANDS for no band
>>> preference, etc.)?
>>
>> Ok. I was not sure how to go about this. Our firmware uses an ordered
>> list of selection "keys" with the first being the primary selection
>> key and so on. So there are three "key" types: band, rssi, and
>> rssi_adjust.
>> The latter is not really a selection key, but will do rssi adjustment
>> for BSSes in the specified band.
>
> Ok.
>
>> One of the questions I have is whether the order of a nested list
>> attribute is retained.
>
> It is if you parse it right, but it's not typically something that we
> rely on and take advantage of, so I wouldn't want to do it that way.
> Also, I'm not really sure it'd really be what we wanted to do anyway?

ok

> It seems though that we might need to allow for other drivers having
> other selection criteria, and having a validity flag for each? That
> could go some of the way.

So do we want want a dedicated "bss selection capability" flag iso 
extended feature in which the driver can indicate the supported 
selection criteria to user-space? Guess so.

> To really fully replicate your firmware's capabilities seems difficult,
> though I also don't really see much point, or are you saying you could
> put "rssi" first? But the way you described it in nl80211, with "band"
> being a "tie breaker", it sounds like really "rssi" comes first,
> usually, followed by rssi_adjust and band?

The firmware api is pretty flexible, but I did not want to introduce 
that in nl80211. So I limited it to the two use-cases I know of 1) 
"band, rssi", and 2) "rssi_adjust, rssi". In terms of netlink attributes 
I played a trick in reusing ATTR_BSS_SELECT_BAND_PREF. When 
ATTR_BSS_SELECT_RSSI_ADJUST is passed the ATTR_BSS_SELECT_BAND_PREF is 
used to determine in which band the rssi is adjusted. So "band" and 
"rssi_adjust" are mutual exclusive.

> The other way - band first - could also be done with a huge rssi_adjust
> though (as I said before), so I don't really see much value in having
> all this complexity to start with?
>
>> Ok. Will elaborate. In follow-up email I raise question whether this
>> could/should be a signed value. Any opinion on this?
>
> I didn't see that, but yeah - good question. Would it be supported by
> firmware?

To be honest I had to check, but yes it does.

> But logically - does it even make sense? If you already prefer that
> band, why give it a boost still? Just disable RSSI? Hmm.

I hope the use-cases mentioned clarify this.

Regards,
Arend