On 19-1-2016 14:20, Johannes Berg wrote: > On Mon, 2016-01-18 at 10:34 +0100, Arend van Spriel wrote: > >>> I'd prefer nl80211_bss_select_attr in this name and the constants >>> too, >>> so it's more obvious that it doesn't belong to the top-level >>> namespace. >> >> Ok. Did not know that convention so it was not that obvious to me ;-) >> Will change it. > > I don't think we really follow it everywhere, and - hindsight being > 20/20 - I think that we should perhaps have chosen shorter prefixes :) > >>>> + * @__NL80211_ATTR_BSS_SELECT_INVALID: reserved. >>>> + * @NL80211_ATTR_BSS_SELECT_PRIMITIVE: Indicates what criteria >>>> are to >>>> + *> > be used for bss selection. Value according >>>> + * %enum nl80211_bss_select_primitive. >>> >>> This I don't understand now. Wouldn't the given attributes just be >>> used? >> >> The primitive just indicates the requested bss selection criteria and >> determines what other attributes are to be expected. Could determine >> it by looking at the other attributes, but that would make it harder >> to validate the request. This way it also makes them mutually >> exclusive. > > I still don't really understand - if a given attribute just gives data > about the remaining attributes, how does it make a difference? You get > all the attributes at once, after all, and aren't really forced to > parse them as they trickle in. True. Ok. let me try without this. >>> I was thinking you'd keep the NLA_FLAG "RSSI" preference, and use >>> the attribute values for the bitmap ... >> >> You lost me here. > > I meant: use BIT(NL80211_BSS_SELECT_ATTR_BAND_PREF) instead of the > separate NL80211_BSS_SELECT_BAND_PREF. > > Keeping the NLA_FLAG and all that was just a reference to the > discussion above. > >>> RSSI (priority 100) >>> BAND_PREF (priority 1) >>> RSSI_ADJUST (priority 1) [since it's mutually exclusive with >>> BAND_PREF] >> >> Not sure about the priority. What I should document is that BAND_PREF >> and RSSI_ADJUST also do RSSI based selection as a second step. As a >> (possibly important) side note our firmware api allows multiple >> primitives, but RSSI must be one of them as it will reject the >> configuration otherwise. As such I could combine RSSI and RSSI_ADJUST >> as RSSI would be RSSI_ADJUST(band=unspec, delta=0). > > Ok, that's a different way of thinking about it. > > I was thinking about it as a kind of small programmable state-machine > or pipeline in the BSS selection pipeline, so if you have > > band_pref, rssi > > you basically have a first "element" in the pipeline that throws away > the BSSes that don't match the right band, and a second one that picks > the one with the highest RSSI. I looked at our firmware and it has a kinda pipeline, but it uses a weighted score. So for "band_pref, rssi" the band_pref score would have more weight than the rssi score and bss-es are sorted based on the score. So not really throwing things away. > If I understand you correctly, you're thinking about it more in terms > of the overall behaviour. That's perfectly fine with me, but then we > should document that more clearly. Agree. > Just to make sure I understand - you're basically saying that > > band_pref > > would mean > * throw away BSSes not matching the right band > * pick the one with the highest RSSI > > which basically makes it mutually exclusive with any of the other > attributes you suggested, where I was thinking that you'd pretty much > always have to specify multiple attributes to get a proper "pipeline". > > > Your way definitely has advantages too, particularly that you don't > need that whole discussion about priorities/ordering, which is good. > > But then I understand the whole point of the "primitive" even less, > since it should be trivial to check that of multiple attributes only a > single one is specified? Not really a single one as rssi_adjust needs two attributes, ie. band and rssi_delta. Still you are right and we can probably drop the primitive. Regards, Arend -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
