From owner-p4-projects@FreeBSD.ORG Fri Feb 15 05:19:33 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id A487016A421; Fri, 15 Feb 2008 05:19:33 +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 6758E16A41A; Fri, 15 Feb 2008 05:19:33 +0000 (UTC) (envelope-from sam@errno.com) Received: from ebb.errno.com (ebb.errno.com [69.12.149.25]) by mx1.freebsd.org (Postfix) with ESMTP id 07D9613C442; Fri, 15 Feb 2008 05:19:32 +0000 (UTC) (envelope-from sam@errno.com) Received: from Macintosh-2.local ([10.0.0.196]) (authenticated bits=0) by ebb.errno.com (8.13.6/8.12.6) with ESMTP id m1F5JW1d013082 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 14 Feb 2008 21:19:32 -0800 (PST) (envelope-from sam@errno.com) Message-ID: <47B520E4.4000604@errno.com> Date: Thu, 14 Feb 2008 21:19:32 -0800 From: Sam Leffler Organization: Errno Consulting User-Agent: Thunderbird 2.0.0.9 (Macintosh/20071031) MIME-Version: 1.0 To: Sepherosa Ziehau References: <200802150450.m1F4oGPj000909@repoman.freebsd.org> In-Reply-To: <200802150450.m1F4oGPj000909@repoman.freebsd.org> X-Enigmail-Version: 0.95.6 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-DCC--Metrics: ebb.errno.com; whitelist Cc: Perforce Change Reviews Subject: Re: PERFORCE change 135423 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: Fri, 15 Feb 2008 05:19:33 -0000 Sepherosa Ziehau wrote: > http://perforce.freebsd.org/chv.cgi?CH=135423 > > Change 135423 by sephe@sephe_zealot:sam_wifi on 2008/02/15 04:49:16 > > Don't include multicast packets into TX rate control calculation. > > Noticed by: sam > Obtained from: DragonFly I didn't get this from dfly. Also, my change handled ACM which yours cannot if you are basing decisions on mcast. Finally you appear to be freeing the node in the start path instead of on tx done which is wrong--you must keep the node around to do things like TXCB. I suggest you just apply my patch. Sam > > Affected files ... > > .. //depot/projects/wifi/sys/dev/bwi/if_bwi.c#16 edit > > Differences ... > > ==== //depot/projects/wifi/sys/dev/bwi/if_bwi.c#16 (text+ko) ==== > > @@ -32,7 +32,6 @@ > * SUCH DAMAGE. > * > * $DragonFly: src/sys/dev/netif/bwi/if_bwi.c,v 1.18 2008/01/15 09:01:13 sephe Exp $ > - * 1.8 is not merged > */ > > #include > @@ -118,7 +117,7 @@ > static void bwi_stop(struct bwi_softc *); > static int bwi_newbuf(struct bwi_softc *, int, int); > static int bwi_encap(struct bwi_softc *, int, struct mbuf *, > - struct ieee80211_node *); > + struct ieee80211_node **); > > static void bwi_init_rxdesc_ring32(struct bwi_softc *, uint32_t, > bus_addr_t, int, int); > @@ -1383,7 +1382,7 @@ > } > wh = NULL; /* Catch any invalid use */ > > - if (bwi_encap(sc, idx, m, ni) != 0) { > + if (bwi_encap(sc, idx, m, &ni) != 0) { > /* 'm' is freed in bwi_encap() if we reach here */ > if (ni != NULL) > ieee80211_free_node(ni); > @@ -2922,9 +2921,10 @@ > > static int > bwi_encap(struct bwi_softc *sc, int idx, struct mbuf *m, > - struct ieee80211_node *ni) > + struct ieee80211_node **ni0) > { > struct ieee80211com *ic = &sc->sc_ic; > + struct ieee80211_node *ni = *ni0; > struct bwi_ring_data *rd = &sc->sc_tx_rdata[BWI_TX_DATA_RING]; > struct bwi_txbuf_data *tbd = &sc->sc_tx_bdata[BWI_TX_DATA_RING]; > struct bwi_txbuf *tb = &tbd->tbd_buf[idx]; > @@ -2935,7 +2935,7 @@ > uint32_t mac_ctrl; > uint16_t phy_ctrl; > bus_addr_t paddr; > - int pkt_len, error; > + int pkt_len, error, mcast_pkt = 0; > #if 0 > const uint8_t *p; > int i; > @@ -2954,9 +2954,10 @@ > * Find TX rate > */ > bzero(tb->tb_rate_idx, sizeof(tb->tb_rate_idx)); > - if (IEEE80211_IS_MULTICAST(wh->i_addr1)) > + if (IEEE80211_IS_MULTICAST(wh->i_addr1)) { > rate = rate_fb = ic->ic_mcast_rate; > - else if (ic->ic_fixed_rate == IEEE80211_FIXED_RATE_NONE) { > + mcast_pkt = 1; > + } else if (ic->ic_fixed_rate == IEEE80211_FIXED_RATE_NONE) { > rate = ni->ni_rates.rs_rates[ni->ni_txrate] & > IEEE80211_RATE_VAL; > rate_fb = (ni->ni_txrate > 0) ? > @@ -2999,7 +3000,7 @@ > bcopy(wh->i_fc, hdr->txh_fc, sizeof(hdr->txh_fc)); > bcopy(wh->i_addr1, hdr->txh_addr1, sizeof(hdr->txh_addr1)); > > - if (!IEEE80211_IS_MULTICAST(wh->i_addr1)) { > + if (!mcast_pkt) { > uint16_t dur; > uint8_t ack_rate; > > @@ -3070,6 +3071,11 @@ > > bus_dmamap_sync(sc->sc_buf_dtag, tb->tb_dmap, BUS_DMASYNC_PREWRITE); > > + if (mcast_pkt) { > + /* Don't involve mcast packets into TX rate control */ > + ieee80211_free_node(ni); > + *ni0 = ni = NULL; > + } > tb->tb_mbuf = m; > tb->tb_ni = ni; > > >