Re: timeout.c in bsd_tcpip

[Andrew Lunn <andrew dot lunn at ascom dot ch>]

> The attachment the two logs of an application run on my machine. 
> Headed by BEFORE is the one without my patch, and headed by AFTER 
> is the one with my patch. You can see some timers such as pffasttimo, 
> pfslowtimo, in_rtqtimo and arptimer, which is expected to be kicked off
> at init routine such as domaininit(), in_inithead() or arp_ifinit() 
> by calling the timeout(), are missing at BEFORE. This is because
> timeout() fails to allocate time entry when it is called second time 
> or later. You can see if_slowtimo which is kicked off by first call of 
> timeout() in both cases. This is because the trace is done 
> not on the alley but on the list by following e->next in timeout(), 
> failing to allocate new empty entry. The trace should be done 
> on the alley(_timeout) by not following e->next, I think. 
> So should be done in untimeout(), either.
> On the other hand, in do_timeout(), the trace should be done on the 
> list headed by timeout from the top, not _timeout.

I get it now. _timeout is the array which is used for storage of the
timeouts. timeout is a pointer and is the head of the linked list.

do_timeout() currently walks the linked list starting from the first
entry in the array. This is not necessarily the head of the linked
list, eg the first entry may of expired. In this case, the subsequent
entries are never looked at because the next pointer is NULL. Thats
what the first hunk of the patch changes. It now walked from the head
of the linked list.

timeout() is responsible for adding new timeouts to the linked
list. It should be looking at the array for empty slots. So using
_timeout is correct. What is wrong is how it moves from slot to
slot. The old code tries to walk a linked list. This is wrong. There
is no linked list of empty slots. The second hunk changes this so that
it simply tries each entry in the array in turn until it finds one
which is not in use, or the end of the array is reached. 

The third hunk changes untimeout(). This removes a timeout. Again the
old code walked a linked list starting from the first entry in the
array and not the head of the list. You changed it to look at all
elements in the array one by one. This will work, but maybe its more
efficient to walk the linked list, but start from the head, ie
timeout, not _timeout. What do you think?

Apart from the minor point for the last hunk, im now happy with this
patch. I've not actually run it, but i know understand it.

Garry, what do you think? This is yours and Hugo's code. Ok to commit?

       Andrew