Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 10 Mar 2012 04:14:04 +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: r232764 - head/sys/dev/ath
Message-ID:  <201203100414.q2A4E4n5039195@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: adrian
Date: Sat Mar 10 04:14:04 2012
New Revision: 232764
URL: http://svn.freebsd.org/changeset/base/232764

Log:
  Don't flood the cabq/mcastq with frames.
  
  In a very noisy 2.4GHz environment (with HT/40 enabled, making it worse)
  I saw the following occur:
  
  * the air was considered "busy" a lot of the time;
  * the cabq time is quite short due to staggered beacons being enabled;
  * it just wasn't able to keep up TX'ing CABQ frames;
  * .. and the cabq would swallow up all the TX ath_buf's.
  
  This patch introduces a twiddle which allows the maximum cabq depth to be
  set, forcing further frames to be dropped.
  
  It defaults to the TX buffer count at the moment, so the default behaviour
  isn't changed.
  
  I've also started fleshing out a similar setup for the data path, so
  it doesn't swallow up all the available TX buffers and preventing management
  frames (such as ADDBA) out.
  
  PR:		kern/165895

Modified:
  head/sys/dev/ath/if_ath.c
  head/sys/dev/ath/if_ath_sysctl.c
  head/sys/dev/ath/if_ath_tx.c
  head/sys/dev/ath/if_athioctl.h
  head/sys/dev/ath/if_athvar.h

Modified: head/sys/dev/ath/if_ath.c
==============================================================================
--- head/sys/dev/ath/if_ath.c	Sat Mar 10 04:02:52 2012	(r232763)
+++ head/sys/dev/ath/if_ath.c	Sat Mar 10 04:14:04 2012	(r232764)
@@ -637,6 +637,19 @@ ath_attach(u_int16_t devid, struct ath_s
 #endif
 
 	/*
+	 * TODO: enforce that at least this many frames are available
+	 * in the txbuf list before allowing data frames (raw or
+	 * otherwise) to be transmitted.
+	 */
+	sc->sc_txq_data_minfree = 10;
+	/*
+	 * Leave this as default to maintain legacy behaviour.
+	 * Shortening the cabq/mcastq may end up causing some
+	 * undesirable behaviour.
+	 */
+	sc->sc_txq_mcastq_maxdepth = ath_txbuf;
+
+	/*
 	 * Allow the TX and RX chainmasks to be overridden by
 	 * environment variables and/or device.hints.
 	 *

Modified: head/sys/dev/ath/if_ath_sysctl.c
==============================================================================
--- head/sys/dev/ath/if_ath_sysctl.c	Sat Mar 10 04:02:52 2012	(r232763)
+++ head/sys/dev/ath/if_ath_sysctl.c	Sat Mar 10 04:14:04 2012	(r232764)
@@ -605,6 +605,17 @@ ath_sysctlattach(struct ath_softc *sc)
 		"tid_hwq_hi", CTLFLAG_RW, &sc->sc_tid_hwq_hi, 0,
 		"");
 
+#if 0
+	SYSCTL_ADD_INT(ctx, SYSCTL_CHILDREN(tree), OID_AUTO,
+		"txq_data_minfree", CTLFLAG_RW, &sc->sc_txq_data_minfree,
+		0, "Minimum free buffers before adding a data frame"
+		" to the TX queue");
+#endif
+	SYSCTL_ADD_INT(ctx, SYSCTL_CHILDREN(tree), OID_AUTO,
+		"txq_mcastq_maxdepth", CTLFLAG_RW,
+		&sc->sc_txq_mcastq_maxdepth, 0,
+		"Maximum buffer depth for multicast/broadcast frames");
+
 #ifdef IEEE80211_SUPPORT_TDMA
 	if (ath_hal_macversion(ah) > 0x78) {
 		sc->sc_tdmadbaprep = 2;
@@ -885,7 +896,10 @@ ath_sysctl_stats_attach(struct ath_softc
 	    &sc->sc_stats.ast_rx_intr, 0, "RX interrupts");
 	SYSCTL_ADD_UINT(ctx, child, OID_AUTO, "ast_tx_intr", CTLFLAG_RD,
 	    &sc->sc_stats.ast_tx_intr, 0, "TX interrupts");
-
+	SYSCTL_ADD_UINT(ctx, child, OID_AUTO, "ast_tx_mcastq_overflow",
+	    CTLFLAG_RD, &sc->sc_stats.ast_tx_mcastq_overflow, 0,
+	    "Number of multicast frames exceeding maximum mcast queue depth");
+	
 	/* Attach the RX phy error array */
 	ath_sysctl_stats_attach_rxphyerr(sc, child);
 }

