From owner-p4-projects@FreeBSD.ORG Sat Feb 9 23:01:53 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 38B2F16A41A; Sat, 9 Feb 2008 23:01:53 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D9BFA16A418 for ; Sat, 9 Feb 2008 23:01:52 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id CB7FF13C461 for ; Sat, 9 Feb 2008 23:01:52 +0000 (UTC) (envelope-from sam@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.1/8.14.1) with ESMTP id m19N1qT7005564 for ; Sat, 9 Feb 2008 23:01:52 GMT (envelope-from sam@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id m19N1qFZ005561 for perforce@freebsd.org; Sat, 9 Feb 2008 23:01:52 GMT (envelope-from sam@freebsd.org) Date: Sat, 9 Feb 2008 23:01:52 GMT Message-Id: <200802092301.m19N1qFZ005561@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to sam@freebsd.org using -f From: Sam Leffler To: Perforce Change Reviews Cc: Subject: PERFORCE change 135128 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 Feb 2008 23:01:53 -0000 http://perforce.freebsd.org/chv.cgi?CH=135128 Change 135128 by sam@sam_ebb on 2008/02/09 23:01:20 o move policy that determines when to enable ampdu to a new ic_ampdu_enable method so drivers can override (e.g. to incorporate rate control/link condition state) o pre-calculate the time to retry addba request and allow drivers to override the default setting Affected files ... .. //depot/projects/vap/sys/net80211/ieee80211_ddb.c#6 edit .. //depot/projects/vap/sys/net80211/ieee80211_ht.c#13 edit .. //depot/projects/vap/sys/net80211/ieee80211_ht.h#9 edit .. //depot/projects/vap/sys/net80211/ieee80211_output.c#30 edit .. //depot/projects/vap/sys/net80211/ieee80211_var.h#27 edit Differences ... ==== //depot/projects/vap/sys/net80211/ieee80211_ddb.c#6 (text+ko) ==== @@ -215,8 +215,8 @@ db_printf("%s token %u qbytes %d qframes %d seqstart %u start %u wnd %u\n", sep, tap->txa_token, tap->txa_qbytes, tap->txa_qframes, tap->txa_seqstart, tap->txa_start, tap->txa_wnd); - db_printf("%s attempts %d lastrequest %d\n", - sep, tap->txa_attempts, tap->txa_lastrequest); + db_printf("%s attempts %d nextrequest %d\n", + sep, tap->txa_attempts, tap->txa_nextrequest); /* XXX packet q + timer */ } ==== //depot/projects/vap/sys/net80211/ieee80211_ht.c#13 (text+ko) ==== @@ -105,6 +105,8 @@ } SYSINIT(wlan_ht, SI_SUB_DRIVERS, SI_ORDER_FIRST, ieee80211_ht_setup, NULL); +static int ieee80211_ampdu_enable(struct ieee80211_node *ni, + struct ieee80211_tx_ampdu *tap); static int ieee80211_addba_request(struct ieee80211_node *ni, struct ieee80211_tx_ampdu *tap, int dialogtoken, int baparamset, int batimeout); @@ -122,6 +124,7 @@ /* setup default aggregation policy */ ic->ic_recv_action = ieee80211_aggr_recv_action; ic->ic_send_action = ieee80211_send_action; + ic->ic_ampdu_enable = ieee80211_ampdu_enable; ic->ic_addba_request = ieee80211_addba_request; ic->ic_addba_response = ieee80211_addba_response; ic->ic_addba_stop = ieee80211_addba_stop; @@ -1244,7 +1247,7 @@ callout_reset(&tap->txa_timer, ieee80211_addba_timeout, addba_timeout, tap); tap->txa_flags |= IEEE80211_AGGR_XCHGPEND; - tap->txa_lastrequest = ticks; + tap->txa_nextrequest = ticks + ieee80211_addba_timeout; } static void @@ -1535,6 +1538,39 @@ */ /* + * Check if A-MPDU should be requested/enabled for a stream. + * We require a traffic rate above a per-AC threshold and we + * also handle backoff from previous failed attempts. + * + * Drivers may override this method to bring in information + * such as link state conditions in making the decision. + */ +static int +ieee80211_ampdu_enable(struct ieee80211_node *ni, + struct ieee80211_tx_ampdu *tap) +{ + struct ieee80211vap *vap = ni->ni_vap; + + if (tap->txa_avgpps < vap->iv_ampdu_mintraffic[tap->txa_ac]) + return 0; + /* XXX check rssi? */ + if (tap->txa_attempts >= ieee80211_addba_maxtries && + ticks < tap->txa_nextrequest) { + /* + * Don't retry too often; txa_nextrequest is set + * to the minimum interval we'll retry after + * ieee80211_addba_maxtries failed attempts are made. + */ + return 0; + } + IEEE80211_NOTE(vap, IEEE80211_MSG_11N, ni, + "%s: enable AMPDU on %s, avgpps %d pkts %d", + __func__, ieee80211_wme_acnames[tap->txa_ac], + tap->txa_avgpps, tap->txa_pkts); + return 1; +} + +/* * Request A-MPDU tx aggregation. Setup local state and * issue an ADDBA request. BA use will only happen after * the other end replies with ADDBA response. @@ -1555,24 +1591,9 @@ callout_init(&tap->txa_timer, CALLOUT_MPSAFE); tap->txa_flags |= IEEE80211_AGGR_SETUP; } - if (tap->txa_attempts >= ieee80211_addba_maxtries && - (ticks - tap->txa_lastrequest) < ieee80211_addba_backoff) { - /* - * Don't retry too often; ieee80211_addba_backoff - * defines the minimum interval we'll retry after - * ieee80211_addba_maxtries failed attempts to - * negotiate use. - */ - return 0; - } /* XXX hack for not doing proper locking */ tap->txa_flags &= ~IEEE80211_AGGR_NAK; - IEEE80211_NOTE(ni->ni_vap, IEEE80211_MSG_11N, ni, - "%s: enable AMPDU on %s, avgpps %d pkts %d", - __func__, ieee80211_wme_acnames[tap->txa_ac], - tap->txa_avgpps, tap->txa_pkts); - dialogtoken = (tokens+1) % 63; /* XXX */ tid = WME_AC_TO_TID(tap->txa_ac); @@ -1593,7 +1614,9 @@ __func__, tap->txa_ac); /* defer next try so we don't slam the driver with requests */ tap->txa_attempts = ieee80211_addba_maxtries; - tap->txa_lastrequest = ticks; + /* NB: check in case driver wants to override */ + if (tap->txa_nextrequest <= ticks) + tap->txa_nextrequest = ticks + ieee80211_addba_backoff; return 0; } tokens = dialogtoken; /* allocate token */ ==== //depot/projects/vap/sys/net80211/ieee80211_ht.h#9 (text+ko) ==== @@ -54,8 +54,8 @@ ieee80211_seq txa_seqstart; ieee80211_seq txa_start; uint16_t txa_wnd; /* BA window size */ - uint8_t txa_attempts; /* # setup attempts */ - int txa_lastrequest;/* time of last ADDBA request */ + uint8_t txa_attempts; /* # ADDBA requests w/o a response */ + int txa_nextrequest;/* soonest to make next ADDBA request */ struct ifqueue txa_q; /* packet queue */ struct callout txa_timer; void *txa_private; /* driver-private storage */ ==== //depot/projects/vap/sys/net80211/ieee80211_output.c#30 (text+ko) ==== @@ -1013,14 +1013,14 @@ /* * Check if A-MPDU tx aggregation is setup or if we * should try to enable it. The sta must be associated - * with HT and A-MPDU enabled for use. When traffic - * passes a threshold we issue an ADDBA request and - * wait for a reply. The frame being encapsulated - * will go out w/o using A-MPDU, or possibly it might - * be collected by the driver and held/retransmit. - * ieee80211_ampdu_request handles staggering requests - * in case the receiver NAK's us or we are otherwise - * unable to establish a BA stream. + * with HT and A-MPDU enabled for use. When the policy + * routine decides we should enable A-MPDU we issue an + * ADDBA request and wait for a reply. The frame being + * encapsulated will go out w/o using A-MPDU, or possibly + * it might be collected by the driver and held/retransmit. + * The default ic_ampdu_enable routine handles staggering + * ADDBA requests in case the receiver NAK's us or we are + * otherwise unable to establish a BA stream. */ if ((ni->ni_flags & IEEE80211_NODE_AMPDU_TX) && (vap->iv_flags_ext & IEEE80211_FEXT_AMPDU_TX)) { @@ -1033,7 +1033,7 @@ */ qos[0] |= IEEE80211_QOS_ACKPOLICY_BA; } else if (!IEEE80211_AMPDU_REQUESTED(tap) && - tap->txa_avgpps >= vap->iv_ampdu_mintraffic[tap->txa_ac]) { + ic->ic_ampdu_enable(ni, tap)) { /* * Not negotiated yet, request service. */ ==== //depot/projects/vap/sys/net80211/ieee80211_var.h#27 (text+ko) ==== @@ -249,6 +249,9 @@ int (*ic_send_action)(struct ieee80211_node *, int category, int action, uint16_t args[4]); + /* check if A-MPDU should be enabled this station+ac */ + int (*ic_ampdu_enable)(struct ieee80211_node *, + struct ieee80211_tx_ampdu *); /* start/stop doing A-MPDU tx aggregation for a station */ int (*ic_addba_request)(struct ieee80211_node *, struct ieee80211_tx_ampdu *,