Subject: port-i386/26906: i386 dp8390.c counts if_ipackets twice
To: None <gnats-bugs@gnats.NetBSD.org>
From: at <djb_pizza@www.netbsd.org, ieee.org@www.netbsd.org>
List: netbsd-bugs
Date: 09/10/2004 18:57:44
>Number:         26906
>Category:       port-i386
>Synopsis:       i386 dp8390.c counts if_ipackets twice
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    port-i386-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Sep 10 18:58:00 UTC 2004
>Closed-Date:
>Last-Modified:
>Originator:     Dave Barnes
>Release:        2.0_BETA
>Organization:
Consult Engr
>Environment:
NetBSD PS2 2.0_BETA NetBSD 2.0_BETA (2004/08/16 DJB.ps2:2.0_BETA) #10: Fri Sep 10 11:46:33 CDT 2004  root@PapaJohn:/usr/src/sys/arch/i386/compile/DJB i386

>Description:
Any ethernet driver that uses dp8390_rint(sc) from dp8390.c will count received packets twice.

Code fragment from dp8390_rint showing call to dp8390_read and increment of if_ipackets:
==================================
		/*
		 * Be fairly liberal about what we allow as a "reasonable"
		 * length so that a [crufty] packet will make it to BPF (and
		 * can thus be analyzed).  Note that all that is really
		 * important is that we have a length that will fit into one
		 * mbuf cluster or less; the upper layer protocols can then
		 * figure out the length from their own length field(s).
		 */
		if (len <= MCLBYTES &&
		    packet_hdr.next_packet >= sc->rec_page_start &&
		    packet_hdr.next_packet < sc->rec_page_stop) {
			/* Go get packet. */
			dp8390_read(sc,
			    packet_ptr + sizeof(struct dp8390_ring),
			    len - sizeof(struct dp8390_ring));
			++sc->sc_ec.ec_if.if_ipackets;
		} else {
			/* Really BAD.  The ring pointers are corrupted. */
			log(LOG_ERR, "%s: NIC memory corrupt - "
			    "invalid packet length %d\n",
			    sc->sc_dev.dv_xname, len);
			++sc->sc_ec.ec_if.if_ierrors;
=====================================================
Code fragment from dp8390_read showing if_ipackets incremented:
=====================================================
void
dp8390_read(sc, buf, len)
	struct dp8390_softc *sc;
	int buf;
	u_short len;
{
	struct ifnet *ifp = &sc->sc_ec.ec_if;
	struct mbuf *m;

	/* Pull packet off interface. */
	m = dp8390_get(sc, buf, len);
	if (m == 0) {
		ifp->if_ierrors++;
		return;
	}

	ifp->if_ipackets++;

#if NBPFILTER > 0

>How-To-Repeat:
# netstat -I we0
Name  Mtu   Network       Address              Ipkts Ierrs    Opkts Oerrs Colls
we0   1500  <Link>        00:00:c0:b3:1c:44      148     0       75     0     0
we0   1500  10/24         PS2                    148     0       75     0     0
# ping -f 10.0.0.104
PING anchovy (10.0.0.104): 56 data bytes
^C.

----anchovy PING Statistics----
4551 packets transmitted, 4551 packets received, 0.0% packet loss
round-trip min/avg/max/stddev = 1.339/1.487/5.047/0.239 ms
  635.0 packets/sec sent,  635.4 packets/sec received
# netstat -I we0
Name  Mtu   Network       Address              Ipkts Ierrs    Opkts Oerrs Colls
we0   1500  <Link>        00:00:c0:b3:1c:44     9432     0     4731     0     0
we0   1500  10/24         PS2                   9432     0     4731     0     0
#

Received packet counts are about 2x transmit counts
>Fix:
--- dev/ic/dp8390.c.orig	2004-09-10 13:37:14.000000000 -0500
+++ dev/ic/dp8390.c	2004-09-10 13:38:32.000000000 -0500
@@ -617,7 +617,9 @@
 			dp8390_read(sc,
 			    packet_ptr + sizeof(struct dp8390_ring),
 			    len - sizeof(struct dp8390_ring));
+			/* redundant, if_ipackets incremented in dp8390_read() 
 			++sc->sc_ec.ec_if.if_ipackets;
+			*/
 		} else {
 			/* Really BAD.  The ring pointers are corrupted. */
 			log(LOG_ERR, "%s: NIC memory corrupt - "

>Release-Note:
>Audit-Trail:
>Unformatted: