From owner-freebsd-wireless@FreeBSD.ORG Tue May 22 06:36:56 2012 Return-Path: Delivered-To: freebsd-wireless@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 4DADF106566B for ; Tue, 22 May 2012 06:36:56 +0000 (UTC) (envelope-from adrian.chadd@gmail.com) Received: from mail-pz0-f54.google.com (mail-pz0-f54.google.com [209.85.210.54]) by mx1.freebsd.org (Postfix) with ESMTP id 2290E8FC12 for ; Tue, 22 May 2012 06:36:56 +0000 (UTC) Received: by dadv36 with SMTP id v36so8429899dad.13 for ; Mon, 21 May 2012 23:36:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:date:x-google-sender-auth:message-id:subject :from:to:content-type; bh=wpxB3mcmKPAdl4P4M3z+5+jsNxAf8f01CXtbx1aat+c=; b=vALQVsKp1WsbXN1+t80kuFqXIbiobqgTCDCUiQump+PjmeXIl4kBzFzBSaKmr5Csk4 9HYltNZTOozM/pkZy3Pwy8MzDfRq/uFDLSWYwMAB66QZSaetetJx588lWV2wC3E7p7v5 MntMs48mf2auWduC/KajpGHnXsJXrqglwFM45wQVRRXTbp7XvaQRYW0Ah5Qo41k3J7VP qeBwYt1TQDLkjoPbNzCI52OSob57jPMrS0c+r/ROLy11AJLy5OPSv4276RSNY5Cw0Kpd g0nfZ8PksAkQo87FS+AbzOCSHVAQZjNu9UtRlJ74J9dau5YmrhZFFt+KHw5bnr5iGzXZ 59KQ== MIME-Version: 1.0 Received: by 10.68.234.35 with SMTP id ub3mr76945310pbc.8.1337668609951; Mon, 21 May 2012 23:36:49 -0700 (PDT) Sender: adrian.chadd@gmail.com Received: by 10.142.203.2 with HTTP; Mon, 21 May 2012 23:36:49 -0700 (PDT) Date: Mon, 21 May 2012 23:36:49 -0700 X-Google-Sender-Auth: -L8A8EgE9zZ9zCftrKo4_jfFo0U Message-ID: From: Adrian Chadd To: freebsd-wireless@freebsd.org Content-Type: text/plain; charset=ISO-8859-1 Subject: [net80211] Fix some corner cases in BAR TX handling X-BeenThere: freebsd-wireless@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Discussions of 802.11 stack, tools device driver development." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 22 May 2012 06:36:56 -0000 Hi, This patch fixes a couple of rather annoying corner cases that I've seen with BAR TX handling in ath(4). They showed up because I am using a very cut down set of buffers in ath(4) (128 tx and 128 rx) rather than the older number (512) and I can easily exhaust them whilst the TID is paused. I'll soon implement some changes which will leave a "reserved" set of frames for management - and if I can line all the stars up correctly, have the management frames go out on the highest priority TX queue. Specifically: * ieee80211_send_bar() disables the timer and doesn't restart it unless ic_raw_xmit() suceeds; * if the TX fails and the timeout occurs via bar_timeout(), it'll try calling ieee80211_send_bar() to retransmit - but if that fails, the timer will never occur. * Callbacks are only done via bar_tx_complete(), which is not called if ieee80211_send_bar() fails to queue any frames to the driver. * If the timer does get started again, BARPEND may have been cleared (due to a failed ieee80211_send_bar() attempt to call ic_raw_xmit()) and it won't actually be processed, causing the attempts to fail. Again. So to maintain some semblence of predicable behaviour, I've done the following: * The ic_bar_response() handler is called just before AMPDU is disabled, giving the driver a chance to be told that the max number of retries has been reached. I don't like how this is the only real way to know that the BAR TX attempt has totally failed; I'll worry about that later. * If a retransmit attempt fails, set BARPEND and restart the timer. I've verified that this fixes all the weird BAR TX stalls I've seen. Thanks, Adrian Index: net80211/ieee80211_ht.c =================================================================== --- net80211/ieee80211_ht.c (revision 235772) +++ net80211/ieee80211_ht.c (working copy) @@ -2166,6 +2166,9 @@ } } +/* XXX */ +static void bar_start_timer(struct ieee80211_tx_ampdu *tap); + static void bar_timeout(void *arg) { @@ -2184,11 +2187,34 @@ 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); + } } }