From: Johannes Berg <[email protected]>
Instead of duplicating the rbtree functions, pass
an argument to the compare function. This removes
the code duplication for the two searches.
Signed-off-by: Johannes Berg <[email protected]>
---
net/wireless/scan.c | 79 ++++++++++++-----------------------------------------
1 file changed, 17 insertions(+), 62 deletions(-)
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index ca367c5..72ef3b2 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -425,34 +425,13 @@ static int cmp_bss_core(struct cfg80211_bss *a, struct cfg80211_bss *b)
}
static int cmp_bss(struct cfg80211_bss *a,
- struct cfg80211_bss *b)
-{
- const struct cfg80211_bss_ies *a_ies, *b_ies;
- int r;
-
- r = cmp_bss_core(a, b);
- if (r)
- return r;
-
- a_ies = rcu_access_pointer(a->ies);
- if (!a_ies)
- return -1;
- b_ies = rcu_access_pointer(b->ies);
- if (!b_ies)
- return 1;
-
- return cmp_ies(WLAN_EID_SSID,
- a_ies->data, a_ies->len,
- b_ies->data, b_ies->len);
-}
-
-static int cmp_hidden_bss(struct cfg80211_bss *a, struct cfg80211_bss *b)
+ struct cfg80211_bss *b,
+ bool hide_ssid)
{
const struct cfg80211_bss_ies *a_ies, *b_ies;
const u8 *ie1;
const u8 *ie2;
- int i;
- int r;
+ int i, r;
r = cmp_bss_core(a, b);
if (r)
@@ -469,14 +448,9 @@ static int cmp_hidden_bss(struct cfg80211_bss *a, struct cfg80211_bss *b)
ie2 = cfg80211_find_ie(WLAN_EID_SSID, b_ies->data, b_ies->len);
/*
- * Key comparator must use same algorithm in any rb-tree
- * search function (order is important), otherwise ordering
- * of items in the tree is broken and search gives incorrect
- * results. This code uses same order as cmp_ies() does.
- *
- * Note that due to the differring behaviour with hidden SSIDs
- * this function only works when "b" is the tree element and
- * "a" is the key we're looking for.
+ * Note that with "hide_ssid", the function returns a match if
+ * the already-present BSS ("b") is a hidden SSID beacon for
+ * the new BSS ("a").
*/
/* sort missing IE before (left of) present IE */
@@ -485,14 +459,17 @@ static int cmp_hidden_bss(struct cfg80211_bss *a, struct cfg80211_bss *b)
if (!ie2)
return 1;
- /* zero-size SSID is used as an indication of the hidden bss */
- if (!ie2[1])
+ /* zero-length SSID is used as an indication of the hidden bss */
+ if (hide_ssid && !ie2[1])
return 0;
/* sort by length first, then by contents */
if (ie1[1] != ie2[1])
return ie2[1] - ie1[1];
+ if (!hide_ssid)
+ return memcmp(ie1 + 2, ie2 + 2, ie1[1]);
+
/*
* zeroed SSID ie is another indication of a hidden bss;
* if it isn't zeroed just return the regular sort value
@@ -584,7 +561,7 @@ static void rb_insert_bss(struct cfg80211_registered_device *dev,
parent = *p;
tbss = rb_entry(parent, struct cfg80211_internal_bss, rbn);
- cmp = cmp_bss(&bss->pub, &tbss->pub);
+ cmp = cmp_bss(&bss->pub, &tbss->pub, false);
if (WARN_ON(!cmp)) {
/* will sort of leak this BSS */
@@ -603,30 +580,8 @@ static void rb_insert_bss(struct cfg80211_registered_device *dev,
static struct cfg80211_internal_bss *
rb_find_bss(struct cfg80211_registered_device *dev,
- struct cfg80211_internal_bss *res)
-{
- struct rb_node *n = dev->bss_tree.rb_node;
- struct cfg80211_internal_bss *bss;
- int r;
-
- while (n) {
- bss = rb_entry(n, struct cfg80211_internal_bss, rbn);
- r = cmp_bss(&res->pub, &bss->pub);
-
- if (r == 0)
- return bss;
- else if (r < 0)
- n = n->rb_left;
- else
- n = n->rb_right;
- }
-
- return NULL;
-}
-
-static struct cfg80211_internal_bss *
-rb_find_hidden_bss(struct cfg80211_registered_device *dev,
- struct cfg80211_internal_bss *res)
+ struct cfg80211_internal_bss *res,
+ bool hidden)
{
struct rb_node *n = dev->bss_tree.rb_node;
struct cfg80211_internal_bss *bss;
@@ -634,7 +589,7 @@ rb_find_hidden_bss(struct cfg80211_registered_device *dev,
while (n) {
bss = rb_entry(n, struct cfg80211_internal_bss, rbn);
- r = cmp_hidden_bss(&res->pub, &bss->pub);
+ r = cmp_bss(&res->pub, &bss->pub, hidden);
if (r == 0)
return bss;
@@ -684,7 +639,7 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
return NULL;
}
- found = rb_find_bss(dev, tmp);
+ found = rb_find_bss(dev, tmp, false);
if (found) {
found->pub.beacon_interval = tmp->pub.beacon_interval;
@@ -739,7 +694,7 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
/* TODO: The code is not trying to update existing probe
* response bss entries when beacon ies are
* getting changed. */
- hidden = rb_find_hidden_bss(dev, tmp);
+ hidden = rb_find_bss(dev, tmp, true);
if (hidden)
copy_hidden_ies(tmp, hidden);
--
1.8.0
On Tue, 2013-01-29 at 17:42 -0800, Thomas Pedersen wrote:
> FWIW cfg80211_get_mesh() has no users either.
Oh, I'll kill it too then :-)
> > + if (WLAN_CAPABILITY_IS_STA_BSS(a->capability))
> > + ie1 = cfg80211_find_ie(WLAN_EID_MESH_ID,
> > + a_ies->data, a_ies->len);
> > + if (WLAN_CAPABILITY_IS_STA_BSS(a->capability))
>
> You probably wanted:
>
> if (WLAN_CAPABILITY_IS_STA_BSS(b->capability))
Yup, indeed, thanks.
johannes
On Wed, Jan 30, 2013 at 12:45:20AM +0100, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> Instead of first checking if a BSS is an MBSS
> and then doing the comparisons, inline it all
> into the BSS comparison function. This avoids
> doing the IE searches twice and is also a lot
> less code.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> net/wireless/scan.c | 120 +++++++++++++++++-----------------------------------
> 1 file changed, 39 insertions(+), 81 deletions(-)
>
> diff --git a/net/wireless/scan.c b/net/wireless/scan.c
> index 72ef3b2..5fd7c47 100644
> --- a/net/wireless/scan.c
> +++ b/net/wireless/scan.c
> @@ -288,26 +288,6 @@ const u8 *cfg80211_find_vendor_ie(unsigned int oui, u8 oui_type,
> }
> EXPORT_SYMBOL(cfg80211_find_vendor_ie);
>
> -static int cmp_ies(u8 num, const u8 *ies1, int len1, const u8 *ies2, int len2)
> -{
> - const u8 *ie1 = cfg80211_find_ie(num, ies1, len1);
> - const u8 *ie2 = cfg80211_find_ie(num, ies2, len2);
> -
> - /* equal if both missing */
> - if (!ie1 && !ie2)
> - return 0;
> - /* sort missing IE before (left of) present IE */
> - if (!ie1)
> - return -1;
> - if (!ie2)
> - return 1;
> -
> - /* sort by length first, then by contents */
> - if (ie1[1] != ie2[1])
> - return ie2[1] - ie1[1];
> - return memcmp(ie1 + 2, ie2 + 2, ie1[1]);
> -}
> -
FWIW cfg80211_get_mesh() has no users either.
> static int cmp_bss(struct cfg80211_bss *a,
> struct cfg80211_bss *b,
> bool hide_ssid)
> {
> const struct cfg80211_bss_ies *a_ies, *b_ies;
> - const u8 *ie1;
> - const u8 *ie2;
> + const u8 *ie1 = NULL;
> + const u8 *ie2 = NULL;
> int i, r;
>
> - r = cmp_bss_core(a, b);
> - if (r)
> - return r;
> + if (a->channel != b->channel)
> + return b->channel->center_freq - a->channel->center_freq;
>
> a_ies = rcu_access_pointer(a->ies);
> if (!a_ies)
> @@ -444,6 +367,41 @@ static int cmp_bss(struct cfg80211_bss *a,
> if (!b_ies)
> return 1;
>
> + if (WLAN_CAPABILITY_IS_STA_BSS(a->capability))
> + ie1 = cfg80211_find_ie(WLAN_EID_MESH_ID,
> + a_ies->data, a_ies->len);
> + if (WLAN_CAPABILITY_IS_STA_BSS(a->capability))
You probably wanted:
if (WLAN_CAPABILITY_IS_STA_BSS(b->capability))
--
Thomas
From: Johannes Berg <[email protected]>
Instead of first checking if a BSS is an MBSS
and then doing the comparisons, inline it all
into the BSS comparison function. This avoids
doing the IE searches twice and is also a lot
less code.
Signed-off-by: Johannes Berg <[email protected]>
---
net/wireless/scan.c | 120 +++++++++++++++++-----------------------------------
1 file changed, 39 insertions(+), 81 deletions(-)
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 72ef3b2..5fd7c47 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -288,26 +288,6 @@ const u8 *cfg80211_find_vendor_ie(unsigned int oui, u8 oui_type,
}
EXPORT_SYMBOL(cfg80211_find_vendor_ie);
-static int cmp_ies(u8 num, const u8 *ies1, int len1, const u8 *ies2, int len2)
-{
- const u8 *ie1 = cfg80211_find_ie(num, ies1, len1);
- const u8 *ie2 = cfg80211_find_ie(num, ies2, len2);
-
- /* equal if both missing */
- if (!ie1 && !ie2)
- return 0;
- /* sort missing IE before (left of) present IE */
- if (!ie1)
- return -1;
- if (!ie2)
- return 1;
-
- /* sort by length first, then by contents */
- if (ie1[1] != ie2[1])
- return ie2[1] - ie1[1];
- return memcmp(ie1 + 2, ie2 + 2, ie1[1]);
-}
-
static bool is_bss(struct cfg80211_bss *a, const u8 *bssid,
const u8 *ssid, size_t ssid_len)
{
@@ -331,29 +311,6 @@ static bool is_bss(struct cfg80211_bss *a, const u8 *bssid,
return memcmp(ssidie + 2, ssid, ssid_len) == 0;
}
-static bool is_mesh_bss(struct cfg80211_bss *a)
-{
- const struct cfg80211_bss_ies *ies;
- const u8 *ie;
-
- if (!WLAN_CAPABILITY_IS_STA_BSS(a->capability))
- return false;
-
- ies = rcu_access_pointer(a->ies);
- if (!ies)
- return false;
-
- ie = cfg80211_find_ie(WLAN_EID_MESH_ID, ies->data, ies->len);
- if (!ie)
- return false;
-
- ie = cfg80211_find_ie(WLAN_EID_MESH_CONFIG, ies->data, ies->len);
- if (!ie)
- return false;
-
- return true;
-}
-
static bool is_mesh(struct cfg80211_bss *a,
const u8 *meshid, size_t meshidlen,
const u8 *meshcfg)
@@ -391,51 +348,17 @@ static bool is_mesh(struct cfg80211_bss *a,
sizeof(struct ieee80211_meshconf_ie) - 2) == 0;
}
-static int cmp_bss_core(struct cfg80211_bss *a, struct cfg80211_bss *b)
-{
- const struct cfg80211_bss_ies *a_ies, *b_ies;
- int r;
-
- if (a->channel != b->channel)
- return b->channel->center_freq - a->channel->center_freq;
-
- if (is_mesh_bss(a) && is_mesh_bss(b)) {
- a_ies = rcu_access_pointer(a->ies);
- if (!a_ies)
- return -1;
- b_ies = rcu_access_pointer(b->ies);
- if (!b_ies)
- return 1;
-
- r = cmp_ies(WLAN_EID_MESH_ID,
- a_ies->data, a_ies->len,
- b_ies->data, b_ies->len);
- if (r)
- return r;
- return cmp_ies(WLAN_EID_MESH_CONFIG,
- a_ies->data, a_ies->len,
- b_ies->data, b_ies->len);
- }
-
- /*
- * we can't use compare_ether_addr here since we need a < > operator.
- * The binary return value of compare_ether_addr isn't enough
- */
- return memcmp(a->bssid, b->bssid, sizeof(a->bssid));
-}
-
static int cmp_bss(struct cfg80211_bss *a,
struct cfg80211_bss *b,
bool hide_ssid)
{
const struct cfg80211_bss_ies *a_ies, *b_ies;
- const u8 *ie1;
- const u8 *ie2;
+ const u8 *ie1 = NULL;
+ const u8 *ie2 = NULL;
int i, r;
- r = cmp_bss_core(a, b);
- if (r)
- return r;
+ if (a->channel != b->channel)
+ return b->channel->center_freq - a->channel->center_freq;
a_ies = rcu_access_pointer(a->ies);
if (!a_ies)
@@ -444,6 +367,41 @@ static int cmp_bss(struct cfg80211_bss *a,
if (!b_ies)
return 1;
+ if (WLAN_CAPABILITY_IS_STA_BSS(a->capability))
+ ie1 = cfg80211_find_ie(WLAN_EID_MESH_ID,
+ a_ies->data, a_ies->len);
+ if (WLAN_CAPABILITY_IS_STA_BSS(a->capability))
+ ie2 = cfg80211_find_ie(WLAN_EID_MESH_ID,
+ b_ies->data, b_ies->len);
+ if (ie1 && ie2) {
+ int mesh_id_cmp;
+
+ if (ie1[1] == ie2[1])
+ mesh_id_cmp = memcmp(ie1 + 2, ie2 + 2, ie1[1]);
+ else
+ mesh_id_cmp = ie2[1] - ie1[1];
+
+ ie1 = cfg80211_find_ie(WLAN_EID_MESH_CONFIG,
+ a_ies->data, a_ies->len);
+ ie2 = cfg80211_find_ie(WLAN_EID_MESH_CONFIG,
+ b_ies->data, b_ies->len);
+ if (ie1 && ie2) {
+ if (mesh_id_cmp)
+ return mesh_id_cmp;
+ if (ie1[1] != ie2[1])
+ return ie2[1] - ie1[1];
+ return memcmp(ie1 + 2, ie2 + 2, ie1[1]);
+ }
+ }
+
+ /*
+ * we can't use compare_ether_addr here since we need a < > operator.
+ * The binary return value of compare_ether_addr isn't enough
+ */
+ r = memcmp(a->bssid, b->bssid, sizeof(a->bssid));
+ if (r)
+ return r;
+
ie1 = cfg80211_find_ie(WLAN_EID_SSID, a_ies->data, a_ies->len);
ie2 = cfg80211_find_ie(WLAN_EID_SSID, b_ies->data, b_ies->len);
--
1.8.0
On Wed, 2013-01-30 at 00:45 +0100, Johannes Berg wrote:
> @@ -469,14 +448,9 @@ static int cmp_hidden_bss(struct cfg80211_bss *a, struct cfg80211_bss *b)
> ie2 = cfg80211_find_ie(WLAN_EID_SSID, b_ies->data, b_ies->len);
>
> /*
> - * Key comparator must use same algorithm in any rb-tree
> - * search function (order is important), otherwise ordering
> - * of items in the tree is broken and search gives incorrect
> - * results. This code uses same order as cmp_ies() does.
> - *
> - * Note that due to the differring behaviour with hidden SSIDs
> - * this function only works when "b" is the tree element and
> - * "a" is the key we're looking for.
> + * Note that with "hide_ssid", the function returns a match if
> + * the already-present BSS ("b") is a hidden SSID beacon for
> + * the new BSS ("a").
> */
>
Need to insert
if (!ie1 && !ie2)
return 0;
here.
johannes
On Wed, 2013-01-30 at 00:45 +0100, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> Instead of duplicating the rbtree functions, pass
> an argument to the compare function. This removes
> the code duplication for the two searches.
Applied (the version with the !ie1 && !ie2 fix)
johannes