kern/168170: commit references a PR

[dfilter service]

The following reply was made to PR kern/168170; it has been noted by GNATS.

From: dfilter at FreeBSD.ORG (dfilter service)
To: bug-followup at FreeBSD.org
Cc:  
Subject: Re: kern/168170: commit references a PR
Date: Tue, 22 May 2012 19:37:22 +0000 (UTC)

 Author: adrian
 Date: Tue May 22 19:37:12 2012
 New Revision: 235801
 URL: http://svn.freebsd.org/changeset/base/235801
 
 Log:
   Fix some corner cases in the ieee80211_send_bar() handling.
   
   * If the first call succeeded but failed to transmit, a timer would
     reschedule it via bar_timeout().  Unfortunately bar_timeout() didn't
     check the return value from the ieee80211_send_bar() reattempt and
     if that failed (eg the driver ic_raw_xmit() failed), it would never
     re-arm the timer.
   
   * If BARPEND is cleared (which ieee80211_send_bar() will do if it can't
     TX), then re-arming the timer isn't enough - once bar_timeout() occurs,
     it'll see BARPEND is 0 and not run through the rest of the routine.
     So when rearming the timer, also set that flag.
   
   * If the TX wasn't occuring, bar_tx_complete() wouldn't be called and the
     driver callback wouldn't be called either.  So the driver had no idea
     that the BAR TX attempt had failed.  In the ath(4) case, TX would stay
     paused.
   
     (There's no callback to indicate that BAR TX had failed or not;
     only a "BAR TX was attempted".  That's a separate, later problem.)
   
     So call the driver callback (ic_bar_response()) before the ADDBA session
     is torn down, so it has a chance of being notified that things didn't
     quite go to plan.
   
   I've verified that yes, this does suspend traffic for ath(4), retry BAR
   TX even if the driver is failing ic_raw_xmit(), and then eventually giving
   up and sending a DELBA.  I'll address the "out of ath_buf" issue in ath(4)
   in a subsequent commit - this commit just fixes the edge case where any
   driver is (way) out of internal buffers/descriptors and fails frame TX.
   
   PR:		kern/168170
   Reviewed by:	bschmidt
   MFC after:	1 month
 
 Modified:
   head/sys/net80211/ieee80211_ht.c
 
 Modified: head/sys/net80211/ieee80211_ht.c
 ==============================================================================
 --- head/sys/net80211/ieee80211_ht.c	Tue May 22 18:31:56 2012	(r235800)
 +++ head/sys/net80211/ieee80211_ht.c	Tue May 22 19:37:12 2012	(r235801)
 @@ -2166,6 +2166,9 @@ ieee80211_ampdu_stop(struct ieee80211_no
  	}
  }
  
 +/* XXX */
 +static void bar_start_timer(struct ieee80211_tx_ampdu *tap);
 +
  static void
  bar_timeout(void *arg)
  {
 @@ -2184,11 +2187,34 @@ bar_timeout(void *arg)
  		return;
  	/* XXX ? */
  	if (tap->txa_attempts >= ieee80211_bar_maxtries) {
 +		struct ieee80211com *ic = ni->ni_ic;
 +
  		ni->ni_vap->iv_stats.is_ampdu_bar_tx_fail++;
 +		/*
 +		 * If (at least) the last BAR TX timeout was due to
 +		 * an ieee80211_send_bar() failures, then we need
 +		 * to make sure we notify the driver that a BAR
 +		 * TX did occur and fail.  This gives the driver
 +		 * a chance to undo any queue pause that may
 +		 * have occured.
 +		 */
 +		ic->ic_bar_response(ni, tap, 1);
  		ieee80211_ampdu_stop(ni, tap, IEEE80211_REASON_TIMEOUT);
  	} else {
  		ni->ni_vap->iv_stats.is_ampdu_bar_tx_retry++;
 -		ieee80211_send_bar(ni, tap, tap->txa_seqpending);
 +		if (ieee80211_send_bar(ni, tap, tap->txa_seqpending) != 0) {
 +			/*
 +			 * If ieee80211_send_bar() fails here, the
 +			 * timer may have stopped and/or the pending
 +			 * flag may be clear.  Because of this,
 +			 * fake the BARPEND and reset the timer.
 +			 * A retransmission attempt will then occur
 +			 * during the next timeout.
 +			 */
 +			/* XXX locking */
 +			tap->txa_flags |= IEEE80211_AGGR_BARPEND;
 +			bar_start_timer(tap);
 +		}
  	}
  }
  
 _______________________________________________
 svn-src-all at freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/svn-src-all
 To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"