On Mon, Apr 30, 2012 at 10:30, Mohammed Shafi <shafi.wireless@xxxxxxxxx> wrote:
> Hi Emmanuel,
>
> On Mon, Apr 30, 2012 at 10:58 AM, Emmanuel Grumbach <egrumbach@xxxxxxxxx> wrote:
>> For quite a while now (not sure I can tell exactly for how long) we
>> see issues in association and scan list.
>> We send probe before authentication, get the probe response but never
>> send the authentication.
>> Moreover a lot of entries in the BSS list are duplicated.
>>
>> I began to look at it and ended up to understand that these 2 issues
>> are related: we just can't find an existing BSS in the BSS table.
>> Obviously this causes the second issue. The reason was it breaks the
>> association is that ieee80211_probe_auth will never be able to find
>> the IEs of the probe response since we couldn't fetch the BSS when we
>> parsed the probe response. In short:
>>
>> if (auth_data->bss->proberesp_ies) {
>> always return false.... and we fall back to send yet another probe request.
>>
>> As you probably know, the BSS table is implemented with an Red Black
>> Tree which requires its elements to be comparable. The compare
>> function compares the BSSID which is not always unique (there can be
>> several SSIDs on the same BSSID), so all the IEs are also compared.
>> But is this a good idea ?
>> It seems that since the IEs of an BSS may change from time to time
>> this compare function is not consistent...
>>
>> Just for playing I always return a positive value in cmp_bss (to have
>> all the nodes serialized and avoid the possibility to miss a existing
>> node) and don't rebalance the tree after insertion... the bug
>> disappeared.
>>
>> Thought ?
>>
>> Emmanuel Grumbach
>> egrumbach@xxxxxxxxx
>> --
>> 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
>
> we are comparing the BSSID of the new entry with the old entry (ie) we
> would return positive (with your latest fix)
> if the BSSID of the new entry > BSSID of the old entry, should not we
> do the same for comparing frequency, ie length and
> ie content.
> just got a doubt if this is by design we have things like this.
>
>
Frankly I didn't think about this :-), but I think the current
behavior is correct even if it is not intuitive. All we need a
consistent comparison operator. But I don't mind folding you
suggestion retest and resubmit.
> net/wireless/scan.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/wireless/scan.c b/net/wireless/scan.c
> index 70faadf..fda4eef 100644
> --- a/net/wireless/scan.c
> +++ b/net/wireless/scan.c
> @@ -271,7 +271,7 @@ static int cmp_ies(u8 num, u8 *ies1, size_t len1,
> u8 *ies2, size_t len2)
>
> /* sort by length first, then by contents */
> if (ie1[1] != ie2[1])
> - return ie2[1] - ie1[1];
> + return ie1[1] - ie2[1];
> return memcmp(ie1 + 2, ie2 + 2, ie1[1]);
> }
>
> @@ -361,7 +361,7 @@ static int cmp_bss_core(struct cfg80211_bss *a,
> int r;
>
> if (a->channel != b->channel)
> - return b->channel->center_freq - a->channel->center_freq;
> + return a->channel->center_freq - b->channel->center_freq;
>
> if (is_mesh_bss(a)&& is_mesh_bss(b)) {
> r = cmp_ies(WLAN_EID_MESH_ID,
> @@ -433,7 +433,7 @@ static int cmp_hidden_bss(struct cfg80211_bss *a,
>
> /* sort by length first, then by contents */
> if (ie1[1] != ie2[1])
> - return ie2[1] - ie1[1];
> + return ie1[1] - ie2[1];
>
> /* zeroed SSID ie is another indication of a hidden bss */
> for (i = 0; i< ie2[1]; i++)
>
>
> --
> thanks,
> shafi
--
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
