eepro100.c: configuration bug

Michael Barthelow m.barthelow@f5.com
Mon Mar 29 11:24:06 1999


This message is in MIME format. Since your mail reader does not understand
this format, some or all of this message may not be legible.

------_=_NextPart_001_01BE7A00.76F54320
Content-Type: text/plain;
	charset="iso-8859-1"

I have found what I believe to be a bug in the eepro100.c driver which can
result in excessive collisions and crc errors, particularly in a full duplex
environment with bidirectional traffic.

The problem is that bit 7 of configuration byte 5 is being used as "DMA
Maximum Byte Count Disable" but in fact, asserting this bit *enables* the
DMA Maximum Byte Counters. With the default configuration of config byte 4 =
0 and config byte 5 = 0x80 we have the worst possible configuration: we've
enabled the Maximum byte counters and set the counters to zero.

>From the Intel programmer's manual:

"It is recommended that software does not enable the DMA maximum byte
counters. Enabling the counters and placing relatively low values in them
could result in shorter bursts on the PCI bus. This could reduce system
performance and lead to transmit underruns or receive overruns. If the
counters are enabled, then it is suggested to use fairly large values in the
counters (in other words, values greater than 50 decimal"

I believe the collisions and crc errors are caused by transmit underruns,
because with zero value counters, the Tx & Rx threads are thrashing in
contending for the bus.

So in the 1.06 driver, line 377 should change from:

         22, 0x08, 0, 0,  0, 0x80, 0x32, 0x03,  1, /* 1=Use MII  0=Use AUI
*/
to:
         22, 0x08, 0, 0,  0, 0, 0x32, 0x03,  1, /* 1=Use MII  0=Use AUI */

and line 382 should change from:

         22, 0x08, 0, 1,  0, 0x80, 0x22, 0x03,  1, /* 1=Use MII  0=Use AUI
*/
to:
         22, 0x08, 0, 1,  0, 0, 0x22, 0x03,  1, /* 1=Use MII  0=Use AUI */


This problem may not show up in all systems because of another bug in the
driver. In lines 32-34 we have:

/* Tx/Rx DMA burst length, 0-127, 0 == no preemption, tx==128 -> disabled.
*/
static int txdmacount = 128;
static int rxdmacount = 0;

we set txdmacount to 128 (0x80) thinking this disables pre-emption (it
enables it). Then later in set_rx_mode, when we adjust the multicast
filtering and need to input a full config command, we construct byte 5 in
line 1450:

                 config_cmd_data[5] = txdmacount + 0x80; 

I think the 0x80 is added here thinking to disable the DMA maximum byte
counters. But since txdmacount has been initialized to 0x80, bit 7 carries
out of the byte and we are left with zero, which does indeed disable the
byte counters!

Does this make sense?

Michael Barthelow
F5 Networks, Inc.


------_=_NextPart_001_01BE7A00.76F54320
Content-Type: text/html;
	charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">



eepro100.c: configuration bug



I have found what I believe to be a = bug in the eepro100.c driver which can result in excessive collisions = and crc errors, particularly in a full duplex environment with = bidirectional traffic.

The problem is that bit 7 of = configuration byte 5 is being used as "DMA Maximum Byte Count = Disable" but in fact, asserting this bit *enables* the DMA Maximum = Byte Counters. With the default configuration of config byte 4 =3D 0 = and config byte 5 =3D 0x80 we have the worst possible configuration: = we've enabled the Maximum byte counters and set the counters to = zero.

From the Intel programmer's = manual:

"It is recommended that software = does not enable the DMA maximum byte counters. Enabling the counters = and placing relatively low values in them could result in shorter = bursts on the PCI bus. This could reduce system performance and lead to = transmit underruns or receive overruns. If the counters are enabled, = then it is suggested to use fairly large values in the counters (in = other words, values greater than 50 decimal"

I believe the collisions and crc = errors are caused by transmit underruns, because with zero value = counters, the Tx & Rx threads are thrashing in contending for the = bus.

So in the 1.06 driver, line 377 should = change from:

         22, = 0x08, 0, 0,  0, 0x80, 0x32, 0x03,  1, /* 1=3DUse MII  = 0=3DUse AUI */
to:
         22, = 0x08, 0, 0,  0, 0, 0x32, 0x03,  1, /* 1=3DUse MII  = 0=3DUse AUI */

and line 382 should change = from:

         22, = 0x08, 0, 1,  0, 0x80, 0x22, 0x03,  1, /* 1=3DUse MII  = 0=3DUse AUI */
to:
         22, = 0x08, 0, 1,  0, 0, 0x22, 0x03,  1, /* 1=3DUse MII  = 0=3DUse AUI */


This problem may not show up in all = systems because of another bug in the driver. In lines 32-34 we = have:

/* Tx/Rx DMA burst length, 0-127, 0 = =3D=3D no preemption, tx=3D=3D128 -> disabled. */
static int txdmacount =3D 128;
static int rxdmacount =3D 0;

we set txdmacount to 128 (0x80) = thinking this disables pre-emption (it enables it). Then later in = set_rx_mode, when we adjust the multicast filtering and need to input a = full config command, we construct byte 5 in line 1450:

         &nb= sp;       config_cmd_data[5] =3D = txdmacount + 0x80;

I think the 0x80 is added here = thinking to disable the DMA maximum byte counters. But since txdmacount = has been initialized to 0x80, bit 7 carries out of the byte and we are = left with zero, which does indeed disable the byte counters!

Does this make sense?

Michael Barthelow
F5 Networks, Inc.

------_=_NextPart_001_01BE7A00.76F54320--