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>