Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 14 Aug 2003 09:14:07 -0700 (PDT)
From:      Sam Leffler <sam@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 36099 for review
Message-ID:  <200308141614.h7EGE7kd056129@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=36099

Change 36099 by sam@sam_ebb on 2003/08/14 09:13:53

	Revamp output path for management frames to eliminate redundant
	locking that causes problems and to correct reference counting
	bogosity that occurs when stations are timed out due to inactivity
	(in AP mode).  On output the refcnt'd node is stashed in the
	pkthdr's recvif field (yech) and retrieved by the driver.  This
	eliminates an unref/ref scenario and related node table unlock/lock
	due to the driver looking up the node.  This is important
	particularly for when stations are timed out as this causes a lock
	order reversal that can result in a deadlock.  As a byproduct we
	also reduce the overhead for sending management frames (minimal).
	Additional fallout from this is a change to ieee80211_encap to
	return a refcn't node for tieing to the outbound frame.  Node
	refcnts are not reclaimed until after a frame is completely
	processed (e.g. in the tx interrupt handler).  This is especially
	important for timedout stations as this deref will be the final
	one causing the node entry to be reclaimed.
	
	Additional semi-related changes:
	o replace m_copym use with m_copypacket (optimization)
	o add assert to verify ic_bss is never free'd during normal operation
	o add comments explaining calling conventions by drivers for frames
	  going in each direction
	o remove extraneous code that "cannot be executed" (e.g. because
	  pointers may never be null)

Affected files ...

.. //depot/projects/netperf/sys/dev/ath/if_ath.c#6 edit
.. //depot/projects/netperf/sys/dev/wi/if_wi.c#3 edit
.. //depot/projects/netperf/sys/net80211/ieee80211_input.c#3 edit
.. //depot/projects/netperf/sys/net80211/ieee80211_node.c#4 edit
.. //depot/projects/netperf/sys/net80211/ieee80211_output.c#2 edit
.. //depot/projects/netperf/sys/net80211/ieee80211_proto.c#2 edit
.. //depot/projects/netperf/sys/net80211/ieee80211_proto.h#3 edit

Differences ...

==== //depot/projects/netperf/sys/dev/ath/if_ath.c#6 (text+ko) ====

@@ -706,7 +706,7 @@
 			/*
 			 * Encapsulate the packet in prep for transmission.
 			 */
