From owner-svn-src-user@FreeBSD.ORG Sun Aug 14 16:03:26 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 97CB0106566B; Sun, 14 Aug 2011 16:03:26 +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 86F8D8FC13; Sun, 14 Aug 2011 16:03:26 +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 p7EG3Qcb070596; Sun, 14 Aug 2011 16:03:26 GMT (envelope-from adrian@svn.freebsd.org) Received: (from adrian@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p7EG3QtO070594; Sun, 14 Aug 2011 16:03:26 GMT (envelope-from adrian@svn.freebsd.org) Message-Id: <201108141603.p7EG3QtO070594@svn.freebsd.org> From: Adrian Chadd Date: Sun, 14 Aug 2011 16:03:26 +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: r224867 - 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: Sun, 14 Aug 2011 16:03:26 -0000 Author: adrian Date: Sun Aug 14 16:03:26 2011 New Revision: 224867 URL: http://svn.freebsd.org/changeset/base/224867 Log: Begin to attempt to handle sending BAR frames. This doesn't work yet and will panic the kernel. BAR frames should be generated to resync the remote side with where the block-ack window left edge is - eg, after some packet loss. The problem here is that said BAR frames will be generated from: * the TX packet completion handler; and * via a timer callout if the BAR TX wasn't successful. The former will occur with the hardware TXQ lock held, as that's currently how I've implemented the locking (ie, the hardware TXQ protects the TXQ and the TID state.) This results in lock recursion (as we're recursing back through net80211 and then back into the driver via ic->ic_raw_xmit()) and thus TX'ing a BAR frame will very much fail. The problem here (among other things) is that TX'ing a BAR frame should occur after all the hardware-queued frames for that AC, in case there's some other packets (which may or may not be successfully transmitted in the future), and in the same hardware queue. This means we can't just use the normal software queue method, as the TID should be paused when a BAR frame is TX'ed. Hence, the direct dispatch to hardware. Anyway, this is all quite messy. I think I'm going to have to bite the bullet and (re) introduce more fine-grained locking in a subsequent commit before supporting this in a clean fashion is even possible. (The other option is to modify the ieee80211_send_bar()) function to just schedule a callback even and have the TX occur from there, rather than recurse through the driver. I may end up doing that at a later date, but I really need to fix the locking anyway. So, I'll just do that first.) 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 Sun Aug 14 14:36:32 2011 (r224866) +++ user/adrian/if_ath_tx/sys/dev/ath/if_ath_tx.c Sun Aug 14 16:03:26 2011 (r224867) @@ -1010,7 +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; + uint8_t type, subtype; /* * Determine the target hardware queue. @@ -1032,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); + type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK; subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK; /* A-MPDU TX */ @@ -1097,9 +1098,38 @@ ath_tx_start(struct ath_softc *sc, struc * destination hardware queue. Don't bother software * queuing it. */ + /* + * If it's a BAR frame, do a direct dispatch to the + * destination hardware queue. Don't bother software + * queuing it, as the TID will now be paused. + */ if (ismcast) ath_tx_handoff_mcast(sc, txq, bf); - else { + else if (type == IEEE80211_FC0_TYPE_CTL && + subtype == IEEE80211_FC0_SUBTYPE_BAR) { + /* + * XXX The following is dirty but needed for now. + * + * Because of the current way locking is implemented, + * we may end up here because of a call to + * ieee80211_send_bar() from a call inside the completion + * handler. This will have the hardware TXQ locked, + * protecting the TXQ and the node TID state in question. + * + * So we (a) can't SWQ queue to it, as it'll be queued + * on the same TID which will be paused, and (b) the TXQ + * will be locked anyway, so grabbing the lock will cause + * recursion. + * + * The longer term issue is that the TXQ lock is being held + * for so damned long, and that must be addressed before this + * stuff is merged into -HEAD. + */ + device_printf(sc->sc_dev, "%s: BAR: TX'ing direct\n", __func__); + ATH_TXQ_LOCK(txq); + ath_tx_handoff(sc, txq, bf); + ATH_TXQ_UNLOCK(txq); + } else { ATH_TXQ_LOCK(txq); /* add to software queue */ ath_tx_swq(sc, ni, txq, bf); @@ -1108,15 +1138,16 @@ ath_tx_start(struct ath_softc *sc, struc ATH_TXQ_UNLOCK(txq); } #else - /* * For now, since there's no software queue, * direct-dispatch to the hardware. */ + ATH_TXQ_LOCK(txq); if (ismcast) ath_tx_handoff_mcast(sc, txq, bf); else ath_tx_handoff_hw(sc, txq, bf); + ATH_TXQ_UNLOCK(txq); #endif return 0; @@ -2093,9 +2124,12 @@ ath_tx_aggr_retry_unaggr(struct ath_soft int tid = bf->bf_state.bfs_tid; struct ath_tid *atid = &an->an_tid[tid]; struct ath_txq *txq = sc->sc_ac2q[atid->ac]; + struct ieee80211_tx_ampdu *tap; ATH_TXQ_LOCK_ASSERT(txq); + tap = ath_tx_get_tx_tid(an, tid); + if (bf->bf_state.bfs_retries >= SWMAX_RETRIES) { sc->sc_stats.ast_tx_swretrymax++; @@ -2107,8 +2141,18 @@ ath_tx_aggr_retry_unaggr(struct ath_soft /* Pause the TID */ /* Send BAR frame */ + /* + * XXX This causes the kernel to recurse + * XXX back into the net80211 layer, then back out + * XXX to the TX queue (via ic->ic_raw_xmit()). + * XXX Because the TXQ lock is held here, + * XXX this will cause a lock recurse panic. + */ device_printf(sc->sc_dev, "%s: TID %d: send BAR\n", __func__, tid); +#if 0 + ieee80211_send_bar(ni, tap, ni->ni_txseqs[tid]); +#endif /* Free buffer, bf is free after this call */ ath_tx_default_comp(sc, bf, 0);