Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 7 Oct 2005 12:06:05 +0400
From:      "Yuriy N. Shkandybin" <jura@networks.ru>
To:        <freebsd-net@freebsd.org>
Cc:        jhb@freebsd.org
Subject:   if_nge & if_lge drivers
Message-ID:  <092e01c5cb15$f7fe5840$6504010a@Jura>

next in thread | raw e-mail | index | archive | help

[-- Attachment #1 --]
Hello.

I saw John Baldwin commit to if_lge.c rev 1.43 and perform same changes for if_nge.c
I've tested it and it  works.
Patch in attachment or available from
http://www.netams.com/if_nge.c.patch



Also i've noticed if_lge affected same problem i've met nge.
In if_lgereg.h we have
struct lge_list_data {
        struct lge_rx_desc      lge_rx_list[LGE_RX_LIST_CNT];
        struct lge_tx_desc      lge_tx_list[LGE_TX_LIST_CNT];
};

In if_lge.c
524: sc->lge_ldata = contigmalloc(sizeof(struct lge_list_data), M_DEVBUF,
            M_NOWAIT, 0, 0xffffffff, PAGE_SIZE, 0);

So lge_rx_list and lge_tx_list  might be initialized with garbage in it.

But in lge_stop() there is:
/*
         * Free data in the RX lists.
         */
        for (i = 0; i < LGE_RX_LIST_CNT; i++) {
                if (sc->lge_ldata->lge_rx_list[i].lge_mbuf != NULL) {
                        m_freem(sc->lge_ldata->lge_rx_list[i].lge_mbuf);
                        sc->lge_ldata->lge_rx_list[i].lge_mbuf = NULL;
                }
        }

And lge_stop() called from lge_init()  (if_lge.c line 1242)
So m_freem() called on garbage from lge_rx_list!

I suggest to add M_ZERO to contigmalloc() flags for both if_nge.c and if_lge.c


Jura


[-- Attachment #2 --]
--- if_nge.c.orig	Fri Oct  7 10:51:29 2005
+++ if_nge.c	Fri Oct  7 11:49:09 2005
@@ -742,7 +742,7 @@
 	}
 
 	if (i == NGE_TIMEOUT)
-		printf("nge%d: reset never completed\n", sc->nge_unit);
+		if_printf(sc->nge_ifp, "reset never completed\n");
 
 	/* Wait a little while for the chip to get its brains in order. */
 	DELAY(1000);
@@ -791,13 +791,10 @@
 {
 	u_char			eaddr[ETHER_ADDR_LEN];
 	struct nge_softc	*sc;
-	struct ifnet		*ifp;
-	int			unit, error = 0, rid;
-	const char		*sep = "";
+	struct ifnet		*ifp=NULL;
+	int			error = 0, rid;
 
 	sc = device_get_softc(dev);
-	unit = device_get_unit(dev);
-	bzero(sc, sizeof(struct nge_softc));
 
 	NGE_LOCK_INIT(sc, device_get_nameunit(dev));
 	/*
@@ -809,7 +806,7 @@
 	sc->nge_res = bus_alloc_resource_any(dev, NGE_RES, &rid, RF_ACTIVE);
 
 	if (sc->nge_res == NULL) {
-		printf("nge%d: couldn't map ports/memory\n", unit);
+		device_printf(dev, "couldn't map ports/memory\n");
 		error = ENXIO;
 		goto fail;
 	}
@@ -823,11 +820,20 @@
 	    RF_SHAREABLE | RF_ACTIVE);
 
 	if (sc->nge_irq == NULL) {
-		printf("nge%d: couldn't map interrupt\n", unit);
-		bus_release_resource(dev, NGE_RES, NGE_RID, sc->nge_res);
+		device_printf(dev, "couldn't map interrupt\n");
 		error = ENXIO;
 		goto fail;
 	}
+	
+	/*
+	* Hookup IRQ last.
+	*/
+	error = bus_setup_intr(dev, sc->nge_irq, INTR_TYPE_NET | INTR_MPSAFE,
+	    nge_intr, sc, &sc->nge_intrhand);
+	if (error) {
+		device_printf(dev, "couldn't set up irq\n");
+		goto fail;
+	}
 
 	/* Reset the adapter. */
 	nge_reset(sc);
@@ -839,25 +845,19 @@
 	nge_read_eeprom(sc, (caddr_t)&eaddr[2], NGE_EE_NODEADDR + 1, 1, 0);
 	nge_read_eeprom(sc, (caddr_t)&eaddr[0], NGE_EE_NODEADDR + 2, 1, 0);
 
-	sc->nge_unit = unit;
-
 	/* XXX: leaked on error */
 	sc->nge_ldata = contigmalloc(sizeof(struct nge_list_data), M_DEVBUF,
-	    M_NOWAIT, 0, 0xffffffff, PAGE_SIZE, 0);
+	    M_NOWAIT|M_ZERO, 0, 0xffffffff, PAGE_SIZE, 0);
 
 	if (sc->nge_ldata == NULL) {
-		printf("nge%d: no memory for list buffers!\n", unit);
-		bus_release_resource(dev, SYS_RES_IRQ, 0, sc->nge_irq);
-		bus_release_resource(dev, NGE_RES, NGE_RID, sc->nge_res);
+		device_printf(dev, "no memory for list buffers!\n");
 		error = ENXIO;
 		goto fail;
 	}
 
 	ifp = sc->nge_ifp = if_alloc(IFT_ETHER);
 	if (ifp == NULL) {
-		printf("nge%d: can not if_alloc()\n", unit);
-		bus_release_resource(dev, SYS_RES_IRQ, 0, sc->nge_irq);
-		bus_release_resource(dev, NGE_RES, NGE_RID, sc->nge_res);
+		device_printf(dev, "can not if_alloc()\n");
 		error = ENOSPC;
 		goto fail;
 	}
@@ -893,19 +893,13 @@
 			ifmedia_init(&sc->nge_ifmedia, 0, nge_ifmedia_upd, 
 				nge_ifmedia_sts);
 #define	ADD(m, c)	ifmedia_add(&sc->nge_ifmedia, (m), (c), NULL)
-#define PRINT(s)	printf("%s%s", sep, s); sep = ", "
 			ADD(IFM_MAKEWORD(IFM_ETHER, IFM_NONE, 0, 0), 0);
-			device_printf(dev, " ");
 			ADD(IFM_MAKEWORD(IFM_ETHER, IFM_1000_SX, 0, 0), 0);
-			PRINT("1000baseSX");
 			ADD(IFM_MAKEWORD(IFM_ETHER, IFM_1000_SX, IFM_FDX, 0),0);
-			PRINT("1000baseSX-FDX");
 			ADD(IFM_MAKEWORD(IFM_ETHER, IFM_AUTO, 0, 0), 0);
-			PRINT("auto");
-	    
-			printf("\n");
 #undef ADD
-#undef PRINT
+			device_printf(dev, " 1000baseSX, 1000baseSX-FDX, auto\n");
+			
 			ifmedia_set(&sc->nge_ifmedia, 
 				IFM_MAKEWORD(IFM_ETHER, IFM_AUTO, 0, 0));
 	    
@@ -916,11 +910,7 @@
 				| NGE_GPIO_GP3_IN | NGE_GPIO_GP4_IN);
 	    
 		} else {
-			printf("nge%d: MII without any PHY!\n", sc->nge_unit);
-			if_free(ifp);
-			bus_release_resource(dev, SYS_RES_IRQ, 0, sc->nge_irq);
-			bus_release_resource(dev, NGE_RES, NGE_RID, 
-					 sc->nge_res);
+			device_printf(dev, "MII without any PHY!\n");
 			error = ENXIO;
 			goto fail;
 		}
