Re: [ECOS] uipc_socket.c (and cyg_tcp_maxidle)

[Will Lentz <will_lentz at trimble dot com>]

Hi Hans,

zfreei() is a function, and doesn't directly modify its first argument.
But - it passes "so" as its second argument, and it modifies the value
of so->so_zone through that.  Looking at zfreei:
{
    elem *p = (elem *)item;  // item here is "so"

    log(LOG_MDEBUG, "zfreei to %s <= %p\n", zone->name, p);
    p->next = zone->pool;  // p->next corresponds to so->so_zone in
memory
    zone->pool = p;
    zone->free++;
    zone->alloc_frees++;
}

It's true that wakeup() doesn't care if it is passed a valid argument,
but if it gets the wrong value then it doesn't wake up the correct
people :-).

Maybe zfreei() shouldn't be passed "so" as its second argument in
sodealloc?  Can anyone comment on why this is done?  It looks fishy
casting a "so" pointer to an "elem *" and then mucking with it...  Every
other example of zfreei() I can find passes the value returned from
zalloci() as the second argument of zfreei().

Thanks,
Will

P.S. I also found that changing cyg_tcp_maxidle helps, but I still think
there is some problem.

On Thu, 2005-06-16 at 09:01 +0200, Hans Hübner wrote:
> On Wed, 15 Jun 2005, Will Lentz wrote:
> 
> > I may have found a potential bug in
> > packages/net/bsd_tcpip/current/src/sys/kern/uipc_socket.c (or I may be
> > completely wrong :-).
> >
> > At the end of sodealloc(), the following code exists:
> >       zfreei(so->so_zone, so);
> >       wakeup(so->so_zone);
> > The problem is that zfreei() changes so->so_zone.  Shouldn't wakeup() be
> > done on the original so->so_zone?  I only noticed this problem by:
> > 1- while(1) {
> >   sock = socket( AF_INET, SOCK_STREAM, IPPROTO_TCP );
> >   connect( sock, ... );
> >   close( sock );
> >   }
> >   Eventually this pauses in socket() (in cyg_tsleep()) when you run out
> > of eCos sockets.
> >
> > 2- After 2*MSL or so, cyg_wakeup() gets called with chan == 0x0.  Why?
> > The zfreei() call in sodealloc() changes so->so_zone to 0 before the
> > wakeup() call.
> 
> If I read the source correctly, zfreei() is a function, not a macro, and it 
> is passed the zone argument by value (as is default with C).  Thus, zfreei() 
> cannot change the so structure itself and the value of so->so_zone will be 
> unmodified after the call.  sleep() and wakeup() take void* as argument and do 
> not interpret the value, so to them it is not of concern whether the passed 
> pointer is actually valid.
> 
> I looked into this because I had some problems with eCos and an embedded web 
> server.  Seemingly, the server worked fine but stopped serving after some 10 
> requests had been processed.  Looking into this, I found that the number of 
> sockets in our default configuration was very low (16).  As TCP sockets stay 
> allocated after the connection has closed until the tcp_maxidle period has 
> expired, no new connections could be accepted until that period (which is in 
> the minutes range in the default configuration) passed.
> 
> Not having looked at the TCP specification, I decided that reducing the 
> tcp_maxidle parameter to a very low value combined with an increase in the 
> number of sockets would help me out:
> 
>    extern int cyg_tcp_maxidle;
>    cyg_tcp_maxidle = 200;
> 
> I am still not completely satisfied with the solution, because I don't really 
> know whether this change will result in bad interactions with other TCP 
> network partners.  Also, I have found the numbers reported for TCP 
> accept/established/close do not correspond and I suspect that there are in 
> fact other bugs which get triggered by hitting the socket limit very often.
> 
> I hope this is useful to some.
> 
> Regards,
> Hans

--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss