On 01/05/2016 05:31 PM, Johannes Berg wrote:

On Tue, 2016-01-05 at 10:50 +0100, Arend van Spriel wrote:

quoted

quoted

quoted

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

quoted

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?

quoted

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