Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 30 Jan 2017 01:11:31 +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: r312972 - head/sys/net80211
Message-ID:  <201701300111.v0U1BV6L081269@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Mon Jan 30 01:11:30 2017
New Revision: 312972
URL: https://svnweb.freebsd.org/changeset/base/312972

Log:
  [net80211] address seqno allocation for group addressed frames
  
  After some digging and looking at packet traces, it looks like the
  sequence number allocation being done by net80211 doesn't meet
  802.11-2012.
  
  Specifically, group addressed frames (broadcast, multicast) have
  sequence numbers allocated from a separate pool, even if they're
  QoS frames.
  
  This patch starts to try and address this, both on transmit and
  receive.
  
  * When receiving, don't throw away multicast frames for now.
    It's sub-optimal, but until we correctly track group addressed
    frames via another TID counter, this is the best we can do.
  
  * When doing A-MPDU checks, don't include group addressed frames
    in the sequence number checks.
  
  * When transmitting, don't allocate group frame sequence numbers
    from the TID, instead use the NONQOS TID for allocation.
  
  This may fix iwn(4) 11n because I /think/ this was one of the
  handful of places where ni_txseqs[] was being assigned /outside/
  of the driver itself.
  
  This however doesn't completely fix things - notably the way that
  TID assignment versus WME assignment for driver hardware queues
  will mess up multicast ordering. For example, if all multicast
  QoS frames come from one sequence number space but they're
  expected to obey the QoS value assigned, they'll end up in
  different queues in the hardware and go out in different
  orders.
  
  I can't fix that right now and indeed fixing it will require some
  pretty heavy lifting of both the WME<->TID QoS assignment, as well
  as figuring out what the correct way for drivers to behave.
  
  For example, both iwn(4) and ath(4) shouldn't put QoS multicast
  traffic into the same output queue as aggregate traffic, because
  the sequence numbers are all wrong. So perhaps the correct thing
  to do there is ignore the WME/TID for QoS traffic and map it all
  to the best effort queue or something, and ensure it doesn't
  muck up the TID/blockack window tracking. However, I'm /pretty/
  sure that is still going to happen.
  
  .. maybe I should disable multicast QoS frames in general as well,
  but I don't know what that'll do for whatever the current state
  of 802.11s mesh support is.
  
  Tested:
  
  * STA mode, ath10k NIC
  * AP mode, AR9344/AR9580 AP
  * iperf tcp/udp tests with concurrent multicast QoS traffic.
  
  Before this, iperfs would fail pretty quickly because the sending
  AP would start sending out QoS multicast frames that would be
  out of order from the rest of the TID traffic, causing the blockack
  window to get way, way out of sync.
  
  This now doesn't occur.
  
  TODO:
  
  * verify which QoS frames SHOULD be tagged as M_AMPDU_MPDU.
    For example, QoS NULL frames shouldn't be tagged!
  
  Reviewed by: avos
  Differential Revision: https://reviews.freebsd.org/D9357

Modified:
  head/sys/net80211/ieee80211_ht.c
  head/sys/net80211/ieee80211_input.h
  head/sys/net80211/ieee80211_output.c

Modified: head/sys/net80211/ieee80211_ht.c
==============================================================================
--- head/sys/net80211/ieee80211_ht.c	Sun Jan 29 22:38:13 2017	(r312971)
+++ head/sys/net80211/ieee80211_ht.c	Mon Jan 30 01:11:30 2017	(r312972)
@@ -827,6 +827,16 @@ ieee80211_ampdu_reorder(struct ieee80211
 		 */
 		return PROCESS;
 	}
+
+	/*
+	 * 802.11-2012 9.3.2.10 - Duplicate detection and recovery.
+	 *
+	 * Multicast QoS data frames are checked against a different
+	 * counter, not the per-TID counter.
+	 */
+	if (IEEE80211_IS_MULTICAST(wh->i_addr1))
+		return PROCESS;
+
 	if (IEEE80211_IS_DSTODS(wh))
 		tid = ((struct ieee80211_qosframe_addr4 *)wh)->i_qos[0];
 	else

