Date: Mon, 26 Mar 2012 16:10:11 GMT From: dfilter@FreeBSD.ORG (dfilter service) To: freebsd-wireless@FreeBSD.org Subject: Re: kern/166357: commit references a PR Message-ID: <201203261610.q2QGABvG001884@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/166357; it has been noted by GNATS. From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: kern/166357: commit references a PR Date: Mon, 26 Mar 2012 16:05:38 +0000 (UTC) Author: adrian Date: Mon Mar 26 16:05:19 2012 New Revision: 233514 URL: http://svn.freebsd.org/changeset/base/233514 Log: Use the assigned sequence number when checking if a retried packet is within the BAW. This regression was introduced in ane earlier commit by me to fix the BAW seqno allocation-but-not-insertion-into-BAW race. Since it was only ever using the to-be allocated sequence number, any frame retries with the first frame in the BAW still in the software queue would have constantly failed, as ni_txseqs[tid] would always be outside the BAW. TODO: * Extract out the mostly common code here in the agg and non-agg ADDBA case and stuff it into a single function. PR: kern/166357 Modified: head/sys/dev/ath/if_ath_tx.c head/sys/dev/ath/if_ath_tx_ht.c Modified: head/sys/dev/ath/if_ath_tx.c ============================================================================== --- head/sys/dev/ath/if_ath_tx.c Mon Mar 26 15:38:17 2012 (r233513) +++ head/sys/dev/ath/if_ath_tx.c Mon Mar 26 16:05:19 2012 (r233514) @@ -2348,7 +2348,6 @@ ath_tx_xmit_aggr(struct ath_softc *sc, s struct ath_tid *tid = &an->an_tid[bf->bf_state.bfs_tid]; struct ath_txq *txq = bf->bf_state.bfs_txq; struct ieee80211_tx_ampdu *tap; - int seqno; ATH_TXQ_LOCK_ASSERT(txq); @@ -2384,13 +2383,31 @@ ath_tx_xmit_aggr(struct ath_softc *sc, s * the TIDs that map to it. Ugh. */ if (bf->bf_state.bfs_dobaw) { - if (! BAW_WITHIN(tap->txa_start, tap->txa_wnd, - ni->ni_txseqs[bf->bf_state.bfs_tid])) { + ieee80211_seq seqno; + + /* + * If the sequence number is allocated, use it. + * Otherwise, use the sequence number we WOULD + * allocate. + */ + if (bf->bf_state.bfs_seqno_assigned) + seqno = SEQNO(bf->bf_state.bfs_seqno); + else + seqno = ni->ni_txseqs[bf->bf_state.bfs_tid]; + + /* + * Check whether either the currently allocated + * sequence number _OR_ the to-be allocated + * sequence number is inside the BAW. + */ + if (! BAW_WITHIN(tap->txa_start, tap->txa_wnd, seqno)) { ATH_TXQ_INSERT_TAIL(tid, bf, bf_list); ath_tx_tid_sched(sc, tid); return; } if (! bf->bf_state.bfs_seqno_assigned) { + int seqno; + seqno = ath_tx_tid_seqno_assign(sc, ni, bf, bf->bf_m); if (seqno < 0) { device_printf(sc->sc_dev, Modified: head/sys/dev/ath/if_ath_tx_ht.c ============================================================================== --- head/sys/dev/ath/if_ath_tx_ht.c Mon Mar 26 15:38:17 2012 (r233513) +++ head/sys/dev/ath/if_ath_tx_ht.c Mon Mar 26 16:05:19 2012 (r233514) @@ -652,7 +652,6 @@ ath_tx_form_aggr(struct ath_softc *sc, s int status = ATH_AGGR_DONE; int prev_frames = 0; /* XXX for AR5416 burst, not done here */ int prev_al = 0; /* XXX also for AR5416 burst */ - int seqno; ATH_TXQ_LOCK_ASSERT(sc->sc_ac2q[tid->ac]); @@ -747,13 +746,32 @@ ath_tx_form_aggr(struct ath_softc *sc, s * see ath_tx_xmit_aggr() for more info. */ if (bf->bf_state.bfs_dobaw) { + ieee80211_seq seqno; + + /* + * If the sequence number is allocated, use it. + * Otherwise, use the sequence number we WOULD + * allocate. + */ + if (bf->bf_state.bfs_seqno_assigned) + seqno = SEQNO(bf->bf_state.bfs_seqno); + else + seqno = ni->ni_txseqs[bf->bf_state.bfs_tid]; + + /* + * Check whether either the currently allocated + * sequence number _OR_ the to-be allocated + * sequence number is inside the BAW. + */ if (! BAW_WITHIN(tap->txa_start, tap->txa_wnd, - ni->ni_txseqs[bf->bf_state.bfs_tid])) { + seqno)) { status = ATH_AGGR_BAW_CLOSED; break; } + /* XXX check for bfs_need_seqno? */ if (! bf->bf_state.bfs_seqno_assigned) { + int seqno; seqno = ath_tx_tid_seqno_assign(sc, ni, bf, bf->bf_m); if (seqno < 0) { device_printf(sc->sc_dev, _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203261610.q2QGABvG001884>