Date: Mon, 1 Aug 2005 17:56:49 GMT From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 81299 for review Message-ID: <200508011756.j71Hunne012782@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=81299 Change 81299 by sam@sam_ebb on 2005/08/01 17:56:25 checkpoint changes to move keycache up to the 802.11 layer to fix race conditions; still needsz fixup for split tx/rx keys used for tkip on some cards Affected files ... .. //depot/projects/wifi/sys/dev/ath/if_ath.c#93 edit .. //depot/projects/wifi/sys/dev/ath/if_athvar.h#40 edit .. //depot/projects/wifi/sys/net80211/ieee80211_crypto.c#15 edit .. //depot/projects/wifi/sys/net80211/ieee80211_node.c#57 edit .. //depot/projects/wifi/sys/net80211/ieee80211_node.h#27 edit Differences ... ==== //depot/projects/wifi/sys/dev/ath/if_ath.c#93 (text+ko) ==== @@ -1938,32 +1938,17 @@ struct ath_softc *sc = ic->ic_ifp->if_softc; struct ath_hal *ah = sc->sc_ah; const struct ieee80211_cipher *cip = k->wk_cipher; - struct ieee80211_node *ni; u_int keyix = k->wk_keyix; DPRINTF(sc, ATH_DEBUG_KEYCACHE, "%s: delete key %u\n", __func__, keyix); ath_hal_keyreset(ah, keyix); /* - * Check the key->node map and flush any ref. - */ - ni = sc->sc_keyixmap[keyix]; - if (ni != NULL) { - ieee80211_free_node(ni); - sc->sc_keyixmap[keyix] = NULL; - } - /* * Handle split tx/rx keying required for TKIP with h/w MIC. */ if (cip->ic_cipher == IEEE80211_CIPHER_TKIP && - (k->wk_flags & IEEE80211_KEY_SWMIC) == 0 && sc->sc_splitmic) { + (k->wk_flags & IEEE80211_KEY_SWMIC) == 0 && sc->sc_splitmic) ath_hal_keyreset(ah, keyix+32); /* RX key */ - ni = sc->sc_keyixmap[keyix+32]; - if (ni != NULL) { /* as above... */ - ieee80211_free_node(ni); - sc->sc_keyixmap[keyix+32] = NULL; - } - } if (keyix >= IEEE80211_WEP_NKID) { /* * Don't touch keymap entries for global keys so @@ -2973,6 +2958,7 @@ ds = bf->bf_desc; ds->ds_link = bf->bf_daddr; /* link to self */ ds->ds_data = bf->bf_segs[0].ds_addr; + ds->ds_vdata = mtod(m, u_int32_t);/*XXX*/ ath_hal_setuprxdesc(ah, ds , m->m_len /* buffer size */ , 0 @@ -3294,51 +3280,22 @@ /* * Locate the node for sender, track state, and then * pass the (referenced) node up to the 802.11 layer - * for its use. If the sender is unknown spam the - * frame; it'll be dropped where it's not wanted. + * for its use. + */ + ni = ieee80211_find_rxnode_withkey(ic, + mtod(m, const struct ieee80211_frame_min *), + ds->ds_rxstat.rs_keyix == HAL_RXKEYIX_INVALID ? + IEEE80211_KEYIX_NONE : ds->ds_rxstat.rs_keyix); + /* + * Track rx rssi and do any rx antenna management. + */ + an = ATH_NODE(ni); + ATH_RSSI_LPF(an->an_avgrssi, ds->ds_rxstat.rs_rssi); + /* + * Send frame up for processing. */ - if (ds->ds_rxstat.rs_keyix != HAL_RXKEYIX_INVALID && - (ni = sc->sc_keyixmap[ds->ds_rxstat.rs_keyix]) != NULL && - ieee80211_node_refcnt(ni) > 1) { - /* - * Fast path: node is present in the key map; - * grab a reference for processing the frame. - */ - an = ATH_NODE(ieee80211_ref_node(ni)); - ATH_RSSI_LPF(an->an_avgrssi, ds->ds_rxstat.rs_rssi); - type = ieee80211_input(ic, m, ni, - ds->ds_rxstat.rs_rssi, ds->ds_rxstat.rs_tstamp); - } else { - /* - * Locate the node for sender, track state, and then - * pass the (referenced) node up to the 802.11 layer - * for its use. - */ - ni = ieee80211_find_rxnode(ic, - mtod(m, const struct ieee80211_frame_min *)); - /* - * Track rx rssi and do any rx antenna management. - */ - an = ATH_NODE(ni); - ATH_RSSI_LPF(an->an_avgrssi, ds->ds_rxstat.rs_rssi); - /* - * Send frame up for processing. - */ - type = ieee80211_input(ic, m, ni, - ds->ds_rxstat.rs_rssi, ds->ds_rxstat.rs_tstamp); - if (ni != ic->ic_bss) { - u_int16_t keyix; - /* - * If the station has a key cache slot assigned - * update the key->node mapping table. - */ - keyix = ni->ni_ucastkey.wk_keyix; - if (keyix != IEEE80211_KEYIX_NONE && - sc->sc_keyixmap[keyix] == NULL) - sc->sc_keyixmap[keyix] = - ieee80211_ref_node(ni); - } - } + type = ieee80211_input(ic, m, ni, + ds->ds_rxstat.rs_rssi, ds->ds_rxstat.rs_tstamp); ieee80211_free_node(ni); if (sc->sc_diversity) { /* ==== //depot/projects/wifi/sys/dev/ath/if_athvar.h#40 (text+ko) ==== @@ -256,7 +256,6 @@ HAL_INT sc_imask; /* interrupt mask copy */ u_int sc_keymax; /* size of key cache */ u_int8_t sc_keymap[ATH_KEYBYTES];/* key use bit map */ - struct ieee80211_node *sc_keyixmap[ATH_KEYMAX];/* key ix->node map */ u_int sc_ledpin; /* GPIO pin for driving LED */ u_int sc_ledon; /* pin setting for LED on */ ==== //depot/projects/wifi/sys/net80211/ieee80211_crypto.c#15 (text+ko) ==== @@ -416,6 +416,11 @@ ic->ic_stats.is_crypto_delkey++; /* XXX recovery? */ } + /* + * Clear any unicast key index -> node mapping. + */ + if ((key->wk_flags & IEEE80211_KEY_GROUP) == 0) + ieee80211_node_delkey(&ic->ic_sta, keyix); } cipher_detach(key); memset(key, 0, sizeof(*key)); ==== //depot/projects/wifi/sys/net80211/ieee80211_node.c#57 (text+ko) ==== @@ -31,7 +31,7 @@ */ #include <sys/cdefs.h> -__FBSDID("$FreeBSD: src/sys/net80211/ieee80211_node.c,v 1.58 2005/07/27 02:53:09 sam Exp $"); +__FBSDID("$FreeBSD: src/sys/net80211/ieee80211_node.c,v 1.59 2005/07/31 06:12:32 sam Exp $"); #include <sys/param.h> #include <sys/systm.h> @@ -892,6 +892,11 @@ return ni; } +#define IS_CTL(wh) \ + ((wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK) == IEEE80211_FC0_TYPE_CTL) +#define IS_PSPOLL(wh) \ + ((wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK) == IEEE80211_FC0_SUBTYPE_PS_POLL) + /* * Locate the node for sender, track state, and then pass the * (referenced) node up to the 802.11 layer for its use. We @@ -909,10 +914,6 @@ const struct ieee80211_frame_min *wh) #endif { -#define IS_CTL(wh) \ - ((wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK) == IEEE80211_FC0_TYPE_CTL) -#define IS_PSPOLL(wh) \ - ((wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK) == IEEE80211_FC0_SUBTYPE_PS_POLL) struct ieee80211_node_table *nt; struct ieee80211_node *ni; @@ -924,12 +925,65 @@ ni = _ieee80211_find_node(nt, wh->i_addr1); else ni = _ieee80211_find_node(nt, wh->i_addr2); + if (ni == NULL) + ni = ieee80211_ref_node(ic->ic_bss); + IEEE80211_NODE_UNLOCK(nt); + + return ni; +} + +/* + * Like ieee80211_find_rxnode but use the supplied h/w + * key index as a hint to locate the node in the key + * mapping table. If an entry is present at the key + * index we return it; otherwise do a normal lookup and + * update the mapping table if the station has a unicast + * key assigned to it. + */ +struct ieee80211_node * +#ifdef IEEE80211_DEBUG_REFCNT +ieee80211_find_rxnode_withkey_debug(struct ieee80211com *ic, + const struct ieee80211_frame_min *wh, , u_int16_t keyix, + const char *func, int line) +#else +ieee80211_find_rxnode_withkey(struct ieee80211com *ic, + const struct ieee80211_frame_min *wh, u_int16_t keyix) +#endif +{ + struct ieee80211_node_table *nt; + struct ieee80211_node *ni; + + nt = &ic->ic_sta; + IEEE80211_NODE_LOCK(nt); + KASSERT(keyix == IEEE80211_KEYIX_NONE || keyix < 128, + ("keyix %u out of bounds (1)", keyix)); + ni = (keyix != IEEE80211_KEYIX_NONE) ? nt->nt_keyixmap[keyix] : NULL; + if (ni == NULL) { + if (IS_CTL(wh) && !IS_PSPOLL(wh) /*&& !IS_RTS(ah)*/) + ni = _ieee80211_find_node(nt, wh->i_addr1); + else + ni = _ieee80211_find_node(nt, wh->i_addr2); + if (ni == NULL) + ni = ieee80211_ref_node(ic->ic_bss); + /* + * If the station has a unicast key cache slot + * assigned update the key->node mapping table. + */ + keyix = ni->ni_ucastkey.wk_keyix; + KASSERT(keyix == IEEE80211_KEYIX_NONE || keyix < 128, + ("keyix %u out of bounds (2)", keyix)); + /* XXX can keyixmap[keyix] != NULL? */ + if (keyix != IEEE80211_KEYIX_NONE && + nt->nt_keyixmap[keyix] == NULL) + nt->nt_keyixmap[keyix] = ieee80211_ref_node(ni); + } else + ieee80211_ref_node(ni); IEEE80211_NODE_UNLOCK(nt); - return (ni != NULL ? ni : ieee80211_ref_node(ic->ic_bss)); + return ni; +} #undef IS_PSPOLL #undef IS_CTL -} /* * Return a reference to the appropriate node for sending @@ -948,15 +1002,16 @@ /* * The destination address should be in the node table - * unless we are operating in station mode or this is a - * multicast/broadcast frame. + * unless this is a multicast/broadcast frame. We can + * also ptimize station mode operation, all frames go + * to the bss node. */ - if (ic->ic_opmode == IEEE80211_M_STA || IEEE80211_IS_MULTICAST(macaddr)) - return ieee80211_ref_node(ic->ic_bss); - /* XXX can't hold lock across dup_bss 'cuz of recursive locking */ IEEE80211_NODE_LOCK(nt); - ni = _ieee80211_find_node(nt, macaddr); + if (ic->ic_opmode == IEEE80211_M_STA || IEEE80211_IS_MULTICAST(macaddr)) + ni = ieee80211_ref_node(ic->ic_bss); + else + ni = _ieee80211_find_node(nt, macaddr); IEEE80211_NODE_UNLOCK(nt); if (ni == NULL) { @@ -1068,23 +1123,35 @@ "%s (%s:%u) %p<%s> refcnt %d\n", __func__, func, line, ni, ether_sprintf(ni->ni_macaddr), ieee80211_node_refcnt(ni)-1); #endif - if (ieee80211_node_dectestref(ni)) { - /* - * Beware; if the node is marked gone then it's already - * been removed from the table and we cannot assume the - * table still exists. Regardless, there's no need to lock - * the table. - */ - if (ni->ni_table != NULL) { - IEEE80211_NODE_LOCK(nt); + if (nt != NULL) { + IEEE80211_NODE_LOCK(nt); + if (ieee80211_node_dectestref(ni)) _ieee80211_free_node(ni); - IEEE80211_NODE_UNLOCK(nt); - } else - _ieee80211_free_node(ni); + IEEE80211_NODE_UNLOCK(nt); + } else { + _ieee80211_free_node(ni); } } /* + * Callback from the crypto code when a unicast h/w key + * is deleted; clear any mapping table entry for the key. + */ +void +ieee80211_node_delkey(struct ieee80211_node_table *nt, u_int16_t keyix) +{ + struct ieee80211_node *ni; + + IEEE80211_NODE_LOCK(nt); + ni = nt->nt_keyixmap[keyix]; + nt->nt_keyixmap[keyix] = NULL;; + IEEE80211_NODE_UNLOCK(nt); + + if (ni != NULL) + ieee80211_free_node(ni); +} + +/* * Reclaim a node. If this is the last reference count then * do the normal free work. Otherwise remove it from the node * table and mark it gone by clearing the back-reference. @@ -1092,11 +1159,26 @@ static void node_reclaim(struct ieee80211_node_table *nt, struct ieee80211_node *ni) { + u_int16_t keyix; + IEEE80211_NODE_LOCK_ASSERT(nt); + IEEE80211_DPRINTF(ni->ni_ic, IEEE80211_MSG_NODE, "%s: remove %p<%s> from %s table, refcnt %d\n", __func__, ni, ether_sprintf(ni->ni_macaddr), nt->nt_name, ieee80211_node_refcnt(ni)-1); + /* + * Clear any entry in the unicast key mapping table. + * We need to do it here so rx lookups don't find it + * in the mapping table even if it's not in the hash + * table. We cannot depend on the mapping table entry + * being cleared because the node may not be free'd. + */ + keyix = ni->ni_ucastkey.wk_keyix; + if (keyix != IEEE80211_KEYIX_NONE && nt->nt_keyixmap[keyix] == ni) { + nt->nt_keyixmap[keyix] = NULL; + ieee80211_node_decref(ni); + } if (!ieee80211_node_dectestref(ni)) { /* * Other references are present, just remove the @@ -1208,6 +1290,14 @@ IEEE80211_MSG_INACT | IEEE80211_MSG_NODE, ni, "%s", "probe station due to inactivity"); + /* + * Grab a reference before unlocking the table + * so the node cannot be reclaimed before we + * send the frame. ieee80211_send_nulldata + * understands we've done this and reclaims the + * ref for us as needed. + */ + ieee80211_ref_node(ni); IEEE80211_NODE_UNLOCK(nt); ieee80211_send_nulldata(ni); /* XXX stat? */ ==== //depot/projects/wifi/sys/net80211/ieee80211_node.h#27 (text+ko) ==== @@ -210,6 +210,7 @@ ieee80211_node_lock_t nt_nodelock; /* on node table */ TAILQ_HEAD(, ieee80211_node) nt_node; /* information of all nodes */ LIST_HEAD(, ieee80211_node) nt_hash[IEEE80211_NODE_HASHSIZE]; + struct ieee80211_node *nt_keyixmap[128];/* key ix -> node map */ const char *nt_name; /* for debugging */ ieee80211_scan_lock_t nt_scanlock; /* on nt_scangen */ u_int nt_scangen; /* gen# for timeout scan */ @@ -224,14 +225,14 @@ #ifdef IEEE80211_DEBUG_REFCNT void ieee80211_free_node_debug(struct ieee80211_node *, const char *func, int line); -struct ieee80211_node *ieee80211_find_node_debug( - struct ieee80211_node_table *, const u_int8_t *, +struct ieee80211_node *ieee80211_find_node_debug(struct ieee80211_node_table *, + const u_int8_t *, const char *func, int line); -struct ieee80211_node * ieee80211_find_rxnode_debug( - struct ieee80211com *, const struct ieee80211_frame_min *, +struct ieee80211_node * ieee80211_find_rxnode_debug(struct ieee80211com *, + const struct ieee80211_frame_min *, const char *func, int line); -struct ieee80211_node *ieee80211_find_txnode_debug( - struct ieee80211com *, const u_int8_t *, +struct ieee80211_node *ieee80211_find_txnode_debug(struct ieee80211com *, + const u_int8_t *, const char *func, int line); struct ieee80211_node *ieee80211_find_node_with_ssid_debug( struct ieee80211_node_table *, const u_int8_t *macaddr, @@ -249,16 +250,19 @@ ieee80211_find_node_with_ssid_debug(nt, mac, sl, ss, __func__, __LINE__) #else void ieee80211_free_node(struct ieee80211_node *); -struct ieee80211_node *ieee80211_find_node( - struct ieee80211_node_table *, const u_int8_t *); -struct ieee80211_node * ieee80211_find_rxnode( - struct ieee80211com *, const struct ieee80211_frame_min *); -struct ieee80211_node *ieee80211_find_txnode( - struct ieee80211com *, const u_int8_t *); +struct ieee80211_node *ieee80211_find_node(struct ieee80211_node_table *, + const u_int8_t *); +struct ieee80211_node * ieee80211_find_rxnode(struct ieee80211com *, + const struct ieee80211_frame_min *); +struct ieee80211_node * ieee80211_find_rxnode_withkey(struct ieee80211com *, + const struct ieee80211_frame_min *, u_int16_t keyix); +struct ieee80211_node *ieee80211_find_txnode(struct ieee80211com *, + const u_int8_t *); struct ieee80211_node *ieee80211_find_node_with_ssid( struct ieee80211_node_table *, const u_int8_t *macaddr, u_int ssidlen, const u_int8_t *ssid); #endif +void ieee80211_node_delkey(struct ieee80211_node_table *, u_int16_t keyix); void ieee80211_node_timeout(void *arg); typedef void ieee80211_iter_func(void *, struct ieee80211_node *);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200508011756.j71Hunne012782>