Date: Mon, 2 May 2011 14:43:27 +0800 From: Adrian Chadd <adrian@freebsd.org> To: freebsd-wireless@freebsd.org Subject: [net80211] patch: properly handle sequence checking for HT non-aggregation; refactor out shared code Message-ID: <BANLkTimseMFe-=y-1rkK%2BhaAxskV6usSfg@mail.gmail.com>
index | next in thread | raw e-mail
[-- Attachment #1 --]
Hi,
The attached patch does two things:
Firstly, it abstracts out the sequence number checks in
{sta,hostap,mesh,adhoc,wds} into a new function,
ieee80211_check_rxseq(). A lot of the input path code is shared; I'm
not going to try and refactor all of it.
Secondly, it attempts to do the HT mode checking correctly. The
existing code assumes HT == AMPDU, so don't worry about re-ordering.
Instead, my replacement code checks whether QOS/WME is enabled and if
so, checks whether AMPDU-RX is running. If it is, it ignores the
sequence number check. If it isn't running AMPDU-RX, it enforces the
same sequence number check for non-HT modes.
This gets rid of the duplicate packet issues I've seen when doing 11n
(non AMPDU-RX) but it doesn't fix the out of order TCP packet
behaviour plaguing performance.
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,57 @@
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 HAS_SEQ(type) ((type & 0x4) == 0)
+ 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.
+ */
+ if ((wh->i_fc[1] & IEEE80211_FC1_RETRY) &&
+ SEQ_LEQ(rxseq, ni->ni_rxseqs[tid]))
+ return 0;
+
+ return 1;
+}
+
#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?BANLkTimseMFe-=y-1rkK%2BhaAxskV6usSfg>