diff --git a/sys/dev/ath/if_ath_tx.c b/sys/dev/ath/if_ath_tx.c index 1ec23972f283..33a22ef190eb 100644 --- a/sys/dev/ath/if_ath_tx.c +++ b/sys/dev/ath/if_ath_tx.c @@ -2034,6 +2034,8 @@ ath_tx_start(struct ath_softc *sc, struct ieee80211_node *ni, * * Don't assign A-MPDU sequence numbers to group address * frames; they come from a different sequence number space. + * + * TODO: why'd I add the !mcast check here? it skips setting the seqno for ampdu-enabled TID mcast? */ if (is_ampdu_tx && (! IEEE80211_IS_MULTICAST(wh->i_addr1))) { /* diff --git a/sys/dev/iwn/if_iwn.c b/sys/dev/iwn/if_iwn.c index 50d50fdc473c..6fd8fdd3358c 100644 --- a/sys/dev/iwn/if_iwn.c +++ b/sys/dev/iwn/if_iwn.c @@ -4658,6 +4658,12 @@ iwn_tx_data(struct iwn_softc *sc, struct mbuf *m, struct ieee80211_node *ni) } ring = &sc->txq[ac]; + + /* + * TODO: IEEE80211_HT_CHECK_BAW_TX, what about multicast and + * QoS null data frames? + */ + if (m->m_flags & M_AMPDU_MPDU) { uint16_t seqno = ni->ni_txseqs[tid]; diff --git a/sys/dev/rtwn/rtl8192c/r92c_tx.c b/sys/dev/rtwn/rtl8192c/r92c_tx.c index 15beca776b61..a1a144692158 100644 --- a/sys/dev/rtwn/rtl8192c/r92c_tx.c +++ b/sys/dev/rtwn/rtl8192c/r92c_tx.c @@ -333,8 +333,11 @@ r92c_fill_tx_desc(struct rtwn_softc *sc, struct ieee80211_node *ni, uint16_t seqno; if (m->m_flags & M_AMPDU_MPDU) { - seqno = ni->ni_txseqs[tid] % IEEE80211_SEQ_RANGE; - ni->ni_txseqs[tid]++; + seqno = ieee80211_tx_seqno_assign(ni, m); + /* + * NB: We don't set the mbuf seqno here; firmware + * will add it for us. + */ } else seqno = M_SEQNO_GET(m) % IEEE80211_SEQ_RANGE; diff --git a/sys/dev/rtwn/rtl8812a/r12a_tx.c b/sys/dev/rtwn/rtl8812a/r12a_tx.c index 9e0d8e85c0cf..f8f2828a0c44 100644 --- a/sys/dev/rtwn/rtl8812a/r12a_tx.c +++ b/sys/dev/rtwn/rtl8812a/r12a_tx.c @@ -334,8 +334,11 @@ r12a_fill_tx_desc(struct rtwn_softc *sc, struct ieee80211_node *ni, uint16_t seqno; if (m->m_flags & M_AMPDU_MPDU) { - seqno = ni->ni_txseqs[tid]; - ni->ni_txseqs[tid]++; + seqno = ieee80211_tx_seqno_assign(ni, m); + /* + * NB: We don't set the mbuf seqno here; firmware + * will add it for us. + */ } else seqno = M_SEQNO_GET(m) % IEEE80211_SEQ_RANGE; diff --git a/sys/dev/rtwn/usb/rtwn_usb_tx.c b/sys/dev/rtwn/usb/rtwn_usb_tx.c index aa438d5d07b1..abb2c0a3e004 100644 --- a/sys/dev/rtwn/usb/rtwn_usb_tx.c +++ b/sys/dev/rtwn/usb/rtwn_usb_tx.c @@ -189,8 +189,9 @@ rtwn_bulk_tx_callback_qid(struct usb_xfer *xfer, usb_error_t error, int qid) rtwn_usb_txeof(uc, data, 1); if (error != 0) device_printf(sc->sc_dev, - "%s: called; txeof error=%s\n", + "%s: called; txeof qid=%d, error=%s\n", __func__, + qid, usbd_errstr(error)); if (error != USB_ERR_CANCELLED) { usbd_xfer_set_stall(xfer); diff --git a/sys/net80211/ieee80211.h b/sys/net80211/ieee80211.h index eb83d0a40a33..185995b84fdb 100644 --- a/sys/net80211/ieee80211.h +++ b/sys/net80211/ieee80211.h @@ -274,6 +274,16 @@ struct ieee80211_qosframe_addr4 { IEEE80211_FC0_TYPE_DATA, \ IEEE80211_FC0_SUBTYPE_QOS_DATA)) +/* + * Return true if this frame is a QoS NULL data frame. + */ +#define IEEE80211_IS_QOS_NULL(wh) \ + (IEEE80211_IS_FC0_CHECK_VER_TYPE_SUBTYPE(wh, \ + IEEE80211_FC0_VERSION_0, \ + IEEE80211_FC0_TYPE_DATA, \ + IEEE80211_FC0_SUBTYPE_QOS_NULL)) + + #define IEEE80211_FC1_DIR_MASK 0x03 #define IEEE80211_FC1_DIR_NODS 0x00 /* STA->STA */ #define IEEE80211_FC1_DIR_TODS 0x01 /* STA->AP */ @@ -344,6 +354,38 @@ struct ieee80211_qosframe_addr4 { (IEEE80211_FC0_TYPE_MASK | IEEE80211_FC0_SUBTYPE_QOS_DATA)) == \ (IEEE80211_FC0_TYPE_DATA | IEEE80211_FC0_SUBTYPE_QOS_DATA)) +/* + * Is this frame a candidate to add to the block-ack window? + * + * Even if AMPDU TX is enabled on a TID, there are some frames - + * notably QoS multicast frames at the moment - which are not + * allowed to be added to the block-ack window. + * + * This may change if reliable multicast is ever implemented, + * where multicast frames will actually be a part of the TID. + * + * TODO: which frames are added to the block-ack window? + * Find very specifically where in the 802.11-2016 specification + * the block-ack window stuff mentions which frames should + * be going into it. + * + * eg 802.11-2016 10.3.2.11.2 (Transmitter requirements) states + * different sequence number spaces that need to be supported. + * QOS NULL-DATA frames have no sequence numbers (SNS5), QoS + * multicast frames fall into the default (SNS1) and QoS data + * frames directed at a station (SNS5 frames) fall into SNS2. + * + * TODO: ath(4) uses IEEE80211_QOS_HAS_SEQ && !mcast && !qos-null. + * IEEE80211_IS_QOS_DATA() only matches QOS_DATA. + * I'm not yet sure what the "correct" one to use here is; I'll + * need to read the spec first to find out and then make sure + * ath, rtwn and iwn (which do TX aggregation) also work. + */ +#define IEEE80211_HT_CHECK_BAW_TX(wh) \ + (IEEE80211_QOS_HAS_SEQ(wh) && \ + !IEEE80211_IS_MULTICAST(wh->i_addr1) && \ + !IEEE80211_IS_QOS_NULL(wh)) + /* * WME/802.11e information element. */ diff --git a/sys/net80211/ieee80211_output.c b/sys/net80211/ieee80211_output.c index 44903ed366fd..ed48c0142435 100644 --- a/sys/net80211/ieee80211_output.c +++ b/sys/net80211/ieee80211_output.c @@ -892,7 +892,6 @@ ieee80211_send_setup( struct ieee80211vap *vap = ni->ni_vap; struct ieee80211_tx_ampdu *tap; struct ieee80211_frame *wh = mtod(m, struct ieee80211_frame *); - ieee80211_seq seqno; IEEE80211_TX_LOCK_ASSERT(ni->ni_ic); @@ -976,23 +975,10 @@ ieee80211_send_setup( /* NB: zero out i_seq field (for s/w encryption etc) */ *(uint16_t *)&wh->i_seq[0] = 0; } else { - if (IEEE80211_HAS_SEQ(type & IEEE80211_FC0_TYPE_MASK, - type & IEEE80211_FC0_SUBTYPE_MASK)) - /* - * 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; + ieee80211_seq seqno; - *(uint16_t *)&wh->i_seq[0] = - htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT); - M_SEQNO_SET(m, seqno); + seqno = ieee80211_tx_seqno_assign(ni, m); + ieee80211_tx_seqno_set(m, seqno); } if (IEEE80211_IS_MULTICAST(wh->i_addr1)) @@ -1483,7 +1469,6 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni, struct ieee80211_key *key; struct llc *llc; int hdrsize, hdrspace, datalen, addqos, txfrag, is4addr, is_mcast; - ieee80211_seq seqno; int meshhdrsize, meshae; uint8_t *qos; int is_amsdu = 0; @@ -1829,22 +1814,11 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni, * and we don't need the TX lock held. */ 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]++; + ieee80211_seq seqno; + seqno = ieee80211_tx_seqno_assign(ni, m); + ieee80211_tx_seqno_set(m, seqno); + } else { /* * NB: don't assign a sequence # to potential * aggregates; we expect this happens at the @@ -1857,10 +1831,7 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni, * capability; this may also change when we pull * aggregation up into net80211 */ - *(uint16_t *)wh->i_seq = - htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT); - M_SEQNO_SET(m, seqno); - } else { + /* NB: zero out i_seq field (for s/w encryption etc) */ *(uint16_t *)wh->i_seq = 0; } @@ -1870,10 +1841,10 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni, * numbers. If the driver does it, then don't do it here; * and we don't need the TX lock held. */ - seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++; - *(uint16_t *)wh->i_seq = - htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT); - M_SEQNO_SET(m, seqno); + ieee80211_seq seqno; + + seqno = ieee80211_tx_seqno_assign(ni, m); + ieee80211_tx_seqno_set(m, seqno); /* * XXX TODO: we shouldn't allow EAPOL, etc that would @@ -3780,7 +3751,6 @@ ieee80211_beacon_update(struct ieee80211_node *ni, struct mbuf *m, int mcast) struct ieee80211com *ic = ni->ni_ic; int len_changed = 0; uint16_t capinfo; - struct ieee80211_frame *wh; ieee80211_seq seqno; IEEE80211_LOCK(ic); @@ -3849,8 +3819,6 @@ ieee80211_beacon_update(struct ieee80211_node *ni, struct mbuf *m, int mcast) return 1; /* just assume length changed */ } - wh = mtod(m, struct ieee80211_frame *); - /* * XXX TODO Strictly speaking this should be incremented with the TX * lock held so as to serialise access to the non-qos TID sequence @@ -3859,10 +3827,8 @@ ieee80211_beacon_update(struct ieee80211_node *ni, struct mbuf *m, int mcast) * If the driver identifies it does its own TX seqno management then * we can skip this (and still not do the TX seqno.) */ - seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++; - *(uint16_t *)&wh->i_seq[0] = - htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT); - M_SEQNO_SET(m, seqno); + seqno = ieee80211_tx_seqno_assign(ni, m); + ieee80211_tx_seqno_set(m, seqno); /* XXX faster to recalculate entirely or just changes? */ capinfo = ieee80211_getcapinfo(vap, ni->ni_chan); @@ -4194,3 +4160,118 @@ ieee80211_tx_complete(struct ieee80211_node *ni, struct mbuf *m, int status) } m_freem(m); } + +/* + * Determine a sequence number to the given frame. + * + * This should only be called once for an output frame. + * + * 802.11-2016 10.3.2.11.2 (Transmitter requirements) states + * that different sequence number spaces need to be supported. + * + * SNS1 - everything else - non-QoS TID + * SNS2 - Individually addressed QoS data frames - per-TID + * SNS3 - Time priority Management Frames - per-TID + * SNS4 - QMF station transmitting a QMF frame - per-AC + * SNS5 - QoS-NULL (none) + * + * Notably, multicast frames, including QoS multicast frames, + * come from SNS1 (non-QOS TID). + */ +ieee80211_seq +ieee80211_tx_seqno_assign(struct ieee80211_node *ni, struct mbuf *m0) +#define INCR(_l, _sz) (_l) ++; (_l) &= ((_sz) - 1) +{ + struct ieee80211_frame *wh; + int tid; + ieee80211_seq seqno; + uint8_t type, subtype; + + wh = mtod(m0, struct ieee80211_frame *); + type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK; + subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK; + + /* Get the TID; it will return the non-TID TID for non-QoS frames. */ + tid = ieee80211_gettid(wh); + +#if 1 + if (!IEEE80211_HAS_SEQ(type, subtype)) { + /* If it doesn't have a sequence number, assign it zero */ + seqno = 0; + } else if (IEEE80211_IS_QOS_NULL(wh)) { + /* + * Is it a QOS NULL Data frame? Give it a sequence number from + * the default TID (IEEE80211_NONQOS_TID.) We could in theory + * give it an arbitrary sequence number. + * + * The RX path of everything I've looked at doesn't include the + * NULL data frame sequence number in the aggregation state + * updates, so assigning it a sequence number there will cause + * a BAW hole on the RX side. + */ + seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]; + INCR(ni->ni_txseqs[IEEE80211_NONQOS_TID], IEEE80211_SEQ_RANGE); + } else if (IEEE80211_IS_MULTICAST(wh->i_addr1)) { + /* + * Multicast frames - QoS or otherwise - use SNS1 for now. + */ + seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]; + INCR(ni->ni_txseqs[IEEE80211_NONQOS_TID], IEEE80211_SEQ_RANGE); + } else { + /* Assign sequence number from the TID */ + seqno = ni->ni_txseqs[tid]; + INCR(ni->ni_txseqs[tid], IEEE80211_SEQ_RANGE); + } +#else + /* + * Eventually verify/test this: it's more aligned with what 802.11-2016 + * says. + */ + if (!IEEE80211_HAS_SEQ(type, subtype)) { + /* If it doesn't have a sequence number, assign it zero */ + seqno = 0; + } else if (IEEE80211_IS_QOS_NULL(wh)) { + /* + * SNS5: Is it a QOS NULL Data frame? Give it a sequence number + * from the default TID (IEEE80211_NONQOS_TID.) + * + * (We could in theory give it an arbitrary sequence number.) + * + * The RX path of everything I've looked at doesn't include the + * NULL data frame sequence number in the aggregation state + * updates, so assigning it a sequence number there will cause + * a BAW hole on the RX side. + */ + seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]; + INCR(ni->ni_txseqs[IEEE80211_NONQOS_TID], IEEE80211_SEQ_RANGE); + } else if (IEEE80211_IS_QOS_DATA(wh) && + !IEEE80211_IS_MULTICAST(wh->i_addr1)) { + /* SNS1: QoS data, not multicast assign from per-TID space */ + seqno = ni->ni_txseqs[tid]; + INCR(ni->ni_txseqs[tid], IEEE80211_SEQ_RANGE); + } else { + /* SNS1: default */ + seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]; + INCR(ni->ni_txseqs[IEEE80211_NONQOS_TID], IEEE80211_SEQ_RANGE); + } + +#endif + + return seqno; +#undef INCR +} + +/* + * Set the sequence number in the given mbuf. + */ +void +ieee80211_tx_seqno_set(struct mbuf *m0, ieee80211_seq seqno) +{ + struct ieee80211_frame *wh; + + wh = mtod(m0, struct ieee80211_frame *); + + *(uint16_t *)&wh->i_seq[0] = htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT); + + M_SEQNO_SET(m0, seqno); +} diff --git a/sys/net80211/ieee80211_proto.h b/sys/net80211/ieee80211_proto.h index 9ab7e644c89d..555e8ae4c0c6 100644 --- a/sys/net80211/ieee80211_proto.h +++ b/sys/net80211/ieee80211_proto.h @@ -123,6 +123,9 @@ struct mbuf * ieee80211_ff_encap1(struct ieee80211vap *, struct mbuf *, const struct ether_header *); void ieee80211_tx_complete(struct ieee80211_node *, struct mbuf *, int); +ieee80211_seq ieee80211_tx_seqno_assign(struct ieee80211_node *ni, + struct mbuf *m0); +void ieee80211_tx_seqno_set(struct mbuf *m0, ieee80211_seq); /* * The formation of ProbeResponse frames requires guidance to