Date: Thu, 14 Feb 2008 21:19:32 -0800 From: Sam Leffler <sam@errno.com> To: Sepherosa Ziehau <sephe@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 135423 for review Message-ID: <47B520E4.4000604@errno.com> In-Reply-To: <200802150450.m1F4oGPj000909@repoman.freebsd.org> References: <200802150450.m1F4oGPj000909@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <sys/cdefs.h> > @@ -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; > > >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?47B520E4.4000604>