Date: Mon, 2 May 2011 17:01:11 +0800 From: Adrian Chadd <adrian@freebsd.org> To: freebsd-wireless@freebsd.org Subject: Re: [net80211] patch: properly handle sequence checking for HT non-aggregation; refactor out shared code Message-ID: <BANLkTimrLqOFA9w7Lp3w-HTa7JyZeGBU%2BA@mail.gmail.com> In-Reply-To: <BANLkTimseMFe-=y-1rkK%2BhaAxskV6usSfg@mail.gmail.com> References: <BANLkTimseMFe-=y-1rkK%2BhaAxskV6usSfg@mail.gmail.com>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
Hi,
Here's an updated diff. I noticed that with this diff, the number of
TCP out of order complaints dropped dramatically but I was seeing them
when the following occured:
wlan0: [00:23:6c:bf:38:3e] discard duplicate frame, seqno <0,4095>
fragno <0,0> tid 0
This is likely also occuring with non-HT (11a/11bg) QoS operation
chips in -HEAD and -8.
Whenever I saw that above being logged (among lots of other logged
duplicate discards, so I know it wasn't just "serial IO" related) I
saw a burst of out-of-order packets/bytes in the receiving side TCP
stack and a drop in throughput.
The current patch adds the following checks:
* if the last seen seqnum is 4095, then retransmissions of the last
seen fragment is eliminated (like the normal codepath intended to do
but didn't);
* else if the last seen seqnum is 4095, allow any subsequent packet.
This may be wrong for fragments (and I may add a check for rxseq !=
4095)) but any subsequent sequence will have a seqnum of < 4095
because of the 12 bit seqnum wrap.
* else (ie, the last seen seqnum isn't 4095), eliminate retransmits
for seq+frag <= last seen.
This almost entirely eliminates the speed drops I've seen in 11n
non-aggregation mode and almost entirely eliminates the burst of
out-of-order packets/bytes. The only time I'm seeing them now is when
the baseband hangs and the interface is reset.
Adrian
[-- Attachment #2 --]
Index: sys/net80211/ieee80211_sta.c
===================================================================
--- sys/net80211/ieee80211_sta.c (revision 220911)
+++ sys/net80211/ieee80211_sta.c (working copy)
@@ -512,7 +512,6 @@
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 @@
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 @@
m_freem(m);
}
return type;
-#undef SEQ_LEQ
}
static void
Index: sys/net80211/ieee80211_wds.c
===================================================================
--- sys/net80211/ieee80211_wds.c (revision 220911)
+++ sys/net80211/ieee80211_wds.c (working copy)
@@ -406,7 +406,6 @@
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 @@
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 @@
m_freem(m);
}
return type;
-#undef SEQ_LEQ
}
static void
Index: sys/net80211/ieee80211_hostap.c
===================================================================
--- sys/net80211/ieee80211_hostap.c (revision 220911)
+++ sys/net80211/ieee80211_hostap.c (working copy)
@@ -472,7 +472,6 @@
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 @@
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 @@
m_freem(m);
}
return type;
-#undef SEQ_LEQ
}
static void
Index: sys/net80211/ieee80211_mesh.c
===================================================================
--- sys/net80211/ieee80211_mesh.c (revision 220911)
+++ sys/net80211/ieee80211_mesh.c (working copy)
@@ -1040,7 +1040,6 @@
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 @@
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",
Index: sys/net80211/ieee80211_input.c
===================================================================
--- sys/net80211/ieee80211_input.c (revision 220911)
+++ sys/net80211/ieee80211_input.c (working copy)
@@ -760,6 +760,90 @@
return 0;
}
+/*
+ * Check the current frame sequence number against the current TID
+ * state and return whether it's in sequence or should be dropped.
+ */
+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.
+ *
+ * XXX This should be revisited to truely identify/log very
+ * XXX out of order packets (due to vendor bugs), rather than
+ * XXX simply retransmits.
+ */
+
+ /* For now, treat frame seqnum 4095 as special. */
+
+ /*
+ * Drop retransmits on seqnum 4095 for itself.
+ */
+ if ((SEQNO(ni->ni_rxseqs[tid]) == 4095) &&
+ 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.
+ */
+ if (SEQNO(ni->ni_rxseqs[tid]) == 4095)
+ return 1;
+
+ /*
+ * At this point we assume that seqnums below
+ * the current are retransmits/out of order packets.
+ * Eliminate retransmits for those.
+ */
+ 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
+
+}
+
#ifdef IEEE80211_DEBUG
/*
* Debugging support.
Index: sys/net80211/ieee80211_input.h
===================================================================
--- sys/net80211/ieee80211_input.h (revision 220911)
+++ sys/net80211/ieee80211_input.h (working copy)
@@ -157,4 +157,6 @@
int ieee80211_parse_beacon(struct ieee80211_node *, struct mbuf *,
struct ieee80211_scanparams *);
int ieee80211_parse_action(struct ieee80211_node *, struct mbuf *);
+int ieee80211_check_rxseq(struct ieee80211_node *ni,
+ struct ieee80211_frame *wh);
#endif /* _NET80211_IEEE80211_INPUT_H_ */
Index: sys/net80211/ieee80211_adhoc.c
===================================================================
--- sys/net80211/ieee80211_adhoc.c (revision 220911)
+++ sys/net80211/ieee80211_adhoc.c (working copy)
@@ -285,7 +285,6 @@
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 @@
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 @@
m_freem(m);
}
return type;
-#undef SEQ_LEQ
}
static int
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTimrLqOFA9w7Lp3w-HTa7JyZeGBU%2BA>