Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 8 Apr 2014 07:08:59 +0000 (UTC)
From:      Adrian Chadd <adrian@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r264254 - head/sys/dev/ath
Message-ID:  <201404080708.s3878xTb068239@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Tue Apr  8 07:08:59 2014
New Revision: 264254
URL: http://svnweb.freebsd.org/changeset/base/264254

Log:
  Correct the actual definition of ath_tx_tid_filt_comp_single() to
  match how it's used.
  
  This is another bug that led to aggregate traffic hanging because
  the BAW tracking stopped being accurate.  In this instance, a filtered
  frame that exceeded retries would return a non-error, which would
  mean the caller would never remove it from the BAW.  But it wouldn't
  be added to the filtered list, so it would be lost forever.  There'd
  thus be a hole in the BAW that would never get transmitted and
  this leads to a traffic hang.
  
  Tested:
  
  * Routerstation Pro, AR9220 AP

Modified:
  head/sys/dev/ath/if_ath_tx.c

Modified: head/sys/dev/ath/if_ath_tx.c
==============================================================================
--- head/sys/dev/ath/if_ath_tx.c	Tue Apr  8 07:01:27 2014	(r264253)
+++ head/sys/dev/ath/if_ath_tx.c	Tue Apr  8 07:08:59 2014	(r264254)
@@ -3388,10 +3388,13 @@ ath_tx_tid_filt_comp_complete(struct ath
 /*
  * Called when a single (aggregate or otherwise) frame is completed.
  *
- * Returns 1 if the buffer could be added to the filtered list
- * (cloned or otherwise), 0 if the buffer couldn't be added to the
+ * Returns 0 if the buffer could be added to the filtered list
+ * (cloned or otherwise), 1 if the buffer couldn't be added to the
  * filtered list (failed clone; expired retry) and the caller should
  * free it and handle it like a failure (eg by sending a BAR.)
+ *
+ * since the buffer may be cloned, bf must be not touched after this
+ * if the return value is 0.
  */
 static int
 ath_tx_tid_filt_comp_single(struct ath_softc *sc, struct ath_tid *tid,
@@ -3412,7 +3415,8 @@ ath_tx_tid_filt_comp_single(struct ath_s
 		    __func__,
 		    bf,
 		    bf->bf_state.bfs_seqno);
-		return (0);
+		retval = 1; /* error */
+		goto finish;
 	}
 
 	/*
@@ -3432,11 +3436,12 @@ ath_tx_tid_filt_comp_single(struct ath_s
 		DPRINTF(sc, ATH_DEBUG_SW_TX_FILT,
 		    "%s: busy buffer couldn't be cloned (%p)!\n",
 		    __func__, bf);
-		retval = 1;
+		retval = 1; /* error */
 	} else {
 		ath_tx_tid_filt_comp_buf(sc, tid, nbf);
-		retval = 0;
+		retval = 0; /* ok */
 	}
+finish:
 	ath_tx_tid_filt_comp_complete(sc, tid);
 
 	return (retval);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201404080708.s3878xTb068239>