>Number:         1299
>Category:       port-amiga
>Synopsis:       if_ed, on Amiga, rejects valid packets with length > MCLBYTES
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jul 31 18:20:02 1995
>Last-Modified:
>Originator:     Ignatios Souvatzis
>Organization:
		private site
>Release:        950710
>Environment:
	
System: NetBSD beverly 1.0A NetBSD 1.0A (BEVERLY) #146: Sat Jul 29 18:15:40 MET DST 1995 is@beverly:/usr/src/sys/arch/amiga/compile/BEVERLY amiga


>Description:
	As s.b. else described in the amiga-dev, the (new? ported from the ISA
	version?) if_ed.c rejects quite valid input packets with a length > MCLBYTES.

	This is done as kind of a sanity test, which is (IMHO) a NOOP, as the code 
	computes most of the length itself in the lines above, to work around buggy
	hardware chips.

>How-To-Repeat:
	As I read the error description, using the if_ed driver to do NFS will trigger
	the bug.
>Fix:
	My patch just removes the test (as a test to the hardware buffer length doesn't
	make any sense for the reasons stated) but leaves the sanity check for the 
	buffer address in place. I leave the code (how I'd have written it) in place to
	remind my successors what should be there in case the lines above the comment would
	be deleted.

	BTW, s.b. should look at the ISA if_ed code; it might have the same kludge in, 
	which should be corrected similarly to avoid dependencies on the MCLBYTES value.

	This code also removes the cia.h include. After looking at it, I can't discover why
	it is in there. Somebody surely will complain if the driver breaks now. :-) i.s.

*** if_ed.c.dist	Mon Jul 31 23:17:19 1995
--- if_ed.c	Mon Jul 31 23:27:08 1995
***************
*** 57,63 ****
  #include <amiga/dev/zbusvar.h>
  #include <dev/ic/dp8390reg.h>
  #include <amiga/dev/if_edreg.h>
- #include <amiga/amiga/cia.h>		/* XXX? */
  
  #define HYDRA_MANID	2121
  #define HYDRA_PRODID	1
--- 57,62 ----
***************
*** 621,628 ****
  		 * 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. */
--- 620,634 ----
  		 * 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).
+ 		 *
+ 		 * XXX NO. mbuf cluster size has nothing to do with believable 
+ 		 * packet lengths. And as we did compute most of the lenght 
+ 		 * ourselfes, above, anyway, (so the only usable test, IMHO, is
+ 		 * a NOOP) I just drop that test, as it leaded to rejection of 
+ 		 * quite normal packet lengths. 
+ 		 * I guess this should be fixed in the ISA driver, too. i.s.
  		 */
! 		if (/* len <= sc->mem_size && */
  		    packet_hdr.next_packet >= sc->rec_page_start &&
  		    packet_hdr.next_packet < sc->rec_page_stop) {
  			/* Go get packet. */


>Audit-Trail:
>Unformatted:
Ignatios Souvatzis