Hi! Is it possible to get this fix into FreeBSD 7.3-RELEASE?
Without this fix I have serious issues with my bge NICs: - packet loss around 10%. - transferrate (scp) is 10% of expected, 2 MB/sec instead of 20 when testing. - get "Corrupted MAC on input" while synchronizing large directories using rsync over SSH. It seems we also discussed this matter on -fs, where I had issues on a ZFS+NFS file server. After updating to a newer 8.1 install (which has the fix) the problem went away. If this can not make it into 7.3, I and other people using 7.x have to manually patch this or follow RELENG_7 which not everyone wants to. The bge driver is such a common server driver, I think this should be rolled back into affected releases. Regards, Anders. On Thu, Jun 10, 2010 at 06:04:25PM +0000, Pyun YongHyeon wrote: > Author: yongari > Date: Thu Jun 10 18:04:25 2010 > New Revision: 208995 > URL: http://svn.freebsd.org/changeset/base/208995 > > Log: > MFC r208862: > Fix a bug introduced in r199011. When bge(4) reuses loaded RX > buffers it should also reinitialize RX descriptors otherwise some > stale data could be passed to controller. This could end up with > mbuf double free or unexpected NULL pointer dereference in upper > stack. To fix the issue, save loaded buffer's length and > reinitialize RX descriptors with the saved value whenever bge(4) > reuses the loaded RX buffers. > While I'm here, increase the number of RX buffers to 512 from 256. > This simplifies RX buffer handling as well as giving more RX > buffers. Controller supports just fixed number of RX buffers > (i.e. 512) and bge(4) used to rely on hope that our CPU is fast > enough to keep up with the controller. With this change, bge(4) > will use 1MB for RX buffers but I don't think it would cause > problems in these days. > > Reported by: marcel > Tested by: marcel > > Modified: > stable/7/sys/dev/bge/if_bge.c > stable/7/sys/dev/bge/if_bgereg.h > Directory Properties: > stable/7/sys/ (props changed) > stable/7/sys/cddl/contrib/opensolaris/ (props changed) > stable/7/sys/contrib/dev/acpica/ (props changed) > stable/7/sys/contrib/pf/ (props changed) > > Modified: stable/7/sys/dev/bge/if_bge.c > ============================================================================== > --- stable/7/sys/dev/bge/if_bge.c Thu Jun 10 17:59:47 2010 > (r208994) > +++ stable/7/sys/dev/bge/if_bge.c Thu Jun 10 18:04:25 2010 > (r208995) > @@ -400,6 +400,8 @@ static void bge_setpromisc(struct bge_so > static void bge_setmulti(struct bge_softc *); > static void bge_setvlan(struct bge_softc *); > > +static __inline void bge_rxreuse_std(struct bge_softc *, int); > +static __inline void bge_rxreuse_jumbo(struct bge_softc *, int); > static int bge_newbuf_std(struct bge_softc *, int); > static int bge_newbuf_jumbo(struct bge_softc *, int); > static int bge_init_rx_ring_std(struct bge_softc *); > @@ -949,6 +951,7 @@ bge_newbuf_std(struct bge_softc *sc, int > sc->bge_cdata.bge_rx_std_dmamap[i] = sc->bge_cdata.bge_rx_std_sparemap; > sc->bge_cdata.bge_rx_std_sparemap = map; > sc->bge_cdata.bge_rx_std_chain[i] = m; > + sc->bge_cdata.bge_rx_std_seglen[i] = segs[0].ds_len; > r = &sc->bge_ldata.bge_rx_std_ring[sc->bge_std]; > r->bge_addr.bge_addr_lo = BGE_ADDR_LO(segs[0].ds_addr); > r->bge_addr.bge_addr_hi = BGE_ADDR_HI(segs[0].ds_addr); > @@ -1006,6 +1009,11 @@ bge_newbuf_jumbo(struct bge_softc *sc, i > sc->bge_cdata.bge_rx_jumbo_sparemap; > sc->bge_cdata.bge_rx_jumbo_sparemap = map; > sc->bge_cdata.bge_rx_jumbo_chain[i] = m; > + sc->bge_cdata.bge_rx_jumbo_seglen[i][0] = 0; > + sc->bge_cdata.bge_rx_jumbo_seglen[i][1] = 0; > + sc->bge_cdata.bge_rx_jumbo_seglen[i][2] = 0; > + sc->bge_cdata.bge_rx_jumbo_seglen[i][3] = 0; > + > /* > * Fill in the extended RX buffer descriptor. > */ > @@ -1018,18 +1026,22 @@ bge_newbuf_jumbo(struct bge_softc *sc, i > r->bge_addr3.bge_addr_lo = BGE_ADDR_LO(segs[3].ds_addr); > r->bge_addr3.bge_addr_hi = BGE_ADDR_HI(segs[3].ds_addr); > r->bge_len3 = segs[3].ds_len; > + sc->bge_cdata.bge_rx_jumbo_seglen[i][3] = segs[3].ds_len; > case 3: > r->bge_addr2.bge_addr_lo = BGE_ADDR_LO(segs[2].ds_addr); > r->bge_addr2.bge_addr_hi = BGE_ADDR_HI(segs[2].ds_addr); > r->bge_len2 = segs[2].ds_len; > + sc->bge_cdata.bge_rx_jumbo_seglen[i][2] = segs[2].ds_len; > case 2: > r->bge_addr1.bge_addr_lo = BGE_ADDR_LO(segs[1].ds_addr); > r->bge_addr1.bge_addr_hi = BGE_ADDR_HI(segs[1].ds_addr); > r->bge_len1 = segs[1].ds_len; > + sc->bge_cdata.bge_rx_jumbo_seglen[i][1] = segs[1].ds_len; > case 1: > r->bge_addr0.bge_addr_lo = BGE_ADDR_LO(segs[0].ds_addr); > r->bge_addr0.bge_addr_hi = BGE_ADDR_HI(segs[0].ds_addr); > r->bge_len0 = segs[0].ds_len; > + sc->bge_cdata.bge_rx_jumbo_seglen[i][0] = segs[0].ds_len; > break; > default: > panic("%s: %d segments\n", __func__, nsegs); > @@ -1041,12 +1053,6 @@ bge_newbuf_jumbo(struct bge_softc *sc, i > return (0); > } > > -/* > - * The standard receive ring has 512 entries in it. At 2K per mbuf cluster, > - * that's 1MB or memory, which is a lot. For now, we fill only the first > - * 256 ring entries and hope that our CPU is fast enough to keep up with > - * the NIC. > - */ > static int > bge_init_rx_ring_std(struct bge_softc *sc) > { > @@ -1054,7 +1060,7 @@ bge_init_rx_ring_std(struct bge_softc *s > > bzero(sc->bge_ldata.bge_rx_std_ring, BGE_STD_RX_RING_SZ); > sc->bge_std = 0; > - for (i = 0; i < BGE_SSLOTS; i++) { > + for (i = 0; i < BGE_STD_RX_RING_CNT; i++) { > if ((error = bge_newbuf_std(sc, i)) != 0) > return (error); > BGE_INC(sc->bge_std, BGE_STD_RX_RING_CNT); > @@ -1063,8 +1069,8 @@ bge_init_rx_ring_std(struct bge_softc *s > bus_dmamap_sync(sc->bge_cdata.bge_rx_std_ring_tag, > sc->bge_cdata.bge_rx_std_ring_map, BUS_DMASYNC_PREWRITE); > > - sc->bge_std = i - 1; > - bge_writembx(sc, BGE_MBX_RX_STD_PROD_LO, sc->bge_std); > + sc->bge_std = 0; > + bge_writembx(sc, BGE_MBX_RX_STD_PROD_LO, BGE_STD_RX_RING_CNT - 1); > > return (0); > } > @@ -1106,14 +1112,14 @@ bge_init_rx_ring_jumbo(struct bge_softc > bus_dmamap_sync(sc->bge_cdata.bge_rx_jumbo_ring_tag, > sc->bge_cdata.bge_rx_jumbo_ring_map, BUS_DMASYNC_PREWRITE); > > - sc->bge_jumbo = i - 1; > + sc->bge_jumbo = 0; > > rcb = &sc->bge_ldata.bge_info.bge_jumbo_rx_rcb; > rcb->bge_maxlen_flags = BGE_RCB_MAXLEN_FLAGS(0, > BGE_RCB_FLAG_USE_EXT_RX_BD); > CSR_WRITE_4(sc, BGE_RX_JUMBO_RCB_MAXLEN_FLAGS, rcb->bge_maxlen_flags); > > - bge_writembx(sc, BGE_MBX_RX_JUMBO_PROD_LO, sc->bge_jumbo); > + bge_writembx(sc, BGE_MBX_RX_JUMBO_PROD_LO, BGE_JUMBO_RX_RING_CNT - 1); > > return (0); > } > @@ -3299,6 +3305,33 @@ bge_reset(struct bge_softc *sc) > return(0); > } > > +static __inline void > +bge_rxreuse_std(struct bge_softc *sc, int i) > +{ > + struct bge_rx_bd *r; > + > + r = &sc->bge_ldata.bge_rx_std_ring[sc->bge_std]; > + r->bge_flags = BGE_RXBDFLAG_END; > + r->bge_len = sc->bge_cdata.bge_rx_std_seglen[i]; > + r->bge_idx = i; > + BGE_INC(sc->bge_std, BGE_STD_RX_RING_CNT); > +} > + > +static __inline void > +bge_rxreuse_jumbo(struct bge_softc *sc, int i) > +{ > + struct bge_extrx_bd *r; > + > + r = &sc->bge_ldata.bge_rx_jumbo_ring[sc->bge_jumbo]; > + r->bge_flags = BGE_RXBDFLAG_JUMBO_RING | BGE_RXBDFLAG_END; > + r->bge_len0 = sc->bge_cdata.bge_rx_jumbo_seglen[i][0]; > + r->bge_len1 = sc->bge_cdata.bge_rx_jumbo_seglen[i][1]; > + r->bge_len2 = sc->bge_cdata.bge_rx_jumbo_seglen[i][2]; > + r->bge_len3 = sc->bge_cdata.bge_rx_jumbo_seglen[i][3]; > + r->bge_idx = i; > + BGE_INC(sc->bge_jumbo, BGE_JUMBO_RX_RING_CNT); > +} > + > /* > * Frame reception handling. This is called if there's a frame > * on the receive return list. > @@ -3362,24 +3395,24 @@ bge_rxeof(struct bge_softc *sc, uint16_t > jumbocnt++; > m = sc->bge_cdata.bge_rx_jumbo_chain[rxidx]; > if (cur_rx->bge_flags & BGE_RXBDFLAG_ERROR) { > - BGE_INC(sc->bge_jumbo, BGE_JUMBO_RX_RING_CNT); > + bge_rxreuse_jumbo(sc, rxidx); > continue; > } > if (bge_newbuf_jumbo(sc, rxidx) != 0) { > - BGE_INC(sc->bge_jumbo, BGE_JUMBO_RX_RING_CNT); > + bge_rxreuse_jumbo(sc, rxidx); > ifp->if_iqdrops++; > continue; > } > BGE_INC(sc->bge_jumbo, BGE_JUMBO_RX_RING_CNT); > } else { > stdcnt++; > + m = sc->bge_cdata.bge_rx_std_chain[rxidx]; > if (cur_rx->bge_flags & BGE_RXBDFLAG_ERROR) { > - BGE_INC(sc->bge_std, BGE_STD_RX_RING_CNT); > + bge_rxreuse_std(sc, rxidx); > continue; > } > - m = sc->bge_cdata.bge_rx_std_chain[rxidx]; > if (bge_newbuf_std(sc, rxidx) != 0) { > - BGE_INC(sc->bge_std, BGE_STD_RX_RING_CNT); > + bge_rxreuse_std(sc, rxidx); > ifp->if_iqdrops++; > continue; > } > > Modified: stable/7/sys/dev/bge/if_bgereg.h > ============================================================================== > --- stable/7/sys/dev/bge/if_bgereg.h Thu Jun 10 17:59:47 2010 > (r208994) > +++ stable/7/sys/dev/bge/if_bgereg.h Thu Jun 10 18:04:25 2010 > (r208995) > @@ -2561,6 +2561,8 @@ struct bge_chain_data { > struct mbuf *bge_tx_chain[BGE_TX_RING_CNT]; > struct mbuf *bge_rx_std_chain[BGE_STD_RX_RING_CNT]; > struct mbuf *bge_rx_jumbo_chain[BGE_JUMBO_RX_RING_CNT]; > + int bge_rx_std_seglen[BGE_STD_RX_RING_CNT]; > + int bge_rx_jumbo_seglen[BGE_JUMBO_RX_RING_CNT][4]; > }; > > struct bge_dmamap_arg { > _______________________________________________ > svn-src-stabl...@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/svn-src-stable-7 > To unsubscribe, send any mail to "svn-src-stable-7-unsubscr...@freebsd.org" -- Anders. _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
