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

Submitter:  Rob Stedman <robstedman1>
Submitted:  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:  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>
Group 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>
Group 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>
Group Member
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>
Group 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>
Group Member
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>
Group Member
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>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

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)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    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.13-d3ae.
    Corresponding source code