From owner-svn-src-user@FreeBSD.ORG Sat Aug 13 10:43:56 2011 Return-Path: Delivered-To: svn-src-user@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id C3C32106566C; Sat, 13 Aug 2011 10:43:56 +0000 (UTC) (envelope-from adrian@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id B25C68FC22; Sat, 13 Aug 2011 10:43:56 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p7DAhuQG010345; Sat, 13 Aug 2011 10:43:56 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p7DAhu7S010342; Sat, 13 Aug 2011 10:43:56 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201108131043.p7DAhu7S010342@svn.freebsd.org> From: Adrian Chadd Date: Sat, 13 Aug 2011 10:43:56 +0000 (UTC) To: src-committers@freebsd.org, svn-src-user@freebsd.org X-SVN-Group: user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r224813 - user/adrian/if_ath_tx/sys/dev/ath X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 13 Aug 2011 10:43:56 -0000 Author: adrian Date: Sat Aug 13 10:43:56 2011 New Revision: 224813 URL: http://svn.freebsd.org/changeset/base/224813 Log: This deviates a bit from the reference code; I may end up backing this out at a later stage. This unifies the "is this packet supposed to be considered as part of the BAW?" logic. I've added a bit in the ath_buf.bf_state struct which indicates whether it's going to be considered for inclusion or not. Packets that aren't part of the BAW calculation are thus not passed to the BAW add or update code. Eventually, the aggregate forming code will also not include this packet as part of an aggregate and will be sent as a non-aggregate packet. This fixes the hangs, interface resets and general unhappiness that has been happening with my recent changes. Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Sat Aug 13 10:43:21 2011 (r224812) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Sat Aug 13 10:43:56 2011 (r224813) @@ -1010,6 +1010,7 @@ ath_tx_start(struct ath_softc *sc, struc const struct ieee80211_frame *wh; int is_ampdu, is_ampdu_tx, is_ampdu_pending; ieee80211_seq seqno; + uint8_t subtype; /* * Determine the target hardware queue. @@ -1031,6 +1032,7 @@ ath_tx_start(struct ath_softc *sc, struc txq = sc->sc_ac2q[pri]; wh = mtod(m0, struct ieee80211_frame *); ismcast = IEEE80211_IS_MULTICAST(wh->i_addr1); + subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK; /* A-MPDU TX */ is_ampdu_tx = ath_tx_ampdu_running(sc, ATH_NODE(ni), tid); @@ -1043,15 +1045,27 @@ ath_tx_start(struct ath_softc *sc, struc /* Multicast frames go onto the software multicast queue */ if (ismcast) txq = &avp->av_mcastq; - if ((! is_ampdu) && (vap->iv_ps_sta || avp->av_mcastq.axq_depth)) + + if ((! is_ampdu) && (vap->iv_ps_sta || avp->av_mcastq.axq_depth)) txq = &avp->av_mcastq; /* Do the generic frame setup */ /* A-MPDU TX? Manually set sequence number */ /* Don't do it whilst pending; the net80211 layer still assigns them */ - if (is_ampdu_tx) + if (is_ampdu_tx) { + /* + * Always set a seqno; this function will + * handle making sure that null data frames + * don't get a sequence number from the current + * TID and thus mess with the BAW. + */ seqno = ath_tx_tid_seqno_assign(sc, ni, bf, m0); + if (IEEE80211_QOS_HAS_SEQ(wh) && + subtype != IEEE80211_FC0_SUBTYPE_QOS_NULL) { + bf->bf_state.bfs_dobaw = 1; + } + } /* * If needed, the sequence number has been assigned. @@ -1708,6 +1722,7 @@ ath_tx_swq(struct ath_softc *sc, struct bf->bf_state.bfs_tid = tid; bf->bf_state.bfs_txq = txq; bf->bf_state.bfs_pri = pri; + bf->bf_state.bfs_dobaw = 0; /* Queue frame to the tail of the software queue */ ATH_TXQ_INSERT_TAIL(atid, bf, bf_list); @@ -1839,7 +1854,8 @@ ath_tx_tid_free_pkts(struct ath_softc *s * If the current TID is running AMPDU, update * the BAW. */ - if (ath_tx_ampdu_running(sc, an, tid)) + if (ath_tx_ampdu_running(sc, an, tid) && + bf->bf_state.bfs_dobaw) ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno)); ATH_TXQ_REMOVE_HEAD(atid, bf_list); @@ -1994,8 +2010,9 @@ ath_tx_cleanup(struct ath_softc *sc, str bf_next = STAILQ_NEXT(bf, bf_list); STAILQ_REMOVE(&atid->axq_q, bf, ath_buf, bf_list); atid->axq_depth--; - ath_tx_update_baw(sc, an, atid, - SEQNO(bf->bf_state.bfs_seqno)); + if (bf->bf_state.bfs_dobaw) + ath_tx_update_baw(sc, an, atid, + SEQNO(bf->bf_state.bfs_seqno)); /* * Call the default completion handler with "fail" just * so upper levels are suitably notified about this. @@ -2083,7 +2100,9 @@ ath_tx_aggr_retry_unaggr(struct ath_soft sc->sc_stats.ast_tx_swretrymax++; /* Update BAW anyway */ - ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno)); + if (bf->bf_state.bfs_dobaw) + ath_tx_update_baw(sc, an, atid, + SEQNO(bf->bf_state.bfs_seqno)); /* Free buffer, bf is free after this call */ ath_tx_default_comp(sc, bf, 0); @@ -2150,7 +2169,8 @@ ath_tx_aggr_comp(struct ath_softc *sc, s /* Success? Complete */ DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: TID=%d, seqno %d\n", __func__, tid, SEQNO(bf->bf_state.bfs_seqno)); - ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno)); + if (bf->bf_state.bfs_dobaw) + ath_tx_update_baw(sc, an, atid, SEQNO(bf->bf_state.bfs_seqno)); ath_tx_default_comp(sc, bf, fail); /* bf is freed at this point */ @@ -2168,9 +2188,6 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft struct ath_txq *txq; struct ath_tid *atid = &an->an_tid[tid]; struct ieee80211_tx_ampdu *tap; - int check_baw; - uint8_t subtype; - const struct ieee80211_frame *wh; DPRINTF(sc, ATH_DEBUG_SW_TX, "%s: tid=%d\n", __func__, tid); @@ -2181,7 +2198,6 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft __func__); for (;;) { - check_baw = 1; bf = STAILQ_FIRST(&atid->axq_q); if (bf == NULL) { break; @@ -2195,22 +2211,8 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft device_printf(sc->sc_dev, "%s: TXQ: tid=%d, ac=%d, bf tid=%d\n", __func__, tid, atid->ac, bf->bf_state.bfs_tid); - /* - * Some packets aren't going to fall within the BAW. - * If they're non-sequence QOS packets that sit inside this TID, - * or they're a null data frame. - * This is quite messy and I should make the seqno code and - * this code share the same decision logic. - */ - wh = mtod(bf->bf_m, const struct ieee80211_frame *); - if (! IEEE80211_QOS_HAS_SEQ(wh)) - check_baw = 0; - subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK; - if (subtype == IEEE80211_FC0_SUBTYPE_QOS_NULL) - check_baw = 0; - - /* XXX check if seqno is outside of BAW, if so don't queue it */ - if (check_baw == 1 && + /* Check if seqno is outside of BAW, if so don't queue it */ + if (bf->bf_state.bfs_dobaw && (! BAW_WITHIN(tap->txa_start, tap->txa_wnd, SEQNO(bf->bf_state.bfs_seqno)))) { DPRINTF(sc, ATH_DEBUG_SW_TX_BAW, @@ -2221,7 +2223,7 @@ ath_tx_tid_hw_queue_aggr(struct ath_soft } /* Don't add packets to the BAW that don't contribute to it */ - if (check_baw == 1) + if (bf->bf_state.bfs_dobaw) ath_tx_addto_baw(sc, an, atid, bf); /* Modified: user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h ============================================================================== --- user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Sat Aug 13 10:43:21 2011 (r224812) +++ user/adrian/if_ath_tx/sys/dev/ath/if_athvar.h Sat Aug 13 10:43:56 2011 (r224813) @@ -207,6 +207,7 @@ struct ath_buf { int bfs_aggr:1; /* part of aggregate? */ int bfs_aggrburst:1; /* part of aggregate burst? */ int bfs_isretried:1; /* retried frame? */ + int bfs_dobaw:1; /* actually check against BAW? */ } bf_state; }; typedef STAILQ_HEAD(, ath_buf) ath_bufhead;