Modified: head/sys/net80211/ieee80211_input.h
==============================================================================
--- head/sys/net80211/ieee80211_input.h	Sun Jan 29 22:38:13 2017	(r312971)
+++ head/sys/net80211/ieee80211_input.h	Mon Jan 30 01:11:30 2017	(r312972)
@@ -149,6 +149,12 @@ ishtinfooui(const uint8_t *frm)
  * (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.
+ *
+ * XXX TODO: handle sequence number space wrapping with dropped frames;
+ * especially in high interference conditions under high traffic load
+ * The RX AMPDU reorder code also needs it.
+ *
+ * XXX TODO: update for 802.11-2012 9.3.2.10 Duplicate Detection and Recovery.
  */
 static __inline int
 ieee80211_check_rxseq(struct ieee80211_node *ni, struct ieee80211_frame *wh,
@@ -175,6 +181,13 @@ ieee80211_check_rxseq(struct ieee80211_n
 	if (! IEEE80211_HAS_SEQ(type, subtype))
 		return 1;
 
+	/*
+	 * Always allow multicast frames for now - QoS (any TID)
+	 * or not.
+	 */
+	if (IEEE80211_IS_MULTICAST(wh->i_addr1))
+		return 1;
+
 	tid = ieee80211_gettid(wh);
 
 	/*

Modified: head/sys/net80211/ieee80211_output.c
==============================================================================
--- head/sys/net80211/ieee80211_output.c	Sun Jan 29 22:38:13 2017	(r312971)
+++ head/sys/net80211/ieee80211_output.c	Mon Jan 30 01:11:30 2017	(r312972)
@@ -122,9 +122,7 @@ ieee80211_vap_pkt_send_dest(struct ieee8
 {
 	struct ieee80211com *ic = vap->iv_ic;
 	struct ifnet *ifp = vap->iv_ifp;
-#ifdef IEEE80211_SUPPORT_SUPERG
 	int mcast;
-#endif
 
 	if ((ni->ni_flags & IEEE80211_NODE_PWR_MGT) &&
 	    (m->m_flags & M_PWR_SAV) == 0) {
@@ -164,9 +162,7 @@ ieee80211_vap_pkt_send_dest(struct ieee8
 	 * interface it (might have been) received on.
 	 */
 	m->m_pkthdr.rcvif = (void *)ni;
-#ifdef IEEE80211_SUPPORT_SUPERG
 	mcast = (m->m_flags & (M_MCAST | M_BCAST)) ? 1: 0;
-#endif
 
 	BPF_MTAP(ifp, m);		/* 802.3 tx */
 
@@ -181,10 +177,15 @@ ieee80211_vap_pkt_send_dest(struct ieee8
 	 * The default ic_ampdu_enable routine handles staggering
 	 * ADDBA requests in case the receiver NAK's us or we are
 	 * otherwise unable to establish a BA stream.
+	 *
+	 * Don't treat group-addressed frames as candidates for aggregation;
+	 * net80211 doesn't support 802.11aa-2012 and so group addressed
+	 * frames will always have sequence numbers allocated from the NON_QOS
+	 * TID.
 	 */
 	if ((ni->ni_flags & IEEE80211_NODE_AMPDU_TX) &&
 	    (vap->iv_flags_ht & IEEE80211_FHT_AMPDU_TX)) {
-		if ((m->m_flags & M_EAPOL) == 0) {
+		if ((m->m_flags & M_EAPOL) == 0 && (! mcast)) {
 			int tid = WME_AC_TO_TID(M_WME_GETAC(m));
 			struct ieee80211_tx_ampdu *tap = &ni->ni_tx_ampdu[tid];
 
@@ -776,12 +777,20 @@ ieee80211_send_setup(
 	 * requiring the TX lock.
 	 */
 	tap = &ni->ni_tx_ampdu[tid];
-	if (tid != IEEE80211_NONQOS_TID && IEEE80211_AMPDU_RUNNING(tap))
+	if (tid != IEEE80211_NONQOS_TID && IEEE80211_AMPDU_RUNNING(tap)) {
 		m->m_flags |= M_AMPDU_MPDU;
-	else {
+	} else {
 		if (IEEE80211_HAS_SEQ(type & IEEE80211_FC0_TYPE_MASK,
 				      type & IEEE80211_FC0_SUBTYPE_MASK))
-			seqno = ni->ni_txseqs[tid]++;
+			/*
+			 * 802.11-2012 9.3.2.10 - QoS multicast frames
+			 * come out of a different seqno space.
+			 */
+			if (IEEE80211_IS_MULTICAST(wh->i_addr1)) {
+				seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
+			} else {
+				seqno = ni->ni_txseqs[tid]++;
+			}
 		else
 			seqno = 0;
 
@@ -1239,7 +1248,7 @@ ieee80211_encap(struct ieee80211vap *vap
 	struct ieee80211_frame *wh;
 	struct ieee80211_key *key;
 	struct llc *llc;
-	int hdrsize, hdrspace, datalen, addqos, txfrag, is4addr;
+	int hdrsize, hdrspace, datalen, addqos, txfrag, is4addr, is_mcast;
 	ieee80211_seq seqno;
 	int meshhdrsize, meshae;
 	uint8_t *qos;
@@ -1247,6 +1256,8 @@ ieee80211_encap(struct ieee80211vap *vap
 	
 	IEEE80211_TX_LOCK_ASSERT(ic);
 
+	is_mcast = !! (m->m_flags & (M_MCAST | M_BCAST));
+
 	/*
 	 * Copy existing Ethernet header to a safe place.  The
 	 * rest of the code assumes it's ok to strip it when
@@ -1291,11 +1302,19 @@ ieee80211_encap(struct ieee80211vap *vap
 	 * ap's require all data frames to be QoS-encapsulated
 	 * once negotiated in which case we'll need to make this
 	 * configurable.
-	 * NB: mesh data frames are QoS.
+	 *
+	 * Don't send multicast QoS frames.
+	 * Technically multicast frames can be QoS if all stations in the
+	 * BSS are also QoS.
+	 *
+	 * NB: mesh data frames are QoS, including multicast frames.
 	 */
-	addqos = ((ni->ni_flags & (IEEE80211_NODE_QOS|IEEE80211_NODE_HT)) ||
+	addqos =
+	    (((is_mcast == 0) && (ni->ni_flags &
+	     (IEEE80211_NODE_QOS|IEEE80211_NODE_HT))) ||
 	    (vap->iv_opmode == IEEE80211_M_MBSS)) &&
 	    (m->m_flags & M_EAPOL) == 0;
+
 	if (addqos)
 		hdrsize = sizeof(struct ieee80211_qosframe);
 	else
@@ -1560,6 +1579,22 @@ ieee80211_encap(struct ieee80211vap *vap
 		 */
 		if ((m->m_flags & M_AMPDU_MPDU) == 0) {
 			/*
+			 * 802.11-2012 9.3.2.10 -
+			 *
+			 * If this is a multicast frame then we need
+			 * to ensure that the sequence number comes from
+			 * a separate seqno space and not the TID space.
+			 *
+			 * Otherwise multicast frames may actually cause
+			 * holes in the TX blockack window space and
+			 * upset various things.
+			 */
+			if (IEEE80211_IS_MULTICAST(wh->i_addr1))
+				seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
+			else
+				seqno = ni->ni_txseqs[tid]++;
+
+			/*
 			 * NB: don't assign a sequence # to potential
 			 * aggregates; we expect this happens at the
 			 * point the frame comes off any aggregation q



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