This patch series adds support for configuration of BSS selection done by
the driver and/or firmware. It allows user-space to pass selection criteria
with the NL80211_CMD_CONNECT using NL80211_ATTR_BSS_SELECT. When user-space
does provide this attribute and no explicit BSSID is provided it will pass
the BSS selection criteria to the driver if the driver supports it.
When multiple BSS-es are found belonging to the same ESS, ie. Infra-BSS with
same SSID, the criteria can be used to select a BSS. The criteria defined are:
1. RSSI based selection.
2. Band selection, RSSI based selection.
3. RSSI adjustment in given band, RSSI based selection.
V2:
- incorporate comments from Johannes Berg (may have missed some).
- split nl80211 patch in two separate patches.
Arend van Spriel (3):
nl80211: add extended feature for BSS selection support
nl80211: add bss selection attribute to CONNECT command
brcmfmac: add support for nl80211 BSS_SELECT feature
drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c | 63 ++++++++++++++++
drivers/net/wireless/brcm80211/brcmfmac/common.c | 38 ++++++----
drivers/net/wireless/brcm80211/brcmfmac/core.h | 1 +
include/net/cfg80211.h | 34 +++++++++
include/uapi/linux/nl80211.h | 52 +++++++++++++
net/wireless/core.c | 5 ++
net/wireless/nl80211.c | 86 ++++++++++++++++++++++
7 files changed, 264 insertions(+), 15 deletions(-)
--
1.9.1
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
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
On Thu, 2016-01-14 at 10:15 +0100, Arend van Spriel wrote:
>
> I looked into the suggested mac80211 "demo", but it seems this bss
> selection could be handled in net/wireless/sme.c for drivers support
> .assoc() and friends instead of .connect(). However, I recall Jouni
> suggesting to have NL80211_ATTR_BSS_SELECT in NL80211_CMD_ASSOC as
> well.
> So I guess wpa_s uses that command directly?
>
wpa_s uses assoc(), yeah. This would only be relevant if you actually
used "iw wlan0 connect", but still might be worthwhile as an example?
Dunno.
johannes
On Tue, 2016-01-19 at 23:33 +0100, Arend van Spriel wrote:
> 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.
Yeah, ok - similar result. It makes sense to keep the next ones too, in
case all the ones on the band you preferred weren't usable or something
(due to later conditions, like garbled beacons or whatever.)
> > 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.
Fair enough, not really a single attribute, but a single "behaviour"
attribute; the other (band) attribute would just provide data for the
behavioural one.
I think then I really would prefer one of these options:
* pack two u32 value into a tiny struct in the single "rssi_adjust"
behavioural attribute, so that it really becomes a single one
* make the rssi_adjust behavioural attribute nested with band and
adjustment inside
* use a rssi_adjust_band attribute, rather than reusing the band_pref
one
I think the first option is perfectly reasonable (and we do this in
other places) and makes for the easiest semantics since we can then
just say "specify exactly one of these attributes"?
johannes
On Wed, 2016-01-13 at 10:49 +0100, Arend van Spriel wrote:
>
> +enum nl80211_bss_select_primitive {
> + NL80211_BSS_SELECT_RSSI,
> + NL80211_BSS_SELECT_BAND_PREF,
> + NL80211_BSS_SELECT_ADJUST_RSSI,
> + /* add other primitives before this one */
> + NUM_NL80211_BSS_SELECT
> +};
>
Why not use enum nl80211_attr_bss_select from the next patch for this?
johannes
On 20-1-2016 15:02, Johannes Berg wrote:
> Ilan pointed me to this:
>
> http://androidxref.com/6.0.1_r10/xref/hardware/libhardware_legacy/include/hardware_legacy/gscan.h#351
>
>
> I'd say that would just be a separate primitive, and then Android would
> pick that in some way?
It was not part of my plan, but it seems to fit, yes. Android now uses
vendor command with google oui for gscan if I am not mistaken.
Regards,
Arend
Announce support for extended nl80211 feature BSS_SELECT and
process BSS selection criteria provided in .connect() callback.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c | 63 ++++++++++++++++++++++
drivers/net/wireless/brcm80211/brcmfmac/common.c | 38 +++++++------
drivers/net/wireless/brcm80211/brcmfmac/core.h | 1 +
3 files changed, 87 insertions(+), 15 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c
index deb5f78..a339e41 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c
@@ -237,6 +237,20 @@ module_param_named(roamoff, brcmf_roamoff, int, S_IRUSR);
MODULE_PARM_DESC(roamoff, "do not use internal roaming engine");
+static u8 nl80211_band_to_fwil(enum nl80211_band band)
+{
+ switch (band) {
+ case NL80211_BAND_2GHZ:
+ return WLC_BAND_2G;
+ case NL80211_BAND_5GHZ:
+ return WLC_BAND_5G;
+ default:
+ WARN_ON(1);
+ break;
+ }
+ return 0;
+}
+
static u16 chandef_to_chanspec(struct brcmu_d11inf *d11inf,
struct cfg80211_chan_def *ch)
{
@@ -1722,6 +1736,48 @@ enum nl80211_auth_type brcmf_war_auth_type(struct brcmf_if *ifp,
return type;
}
+static void brcmf_set_join_pref(struct brcmf_if *ifp,
+ struct cfg80211_bss_selection *bss_select)
+{
+ struct brcmf_join_pref_params join_pref_params[2];
+ enum nl80211_band band;
+ int err, i = 0;
+
+ if (!bss_select->primitive) {
+ brcmf_c_set_joinpref_default(ifp);
+ return;
+ }
+
+ join_pref_params[i].len = 2;
+ join_pref_params[i].rssi_gain = 0;
+ switch (bss_select->primitive) {
+ case NL80211_BSS_SELECT_BAND_PREF:
+ join_pref_params[i].type = BRCMF_JOIN_PREF_BAND;
+ band = bss_select->param.band_pref;
+ join_pref_params[i].band = nl80211_band_to_fwil(band);
+ i++;
+ break;
+ case NL80211_BSS_SELECT_ADJUST_RSSI:
+ join_pref_params[i].type = BRCMF_JOIN_PREF_RSSI_DELTA;
+ band = bss_select->param.adjust.band;
+ join_pref_params[i].band = nl80211_band_to_fwil(band);
+ join_pref_params[i].rssi_gain = bss_select->param.adjust.delta;
+ i++;
+ break;
+ case NL80211_BSS_SELECT_RSSI:
+ default:
+ break;
+ }
+ join_pref_params[i].type = BRCMF_JOIN_PREF_RSSI;
+ join_pref_params[i].len = 2;
+ join_pref_params[i].rssi_gain = 0;
+ join_pref_params[i].band = 0;
+ err = brcmf_fil_iovar_data_set(ifp, "join_pref", join_pref_params,
+ sizeof(join_pref_params));
+ if (err)
+ brcmf_err("Set join_pref error (%d)\n", err);
+}
+
static s32
brcmf_cfg80211_connect(struct wiphy *wiphy, struct net_device *ndev,
struct cfg80211_connect_params *sme)
@@ -1858,6 +1914,8 @@ brcmf_cfg80211_connect(struct wiphy *wiphy, struct net_device *ndev,
else
eth_broadcast_addr(ext_join_params->assoc_le.bssid);
+ brcmf_set_join_pref(ifp, &sme->bss_select);
+
if (cfg->channel) {
ext_join_params->assoc_le.chanspec_num = cpu_to_le32(1);
@@ -5920,6 +5978,11 @@ static int brcmf_setup_wiphy(struct wiphy *wiphy, struct brcmf_if *ifp)
wiphy->signal_type = CFG80211_SIGNAL_TYPE_MBM;
wiphy->cipher_suites = __wl_cipher_suites;
wiphy->n_cipher_suites = ARRAY_SIZE(__wl_cipher_suites);
+ wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_BSS_SELECT);
+ wiphy->bss_select_support = BIT(NL80211_BSS_SELECT_RSSI) |
+ BIT(NL80211_BSS_SELECT_BAND_PREF) |
+ BIT(NL80211_BSS_SELECT_ADJUST_RSSI);
+
wiphy->flags |= WIPHY_FLAG_PS_ON_BY_DEFAULT |
WIPHY_FLAG_OFFCHAN_TX |
WIPHY_FLAG_HAS_REMAIN_ON_CHANNEL |
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/common.c b/drivers/net/wireless/brcm80211/brcmfmac/common.c
index fe54844..7c40979 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/common.c
@@ -33,14 +33,34 @@ const u8 ALLFFMAC[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
#define BRCMF_DEFAULT_SCAN_CHANNEL_TIME 40
#define BRCMF_DEFAULT_SCAN_UNASSOC_TIME 40
-/* boost value for RSSI_DELTA in preferred join selection */
+/* default boost value for RSSI_DELTA in preferred join selection */
#define BRCMF_JOIN_PREF_RSSI_BOOST 8
+void brcmf_c_set_joinpref_default(struct brcmf_if *ifp)
+{
+ struct brcmf_join_pref_params join_pref_params[2];
+ int err;
+
+ /* Setup join_pref to select target by RSSI (boost on 5GHz) */
+ join_pref_params[0].type = BRCMF_JOIN_PREF_RSSI_DELTA;
+ join_pref_params[0].len = 2;
+ join_pref_params[0].rssi_gain = BRCMF_JOIN_PREF_RSSI_BOOST;
+ join_pref_params[0].band = WLC_BAND_5G;
+
+ join_pref_params[1].type = BRCMF_JOIN_PREF_RSSI;
+ join_pref_params[1].len = 2;
+ join_pref_params[1].rssi_gain = 0;
+ join_pref_params[1].band = 0;
+ err = brcmf_fil_iovar_data_set(ifp, "join_pref", join_pref_params,
+ sizeof(join_pref_params));
+ if (err)
+ brcmf_err("Set join_pref error (%d)\n", err);
+}
+
int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
{
s8 eventmask[BRCMF_EVENTING_MASK_LEN];
u8 buf[BRCMF_DCMD_SMLEN];
- struct brcmf_join_pref_params join_pref_params[2];
struct brcmf_rev_info_le revinfo;
struct brcmf_rev_info *ri;
char *ptr;
@@ -127,19 +147,7 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
goto done;
}
- /* Setup join_pref to select target by RSSI(with boost on 5GHz) */
- join_pref_params[0].type = BRCMF_JOIN_PREF_RSSI_DELTA;
- join_pref_params[0].len = 2;
- join_pref_params[0].rssi_gain = BRCMF_JOIN_PREF_RSSI_BOOST;
- join_pref_params[0].band = WLC_BAND_5G;
- join_pref_params[1].type = BRCMF_JOIN_PREF_RSSI;
- join_pref_params[1].len = 2;
- join_pref_params[1].rssi_gain = 0;
- join_pref_params[1].band = 0;
- err = brcmf_fil_iovar_data_set(ifp, "join_pref", join_pref_params,
- sizeof(join_pref_params));
- if (err)
- brcmf_err("Set join_pref error (%d)\n", err);
+ brcmf_c_set_joinpref_default(ifp);
/* Setup event_msgs, enable E_IF */
err = brcmf_fil_iovar_data_get(ifp, "event_msgs", eventmask,
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/core.h b/drivers/net/wireless/brcm80211/brcmfmac/core.h
index 2f9101b..f8b4925 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/core.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/core.h
@@ -217,5 +217,6 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success);
void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb);
void brcmf_net_setcarrier(struct brcmf_if *ifp, bool on);
+void brcmf_c_set_joinpref_default(struct brcmf_if *ifp);
#endif /* BRCMFMAC_CORE_H */
--
1.9.1
On 14-1-2016 12:10, Johannes Berg wrote:
> 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.
Ok. Did not know that convention so it was not that obvious to me ;-)
Will change it.
>> + * @__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 was thinking you'd keep the NLA_FLAG "RSSI" preference, and use the
> attribute values for the bitmap ...
You lost me here.
> 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]
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).
>> + * @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".
Indeed. Will change it.
> 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.
They are mutually exclusive as user-space can only give a single primitive.
Regards,
Arend
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.
> > 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.
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.
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?
johannes
Ilan pointed me to this:
http://androidxref.com/6.0.1_r10/xref/hardware/libhardware_legacy/include/hardware_legacy/gscan.h#351
I'd say that would just be a separate primitive, and then Android would
pick that in some way?
johannes
Pass user-space provided bss selection criteria to the driver
in the .connect() callback. It handles three selection primitives:
1. RSSI based selection.
2. Band selection, RSSI based selection.
3. RSSI adjustment in given band, RSSI based selection.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
include/net/cfg80211.h | 29 +++++++++++++++++++
include/uapi/linux/nl80211.h | 28 ++++++++++++++++++-
net/wireless/nl80211.c | 66 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 122 insertions(+), 1 deletion(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 34c4929..68695fd 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1854,6 +1854,33 @@ struct cfg80211_ibss_params {
};
/**
+ * struct cfg80211_bss_select_adjust - BSS selection with RSSI adjustment.
+ *
+ * @band: band of BSS which should match for RSSI level adjustment.
+ * @delta: value of RSSI level adjustment.
+ */
+struct cfg80211_bss_select_adjust {
+ enum nl80211_band band;
+ s8 delta;
+};
+
+/**
+ * struct cfg80211_bss_selection - connection parameters for BSS selection.
+ *
+ * @primitive: primitive for BSS selection.
+ * @param: parameters for given primitive.
+ * @band_pref: preferred band for %NL80211_BSS_SELECT_BAND_PREF primitive.
+ * @adjust: parameters for %NL80211_BSS_SELECT_ADJUST_RSSI primitive.
+ */
+struct cfg80211_bss_selection {
+ enum nl80211_bss_select_primitive primitive;
+ union {
+ enum nl80211_band band_pref;
+ struct cfg80211_bss_select_adjust adjust;
+ } param;
+};
+
+/**
* struct cfg80211_connect_params - Connection parameters
*
* This structure provides information needed to complete IEEE 802.11
@@ -1888,6 +1915,7 @@ struct cfg80211_ibss_params {
* @ht_capa_mask: The bits of ht_capa which are to be used.
* @vht_capa: VHT Capability overrides
* @vht_capa_mask: The bits of vht_capa which are to be used.
+ * @bss_select: criteria to be used for BSS selection.
*/
struct cfg80211_connect_params {
struct ieee80211_channel *channel;
@@ -1910,6 +1938,7 @@ struct cfg80211_connect_params {
struct ieee80211_ht_cap ht_capa_mask;
struct ieee80211_vht_cap vht_capa;
struct ieee80211_vht_cap vht_capa_mask;
+ struct cfg80211_bss_selection bss_select;
};
/**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index a5baf93..9a1b968 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1793,7 +1793,8 @@ enum nl80211_commands {
* extended feature %NL80211_EXT_FEATURE_BSS_SELECT. When used with
* %NL80211_CMD_GET_WIPHY is contains NLA_FLAG type according
* &enum nl80211_bss_select_primitive to indicate what primitives are
- * supported.
+ * supported. When used with %NL80211_CMD_CONNECT is contains the criteria
+ * for BSS selection to be done by driver and/or firmware.
*
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
@@ -4677,4 +4678,29 @@ enum nl80211_bss_select_primitive {
NUM_NL80211_BSS_SELECT
};
+/**
+ * enum nl80211_attr_bss_select - attributes for bss selection.
+ *
+ * @__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.
+ * @NL80211_ATTR_BSS_SELECT_BAND_PREF: Required attribute indicating the
+ * preferred band. Value according %enum nl80211_band.
+ * @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).
+ * @NL80211_ATTR_BSS_SELECT_MAX: highest bss select attribute number.
+ *@__NL80211_ATTR_BSS_SELECT_AFTER_LAST: internal use.
+ */
+enum nl80211_attr_bss_select {
+ __NL80211_ATTR_BSS_SELECT_INVALID,
+ NL80211_ATTR_BSS_SELECT_PRIMITIVE,
+ NL80211_ATTR_BSS_SELECT_BAND_PREF,
+ NL80211_ATTR_BSS_SELECT_RSSI_ADJUST,
+
+ /* keep last */
+ __NL80211_ATTR_BSS_SELECT_AFTER_LAST,
+ NL80211_ATTR_BSS_SELECT_MAX = __NL80211_ATTR_BSS_SELECT_AFTER_LAST - 1
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7a51de0..580c6ef 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -485,6 +485,13 @@ nl80211_plan_policy[NL80211_SCHED_SCAN_PLAN_MAX + 1] = {
[NL80211_SCHED_SCAN_PLAN_ITERATIONS] = { .type = NLA_U32 },
};
+static const struct nla_policy
+nl80211_bss_select_policy[NL80211_ATTR_BSS_SELECT_MAX + 1] = {
+ [NL80211_ATTR_BSS_SELECT_PRIMITIVE] = { .type = NLA_U32 },
+ [NL80211_ATTR_BSS_SELECT_BAND_PREF] = { .type = NLA_U32 },
+ [NL80211_ATTR_BSS_SELECT_RSSI_ADJUST] = { .type = NLA_U8 },
+};
+
static int nl80211_prepare_wdev_dump(struct sk_buff *skb,
struct netlink_callback *cb,
struct cfg80211_registered_device **rdev,
@@ -5773,6 +5780,53 @@ static int validate_scan_freqs(struct nlattr *freqs)
return n_channels;
}
+static int parse_bss_select(struct nlattr *nla,
+ struct cfg80211_bss_selection *bss_select)
+{
+ struct nlattr *attr[NL80211_ATTR_BSS_SELECT_MAX + 1];
+ enum nl80211_band band = NL80211_BAND_2GHZ;
+ int err;
+
+ if (!nla)
+ return 0;
+
+ err = nla_parse(attr, NL80211_ATTR_BSS_SELECT_MAX,
+ nla_data(nla), nla_len(nla), nl80211_bss_select_policy);
+ if (err)
+ return err;
+
+ if (!attr[NL80211_ATTR_BSS_SELECT_PRIMITIVE])
+ return -EINVAL;
+
+ bss_select->primitive =
+ nla_get_u32(attr[NL80211_ATTR_BSS_SELECT_PRIMITIVE]);
+
+ if (bss_select->primitive == NL80211_BSS_SELECT_BAND_PREF ||
+ bss_select->primitive == NL80211_BSS_SELECT_ADJUST_RSSI) {
+ if (!attr[NL80211_ATTR_BSS_SELECT_BAND_PREF])
+ return -EINVAL;
+ band = nla_get_u32(attr[NL80211_ATTR_BSS_SELECT_BAND_PREF]);
+ }
+
+ switch (bss_select->primitive) {
+ case NL80211_BSS_SELECT_BAND_PREF:
+ bss_select->param.band_pref = band;
+ break;
+ case NL80211_BSS_SELECT_ADJUST_RSSI:
+ bss_select->param.adjust.band = band;
+ if (!attr[NL80211_ATTR_BSS_SELECT_RSSI_ADJUST])
+ return -EINVAL;
+ bss_select->param.adjust.delta =
+ nla_get_u8(attr[NL80211_ATTR_BSS_SELECT_RSSI_ADJUST]);
+ break;
+ case NL80211_BSS_SELECT_RSSI:
+ default:
+ break;
+ }
+
+ return 0;
+}
+
static int nl80211_parse_random_mac(struct nlattr **attrs,
u8 *mac_addr, u8 *mac_addr_mask)
{
@@ -7998,6 +8052,18 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
connect.flags |= ASSOC_REQ_USE_RRM;
}
+ /* only do bss selection when no BSSID is specified. */
+ if (!connect.bssid &&
+ wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_BSS_SELECT)) {
+ err = parse_bss_select(info->attrs[NL80211_ATTR_BSS_SELECT],
+ &connect.bss_select);
+ if (err) {
+ kzfree(connkeys);
+ return err;
+ }
+ }
+
wdev_lock(dev->ieee80211_ptr);
err = cfg80211_connect(rdev, dev, &connect, connkeys, NULL);
wdev_unlock(dev->ieee80211_ptr);
--
1.9.1
On Thu, 2016-01-14 at 12:10 +0100, Johannes Berg wrote:
> 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]
>
I do wonder though what should happen if you just specify *only*
BAND_PREF, for example, or only RSSI_ADJUST. The latter doesn't even
make sense without then going to RSSI as the next level, and BAND_PREF
only can't really pick a single AP (just throw out ones that aren't on
the right band)
johannes
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBBcmVuZCB2YW4gU3ByaWVsIFtt
YWlsdG86YXJlbmRAYnJvYWRjb20uY29tXQ0KPiBTZW50OiBXZWRuZXNkYXksIEphbnVhcnkgMjAs
IDIwMTYgMjM6NTQNCj4gVG86IEpvaGFubmVzIEJlcmcNCj4gQ2M6IGxpbnV4LXdpcmVsZXNzOyBQ
ZWVyLCBJbGFuDQo+IFN1YmplY3Q6IFJlOiBbUkZDIFYyIDIvM10gbmw4MDIxMTogYWRkIGJzcyBz
ZWxlY3Rpb24gYXR0cmlidXRlIHRvIENPTk5FQ1QNCj4gY29tbWFuZA0KPiANCj4gDQo+IA0KPiBP
biAyMC0xLTIwMTYgMTU6MDIsIEpvaGFubmVzIEJlcmcgd3JvdGU6DQo+ID4gSWxhbiBwb2ludGVk
IG1lIHRvIHRoaXM6DQo+ID4NCj4gPiBodHRwOi8vYW5kcm9pZHhyZWYuY29tLzYuMC4xX3IxMC94
cmVmL2hhcmR3YXJlL2xpYmhhcmR3YXJlX2xlZ2FjeS9pbmNsDQo+ID4gdWRlL2hhcmR3YXJlX2xl
Z2FjeS9nc2Nhbi5oIzM1MQ0KPiA+DQo+ID4NCj4gPiBJJ2Qgc2F5IHRoYXQgd291bGQganVzdCBi
ZSBhIHNlcGFyYXRlIHByaW1pdGl2ZSwgYW5kIHRoZW4gQW5kcm9pZA0KPiA+IHdvdWxkIHBpY2sg
dGhhdCBpbiBzb21lIHdheT8NCj4gDQo+IEl0IHdhcyBub3QgcGFydCBvZiBteSBwbGFuLCBidXQg
aXQgc2VlbXMgdG8gZml0LCB5ZXMuIEFuZHJvaWQgbm93IHVzZXMgdmVuZG9yDQo+IGNvbW1hbmQg
d2l0aCBnb29nbGUgb3VpIGZvciBnc2NhbiBpZiBJIGFtIG5vdCBtaXN0YWtlbi4NCj4gDQoNClRo
ZXNlIGRvIG5vdCBuZWVkIHRvIG5lY2Vzc2FyaWx5IGJlIHZlbmRvciBjb21tYW5kcyA6KSBXZSBj
YW4gZXh0ZW5kIHlvdXIgd29yayB0byBhbHNvIHN1cHBvcnQgdGhlIGFuZHJvaWQgYmVoYXZpb3Ig
KGp1c3QgcG9pbnRlZCB0aGlzIG91dCBzbyB3ZSB3b3VsZCBub3QgYmUgYmxvY2tlZCB3aGVuIHdh
bnRpbmcgdG8gZXh0ZW5kIGl0KS4NCg0KUmVnYXJkcywNCg0KSWxhbi4NCg==
On 01/13/2016 10:49 AM, Arend van Spriel wrote:
> This patch series adds support for configuration of BSS selection done by
> the driver and/or firmware. It allows user-space to pass selection criteria
> with the NL80211_CMD_CONNECT using NL80211_ATTR_BSS_SELECT. When user-space
> does provide this attribute and no explicit BSSID is provided it will pass
> the BSS selection criteria to the driver if the driver supports it.
>
> When multiple BSS-es are found belonging to the same ESS, ie. Infra-BSS with
> same SSID, the criteria can be used to select a BSS. The criteria defined are:
>
> 1. RSSI based selection.
> 2. Band selection, RSSI based selection.
> 3. RSSI adjustment in given band, RSSI based selection.
>
> V2:
> - incorporate comments from Johannes Berg (may have missed some).
> - split nl80211 patch in two separate patches.
I looked into the suggested mac80211 "demo", but it seems this bss
selection could be handled in net/wireless/sme.c for drivers support
.assoc() and friends instead of .connect(). However, I recall Jouni
suggesting to have NL80211_ATTR_BSS_SELECT in NL80211_CMD_ASSOC as well.
So I guess wpa_s uses that command directly?
Regards,
Arend
> Arend van Spriel (3):
> nl80211: add extended feature for BSS selection support
> nl80211: add bss selection attribute to CONNECT command
> brcmfmac: add support for nl80211 BSS_SELECT feature
>
> drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c | 63 ++++++++++++++++
> drivers/net/wireless/brcm80211/brcmfmac/common.c | 38 ++++++----
> drivers/net/wireless/brcm80211/brcmfmac/core.h | 1 +
> include/net/cfg80211.h | 34 +++++++++
> include/uapi/linux/nl80211.h | 52 +++++++++++++
> net/wireless/core.c | 5 ++
> net/wireless/nl80211.c | 86 ++++++++++++++++++++++
> 7 files changed, 264 insertions(+), 15 deletions(-)
>
Introducing a new extended feature that the driver can use to
indicate the driver/firmware supports configuration of BSS
selection criteria upon CONNECT command. This can be useful
when multiple BSS-es are found belonging to the same ESS,
ie. Infra-BSS with same SSID. The criteria can then be used to
offload selection of a preferred BSS.
This patch adds the nested NL80211_ATTR_BSS_SELECT attribute
in NL80211_CMD_GET_WIPHY indicating supported selection criteria.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Franky (Zhenhui) Lin <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Lei Zhang <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
V2:
- added NL80211_ATTR_BSS_SELECT for NL80211_CMD_GET_WIPHY.
- still adds extended feature although it is redundant now.
---
include/net/cfg80211.h | 5 +++++
include/uapi/linux/nl80211.h | 26 ++++++++++++++++++++++++++
net/wireless/core.c | 5 +++++
net/wireless/nl80211.c | 20 ++++++++++++++++++++
4 files changed, 56 insertions(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 9bcaaf7..34c4929 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3178,6 +3178,9 @@ struct wiphy_vendor_command {
* low rssi when a frame is heard on different channel, then it should set
* this variable to the maximal offset for which it can compensate.
* This value should be set in MHz.
+ * @bss_select_support: bitmask indicating the BSS selection criteria supported
+ * by the driver in the .connect() callback. The bit position maps to the
+ * criteria defined in &enum nl80211_bss_select_primitive.
*/
struct wiphy {
/* assign these fields before you register the wiphy */
@@ -3300,6 +3303,8 @@ struct wiphy {
u8 max_num_csa_counters;
u8 max_adj_channel_rssi_comp;
+ u32 bss_select_support;
+
char priv[0] __aligned(NETDEV_ALIGN);
};
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 5b7b5eb..a5baf93 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1789,6 +1789,11 @@ enum nl80211_commands {
* thus it must not specify the number of iterations, only the interval
* between scans. The scan plans are executed sequentially.
* Each scan plan is a nested attribute of &enum nl80211_sched_scan_plan.
+ * @NL80211_ATTR_BSS_SELECT: nested attribute for driver supporting the
+ * extended feature %NL80211_EXT_FEATURE_BSS_SELECT. When used with
+ * %NL80211_CMD_GET_WIPHY is contains NLA_FLAG type according
+ * &enum nl80211_bss_select_primitive to indicate what primitives are
+ * supported.
*
* @NUM_NL80211_ATTR: total number of nl80211_attrs available
* @NL80211_ATTR_MAX: highest attribute number currently defined
@@ -2164,6 +2169,8 @@ enum nl80211_attrs {
NL80211_ATTR_MAX_SCAN_PLAN_ITERATIONS,
NL80211_ATTR_SCHED_SCAN_PLANS,
+ NL80211_ATTR_BSS_SELECT,
+
/* add attributes here, update the policy in nl80211.c */
__NL80211_ATTR_AFTER_LAST,
@@ -4396,12 +4403,15 @@ enum nl80211_feature_flags {
/**
* enum nl80211_ext_feature_index - bit index of extended features.
* @NL80211_EXT_FEATURE_VHT_IBSS: This driver supports IBSS with VHT datarates.
+ * @NL80211_EXT_FEATURE_BSS_SELECT: This driver supports BSS selection criteria
+ * to be given upon %NL80211_CMD_CONNECT.
*
* @NUM_NL80211_EXT_FEATURES: number of extended features.
* @MAX_NL80211_EXT_FEATURES: highest extended feature index.
*/
enum nl80211_ext_feature_index {
NL80211_EXT_FEATURE_VHT_IBSS,
+ NL80211_EXT_FEATURE_BSS_SELECT,
/* add new features before the definition below */
NUM_NL80211_EXT_FEATURES,
@@ -4651,4 +4661,20 @@ enum nl80211_sched_scan_plan {
__NL80211_SCHED_SCAN_PLAN_AFTER_LAST - 1
};
+/**
+ * enum nl80211_bss_select_primitive - primitives for bss selection.
+ *
+ * @NL80211_BSS_SELECT_RSSI:
+ * @NL80211_BSS_SELECT_BAND_PREF:
+ * @NL80211_BSS_SELECT_ADJUST_RSSI:
+ * @NUM_NL80211_BSS_SELECT: Must be kept last.
+ */
+enum nl80211_bss_select_primitive {
+ NL80211_BSS_SELECT_RSSI,
+ NL80211_BSS_SELECT_BAND_PREF,
+ NL80211_BSS_SELECT_ADJUST_RSSI,
+ /* add other primitives before this one */
+ NUM_NL80211_BSS_SELECT
+};
+
#endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 3a9c41b..73ab509 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -626,6 +626,11 @@ int wiphy_register(struct wiphy *wiphy)
!rdev->ops->set_mac_acl)))
return -EINVAL;
+ if (WARN_ON(wiphy_ext_feature_isset(wiphy,
+ NL80211_EXT_FEATURE_BSS_SELECT) &&
+ !wiphy->bss_select_support))
+ return -EINVAL;
+
if (wiphy->addresses)
memcpy(wiphy->perm_addr, wiphy->addresses[0].addr, ETH_ALEN);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 72de698..7a51de0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1730,6 +1730,26 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
rdev->wiphy.ext_features))
goto nla_put_failure;
+ if (wiphy_ext_feature_isset(&rdev->wiphy,
+ NL80211_EXT_FEATURE_BSS_SELECT) &&
+ rdev->wiphy.bss_select_support) {
+ struct nlattr *nested;
+ u32 bss_select_support = rdev->wiphy.bss_select_support;
+
+ nested = nla_nest_start(msg, NL80211_ATTR_BSS_SELECT);
+ if (!nested)
+ goto nla_put_failure;
+
+ i = 0;
+ while (bss_select_support) {
+ if ((bss_select_support & 1) &&
+ nla_put_flag(msg, i))
+ goto nla_put_failure;
+ i++;
+ bss_select_support >>= 1;
+ }
+ nla_nest_end(msg, nested);
+ }
/* done */
state->split_start = 0;
break;
--
1.9.1