-			m = ieee80211_encap(ifp, m);
+			m = ieee80211_encap(ifp, m, &ni);
 			if (m == NULL) {
 				DPRINTF(("ath_start: encapsulation failure\n"));
 				sc->sc_stats.ast_tx_encap++;
@@ -716,6 +716,18 @@
 			if (ic->ic_flags & IEEE80211_F_WEPON)
 				wh->i_fc[1] |= IEEE80211_FC1_WEP;
 		} else {
+			/*
+			 * Hack!  The referenced node pointer is in the
+			 * rcvif field of the packet header.  This is
+			 * placed there by ieee80211_mgmt_output because
+			 * we need to hold the reference with the frame
+			 * and there's no other way (other than packet
+			 * tags which we consider too expensive to use)
+			 * to pass it along.
+			 */
+			ni = (struct ieee80211_node *) m->m_pkthdr.rcvif;
+			m->m_pkthdr.rcvif = NULL;
+
 			wh = mtod(m, struct ieee80211_frame *);
 			if ((wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK) ==
 			    IEEE80211_FC0_SUBTYPE_PROBE_RESP) {
@@ -735,26 +747,6 @@
 		if (ic->ic_rawbpf)
 			bpf_mtap(ic->ic_rawbpf, m);
 
-		if (ic->ic_opmode != IEEE80211_M_STA) {
-			ni = ieee80211_find_node(ic, wh->i_addr1);
-			if (ni == NULL) {
-				/*
-				 * When not in station mode the destination
-				 * address should always be in the node table
-				 * unless this is a multicast/broadcast frame.
-				 */
-				if (!IEEE80211_IS_MULTICAST(wh->i_addr1) &&
-				    (wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK) ==
-				    IEEE80211_FC0_TYPE_DATA) {
-					m_freem(m);
-					sc->sc_stats.ast_tx_nonode++;
-					goto bad;
-				}
-				ni = ic->ic_bss;
-			}
-		} else
-			ni = ic->ic_bss;
-
 		if (sc->sc_drvbpf) {
 			struct mbuf *mb;
 
@@ -785,6 +777,8 @@
 			TAILQ_INSERT_TAIL(&sc->sc_txbuf, bf, bf_list);
 			mtx_unlock(&sc->sc_txbuflock);
 			ifp->if_oerrors++;
+			if (ni && ni != ic->ic_bss)
+				ieee80211_free_node(ic, ni);
 			continue;
 		}
 
@@ -1745,7 +1739,7 @@
 	DPRINTF2(("ath_tx_start: m %p len %u\n", m0, pktlen));
 	bus_dmamap_sync(sc->sc_dmat, bf->bf_dmamap, BUS_DMASYNC_PREWRITE);
 	bf->bf_m = m0;
-	bf->bf_node = ni;
+	bf->bf_node = ni;			/* NB: held reference */
 
 	/* setup descriptors */
 	ds = bf->bf_desc;
@@ -1929,7 +1923,8 @@
 	struct ath_softc *sc = arg;
 	struct ath_hal *ah = sc->sc_ah;
 	struct ath_buf *bf;
-	struct ifnet *ifp = &sc->sc_ic.ic_if;
+	struct ieee80211com *ic = &sc->sc_ic;
+	struct ifnet *ifp = &ic->ic_if;
 	struct ath_desc *ds;
 	struct ieee80211_node *ni;
 	struct ath_node *an;
@@ -1984,6 +1979,15 @@
 			sc->sc_stats.ast_tx_longretry += lr;
 			if (sr + lr)
 				an->an_tx_retr++;
+			/*
+			 * Reclaim reference to node.
+			 *
+			 * NB: the node may be reclaimed here if, for example
+			 *     this is a DEAUTH message that was sent and the
+			 *     node was timed out due to inactivity.
+			 */
+			if (ni != ic->ic_bss)
+				ieee80211_free_node(ic, ni);
 		}
 		bus_dmamap_sync(sc->sc_dmat, bf->bf_dmamap,
 		    BUS_DMASYNC_POSTWRITE);

==== //depot/projects/netperf/sys/dev/wi/if_wi.c#3 (text+ko) ====

@@ -834,10 +834,11 @@
 {
 	struct wi_softc	*sc = ifp->if_softc;
 	struct ieee80211com *ic = &sc->sc_ic;
+	struct ieee80211_node *ni;
 	struct ieee80211_frame *wh;
 	struct mbuf *m0;
 	struct wi_frame frmhdr;
-	int cur, fid, off;
+	int cur, fid, off, error;
 	WI_LOCK_DECL();
 
 	WI_LOCK(sc);
@@ -861,6 +862,18 @@
 				break;
 			}
 			IF_DEQUEUE(&ic->ic_mgtq, m0);
+			/*
+			 * Hack!  The referenced node pointer is in the
+			 * rcvif field of the packet header.  This is
+			 * placed there by ieee80211_mgmt_output because
+			 * we need to hold the reference with the frame
+			 * and there's no other way (other than packet
+			 * tags which we consider too expensive to use)
+			 * to pass it along.
+			 */
+			ni = (struct ieee80211_node *) m0->m_pkthdr.rcvif;
+			m0->m_pkthdr.rcvif = NULL;
+
 			m_copydata(m0, 4, ETHER_ADDR_LEN * 2,
 			    (caddr_t)&frmhdr.wi_ehdr);
 			frmhdr.wi_ehdr.ether_type = 0;
@@ -883,26 +896,12 @@
 			BPF_MTAP(ifp, m0);
 #endif
 
-			if ((m0 = ieee80211_encap(ifp, m0)) == NULL) {
+			m0 = ieee80211_encap(ifp, m0, &ni);
+			if (m0 == NULL) {
 				ifp->if_oerrors++;
 				continue;
 			}
                         wh = mtod(m0, struct ieee80211_frame *);
-			if (ic->ic_opmode == IEEE80211_M_HOSTAP &&
-			    !IEEE80211_IS_MULTICAST(wh->i_addr1) &&
-			    (wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK) ==
-			    IEEE80211_FC0_TYPE_DATA) {
-				struct ieee80211_node *ni =
-					ieee80211_find_node(ic, wh->i_addr1);
-			        int err = (ni == NULL || ni->ni_associd == 0);
-				if (ni != NULL)
-					ieee80211_unref_node(&ni);
-			        if (err) {
-					m_freem(m0);
-					ifp->if_oerrors++;
-					continue;
-				}
-			}
 			if (ic->ic_flags & IEEE80211_F_WEPON)
 				wh->i_fc[1] |= IEEE80211_FC1_WEP;
 
@@ -916,6 +915,8 @@
 		    (wh->i_fc[1] & IEEE80211_FC1_WEP)) {
 			if ((m0 = ieee80211_wep_crypt(ifp, m0, 1)) == NULL) {
 				ifp->if_oerrors++;
+				if (ni && ni != ic->ic_bss)
+					ieee80211_free_node(ic, ni);
 				continue;
 			}
 			frmhdr.wi_tx_ctl |= htole16(WI_TXCNTL_NOCRYPT);
@@ -943,13 +944,15 @@
 			wi_dump_pkt(&frmhdr, NULL, -1);
 		fid = sc->sc_txd[cur].d_fid;
 		off = sizeof(frmhdr);
-		if (wi_write_bap(sc, fid, 0, &frmhdr, sizeof(frmhdr)) != 0 ||
-		    wi_mwrite_bap(sc, fid, off, m0, m0->m_pkthdr.len) != 0) {
+		error = wi_write_bap(sc, fid, 0, &frmhdr, sizeof(frmhdr)) != 0
+		     || wi_mwrite_bap(sc, fid, off, m0, m0->m_pkthdr.len) != 0;
+		m_freem(m0);
+		if (ni && ni != ic->ic_bss)
+			ieee80211_free_node(ic, ni);
+		if (error) {
 			ifp->if_oerrors++;
-			m_freem(m0);
 			continue;
 		}
-		m_freem(m0);
 		sc->sc_txd[cur].d_len = off;
 		if (sc->sc_txcur == cur) {
 			if (wi_cmd(sc, WI_CMD_TX | WI_RECLAIM, fid, 0, 0)) {

==== //depot/projects/netperf/sys/net80211/ieee80211_input.c#3 (text+ko) ====

@@ -233,7 +233,7 @@
 		if (ic->ic_opmode == IEEE80211_M_HOSTAP) {
 			eh = mtod(m, struct ether_header *);
 			if (ETHER_IS_MULTICAST(eh->ether_dhost)) {
-				m1 = m_copym(m, 0, M_COPYALL, M_DONTWAIT);
+				m1 = m_copypacket(m, M_DONTWAIT);
 				if (m1 == NULL)
 					ifp->if_oerrors++;
 				else
@@ -741,10 +741,11 @@
 			IEEE80211_DPRINTF(("%s: rate negotiation failed: %s\n",
 				__func__,ether_sprintf(wh->i_addr2)));
 		} else {
-			IEEE80211_SEND_MGMT(ic, ni, IEEE80211_FC0_SUBTYPE_PROBE_RESP,
-			    0);
+			IEEE80211_SEND_MGMT(ic, ni,
+				IEEE80211_FC0_SUBTYPE_PROBE_RESP, 0);
 		}
 		if (allocbs) {
+			/* XXX just use free? */
 			if (ic->ic_opmode == IEEE80211_M_HOSTAP)
 				ieee80211_free_node(ic, ni);
 			else
@@ -798,13 +799,12 @@
 				allocbs = 1;
 			} else
 				allocbs = 0;
-			IEEE80211_SEND_MGMT(ic, ni, IEEE80211_FC0_SUBTYPE_AUTH, 2);
+			IEEE80211_SEND_MGMT(ic, ni,
+				IEEE80211_FC0_SUBTYPE_AUTH, 2);
 			if (ifp->if_flags & IFF_DEBUG)
 				if_printf(ifp, "station %s %s authenticated\n",
 				    (allocbs ? "newly" : "already"),
 				    ether_sprintf(ni->ni_macaddr));
-			if (allocbs)
-				ieee80211_unref_node(&ni);
 			break;
 
 		case IEEE80211_M_STA:
@@ -912,7 +912,8 @@
 			IEEE80211_DPRINTF(("%s: capability mismatch %x for %s\n",
 				__func__, capinfo, ether_sprintf(wh->i_addr2)));
 			ni->ni_associd = 0;
-			IEEE80211_SEND_MGMT(ic, ni, resp, IEEE80211_STATUS_CAPINFO);
+			IEEE80211_SEND_MGMT(ic, ni, resp,
+				IEEE80211_STATUS_CAPINFO);
 			return;
 		}
 		ieee80211_setup_rates(ic, ni, rates, xrates,
@@ -922,7 +923,8 @@
 			IEEE80211_DPRINTF(("%s: rate unmatch for %s\n",
 				__func__, ether_sprintf(wh->i_addr2)));
 			ni->ni_associd = 0;
-			IEEE80211_SEND_MGMT(ic, ni, resp, IEEE80211_STATUS_BASIC_RATE);
+			IEEE80211_SEND_MGMT(ic, ni, resp,
+				IEEE80211_STATUS_BASIC_RATE);
 			return;
 		}
 		ni->ni_rssi = rssi;

==== //depot/projects/netperf/sys/net80211/ieee80211_node.c#4 (text+ko) ====

@@ -507,6 +507,8 @@
 void
 ieee80211_free_node(struct ieee80211com *ic, struct ieee80211_node *ni)
 {
+	KASSERT(ni != ic->ic_bss, ("freeing ic_bss"));
+
 	/* XXX need equivalent of atomic_dec_and_test */
 	atomic_subtract_int(&ni->ni_refcnt, 1);
 	if (atomic_cmpset_int(&ni->ni_refcnt, 0, 1)) {
@@ -534,21 +536,22 @@
 
 	mtx_lock(&ic->ic_nodelock);
 	for (ni = TAILQ_FIRST(&ic->ic_node); ni != NULL;) {
-		if (++ni->ni_inact <= IEEE80211_INACT_MAX) {
-			ni = TAILQ_NEXT(ni, ni_list);
-			continue;
-		}
-		/* NB: don't honor reference count */
-		IEEE80211_DPRINTF(("station %s timed out "
+		if (++ni->ni_inact > IEEE80211_INACT_MAX) {
+			IEEE80211_DPRINTF(("station %s timed out "
 			    "due to inactivity (%u secs)\n",
 			    ether_sprintf(ni->ni_macaddr),
 			    ni->ni_inact));
-		nextbs = TAILQ_NEXT(ni, ni_list);
-		IEEE80211_SEND_MGMT(ic, ni,
-		    IEEE80211_FC0_SUBTYPE_DEAUTH,
-		    IEEE80211_REASON_AUTH_EXPIRE);
-		_ieee80211_free_node(ic, ni);
-		ni = nextbs;
+			nextbs = TAILQ_NEXT(ni, ni_list);
+			/*
+			 * Send a deauthenticate frame.
+			 */
+			IEEE80211_SEND_MGMT(ic, ni,
+			    IEEE80211_FC0_SUBTYPE_DEAUTH,
+			    IEEE80211_REASON_AUTH_EXPIRE);
+			ieee80211_free_node(ic, ni);
+			ni = nextbs;
+		} else
+			ni = TAILQ_NEXT(ni, ni_list);
 	}
 	if (!TAILQ_EMPTY(&ic->ic_node))
 		ic->ic_inact_timer = IEEE80211_INACT_WAIT;

==== //depot/projects/netperf/sys/net80211/ieee80211_output.c#2 (text+ko) ====

@@ -66,20 +66,41 @@
 #include <netinet/if_ether.h>
 #endif
 
-int
+/*
+ * Send a management frame to the specified node.  The node pointer
+ * must have a reference as the pointer will be passed to the driver
+ * and potentially held for a long time.  If the frame is successfully
+ * dispatched to the driver, then it is responsible for freeing the
+ * reference (and potentially free'ing up any associated storage).
+ */
+static int
 ieee80211_mgmt_output(struct ifnet *ifp, struct ieee80211_node *ni,
     struct mbuf *m, int type)
 {
 	struct ieee80211com *ic = (void *)ifp;
 	struct ieee80211_frame *wh;
 
-	/* XXX this probably shouldn't be permitted */
-	KASSERT(ni != NULL, ("%s: null node", __func__));
+	KASSERT(ni != NULL, ("null node"));
 	ni->ni_inact = 0;
 
+	/*
+	 * Yech, hack alert!  We want to pass the node down to the
+	 * driver's start routine.  If we don't do so then the start
+	 * routine must immediately look it up again and that can
+	 * cause a lock order reversal if, for example, this frame
+	 * is being sent because the station is being timedout and
+	 * the frame being sent is a DEAUTH message.  We could stick
+	 * this in an m_tag and tack that on to the mbuf.  However
+	 * that's rather expensive to do for every frame so instead
+	 * we stuff it in the rcvif field since outbound frames do
+	 * not (presently) use this.
+	 */
 	M_PREPEND(m, sizeof(struct ieee80211_frame), M_DONTWAIT);
 	if (m == NULL)
 		return ENOMEM;
+	KASSERT(m->m_pkthdr.rcvif == NULL, ("rcvif not null"));
+	m->m_pkthdr.rcvif = (void *)ni;
+
 	wh = mtod(m, struct ieee80211_frame *);
 	wh->i_fc[0] = IEEE80211_FC0_VERSION_0 | IEEE80211_FC0_TYPE_MGT | type;
 	wh->i_fc[1] = IEEE80211_FC1_DIR_NODS;
@@ -106,31 +127,57 @@
 			    ether_sprintf(ni->ni_macaddr),
 			    ieee80211_chan2ieee(ic, ni->ni_chan));
 	}
+
 	IF_ENQUEUE(&ic->ic_mgtq, m);
 	ifp->if_timer = 1;
 	(*ifp->if_start)(ifp);
 	return 0;
 }
 
+/*
+ * Encapsulate an outbound data frame.  The mbuf chain is updated and
+ * a reference to the destination node is returned.  If an error is
+ * encountered NULL is returned and the node reference will also be NULL.
+ * 
+ * NB: The caller is responsible for free'ing a returned node reference.
+ *     The convention is ic_bss is not reference counted; the caller must
+ *     maintain that.
+ */
 struct mbuf *
-ieee80211_encap(struct ifnet *ifp, struct mbuf *m)
+ieee80211_encap(struct ifnet *ifp, struct mbuf *m, struct ieee80211_node **pni)
 {
 	struct ieee80211com *ic = (void *)ifp;
 	struct ether_header eh;
 	struct ieee80211_frame *wh;
+	struct ieee80211_node *ni = NULL;
 	struct llc *llc;
-	struct ieee80211_node *ni;
 
 	if (m->m_len < sizeof(struct ether_header)) {
 		m = m_pullup(m, sizeof(struct ether_header));
-		if (m == NULL)
-			return NULL;
+		if (m == NULL) {
+			/* XXX statistic */
+			goto bad;
+		}
 	}
 	memcpy(&eh, mtod(m, caddr_t), sizeof(struct ether_header));
 
-	ni = ieee80211_find_node(ic, eh.ether_dhost);
-	if (ni == NULL)			/*ic_opmode?? XXX*/
-		ni = ieee80211_ref_node(ic->ic_bss);
+	if (ic->ic_opmode != IEEE80211_M_STA) {
+		ni = ieee80211_find_node(ic, eh.ether_dhost);
+		if (ni == NULL) {
+			/*
+			 * When not in station mode the
+			 * destination address should always be
+			 * in the node table unless this is a
+			 * multicast/broadcast frame.
+			 */
+			if (!IEEE80211_IS_MULTICAST(eh.ether_dhost)) {
+				/* ic->ic_stats.st_tx_nonode++; XXX statistic */
+				goto bad;
+			}
+			ni = ic->ic_bss;
+		}
+	} else
+		ni = ic->ic_bss;
 	ni->ni_inact = 0;
 
 	m_adj(m, sizeof(struct ether_header) - sizeof(struct llc));
@@ -142,10 +189,8 @@
 	llc->llc_snap.org_code[2] = 0;
 	llc->llc_snap.ether_type = eh.ether_type;
 	M_PREPEND(m, sizeof(struct ieee80211_frame), M_DONTWAIT);
-	if (m == NULL) {
-		ieee80211_unref_node(&ni);
-		return NULL;
-	}
+	if (m == NULL)
+		goto bad;
 	wh = mtod(m, struct ieee80211_frame *);
 	wh->i_fc[0] = IEEE80211_FC0_VERSION_0 | IEEE80211_FC0_TYPE_DATA;
 	*(u_int16_t *)wh->i_dur = 0;
@@ -173,11 +218,17 @@
 		IEEE80211_ADDR_COPY(wh->i_addr3, eh.ether_shost);
 		break;
 	case IEEE80211_M_MONITOR:
-		m_freem(m), m = NULL;
-		break;
+		goto bad;
 	}
-	ieee80211_unref_node(&ni);
+	*pni = ni;
 	return m;
+bad:
+	if (m != NULL)
+		m_freem(m);
+	if (ni && ni != ic->ic_bss)
+		ieee80211_free_node(ic, ni);
+	*pni = NULL;
+	return NULL;
 }
 
 /*
@@ -240,10 +291,16 @@
 	return m;
 }
 
+/*
+ * Send a management frame.  The node is for the destination (or ic_bss
+ * when in station mode).  Nodes other than ic_bss have their reference
+ * count bumped to reflect our use for an indeterminant time.
+ */
 int
 ieee80211_send_mgmt(struct ieee80211com *ic, struct ieee80211_node *ni,
 	int type, int arg)
 {
+#define	senderr(_x)	do { ret = _x; goto bad; } while (0)
 	struct ifnet *ifp = &ic->ic_if;
 	struct mbuf *m;
 	u_int8_t *frm;
@@ -251,6 +308,15 @@
 	u_int16_t capinfo;
 	int ret, timer;
 
+	KASSERT(ni != NULL, ("null node"));
+
+	/*
+	 * Hold a reference on the node so it doesn't go away until after
+	 * the xmit is complete all the way in the driver.  On error we
+	 * will remove our reference.
+	 */
+	if (ni != ic->ic_bss)
+		ieee80211_ref_node(ni);
 	timer = 0;
 	switch (type) {
 	case IEEE80211_FC0_SUBTYPE_PROBE_REQ:
@@ -265,7 +331,7 @@
 		       + 2 + IEEE80211_RATE_SIZE
 		       + 2 + (IEEE80211_RATE_MAXSIZE - IEEE80211_RATE_SIZE));
 		if (m == NULL)
-			return ENOMEM;
+			senderr(ENOMEM);
 		m->m_data += sizeof(struct ieee80211_frame);
 		frm = mtod(m, u_int8_t *);
 		frm = ieee80211_add_ssid(frm, ic->ic_des_essid, ic->ic_des_esslen);
@@ -295,7 +361,7 @@
 		       + 6
 		       + 2 + (IEEE80211_RATE_MAXSIZE - IEEE80211_RATE_SIZE));
 		if (m == NULL)
-			return ENOMEM;
+			senderr(ENOMEM);
 		m->m_data += sizeof(struct ieee80211_frame);
 		frm = mtod(m, u_int8_t *);
 
@@ -336,7 +402,7 @@
 	case IEEE80211_FC0_SUBTYPE_AUTH:
 		MGETHDR(m, M_DONTWAIT, MT_DATA);
 		if (m == NULL)
-			return ENOMEM;
+			senderr(ENOMEM);
 		MH_ALIGN(m, 2 * 3);
 		m->m_pkthdr.len = m->m_len = 6;
 		frm = mtod(m, u_int8_t *);
@@ -354,7 +420,7 @@
 			    ether_sprintf(ni->ni_macaddr), arg);
 		MGETHDR(m, M_DONTWAIT, MT_DATA);
 		if (m == NULL)
-			return ENOMEM;
+			senderr(ENOMEM);
 		MH_ALIGN(m, 2);
 		m->m_pkthdr.len = m->m_len = 2;
 		*mtod(m, u_int16_t *) = htole16(arg);	/* reason */
@@ -379,7 +445,7 @@
 		       + 2 + IEEE80211_RATE_SIZE
 		       + 2 + (IEEE80211_RATE_MAXSIZE - IEEE80211_RATE_SIZE));
 		if (m == NULL)
-			return ENOMEM;
+			senderr(ENOMEM);
 		m->m_data += sizeof(struct ieee80211_frame);
 		frm = mtod(m, u_int8_t *);
 
@@ -430,7 +496,7 @@
 		       + 2 + IEEE80211_RATE_SIZE
 		       + 2 + (IEEE80211_RATE_MAXSIZE - IEEE80211_RATE_SIZE));
 		if (m == NULL)
-			return ENOMEM;
+			senderr(ENOMEM);
 		m->m_data += sizeof(struct ieee80211_frame);
 		frm = mtod(m, u_int8_t *);
 
@@ -443,19 +509,12 @@
 		*(u_int16_t *)frm = htole16(arg);	/* status */
 		frm += 2;
 
-		if (arg == IEEE80211_STATUS_SUCCESS && ni != NULL)
+		if (arg == IEEE80211_STATUS_SUCCESS)
 			*(u_int16_t *)frm = htole16(ni->ni_associd);
-		else
-			*(u_int16_t *)frm = htole16(0);
 		frm += 2;
 
-		if (ni != NULL) {
-			frm = ieee80211_add_rates(frm, &ni->ni_rates);
-			frm = ieee80211_add_xrates(frm, &ni->ni_rates);
-		} else {
-			frm = ieee80211_add_rates(frm, &ic->ic_bss->ni_rates);
-			frm = ieee80211_add_xrates(frm, &ic->ic_bss->ni_rates);
-		}
+		frm = ieee80211_add_rates(frm, &ni->ni_rates);
+		frm = ieee80211_add_xrates(frm, &ni->ni_rates);
 		m->m_pkthdr.len = m->m_len = frm - mtod(m, u_int8_t *);
 		break;
 
@@ -465,7 +524,7 @@
 			    ether_sprintf(ni->ni_macaddr), arg);
 		MGETHDR(m, M_DONTWAIT, MT_DATA);
 		if (m == NULL)
-			return ENOMEM;
+			senderr(ENOMEM);
 		MH_ALIGN(m, 2);
 		m->m_pkthdr.len = m->m_len = 2;
 		*mtod(m, u_int16_t *) = htole16(arg);	/* reason */
@@ -474,11 +533,19 @@
 	default:
 		IEEE80211_DPRINTF(("%s: invalid mgmt frame type %u\n",
 			__func__, type));
-		return EINVAL;
+		senderr(EINVAL);
+		/* NOTREACHED */
 	}
 
 	ret = ieee80211_mgmt_output(ifp, ni, m, type);
-	if (ret == 0 && timer)
-		ic->ic_mgt_timer = timer;
+	if (ret == 0) {
+		if (timer)
+			ic->ic_mgt_timer = timer;
+	} else {
+bad:
+		if (ni != ic->ic_bss)		/* remove ref we added */
+			ieee80211_free_node(ic, ni);
+	}
 	return ret;
+#undef senderr
 }

==== //depot/projects/netperf/sys/net80211/ieee80211_proto.c#2 (text+ko) ====


==== //depot/projects/netperf/sys/net80211/ieee80211_proto.h#3 (text+ko) ====

@@ -62,9 +62,8 @@
 		struct ieee80211_node *, int, int, u_int32_t);
 extern	int ieee80211_send_mgmt(struct ieee80211com *, struct ieee80211_node *,
 		int, int);
-extern	int ieee80211_mgmt_output(struct ifnet *, struct ieee80211_node *,
-		struct mbuf *, int);
-extern	struct mbuf *ieee80211_encap(struct ifnet *, struct mbuf *);
+extern	struct mbuf *ieee80211_encap(struct ifnet *, struct mbuf *,
+		struct ieee80211_node **);
 extern	struct mbuf *ieee80211_decap(struct ifnet *, struct mbuf *);
 extern	u_int8_t *ieee80211_add_rates(u_int8_t *frm,
 		const struct ieee80211_rateset *);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200308141614.h7EGE7kd056129>