Re: [RFC V2 2/3] nl80211: add bss selection attribute to CONNECT command

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2016-01-13 at 10:49 +0100, Arend van Spriel wrote:
> 
> +/**
> + * enum nl80211_attr_bss_select - attributes for bss selection.

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.

> + * @__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?

I was thinking you'd keep the NLA_FLAG "RSSI" preference, and use the
attribute values for the bitmap ...

But still I think this isn't described very well.

Perhaps a good way of documenting this would be to say each primitive
has a priority? E.g.

    Primitives with a lower document priority are executed first while
    selecting a BSS:

    RSSI (priority 100)
    BAND_PREF (priority 1)
    RSSI_ADJUST (priority 1) [since it's mutually exclusive with
    BAND_PREF]

> + * @NL80211_ATTR_BSS_SELECT_BAND_PREF: Required attribute indicating the
> + *	preferred band. Value according %enum nl80211_band.

Why required?

> + * @NL80211_ATTR_BSS_SELECT_RSSI_ADJUST: When present the value is to be
> + *	added to the RSSI level for BSS-es in the preferred band (s8).

Oh, ok, I see. It's necessary with this too. Still, I wouldn't say
required, in case future extensions don't need it.

You should also more clearly document this though, I think, since in
this case the preferred band isn't really a preferred band any more, it
becomes just the "adjusted band".

I wonder if it'd be simpler to define two band attributes, and make
RSSI_ADJUST and BAND_PREF mutually exclusive, adding RSSI_ADJUST_BAND.
We could also use a complex struct value for the contents of
RSSI_ADJUST, but that doesn't seem great either.

johannes