The function ieee80211_find_node_for_beacon() was added by reyk on 2005.
At the time, net80211 nodes were stored in a hash table keyed on hashes
the node's source MAC addresses.
The purpose of ieee80211_find_node_for_beacon() was to avoid storing
duplicate nodes with the same source MAC address in a given hash bucket.
Hash collisions on differing MAC addresses were valid, but collisions
on the same address had to be avoided.
Later on, the node table data structure was changed from a hash table
to an RB tree which is still in use today. The RB tree can only store a
single node per MAC address. However, find_node_for_beacon() was kept
regardless, now documented to serve a different purpose.
Its new purpose is to tell apart different nodes which happen to use
the same MAC address and hence cannot both be stored in the RB tree.
The idea is to help the net80211 stack to pick the "better" one of two
such APs while it is scanning for access points. The comment also claims
this function would avoid "bloat" of the nodes tree, which is obviously
related to the function's original purpose.
There are several problems with this:
The code which decides which node is "better" can erroneously match
an AP against itself, in case the AP uses a hidden SSID.
This prevents state updates for such APs.
Just a bit further down, the code looks up the same node again
and performs all of the intended updates. Simply skipping the
ieee80211_find_node_for_beacon() check makes state updates work.
(I have received a patch submission which fixes probe responses
for iwm/iwx in order to interop with hidden SSID APs. And that
patch contains a workaround for this problem, which prompted me
to look into it.)
The intention of avoiding "bloat" makes no sense.
There cannot be "bloat" of the node tree. Worst case we could end up
leaking memory when we replace an existing node in the tree with a
new node which has the same address (we would lose track of the pointer
to memory which stores the original node, and would never be able to
free this memory). There is no code in place which would prevent such
a leak. Simply always updating the existing node we have allocated is
the safer choice in the absence of proper collision handling.
My opinion is that this function should simply be removed.
It is an incomplete attempt at dealing with an edge case (MAC address
collisions), which is not a problem this function alone could ever
hope to solve completely.
ok?
diff d32c3633e170510dd593f1e288e78acfddaff882
32e6a77a0992dc0014e72c20005dfd9631b2055b
blob - bb6367fd1d5dd9f4641fedd96491069a8f0878dc
blob + 51d05f49c8b1ea58ea891f7b0f817e3bf94bed08
--- sys/net80211/ieee80211_input.c
+++ sys/net80211/ieee80211_input.c
@@ -1751,37 +1751,7 @@ ieee80211_recv_probe_resp(struct ieee80211com *ic, str
ic->ic_stats.is_rx_chanmismatch++;
return;
}
- /*
- * Use mac, channel and rssi so we collect only the
- * best potential AP with the equal bssid while scanning.
- * Collecting all potential APs may result in bloat of
- * the node tree. This call will return NULL if the node
- * for this APs does not exist or if the new node is the
- * potential better one.
- */
- ni = ieee80211_find_node_for_beacon(ic, wh->i_addr2,
- &ic->ic_channels[chan], ssid, rxi->rxi_rssi);
- if (ni != NULL) {
- /*
- * If we are doing a directed scan for an AP with a hidden SSID
- * we must collect the SSID from a probe response to override
- * a non-zero-length SSID filled with zeroes that we may have
- * received earlier in a beacon.
- */
- if (isprobe && ssid[1] != 0 && ni->ni_essid[0] == '\0') {
- ni->ni_esslen = ssid[1];
- memset(ni->ni_essid, 0, sizeof(ni->ni_essid));
- /* we know that ssid[1] <= IEEE80211_NWID_LEN */
- memcpy(ni->ni_essid, &ssid[2], ssid[1]);
- }
- /* Update channel in case AP has switched */
- if (ic->ic_opmode == IEEE80211_M_STA)
- ni->ni_chan = rni->ni_chan;
-
- return;
- }
-
#ifdef IEEE80211_DEBUG
if (ieee80211_debug > 1 &&
(ni == NULL || ic->ic_state == IEEE80211_S_SCAN ||
@@ -1977,6 +1947,13 @@ ieee80211_recv_probe_resp(struct ieee80211com *ic, str
}
}
+ /*
+ * Set our SSID if we do not know it yet.
+ * If we are doing a directed scan for an AP with a hidden SSID
+ * we must collect the SSID from a probe response to override
+ * a non-zero-length SSID filled with zeroes that we may have
+ * received earlier in a beacon.
+ */
if (ssid[1] != 0 && ni->ni_essid[0] == '\0') {
ni->ni_esslen = ssid[1];
memset(ni->ni_essid, 0, sizeof(ni->ni_essid));
blob - 98cac0edbe5d0a070244293819042423b33e1e68
blob + 691dbc4f48afcdec88f15856c97fc3ebe123db70
--- sys/net80211/ieee80211_node.c
+++ sys/net80211/ieee80211_node.c
@@ -2025,30 +2025,6 @@ ieee80211_find_rxnode(struct ieee80211com *ic,
return ieee80211_ref_node(ni);
}
-struct ieee80211_node *
-ieee80211_find_node_for_beacon(struct ieee80211com *ic,
- const u_int8_t *macaddr, const struct ieee80211_channel *chan,
- const char *ssid, u_int8_t rssi)
-{
- struct ieee80211_node *ni, *keep = NULL;
- int s, score = 0;
-
- if ((ni = ieee80211_find_node(ic, macaddr)) != NULL) {
- s = splnet();
-
- if (ni->ni_chan != chan && ni->ni_rssi >= rssi)
- score++;
- if (ssid[1] == 0 && ni->ni_esslen != 0)
- score++;
- if (score > 0)
- keep = ni;
-
- splx(s);
- }
-
- return (keep);
-}
-
void
ieee80211_node_tx_ba_clear(struct ieee80211_node *ni, int tid)
{
blob - ca7f9cd59bbc04c36318d64eaf8ef2036a07644e
blob + 4edf341acf6ae1ed4e6c3361bcc19a585dab636e
--- sys/net80211/ieee80211_node.h
+++ sys/net80211/ieee80211_node.h
@@ -524,10 +524,6 @@ struct ieee80211_node *ieee80211_find_rxnode(struct ie
const struct ieee80211_frame *);
struct ieee80211_node *ieee80211_find_txnode(struct ieee80211com *,
const u_int8_t *);
-struct ieee80211_node *
- ieee80211_find_node_for_beacon(struct ieee80211com *,
- const u_int8_t *, const struct ieee80211_channel *,
- const char *, u_int8_t);
void ieee80211_release_node(struct ieee80211com *,
struct ieee80211_node *);
void ieee80211_node_cleanup(struct ieee80211com *, struct ieee80211_node *);
