Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 4 May 2011 02:23:59 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r221418 - head/sys/net80211
Message-ID:  <201105040223.p442Nx1r026636@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Wed May  4 02:23:59 2011
New Revision: 221418
URL: http://svn.freebsd.org/changeset/base/221418

Log:
  Fix some corner cases in the net80211 sequence number retransmission
  handling.
  
  The current sequence number code does a few things incorrectly:
  
  * It didn't try eliminating duplications from HT nodes. I guess it's assumed
    that out of order / retransmission handling would be handled by the AMPDU RX
    routines. If a HT node isn't doing AMPDU RX, then retransmissions need to
    be eliminated. Since most of my debugging is based on this (as AMPDU TX
    software packet aggregation isn't yet handled), handle this corner case.
  
  * When a sequence number of 4095 was received, any subsequent sequence number
    is going to be (by definition) less than 4095. So if the following sequence
    number (0) doesn't initially occur and the retransmit is received, it's
    incorrectly eliminated by the IEEE80211_FC1_RETRY && SEQ_LEQ() check.
    Try to handle this better.
  
  This almost completely eliminates out of order TCP statistics showing up during
  iperf testing for the 11a, 11g and non-aggregate 11n AMPDU RX case. The only
  other packet loss conditions leading to this are due to baseband resets or
  heavy interference.

Modified:
  head/sys/net80211/ieee80211_adhoc.c
  head/sys/net80211/ieee80211_hostap.c
  head/sys/net80211/ieee80211_input.h
  head/sys/net80211/ieee80211_mesh.c
  head/sys/net80211/ieee80211_sta.c
  head/sys/net80211/ieee80211_wds.c

Modified: head/sys/net80211/ieee80211_adhoc.c
==============================================================================
--- head/sys/net80211/ieee80211_adhoc.c	Wed May  4 01:39:44 2011	(r221417)
+++ head/sys/net80211/ieee80211_adhoc.c	Wed May  4 02:23:59 2011	(r221418)
@@ -285,7 +285,6 @@ doprint(struct ieee80211vap *vap, int su
 static int
 adhoc_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
 {
-#define	SEQ_LEQ(a,b)	((int)((a)-(b)) <= 0)
 #define	HAS_SEQ(type)	((type & 0x4) == 0)
 	struct ieee80211vap *vap = ni->ni_vap;
 	struct ieee80211com *ic = ni->ni_ic;
@@ -412,9 +411,7 @@ adhoc_input(struct ieee80211_node *ni, s
 			    TID_TO_WME_AC(tid) >= WME_AC_VI)
 				ic->ic_wme.wme_hipri_traffic++;
 			rxseq = le16toh(*(uint16_t *)wh->i_seq);
-			if ((ni->ni_flags & IEEE80211_NODE_HT) == 0 &&
-			    (wh->i_fc[1] & IEEE80211_FC1_RETRY) &&
-			    SEQ_LEQ(rxseq, ni->ni_rxseqs[tid])) {
+			if (! ieee80211_check_rxseq(ni, wh)) {
 				/* duplicate, discard */
 				IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_INPUT,
 				    bssid, "duplicate",
@@ -660,7 +657,6 @@ out:
 		m_freem(m);
 	}
 	return type;
-#undef SEQ_LEQ
 }
 
 static int

Modified: head/sys/net80211/ieee80211_hostap.c
==============================================================================
--- head/sys/net80211/ieee80211_hostap.c	Wed May  4 01:39:44 2011	(r221417)
+++ head/sys/net80211/ieee80211_hostap.c	Wed May  4 02:23:59 2011	(r221418)
@@ -472,7 +472,6 @@ doprint(struct ieee80211vap *vap, int su
 static int
 hostap_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
 {
-#define	SEQ_LEQ(a,b)	((int)((a)-(b)) <= 0)
 #define	HAS_SEQ(type)	((type & 0x4) == 0)
 	struct ieee80211vap *vap = ni->ni_vap;
 	struct ieee80211com *ic = ni->ni_ic;
@@ -572,9 +571,7 @@ hostap_input(struct ieee80211_node *ni, 
 			    TID_TO_WME_AC(tid) >= WME_AC_VI)
 				ic->ic_wme.wme_hipri_traffic++;
 			rxseq = le16toh(*(uint16_t *)wh->i_seq);
-			if ((ni->ni_flags & IEEE80211_NODE_HT) == 0 &&
-			    (wh->i_fc[1] & IEEE80211_FC1_RETRY) &&
-			    SEQ_LEQ(rxseq, ni->ni_rxseqs[tid])) {
+			if (! ieee80211_check_rxseq(ni, wh)) {
 				/* duplicate, discard */
 				IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_INPUT,
 				    bssid, "duplicate",
@@ -914,7 +911,6 @@ out:
 		m_freem(m);
 	}
 	return type;
-#undef SEQ_LEQ
 }
 
 static void

Modified: head/sys/net80211/ieee80211_input.h
==============================================================================
--- head/sys/net80211/ieee80211_input.h	Wed May  4 01:39:44 2011	(r221417)
+++ head/sys/net80211/ieee80211_input.h	Wed May  4 02:23:59 2011	(r221418)
@@ -142,6 +142,104 @@ ishtinfooui(const uint8_t *frm)
 	return frm[1] > 3 && LE_READ_4(frm+2) == ((BCM_OUI_HTINFO<<24)|BCM_OUI);
 }
 
+#include <sys/endian.h>		/* For le16toh() */
+
+/*
+ * Check the current frame sequence number against the current TID
+ * state and return whether it's in sequence or should be dropped.
+ *
+ * Since out of order packet and duplicate packet eliminations should
+ * be done by the AMPDU RX code, this routine blindly accepts all
+ * frames from a HT station w/ a TID that is currently doing AMPDU-RX.
+ * HT stations without WME or where the TID is not doing AMPDU-RX
+ * are checked like non-HT stations.
+ *
+ * The routine only eliminates packets whose sequence/fragment
+ * match or are less than the last seen sequence/fragment number
+ * AND are retransmits It doesn't try to eliminate out of order packets.
+ *
+ * Since all frames after sequence number 4095 will be less than 4095
+ * (as the seqnum wraps), handle that special case so packets aren't
+ * incorrectly dropped - ie, if the next packet is sequence number 0
+ * but a retransmit since the initial packet didn't make it.
+ */
+static __inline int
+ieee80211_check_rxseq(struct ieee80211_node *ni, struct ieee80211_frame *wh)
+{
+#define	SEQ_LEQ(a,b)	((int)((a)-(b)) <= 0)
+#define	SEQ_EQ(a,b)	((int)((a)-(b)) == 0)
+#define	HAS_SEQ(type)	((type & 0x4) == 0)
+#define	SEQNO(a)	((a) >> IEEE80211_SEQ_SEQ_SHIFT)
+#define	FRAGNO(a)	((a) & IEEE80211_SEQ_FRAG_MASK)
+	uint16_t rxseq;
+	uint8_t type;
+	uint8_t tid;
+	struct ieee80211_rx_ampdu *rap;
+
+	rxseq = le16toh(*(uint16_t *)wh->i_seq);
+	type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK;
+
+	/* Types with no sequence number are always treated valid */
+	if (! HAS_SEQ(type))
+		return 1;
+
+	tid = ieee80211_gettid(wh);
+
+	/*
+	 * Only do the HT AMPDU check for WME stations; non-WME HT stations
+	 * shouldn't exist outside of debugging. We should at least
+	 * handle that.
+	 */
+	if (tid < WME_NUM_TID) {
+		rap = &ni->ni_rx_ampdu[tid];
+		/* HT nodes currently doing RX AMPDU are always valid */
+		if ((ni->ni_flags & IEEE80211_NODE_HT) &&
+		    (rap->rxa_flags & IEEE80211_AGGR_RUNNING))
+			return 1;
+	}
+
+	/*	
+	 * Otherwise, retries for packets below or equal to the last
+	 * seen sequence number should be dropped.
+	 */
+
+	/*
+	 * Treat frame seqnum 4095 as special due to boundary
+	 * wrapping conditions.
+	 */
+	if (SEQNO(ni->ni_rxseqs[tid]) == 4095) {
+		/*
+		 * Drop retransmits on seqnum 4095/current fragment for itself.
+		 */
+		if (SEQ_EQ(rxseq, ni->ni_rxseqs[tid]) &&
+		    (wh->i_fc[1] & IEEE80211_FC1_RETRY))
+			return 0;
+		/*
+		 * Treat any subsequent frame as fine if the last seen frame
+		 * is 4095 and it's not a retransmit for the same sequence
+		 * number. However, this doesn't capture incorrectly ordered
+	 	 * fragments w/ sequence number 4095. It shouldn't be seen
+		 * in practice, but see the comment above for further info.
+		 */
+		return 1;
+	}
+
+	/*
+	 * At this point we assume that retransmitted seq/frag numbers below
+	 * the current can simply be eliminated.
+	 */
+	if ((wh->i_fc[1] & IEEE80211_FC1_RETRY) &&
+	    SEQ_LEQ(rxseq, ni->ni_rxseqs[tid]))
+		return 0;
+
+	return 1;
+#undef	SEQ_LEQ
+#undef	SEQ_EQ
+#undef	HAS_SEQ
+#undef	SEQNO
+#undef	FRAGNO
+}
+
 void	ieee80211_deliver_data(struct ieee80211vap *,
 		struct ieee80211_node *, struct mbuf *);
 struct mbuf *ieee80211_defrag(struct ieee80211_node *,

Modified: head/sys/net80211/ieee80211_mesh.c
==============================================================================
--- head/sys/net80211/ieee80211_mesh.c	Wed May  4 01:39:44 2011	(r221417)
+++ head/sys/net80211/ieee80211_mesh.c	Wed May  4 02:23:59 2011	(r221418)
@@ -1040,7 +1040,6 @@ mesh_isucastforme(struct ieee80211vap *v
 static int
 mesh_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
 {
-#define	SEQ_LEQ(a,b)	((int)((a)-(b)) <= 0)
 #define	HAS_SEQ(type)	((type & 0x4) == 0)
 	struct ieee80211vap *vap = ni->ni_vap;
 	struct ieee80211com *ic = ni->ni_ic;
@@ -1094,9 +1093,7 @@ mesh_input(struct ieee80211_node *ni, st
 			    TID_TO_WME_AC(tid) >= WME_AC_VI)
 				ic->ic_wme.wme_hipri_traffic++;
 			rxseq = le16toh(*(uint16_t *)wh->i_seq);
-			if ((ni->ni_flags & IEEE80211_NODE_HT) == 0 &&
-			    (wh->i_fc[1] & IEEE80211_FC1_RETRY) &&
-			    SEQ_LEQ(rxseq, ni->ni_rxseqs[tid])) {
+			if (! ieee80211_check_rxseq(ni, wh)) {
 				/* duplicate, discard */
 				IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_INPUT,
 				    wh->i_addr1, "duplicate",

Modified: head/sys/net80211/ieee80211_sta.c
==============================================================================
--- head/sys/net80211/ieee80211_sta.c	Wed May  4 01:39:44 2011	(r221417)
+++ head/sys/net80211/ieee80211_sta.c	Wed May  4 02:23:59 2011	(r221418)
@@ -512,7 +512,6 @@ doprint(struct ieee80211vap *vap, int su
 static int
 sta_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
 {
-#define	SEQ_LEQ(a,b)	((int)((a)-(b)) <= 0)
 #define	HAS_SEQ(type)	((type & 0x4) == 0)
 	struct ieee80211vap *vap = ni->ni_vap;
 	struct ieee80211com *ic = ni->ni_ic;
@@ -591,9 +590,7 @@ sta_input(struct ieee80211_node *ni, str
 			    TID_TO_WME_AC(tid) >= WME_AC_VI)
 				ic->ic_wme.wme_hipri_traffic++;
 			rxseq = le16toh(*(uint16_t *)wh->i_seq);
-			if ((ni->ni_flags & IEEE80211_NODE_HT) == 0 &&
-			    (wh->i_fc[1] & IEEE80211_FC1_RETRY) &&
-			    SEQ_LEQ(rxseq, ni->ni_rxseqs[tid])) {
+			if (! ieee80211_check_rxseq(ni, wh)) {
 				/* duplicate, discard */
 				IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_INPUT,
 				    bssid, "duplicate",
@@ -910,7 +907,6 @@ out:
 		m_freem(m);
 	}
 	return type;
-#undef SEQ_LEQ
 }
 
 static void

Modified: head/sys/net80211/ieee80211_wds.c
==============================================================================
--- head/sys/net80211/ieee80211_wds.c	Wed May  4 01:39:44 2011	(r221417)
+++ head/sys/net80211/ieee80211_wds.c	Wed May  4 02:23:59 2011	(r221418)
@@ -406,7 +406,6 @@ wds_newstate(struct ieee80211vap *vap, e
 static int
 wds_input(struct ieee80211_node *ni, struct mbuf *m, int rssi, int nf)
 {
-#define	SEQ_LEQ(a,b)	((int)((a)-(b)) <= 0)
 #define	HAS_SEQ(type)	((type & 0x4) == 0)
 	struct ieee80211vap *vap = ni->ni_vap;
 	struct ieee80211com *ic = ni->ni_ic;
@@ -495,9 +494,7 @@ wds_input(struct ieee80211_node *ni, str
 		    TID_TO_WME_AC(tid) >= WME_AC_VI)
 			ic->ic_wme.wme_hipri_traffic++;
 		rxseq = le16toh(*(uint16_t *)wh->i_seq);
-		if ((ni->ni_flags & IEEE80211_NODE_HT) == 0 &&
-		    (wh->i_fc[1] & IEEE80211_FC1_RETRY) &&
-		    SEQ_LEQ(rxseq, ni->ni_rxseqs[tid])) {
+		if (! ieee80211_check_rxseq(ni, wh)) {
 			/* duplicate, discard */
 			IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_INPUT,
 			    wh->i_addr1, "duplicate",
@@ -741,7 +738,6 @@ out:
 		m_freem(m);
 	}
 	return type;
-#undef SEQ_LEQ
 }
 
 static void



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