From owner-svn-src-user@FreeBSD.ORG Tue Sep 20 04:20: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 159C81065670; Tue, 20 Sep 2011 04:20: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 05FA48FC19; Tue, 20 Sep 2011 04:20: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 p8K4KtWl009983; Tue, 20 Sep 2011 04:20:55 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p8K4KtG3009981; Tue, 20 Sep 2011 04:20:55 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201109200420.p8K4KtG3009981@svn.freebsd.org> From: Adrian Chadd Date: Tue, 20 Sep 2011 04:20:55 +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: r225685 - 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: Tue, 20 Sep 2011 04:20:56 -0000 Author: adrian Date: Tue Sep 20 04:20:55 2011 New Revision: 225685 URL: http://svn.freebsd.org/changeset/base/225685 Log: Undo the BAR TX stuff (as it's broken); correct what the new LH edge of the BAW should be. The BAR TX stuff I committed earlier is just plain wrong. What should happen is this (in ADDBA mode): * If a software retransmit completely fails, all TX for that given TID should be paused until all existing TXes for said TID are completed (failed or not); * Once all the hardware-queued transmissions are completed, the left-hand edge of the BAW (from the TX point of view) should be correct. Ie, there may be further packets queued in the software TX queue, but the left-hand edge of the BAW will point to one frame past the last hardware-queued frame (again, successful or not, it doesn't matter.) * At that point, a BAR should be sent reflecting what the new BAW left hand edge is. * Once that's been received, TX on the given TID should resume. * If it fails, net80211 will cancel the addba session for us; we just need to resume TX'ing on the TID anyway (newly downgraded to non-aggregation.) Now, the things to keep in mind when implementing the correct-er solution: * There may be plenty of outstanding frames on the hardware TX queue, so you can't just send one BAR for each of those. * Whilst the BAR is being TXed, the node may be reset; this SHOULD correctly be handled (ie, cancelling the frame will cause net80211 to reschedule another one) * You can't just call pause() and resume() like I was doing, as multiple outstanding frames may be on the hardware queue (and multiples ones may fail.) A new method of marking a TID as "BAR pending" needs to be added first. That way the TID can be paused and unpaused a total of once, no matter how many outstanding frames fail before the TID has no more frames on the hardware queue. * Finally, this all only works if tid->hwq_depth is completely accurate. This needs to be verified! Modified: user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c 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 Tue Sep 20 00:37:35 2011 (r225684) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Tue Sep 20 04:20:55 2011 (r225685) @@ -2714,30 +2714,13 @@ ath_tx_aggr_retry_unaggr(struct ath_soft * This'll end up going into net80211 and back out * again, via ic->ic_raw_xmit(). */ - txseq = ni->ni_txseqs[tid]; + txseq = tap->txa_start; + ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); + device_printf(sc->sc_dev, "%s: TID %d: send BAR; seq %d\n", __func__, tid, txseq); - ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); - /* - * It's ok to unlock it now (and it's required at - * the moment!) since we are purging "holes" in the - * tx sequence up to this point; any subsequent - * sequence numbers haven't been yet attempted to - * TX. - */ - if (ieee80211_send_bar(ni, tap, txseq) == 0) { - /* - * Pause the TID if this was successful. - * An un-successful BAR TX would never call - * the BAR complete / timeout methods. - */ - ath_tx_tid_pause(sc, atid); - } else { - /* BAR TX failed */ - device_printf(sc->sc_dev, - "%s: TID %d: BAR TX failed\n", - __func__, tid); - } + + /* XXX TODO: send BAR */ /* Free buffer, bf is free after this call */ ath_tx_default_comp(sc, bf, 0); @@ -2885,31 +2868,13 @@ ath_tx_comp_aggr_error(struct ath_softc * in the ifnet TX context or raw TX context.) */ if (drops) { - int txseq = ni->ni_txseqs[tid->tid]; + int txseq = tap->txa_start; + ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]); device_printf(sc->sc_dev, "%s: TID %d: send BAR; seq %d\n", __func__, tid->tid, txseq); - ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]); - /* - * It's ok to unlock it now (and it's required at - * the moment!) since we are purging "holes" in the - * tx sequence up to this point; any subsequent - * sequence numbers haven't been yet attempted to - * TX. - */ - if (ieee80211_send_bar(ni, tap, txseq) == 0) { - /* - * Pause the TID if this was successful. - * An un-successful BAR TX would never call - * the BAR complete / timeout methods. - */ - ath_tx_tid_pause(sc, tid); - } else { - /* BAR TX failed */ - device_printf(sc->sc_dev, - "%s: TID %d: BAR TX failed\n", - __func__, tid->tid); - } + + /* XXX TODO: send BAR */ } else { ATH_TXQ_UNLOCK(sc->sc_ac2q[tid->ac]); } @@ -3138,7 +3103,7 @@ ath_tx_aggr_comp_aggr(struct ath_softc * * Anything after this point will not yet have been * TXed. */ - txseq = ni->ni_txseqs[tid]; + txseq = tap->txa_start; ATH_TXQ_UNLOCK(sc->sc_ac2q[atid->ac]); if (nframes != nf) @@ -3159,19 +3124,7 @@ ath_tx_aggr_comp_aggr(struct ath_softc * if (drops) { device_printf(sc->sc_dev, "%s: TID %d: send BAR; seq %d\n", __func__, tid, txseq); - if (ieee80211_send_bar(ni, tap, txseq) == 0) { - /* - * Pause the TID if this was successful. - * An un-successful BAR TX would never call - * the BAR complete / timeout methods. - */ - ath_tx_tid_pause(sc, atid); - } else { - /* BAR TX failed */ - device_printf(sc->sc_dev, - "%s: TID %d: BAR TX failed\n", - __func__, tid); - } + /* XXX TODO: send BAR */ } /* Prepend all frames to the beginning of the queue */