Hi Johannes, On Thu, Nov 3, 2011 at 10:39 AM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote: > Hi Dmitry, > >> >> + if (!ie1 && !ie2) >> >> + return 0; >> > I don't think it's valid to leave out the SSID IE, so I'd rather not >> > have that test here. It might interact badly with other uses (mesh?) >> ie1 and ie2 could not NULL at the same time, otherwise the bss >> would be found in the first search attempt. However, I would prefer >> to have some water-proof check here. WARN_ON? BUG_ON? > Maybe a warning? But I don't see why it'll just fall through to the next > line and return -1 if one of them is missing. > -1 means "lesser", which is not true if both of ies are absent. 0 means "equal", which is correct (but logically impossible with the current usage of the comparator). Now I think even WARN_ON is a bad idea, the comparator can be reused for other purposes when both SSID ies are missing. >> 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. > Oh. Good point. Add a comment? > Yes, I will. > Eliad pointed out another thing: It is possible to have multiple SSIDs > hidden behind a single hidden SSID. Therefore, this should update all of > those BSS structs. Also, I'm a tad confused about the matching code > here, this code path executes for both received beacons and probe > responses, but probe responses would typically have found the BSS > already? But what if we only have a beacon and then receive a probe > response? Or vice versa? Some comments would be good outlining how that > works. > I will add some comments in the code. Is a word, the code searches for a beacon of the hidden BSS and copies beacon IEs (with nullified SSID) into the probe response bss entry (with real SSID). It works fine in case of multi-SSID hidden BSS too. > what if we only have a beacon and then receive a probe response? It's the typical usecase for association with hidden BSS. The probe response will be updated with beacon IEs. > Or vice versa? It will not work. Update of beacon IEs of probe response is not implemented. The code is not trying to update existing probe response bss entries when beacon IEs are getting changed. Technically it's possible to update them, but I think it's a bit overkill, I'm addressing only PSM in this commit. Thank you and with best regards, Dmitry -- 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
