svn commit: r208995 - stable/7/sys/dev/bge
Anders Nordby
anders at FreeBSD.org
Tue Jun 29 08:06:22 UTC 2010
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-stable-7 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/svn-src-stable-7
> To unsubscribe, send any mail to "svn-src-stable-7-unsubscribe at freebsd.org"
--
Anders.
More information about the svn-src-stable-7
mailing list