From owner-freebsd-current@FreeBSD.ORG Mon Jan 5 08:13:27 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3A7DF16A4CE; Mon, 5 Jan 2004 08:13:27 -0800 (PST) Received: from gw.catspoiler.org (217-ip-163.nccn.net [209.79.217.163]) by mx1.FreeBSD.org (Postfix) with ESMTP id C70C243D1D; Mon, 5 Jan 2004 08:13:25 -0800 (PST) (envelope-from truckman@FreeBSD.org) Received: from FreeBSD.org (mousie.catspoiler.org [192.168.101.2]) by gw.catspoiler.org (8.12.9p2/8.12.9) with ESMTP id i05GDE7E011556; Mon, 5 Jan 2004 08:13:19 -0800 (PST) (envelope-from truckman@FreeBSD.org) Message-Id: <200401051613.i05GDE7E011556@gw.catspoiler.org> Date: Mon, 5 Jan 2004 08:13:14 -0800 (PST) From: Don Lewis To: silby@silby.com In-Reply-To: <53342.158.6.15.27.1073314708.squirrel@webmail.pair.com> MIME-Version: 1.0 Content-Type: TEXT/plain; charset=us-ascii cc: dejan.lesjak@ijs.si cc: freebsd-current@FreeBSD.org cc: wpaul@FreeBSD.org cc: ryans@gamersimpact.com Subject: Re: 5.2-RC oerrs and collisions on dc0 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Jan 2004 16:13:27 -0000 On 5 Jan, silby@silby.com wrote: >> I just took a closer look at the busdma diff, and this change to >> dc_txeof() looks very suspicious: >> >> - if (!(cur_tx->dc_ctl & DC_TXCTL_LASTFRAG) || >> + if (!(cur_tx->dc_ctl & DC_TXCTL_FIRSTFRAG) || >> cur_tx->dc_ctl & DC_TXCTL_SETUP) { > > I'm current checking e-mail via a webmail interface and I haven't had time > to check over your later posts, but I thought I'd note that the change > above _is_ busdma related; one subtle change in the busdma code was that > the mbuf is now linked to the first fragment in the chain, whereas before > it was linked to the last fragment. So, the change does make sense on the > surface, although I wouldn't be surprised if it broke something subtle. Hmn, in that case skipping the "if" block if either DC_TXCTL_LASTFRAG is set or if DC_TXCTL_FIRSTFRAG is set would be appropriate. Then we would need to execute the code fragment: if (txstat & DC_TXSTAT_ERRSUM) { ifp->if_oerrors++; if (txstat & DC_TXSTAT_EXCESSCOLL) ifp->if_collisions++; if (txstat & DC_TXSTAT_LATECOLL) ifp->if_collisions++; if (!(txstat & DC_TXSTAT_UNDERRUN)) { dc_init(sc); return; } } ifp->if_collisions += (txstat & DC_TXSTAT_COLLCNT) >> 3; if the DC_TXCTL_LASTFRAG is set and execute the code fragment ifp->if_opackets++; if (sc->dc_cdata.dc_tx_chain[idx] != NULL) { m_freem(sc->dc_cdata.dc_tx_chain[idx]); sc->dc_cdata.dc_tx_chain[idx] = NULL; } if the DC_TXCTL_FIRSTFRAGMENT flag is set. I think this would introduce another subtle bug. I'm pretty sure that the chip clears the DC_TXSTAT_OWN bit after it does the DMA for each descriptor, so freeing the mbuf chain as soon as we see the DC_TXSTAT_OWN bit go away on the first descriptor may free the mbuf chain before the chip has copied the entire frame. I think it would be better to revert the change to dc_txeof() and to hang the mbuf chain off the last descriptor.