Date: Thu, 10 Jun 2010 17:53:35 +0000 (UTC) From: Pyun YongHyeon <yongari@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org Subject: svn commit: r208993 - stable/8/sys/dev/bge Message-ID: <201006101753.o5AHrZE6021738@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: yongari Date: Thu Jun 10 17:53:35 2010 New Revision: 208993 URL: http://svn.freebsd.org/changeset/base/208993 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 Approved by: re (bz) Modified: stable/8/sys/dev/bge/if_bge.c stable/8/sys/dev/bge/if_bgereg.h Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) stable/8/sys/geom/sched/ (props changed) Modified: stable/8/sys/dev/bge/if_bge.c ============================================================================== --- stable/8/sys/dev/bge/if_bge.c Thu Jun 10 17:49:36 2010 (r208992) +++ stable/8/sys/dev/bge/if_bge.c Thu Jun 10 17:53:35 2010 (r208993) @@ -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/8/sys/dev/bge/if_bgereg.h ============================================================================== --- stable/8/sys/dev/bge/if_bgereg.h Thu Jun 10 17:49:36 2010 (r208992) +++ stable/8/sys/dev/bge/if_bgereg.h Thu Jun 10 17:53:35 2010 (r208993) @@ -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 {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201006101753.o5AHrZE6021738>