Modified: head/sys/dev/ath/if_ath_tx.c
==============================================================================
--- head/sys/dev/ath/if_ath_tx.c	Sat Mar 10 04:02:52 2012	(r232763)
+++ head/sys/dev/ath/if_ath_tx.c	Sat Mar 10 04:14:04 2012	(r232764)
@@ -1369,7 +1369,7 @@ ath_tx_start(struct ath_softc *sc, struc
 {
 	struct ieee80211vap *vap = ni->ni_vap;
 	struct ath_vap *avp = ATH_VAP(vap);
-	int r;
+	int r = 0;
 	u_int pri;
 	int tid;
 	struct ath_txq *txq;
@@ -1402,6 +1402,30 @@ ath_tx_start(struct ath_softc *sc, struc
 	type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK;
 	subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK;
 
+	/*
+	 * Enforce how deep the multicast queue can grow.
+	 *
+	 * XXX duplicated in ath_raw_xmit().
+	 */
+	if (IEEE80211_IS_MULTICAST(wh->i_addr1)) {
+		ATH_TXQ_LOCK(sc->sc_cabq);
+		ATH_TXQ_LOCK(&avp->av_mcastq);
+
+		if ((sc->sc_cabq->axq_depth + avp->av_mcastq.axq_depth) >
+		    sc->sc_txq_mcastq_maxdepth) {
+			sc->sc_stats.ast_tx_mcastq_overflow++;
+			r = ENOBUFS;
+		}
+
+		ATH_TXQ_UNLOCK(&avp->av_mcastq);
+		ATH_TXQ_UNLOCK(sc->sc_cabq);
+
+		if (r != 0) {
+			m_freem(m0);
+			return r;
+		}
+	}
+
 	/* A-MPDU TX */
 	is_ampdu_tx = ath_tx_ampdu_running(sc, ATH_NODE(ni), tid);
 	is_ampdu_pending = ath_tx_ampdu_pending(sc, ATH_NODE(ni), tid);
@@ -1734,7 +1758,10 @@ ath_raw_xmit(struct ieee80211_node *ni, 
 	struct ifnet *ifp = ic->ic_ifp;
 	struct ath_softc *sc = ifp->if_softc;
 	struct ath_buf *bf;
-	int error;
+	struct ieee80211_frame *wh = mtod(m, struct ieee80211_frame *);
+	struct ieee80211vap *vap = ni->ni_vap;
+	struct ath_vap *avp = ATH_VAP(vap);
+	int error = 0;
 
 	ATH_PCU_LOCK(sc);
 	if (sc->sc_inreset_cnt > 0) {
@@ -1755,6 +1782,31 @@ ath_raw_xmit(struct ieee80211_node *ni, 
 		error = ENETDOWN;
 		goto bad;
 	}
+
+	/*
+	 * Enforce how deep the multicast queue can grow.
+	 *
+	 * XXX duplicated in ath_tx_start().
+	 */
+	if (IEEE80211_IS_MULTICAST(wh->i_addr1)) {
+		ATH_TXQ_LOCK(sc->sc_cabq);
+		ATH_TXQ_LOCK(&avp->av_mcastq);
+
+		if ((sc->sc_cabq->axq_depth + avp->av_mcastq.axq_depth) >
+		    sc->sc_txq_mcastq_maxdepth) {
+			sc->sc_stats.ast_tx_mcastq_overflow++;
+			error = ENOBUFS;
+		}
+
+		ATH_TXQ_UNLOCK(&avp->av_mcastq);
+		ATH_TXQ_UNLOCK(sc->sc_cabq);
+
+		if (error != 0) {
+			m_freem(m);
+			goto bad;
+		}
+	}
+
 	/*
 	 * Grab a TX buffer and associated resources.
 	 */

Modified: head/sys/dev/ath/if_athioctl.h
==============================================================================
--- head/sys/dev/ath/if_athioctl.h	Sat Mar 10 04:02:52 2012	(r232763)
+++ head/sys/dev/ath/if_athioctl.h	Sat Mar 10 04:14:04 2012	(r232764)
@@ -155,7 +155,8 @@ struct ath_stats {
 	u_int32_t	ast_rx_intr;
 	u_int32_t	ast_tx_aggr_ok;		/* aggregate TX ok */
 	u_int32_t	ast_tx_aggr_fail;	/* aggregate TX failed */
-	u_int32_t	ast_pad[2];
+	u_int32_t	ast_tx_mcastq_overflow;	/* multicast queue overflow */
+	u_int32_t	ast_pad[1];
 };
 
 #define	SIOCGATHSTATS	_IOWR('i', 137, struct ifreq)

Modified: head/sys/dev/ath/if_athvar.h
==============================================================================
--- head/sys/dev/ath/if_athvar.h	Sat Mar 10 04:02:52 2012	(r232763)
+++ head/sys/dev/ath/if_athvar.h	Sat Mar 10 04:14:04 2012	(r232764)
@@ -530,6 +530,31 @@ struct ath_softc {
 	int			sc_txchainmask;	/* currently configured TX chainmask */
 	int			sc_rxchainmask;	/* currently configured RX chainmask */
 
+	/* Queue limits */
+
+	/*
+	 * To avoid queue starvation in congested conditions,
+	 * these parameters tune the maximum number of frames
+	 * queued to the data/mcastq before they're dropped.
+	 *
+	 * This is to prevent:
+	 * + a single destination overwhelming everything, including
+	 *   management/multicast frames;
+	 * + multicast frames overwhelming everything (when the
+	 *   air is sufficiently busy that cabq can't drain.)
+	 *
+	 * These implement:
+	 * + data_minfree is the maximum number of free buffers
+	 *   overall to successfully allow a data frame.
+	 *
+	 * + mcastq_maxdepth is the maximum depth allowe dof the avp+cabq
+	 *   queue.  The avp is included in each comparison just to be
+	 *   a little overly conservative and this may end up being
+	 *   unhelpful with multiple VAPs.
+	 */
+	int			sc_txq_data_minfree;
+	int			sc_txq_mcastq_maxdepth;
+
 	/*
 	 * Aggregation twiddles
 	 *



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