buglwIP - A Lightweight TCP/IP stack - Bugs: bug #19161, lwip_select mishandles sub 500us...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #19161: lwip_select mishandles sub 500us delays

Submitted by:  Rob Stedman <robstedman1>
Submitted on:  Tue 27 Feb 2007 07:14:29 AM UTC  
 
Category:  sockets/netconn Severity:  2 - Minor
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  Kieran Mansley <kieranm>
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

( Jump to the original submission)

Mon 26 Mar 2007 11:53:30 AM UTC, comment #6: 

Oh scratch that - all the API code has changed in the last week and this aspect has gone. Forget it :-).

Jonathan Larmour <jifl>
Project Member
Mon 26 Mar 2007 11:49:59 AM UTC, comment #5: 

Okay, it's probably best I assign a new bug for this and take it rather than overload this one.

It just occurred to me to mention it here because in my and no doubt other ports, 1ms is treated as polling, so the fix for this bug in select may not work as expected for those ports.

Jonathan Larmour <jifl>
Project Member
Mon 26 Mar 2007 10:57:48 AM UTC, comment #4: 

I'm not sure I understand quite what the problem is here, but documenting that anything less than 1ms in timeout will be rounded up is a good idea.   Rounding it down to 0 (as it was) was clearly incorrect behaviour.

If we're waiting for 1ms when doing a polling operation that is clearly not right though, because that should by definition return straight away.

If 1ms timeout is being used to by ports to achieve polling-like behaviour that is a bit more problematic as we can't then tell the difference between a genuine 1ms timeout and a "I'd really like a poll" 1ms timeout.  Perhaps this is what you're suggesting the tryfetch/trywait for?  If we could introduce these in a backwardly compatible way (so that ports can use them if they wish, but current semantics maintained) that would be good.

Kieran Mansley <kieranm>
Project MemberIn charge of this item.
Fri 23 Mar 2007 06:14:17 AM UTC, comment #3: 

I see the fix is to tweak it to 1ms, which is fair enough in the current context. The same thing happens in api_lib.c in two places - a 1ms timeout is used in order to be interpreted as a poll. I'd be concerned that ports may unnecessarily wait 1ms though. I know in my port I map 1ms waits to a poll. Perhaps we should just do the right thing and either:
a) document that a 1ms timeout should always be considered a poll - the precise timings of lwip operation being somewhat variable as it is; or
b) provide new polled versions in sys.c: sys_mbox_tryfetch, and sys_sem_trywait, which would just be #defines to sys_arch_mbox_tryfetch and sys_arch_sem_trywait. (Which ports may in turn choose to make #defines to sys_mbox_arch_fetch(mbox, 1) to preserve existing behaviour if they wish).

The latter is clearer, but breaks existing ports. Any opinions out there?

Jonathan Larmour <jifl>
Project Member
Wed 28 Feb 2007 12:54:02 PM UTC, comment #2: 

Fixed checked in to CVS.  Please review and update if there's still a problem.

Kieran Mansley <kieranm>
Project MemberIn charge of this item.
Tue 27 Feb 2007 08:54:36 AM UTC, comment #1: 

Sounds like we need to guarantee that a non-zero timeout results in a non-zero value passed to sys_sem_wait_timeout.  Should be a straightforward fix.

Kieran Mansley <kieranm>
Project MemberIn charge of this item.
Tue 27 Feb 2007 07:14:29 AM UTC, original submission:  

My understanding is lwip_select uses sys_sem_wait_timeout when pending the caller.

If the supplied struct timeval is sub 500us then a timeout of 0 msec is passed to sys_sem_wait_timeout which is wrongly interpreted as wait forever.

Rob Stedman <robstedman1>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by robstedman1 (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-02-28 kieranm StatusNone => Fixed
        Open/ClosedOpen => Closed
    2007-02-27 kieranm Severity3 - Normal => 2 - Minor
        Assigned toNone => kieranm

    Back to the top


    Powered by Savane 3.5