Re: bug in tulip_rx? patch

Wolfgang Walter (wolfgang.walter@stusta.mhn.de)
Wed, 24 Nov 1999 16:49:36 +0100


On Wed, Nov 24, 1999 at 11:10:30AM +0100, Miquel van Smoorenburg wrote:
> In article <cistron.19991124023024.A2289@wuerli.h3.stusta.mhn.de>,
> Wolfgang Walter <wolfgang.walter@stusta.mhn.de> wrote:
> >Since we use 2.2.14pre4 instead of 2.0.38 we observe random hangs of the
> >network interface which is a tulip card.
> >
> >I tried older tulip-drivers, but this happens with all of them.
>
> I have the same happening with the epic100 and eepro100 drivers.
> Since they all were written by Donald, and probably use the
> same skeleton, could it be that this same bug is present in
> those drivers as well ?
>
> [checking .. oh yes, at least the epic100 driver is remarkably similar!]

Probably same reason, indeed.

As this seems to concern not only the tulip driver, I think I should explain
the bug in more detail in this list again instead referring my mail on
tulip-bug. And what my patch does (ok, should do :-) ).

The tulip-driver (and probably other drivers, too) has a ring-buffer design.
Two counters are important:

cur_rx and dirty_rx.

Donald use them a little bit tricky. They are unsigned int. As long as the ring
buffer size is smaller that (and it is, of course), all calculations and
checks even work when they wrap-around (you must be carefull, though, to
calculate with always with unsigned int). Rather nice.

The following entries are owned by the card:

unsigned int i;
for (i=cur_rx; cur_rx + i != dirty_rx + RX_RING_SIZE; i++) {
entry = i % RX_RING_SIZE;
}

The following entries are not owned by the card:

unsigned int i;
for (i=dirty_rx; dirty_rx + i != cur_rx) {
entry = i % RX_RING_SIZE;
}

One can see: usually this meens (if they are not wrapping around)
dirty_rx <= cur_rx <= dirty_rx + RX_RING_SIZE

Speciall situations:

cur_rx == dirty_rx

means: all entries are owned by card.

cur_rx == dirty_rx + RX_RING_SIZE

means: no entry is owned by card.

Every entry owned by the card must have associated memory which means a skb.

If you have a look at the implementation there is another important conditions
which allways hold if tulip_rx finished:

If cur_rx != dirty_rx then the entry dirty_rx % RX_RING_SIZE has
no skb associated.

The bug now is: the tulip driver tries to avoid copying. He often gives the
skb of the entry away and tries to reallocate a new skb for the ring in
tulip_rx.

This is fine as long we do not enter the situation that the entry associated
with cur_rx has no skb as this entry is used by the card for the next packet
received. It is clear that

cur_rx no skb <=> cur_rx == dirty_rx % RX_RING_SIZE

The card now stops receiving. Even if we get later a no receive buffer
interrupt: if we do not succeed in allocation a skb we will get no further
interrupts and so never get the chance to allocate a skb => hang.

There are several workarounds:

1. always allocate immediately a replacement skb. If we do not succeed: drop
the packet.

In this case we insure that every entry has always a skb associated,
therefor after tulip_rx cur_rx == dirty_rx always holds

But this is especially inflexible and a lot of packets could be dropped.

2. always ensure that cur_rx always has an skb associated. After the above
ist is clear that this means:

If cur_rx == dirty_rx % RX_RING_SIZE at the end of tulip_rx we
must associate a skb with cur_rx (and then we may increase) dirty_rx.

To be able to do that we must keep always one skb of our ring buffer.
If we had to give away our last skb, we must drop the packet.

Nr. 2 is what my patch does. This is probably a strategie which works for other
drivers, too.

A comment: if in tulip_rx cur_rx == dirty_rx % RX_RING_SIZE after processing the
new packets the card may already in suspend mode. But it will switch to running
mode automatically when the next packet is received (if I read the manual
correctly). I don't now if a additional receive poll command has advantages.

Excuse my english

Hope this description helps when analyzing other drivers.

Wolfgang Walter

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/