Subject: Re: port-mac68k/32583: mac68k netbsd-2 panics during rcp(1)
To: None <port-mac68k-maintainer@netbsd.org, gnats-admin@netbsd.org,>
From: Hauke Fath <hauke@Espresso.Rhein-Neckar.DE>
List: netbsd-bugs
Date: 09/03/2006 20:30:03
The following reply was made to PR port-mac68k/32583; it has been noted by GNATS.

From: Hauke Fath <hauke@Espresso.Rhein-Neckar.DE>
To: Dave Huang <khym@azeotrope.org>
Cc: Hauke Fath <hauke@Espresso.Rhein-Neckar.DE>,
	Scott Reynolds <scottr@clank.org>, port-mac68k-maintainer@netbsd.org,
	gnats-bugs@netbsd.org
Subject: Re: port-mac68k/32583: mac68k netbsd-2 panics during rcp(1)
Date: Sun, 3 Sep 2006 20:54:48 +0200

 At 3:36 Uhr -0500 3.9.2006, Dave Huang wrote:
 >Ah, yeah, I see the bug Scott's talking about... if the original
 >packet that needs to be padded has an odd length, it'll write one
 >extra word of padding (I don't know if that actually causes any
 >problems though; it seems like it'd be harmless :)
 
 Dave, first of all thanks for picking this up. I've prodded Scott twice via
 email, on May 1 and May 29, but never received an answer.
 
 Unfortunately, the tail of our mail exchange with the last patch he sent
 (the one that worked for me) was not cc'ed to gnats-bugs@netbsd.org as it
 should have been.
 
 >However, Scott's patch has a minor bug where if the packet is exactly
 >one byte too short, the function will write the correct data out to
 >the ethernet chip, but will report that it wrote one byte less than it
 >actually did (i.e., it'll write 60 bytes, but report that it wrote 59).
 >
 >What about this?
 
 Your patch and the last one that Scott provided differ only in that
 
 >Index: if_ae.c
 >===================================================================
 >RCS file: /cvsroot/src/sys/arch/mac68k/dev/if_ae.c,v
 >retrieving revision 1.77
 >diff -u -r1.77 if_ae.c
 >--- if_ae.c	11 Dec 2005 12:18:02 -0000	1.77
 >+++ if_ae.c	3 Sep 2006 08:27:17 -0000
 >@@ -165,15 +165,18 @@
 > 		}
 > 	}
 >
 >+	len = ETHER_MIN_LEN - ETHER_CRC_LEN - totlen;
 > 	if (wantbyte) {
 > 		savebyte[1] = 0;
 > 		bus_space_write_region_2(sc->sc_buft, sc->sc_bufh,
 > 		    buf, (u_int16_t *)savebyte, 1);
 >-		    buf += 2;
 >+		buf += 2;
 >+		if (len-- > 0)
 >+			totlen++;
 
 his has an additional
 
 	        len--;
 
 here.
 
 > 	}
 >-	if (totlen < ETHER_MIN_LEN - ETHER_CRC_LEN) {
 >+	if (len > 0) {
 > 		bus_space_set_region_2(sc->sc_buft, sc->sc_bufh, buf, 0,
 >-		    (ETHER_MIN_LEN - ETHER_CRC_LEN - totlen) >> 1);
 >+		    len >> 1);
 > 		totlen = ETHER_MIN_LEN - ETHER_CRC_LEN;
 > 	}
 > 	return (totlen);
 
 Since I have just returned from a 3.000 km cycle tour, and part of my mind
 is still in the Scottish Highlands, I'll ask you first what difference this
 makes, before making misguided attempts to wrap my mind around the code.  ;)
 
 	hauke
 
 
 --
 "It's never straight up and down"     (DEVO)