Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 10 Jun 2010 18:04:25 +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-7@freebsd.org
Subject:   svn commit: r208995 - stable/7/sys/dev/bge
Message-ID:  <201006101804.o5AI4PEX024259@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
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 {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201006101804.o5AI4PEX024259>