buglwIP - A Lightweight TCP/IP stack - Bugs: bug #48568, timeouts does not support late...

 
 

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

bug #48568: timeouts does not support late firing properly

Submitter:  Sylvain Rochet <gradator>
Submitted:  Wed 20 Jul 2016 10:07:28 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  2.0.0 Beta2
lwIP version:  git head

Wed 27 Jul 2016 07:28:23 PM UTC, comment #4: 

I've fixed this bug, can we continue the diff-time vs. absolute-time in a seperate task, or reuse patch #7855?

Hey, I've tried lwIP on an ATmega just ~2 years ago, andi t worked! (OK, I only had a slipif and needed a slipif-to-ethernet router on my PC...) But that was no a real-world thing, of course... ;-)

Simon Goldschmidt <goldsimon>
Group administrator
Wed 27 Jul 2016 07:20:29 PM UTC, comment #3: 

Absolute timers are great, and faster, because the comparison is obviously the most simple comparison that could be made, well, at least, on 32-bit targets (are there still users of lwIP on 8-bit platforms ?, that doesn't sound like a good design choice nowadays ;-), I'm pretty none of us tried that for a long time).

But, there is always a but, it means we need a new u32_t field in the timeout struct to store the target time, so we can keep the timeout period, we need that in order to be able to restart all timers on wakeup event for devices that sleep, this is what the timeouts_last_time is currently used to. With delta-diff timers we only need one variable, with absolute timers, we need one "timeouts_last_time"-equivalent per timer.

Sylvain Rochet <gradator>
Group Member
Wed 27 Jul 2016 07:06:17 PM UTC, comment #2: 


> This could be fixed by moving to absolute target timers


Given the fact that less-than-32 bit platforms get more uncommon, the arguments for delta-diff timers get less pressing, indeed ;-)

I'd be curious to see a diff to support absolute times. With correct refactoring, it shouldn't be too messy?

We do have patch #7855, but this is far from being a small diff...

Simon Goldschmidt <goldsimon>
Group administrator
Wed 20 Jul 2016 10:09:19 AM UTC, comment #1: 

This could be fixed by moving to absolute target timers instead of the currently used delta-diff timers (which I dislike, due to the problem mentioned below, at least). The main problem is our current support of timers restart on wake up event for devices that sleep to prevent firing all timers at once, that's hard to achieve with absolute target timers.

Or maybe we can find a hack about the timeouts_last_time to take into account the extra period if we are able to detect it.

Sylvain Rochet <gradator>
Group Member
Wed 20 Jul 2016 10:07:28 AM UTC, original submission:  

With the current code, timeouts get delayed when we are in a debugger breakpoint and sys_now() might continues running while we are stopped (e.g. when it uses a clock source from an OS which does not stop).

sys_check_timeouts() increases timeouts_last_time only by the expired timeout's time. If that handler restarts a timeout (as is the case for all lwIP-internal timers), it gets an addditional delay of the elapsed time in the debugger stop period, meaning if we stopped for 2 minutes, the timer will get an extra 2 minutes delay.

Sylvain Rochet <gradator>
Group Member

 

(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 goldsimon (Posted a comment)
  • -email is unavailable- added by gradator (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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-27 goldsimon StatusNone Fixed
        Assigned toNone goldsimon
        Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code