svn commit: r312972 - head/sys/net80211
Adrian Chadd
adrian at freebsd.org
Mon Jan 30 03:15:52 UTC 2017
... and obviously between writing the initial review message and the
commit, I also decided to disable multicast QoS frame generation for
now. It just makes things less terrible.
At some point IBSS/hostap mode should grow "how many stations are
QoS/non-QoS" tracking so this can be dynamic and multicast QoS can
occur.
(ANd for whenever I implement 802.11aa extensions, I'll need it to work..)
-adrian
On 29 January 2017 at 17:11, Adrian Chadd <adrian at freebsd.org> wrote:
> 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
>
More information about the svn-src-all
mailing list