@@ -931,24 +921,21 @@
 	 */
 	ether_ifattach(ifp, eaddr);
 	callout_init(&sc->nge_stat_ch, CALLOUT_MPSAFE);
+	return (0);
 
-	/*
-	 * Hookup IRQ last.
-	 */
-	error = bus_setup_intr(dev, sc->nge_irq, INTR_TYPE_NET | INTR_MPSAFE,
-	    nge_intr, sc, &sc->nge_intrhand);
-	if (error) {
-		/* XXX: resource leaks */
+fail:
+	if (sc->nge_ldata)
+		contigfree(sc->nge_ldata,
+		  sizeof(struct nge_list_data), M_DEVBUF);
+	if (ifp)
 		if_free(ifp);
+	if (sc->nge_intrhand)
+		bus_teardown_intr(dev, sc->nge_irq, sc->nge_intrhand);
+	if (sc->nge_irq)
 		bus_release_resource(dev, SYS_RES_IRQ, 0, sc->nge_irq);
+	if (sc->nge_res)
 		bus_release_resource(dev, NGE_RES, NGE_RID, sc->nge_res);
-		printf("nge%d: couldn't set up irq\n", unit);
-	}
-
-fail:
-
-	if (error)
-		NGE_LOCK_DESTROY(sc);
+	NGE_LOCK_DESTROY(sc);
 	return(error);
 }
 
@@ -1345,8 +1332,8 @@
 			if (CSR_READ_4(sc, NGE_TBI_BMSR) 
 			    & NGE_TBIBMSR_ANEG_DONE) {
 				if (bootverbose)
-					printf("nge%d: gigabit link up\n",
-					    sc->nge_unit);
+					if_printf(sc->nge_ifp, 
+					    "gigabit link up\n");
 				nge_miibus_statchg(sc->nge_miibus);
 				sc->nge_link++;
 				if (ifp->if_snd.ifq_head != NULL)
@@ -1363,8 +1350,8 @@
 				sc->nge_link++;
 				if (IFM_SUBTYPE(mii->mii_media_active) 
 				    == IFM_1000_T && bootverbose)
-					printf("nge%d: gigabit link up\n",
-					    sc->nge_unit);
+					if_printf(sc->nge_ifp, 
+					    "gigabit link up\n");
 				if (ifp->if_snd.ifq_head != NULL)
 					nge_start_locked(ifp);
 			}
@@ -1682,6 +1669,7 @@
 	 * Cancel pending I/O and free all RX/TX buffers.
 	 */
 	nge_stop(sc);
+	nge_reset(sc);
 
 	if (sc->nge_tbi) {
 		mii = NULL;
@@ -1702,8 +1690,8 @@
 
 	/* Init circular RX list. */
 	if (nge_list_rx_init(sc) == ENOBUFS) {
-		printf("nge%d: initialization failed: no "
-			"memory for rx buffers\n", sc->nge_unit);
+		if_printf(sc->nge_ifp, "initialization failed: no "
+			"memory for rx buffers\n");
 		nge_stop(sc);
 		return;
 	}
@@ -2086,7 +2074,7 @@
 	sc = ifp->if_softc;
 
 	ifp->if_oerrors++;
-	printf("nge%d: watchdog timeout\n", sc->nge_unit);
+	if_printf(sc->nge_ifp, "watchdog timeout\n");
 
 	NGE_LOCK(sc);
 	nge_stop(sc);

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?092e01c5cb15$f7fe5840$6504010a>