Date: Wed, 12 Oct 2005 16:10:46 -0400 From: John Baldwin <jhb@freebsd.org> To: "Yuriy N. Shkandybin" <jura@networks.ru> Cc: freebsd-net@freebsd.org Subject: Re: if_nge & if_lge drivers Message-ID: <200510121610.47507.jhb@freebsd.org> In-Reply-To: <092e01c5cb15$f7fe5840$6504010a@Jura> References: <092e01c5cb15$f7fe5840$6504010a@Jura>
next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 07 October 2005 04:06 am, Yuriy N. Shkandybin wrote: > 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 Note that lge() has a bzero() call after the contigmalloc(), but M_ZERO is probably better to use: sc->lge_ldata = contigmalloc(sizeof(struct lge_list_data), M_DEVBUF, M_NOWAIT, 0, 0xffffffff, PAGE_SIZE, 0); ... bzero(sc->lge_ldata, sizeof(struct lge_list_data)); I'll put that in my lge(4) patches and incorporate your nge(4) patches. One issue with your nge(4) patch is that you moved the bus_setup_intr() up when it really should happen after ether_ifattach(). -- John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200510121610.47507.jhb>