buglwIP - A Lightweight TCP/IP stack - Bugs: bug #1902, Timeouts and semaphores/mailboxes...

 
 

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

bug #1902: Timeouts and semaphores/mailboxes are too tightly integrated

Submitter:  Kieran Mansley <kieranm>
Submitted:  Thu 05 Dec 2002 10:33:05 AM UTC
   
 
Category:  None Severity:  1 - Wish
Item Group:  None Status:  Invalid
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Planned Release:  None
lwIP version:  None

Jump to the original submission

Thu 23 Aug 2007 06:18:56 AM UTC, comment #43: 

I've opened task #7235 for the NO_SYS=1 issue. The core locking issue is far far away, I've put a statement about this in task #6994.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 22 Aug 2007 10:42:05 PM UTC, comment #42: 

Simon, can we close it (about coment #41) ?

Frédéric Bernon <fbernon>
Group Member
Fri 13 Jul 2007 11:59:36 AM UTC, comment #41: 

I think those issues should be raised as separate bugs so that we can track them independently.  Simon: could you do that, and then close this one?

Kieran Mansley <kieranm>
Group Member
Fri 15 Jun 2007 12:02:00 PM UTC, comment #40: 

Now that I think about it:
- timeouts don't work with NO_SYS=1 anyway, maybe we should have an implementation for that
- if (for NO_SYS=0) we move to locking the core, we can have a completely different timer handling: simply create one thread that sleeps until the next timer should be called

given that, we could leave this bug open...

Simon Goldschmidt <goldsimon>
Group administrator
Fri 15 Jun 2007 09:16:41 AM UTC, comment #39: 

If everyone else is happy, it's OK with me to close this.

Kieran Mansley <kieranm>
Group Member
Sat 09 Jun 2007 10:19:14 AM UTC, comment #38: 

Kieran, you have open it. Are you agree to close it?

Frédéric Bernon <fbernon>
Group Member
Sat 02 Jun 2007 10:37:16 AM UTC, comment #37: 

I think so, yes.

Simon Goldschmidt <goldsimon>
Group administrator
Sat 02 Jun 2007 10:23:00 AM UTC, comment #36: 

Can we close that?

Frédéric Bernon <fbernon>
Group Member
Tue 22 May 2007 08:54:41 PM UTC, comment #35: 

Ok, it's check in...

Frédéric Bernon <fbernon>
Group Member
Tue 22 May 2007 02:38:37 PM UTC, comment #34: 

So you remove sys_mbox_fetch_timeout() (was only used for LWIP_SO_RCVTIMEO) and use sys_arch_mbox_fetch() instead of sys_mbox_fetch() in api files?

I think that's a good idea, so check it in!

Simon Goldschmidt <goldsimon>
Group administrator
Tue 22 May 2007 01:34:50 PM UTC, comment #33: 

So (and because in forum's thread "lwIP Manual", we have decide to try to do all the port-breakage modifications only in the next release, because there is already some others port-breakages) I propose to check in the patch file attached. If no objects, I will add an information in CHANGELOG and sys_arch.txt to remember that users doesn't have to use lwIP internal features in their application level.


(file #12830)

Frédéric Bernon <fbernon>
Group Member
Mon 21 May 2007 05:06:19 PM UTC, comment #32: 

Silence == tacit agreement. :-)

Frédéric Bernon <fbernon>
Group Member
Mon 21 May 2007 04:43:32 PM UTC, comment #31: 

I don't think there's a problem with that, but I can't be sure - there might be some reasonable situation where someone may want to use the sys API from their own application. Perhaps change it and see who screams ;-).

Jonathan Larmour <jifl>
Group Member
Mon 21 May 2007 04:41:53 PM UTC, comment #30: 

You want to replace calls to sys_mbox_post() by calls to sys_arch_mbox_fetch() ???

Haha, but I think I got your idea (removing timeouts from api threads) and I agree.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 21 May 2007 04:35:19 PM UTC, comment #29: 

No comment about previous remark (comment #28) ? Can I propose a patch file for that?

Frédéric Bernon <fbernon>
Group Member
Wed 16 May 2007 01:01:24 PM UTC, comment #28: 


>- the timeouts/mailbox integration is a feature that is needed by tcpip.c and therefore (at least in my opinion) not a bug.
>- the timeouts/semaphore integration is not needed / lead to bugs in the past since normally a thread is not in a state to let timeouts run if waiting for a semaphore (since most of the time semaphores are used as mutexes in lwip code) therefore the timeout/semaphore integration could be thrown away reducing sys_sem_wait to a direct call to sys_arch_sem_wait


We have already talk about that in several others threads, but isn't it the time to tell that internal lwIP features shouldn't have to be used "outside" lwIP ? I think to sys_mbox_post which could be replace by direct calls to sys_arch_mbox_fetch in most of cases (in fact, I think in all cases, except tcpip_thread)...


Frédéric Bernon <fbernon>
Group Member
Wed 16 May 2007 12:50:55 PM UTC, comment #27: 

I'm agree with you about opening a new "patch" for the timestamp timer changes. Because some new features can be shared to replace the snmp timer (see https://savannah.nongnu.org/patch/?5785), I think the title should have to be something like "Integration of ticks & jiffies in timers"...



Frédéric Bernon <fbernon>
Group Member
Wed 16 May 2007 11:27:15 AM UTC, comment #26: 

OK, what I'm saying is:
- the timeouts/mailbox integration is a feature that is needed by tcpip.c and therefore (at least in my opinion) not a bug.
- the timeouts/semaphore integration is not needed / lead to bugs in the past since normally a thread is not in a state to let timeouts run if waiting for a semaphore (since most of the time semaphores are used as mutexes in lwip code) therefore the timeout/semaphore integration could be thrown away reducing sys_sem_wait to a direct call to sys_arch_sem_wait

I think you originally said you wanted to wait for my timestamp changes to close this bug. All I'm saying now is I think there's no need to wait. If we throw away semaphore/timeout integration we can do it now. If not, we can close this bug as won't fix. Timeout/mailbox integration is needed anyway.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 16 May 2007 11:21:38 AM UTC, comment #25: 

I'm not sure I understand the significance of what you're getting at (I'm not up to speed on the nitty-gritty of this one) but I wouldn't worry too much about breaking things in contrib - a lot of them are already out of date, and are going to be archived, and anything that isn't will have an active maintainer to deal with things like this.

Kieran Mansley <kieranm>
Group Member
Wed 16 May 2007 11:16:32 AM UTC, comment #24: 

Oh, what I forgot: :-)
The reason I'm posting this is that my planned changes only change accuracy of timeouts but don't change the integration of timeouts and sems/mboxes (if not removing sys_sem_wait). So what I wanted to say: maybe we should see the reason of this bug as a feature and open a new "patch" for the timestamp timer changes?

Simon Goldschmidt <goldsimon>
Group administrator
Wed 16 May 2007 11:14:18 AM UTC, comment #23: 

Kieran, as at least one implementation in contrib relies on the integration of timers with semaphores and tcpip.c relies on it with mboxes, maybe we see this as a feature instead of a bug?

I'd say the user must know what he is doing if using lwip_timeout(). But maybe we should remove calls to sys_sem_wait and sys_mbox_fetch from api files (thus removing timeout processing from api threads and not provide this feature to the users). This would effectively make sys_sem_wait unused (besides the contrib files but maybe those files could be modified to use mboxes for that) but we would not have to cope with users implementing timeouts in a wrong way.

Simon Goldschmidt <goldsimon>
Group administrator
Sun 13 May 2007 01:13:56 PM UTC, comment #22: 


>Oh, and if this goes on too long before making progress, maybe we should take out the timer processing from the semaphore functions as a separate action now?


Since I just discovered at least one of the contrib/ports/unix/netif examples relies on semaphore/timer integration, maybe we should simply change all calls to sys_sem_wait(sem) in the core stack into calls to sys_arch_sem_wait(sem, 0) to solve bugs like #19167 ?

Simon Goldschmidt <goldsimon>
Group administrator
Sun 13 May 2007 10:01:27 AM UTC, comment #21: 

OK, I've had a little time to create a new version of sys.h/sys.c (attaching the 2 new files instead of a patch) with the following changes to the original sys layer:

- semaphore functions don't check timers
- all time-related processing is done in jiffies rather than ms to avoid frequent converting
- converting is done through SYS_ARCH_JIFFIES_TO_MS()/SYS_ARCH_MS_TO_JIFFIES() macros

Changes for the ports:
- SYS_ARCH_JIFFIES_TO_MS() / SYS_ARCH_MS_TO_JIFFIES() need to be defined in sys_arch.h
- lwip_timestamp_t (unsigned) and lwip_timediff_t (signed) need to be typedef'ed or defined in cc.h
- mbox and semaphore functions in sys_arch.c get their timeout arguments as jiffies, so don't need to convert any more
- the function sys_jiffies() has to be implemented (returns the jiffies in lwip_timestamp_t)

What remains to be determined
- do we need sys_msleep() as it was (using a timer rather than simply using the OSes sleep-function)? In other words: do timers have to run while sleeping?
- no check for overflow is done, that either relies on a working port or remains to be done, I'm not quite sure where to check it... Maybe we could need 2 defines: SYS_ARCH_MAX_JIFFIES and SYS_ARCH_MAX_MS to check against when calling a timeout function (create timer / mbox_fetch / sem_wait). This is because if JIFFIES_PER_MS < 1 (e.g. ticks per second < 1000), SYS_ARCH_MAX_JIFFIES is what limits a maximum sleep value; but if JIFFIES_PER_MS > 1 (e.g. with a high resolution hardware timer), SYS_ARCH_MAX_SECONDS is what limits a maximum sleep. Because if the hardware timer wraps around at 0xffffffff (max 32bit) and you have to multiply ms by 4000 (as I have to) to get the hw-timer-ticks, the max-wait-time is only 0x10624D instead of 0xffffffff. Hm, I think I did not formulate this very clear, but that's how it is! ;-)


Finally 2 speedup issues:
- I tried to design the code in a way that defining SYS_ARCH_JIFFIES_TO_MS() & SYS_ARCH_MS_TO_JIFFIES() as macros using constants should lead to conversion at compiletime instead of converting at runtime
- As Frédéric has proposed, replacing sys_mbox_fetch calls in the api layer by sys_arch_mbox_fetch (e.g. not using timers in a non-core thread) should give a better performance. Personally, I think this is a good idea.

Oh, and if this goes on too long before making progress, maybe we should take out the timer processing from the semaphore functions as a separate action now? It seems to me this is rather unrelated to a faster/more accurate way of timestamp processing... And it does lead to problems!

(file #12761, file #12762)

Simon Goldschmidt <goldsimon>
Group administrator
Fri 27 Apr 2007 06:18:09 AM UTC, comment #20: 


>As I said we can worry about the specific implementation some other time (or if there's no demand, I guess never!).


I would rather solve this now or insist on 32-bit timers. The problem is that with 16-bit hardware timers and 10 ms ticks (which is only an example, most hardware timers have some odd frequency), the maximum wait time (MAX(s16_t)/100 = max_sec) would be ~330 seconds which is 5 minutes (which is enough for DHCP but not that far away from it!)...

For user-sleeps, we could chain s16_t sleeps to be long enough. For lwIP timers, this does not work: since we don't wait for the time-ticks, a wraparound can always occur.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 27 Apr 2007 12:39:35 AM UTC, comment #19: 

Agreed on pretty much everything. I'm not sure you need OS specific constants for "wait infinite" and "poll" though - instead give the functions different names, and if the port wants to map that to the same underlying function with special arguments, then it can do that with a #define.

As for 16-bit vs 32-bit timers. If we have a SYS_ARCH_MAX_JIFFIES, then I think we have the tools to deal with frequent wraparound. Maybe we could insist on 32-bit timers, but I'm aware that many hardware timers are 16-bit only. As I said we can worry about the specific implementation some other time (or if there's no demand, I guess never!).


Jonathan Larmour <jifl>
Group Member
Thu 26 Apr 2007 07:04:34 PM UTC, comment #18: 

OK, dealing with jiffies only in the stack would improve performance, I think. Since most OSes use them also in their semaphore and mailbox wait functions, we could also modify the sys_arch_sem_* and sys_arch_mbox_* functions eliminating the conversion from ms to jiffies in those functions as well.

Having SYS_ARCH_JIFFIES_TO_MS(jiffies, ms) and SYS_ARCH_MS_TO_JIFFIES(ms, jiffies) along with SYS_ARCH_MAX_JIFFIES should be enough to ensure easy porting. The only thing would be that sys_arch_mbox_*/sem_* would have to deal with OS specific consatns for "wait infinite" / "poll only", the rest would be directly mapped through.

Only conversion needed would be if sys_mbox_wait_timeout() or something is called.

For stack internal functions (like in tcpip_thread), this could be expanded through a define that those timespans are calculated at compile time already.

Regarding 16-bit timestamps: Can't we simply require 32-bit timestamps?

Simon Goldschmidt <goldsimon>
Group administrator
Mon 23 Apr 2007 11:49:17 AM UTC, comment #17: 

I haven't looked in detail either (although it would be nice if someone could!) but this definitely sounds like a step in the right direction.  Thanks for putting in the effort on this.

Kieran Mansley <kieranm>
Group Member
Sat 21 Apr 2007 02:04:42 AM UTC, comment #16: 

I haven't looked at the patch itself yet, but a comment or two based on the description. Rather than a single SYS_ARCH_TICKS_PER_SECOND constant , I think it would be nice for the port to provide macro functions:

SYS_ARCH_JIFFIES_TO_MS(jiffies, ms)
and
SYS_ARCH_MS_TO_JIFFIES(ms, jiffies)

The reason is that on some systems ticks per second is irrational, and we really don't want to do floating point calculations on those. Macros like these give the porters the choice, in particular to deal with rounding.

If it is rational, then of course they would just be something like:
#define SYS_ARCH_JIFFIES_TO_MS(jiffies, ms) \
  (ms) = ((jiffies)/MY_OS_JIFFIES_PER_MS)
but doesn't have to be. Even better are powers of two, when a divide can be avoided, and a shift used instead.

I think great care will be needed with wraparound, especially since these are used with user-provided timeouts (e.g. select()) which can be long. Not so much of a deal with 32-bit timestamps (although wraparound still needs detecting), but it is for 16-bit timers. This could affect the porting abstraction. For example one solution is for the porter to have a define for the maximum value of jiffies before it wraps. You then ensure you wait in individual timeouts in chunks of at most half that value (and do that repeatedly to make up to the original time requested). It also affects conversion between jiffies and ms - a 16-bit timestamp_t won't be able to hold some of the millisecond values used with it. Maybe timers < 32-bit should be a special case? Still something that has to be considered in the design.

I'm slightly concerned that existing OSes will use names like timestamp_t and timediff_t. Symbols named "*_t" are normally reserved for OSes. (e.g. see the last line of the table on http://www.opengroup.org/onlinepubs/007908799/xsh/compilation.html )
Usually you can get away with it, with suitably obscure terms, but "timestamp_t" and "timediff_t" seem less easy. Perhaps just drop the _t, or s/time/jiffy/ or prefix lwip?

I think the rename from timeouts to timers is good, personally. As are sys_timer_create and sys_timer_remove. As of course is the whole idea of using absolute timestamps, although I don't really see much value until jiffies are used internally.

Jonathan Larmour <jifl>
Group Member
Fri 20 Apr 2007 09:27:09 PM UTC, comment #15: 

Finally... I've put together a first patch introducing new timers using timestamps. This solves only semaphore/timer integration, as timer-callbacks are still called inside sys_mbox_fetch() which is a good implementation, I think (as it saves us from locking as opposed to calling timeout handlers from interrupt context). In places where this is not wanted, sys_arch_mbox_fetch(mbox, msg, 0) can directly be called.

OK, now to the patch: it introduces 2 new types (timestamp_t (normally u32_t) and timediff_t (normally s32_t)) which have to be defined in cc.h, uses the function sys_jiffies() (already defined in sys.h but no needed in many ports) and converts jiffies to ms by using a define SYS_ARCH_TICKS_PER_SECOND (to be defined in sys_arch.h). Then, expiration times are stored absolutely in timers.

NOTE: to avoid confusion with timeouts given as an argument to functions like sys_sem_wait_timeout(), I called the lwip timeous 'timers' since I think timeouts is a bad name . Further, I renamed sys_timeout to sys_timer_create and sys_untimeout to sys_timer_remove (and did not include files depending on the new names in the patch!). This must not necessarily be part of CVS, of course. We should discuss that!

Last, I have to say I'm not really content with the patch as it is, as there is much calculation from jiffies to ms which, besides performance loss includes some wrap-arounds corner cases... I'd prefer to handle all timers in jiffies and convert ms-timeouts given as function-arguments into jiffies. That way, simple addition/substraction is enough. Especially seen that most ports must convert ms to jiffies in sys_arch.c ... But that can be changed later, also.

OK, so here's the patch:

(file #12539)

Simon Goldschmidt <goldsimon>
Group administrator
Wed 11 Apr 2007 02:43:26 PM UTC, comment #14: 


>>I think you have to assume the OS clock is accurate - what else can you do!
>OK, but hooking on the timer tick still can make porting a hard thing


I wasn't intending to imply a timer hook is better. Polling the current time is far more portable.

The reason I mentioned lwip_select, is because that calls sys_sem_wait_timeout, which works in terms of lwip timeouts as set by sys_timeout().

So users who have their own timeouts using the lwip infrastructure (e.g. Dmitri), would want that to continue to work, and wouldn't want that support removed. Maybe we should just remove it anyway and make them change, so long as that's a conscious decision.

Alternatively we keep the functions for users, but don't use them inside the stack itself.


Jonathan Larmour <jifl>
Group Member
Wed 11 Apr 2007 07:51:18 AM UTC, comment #13: 

Who me?  I've been avoiding the whole timeouts issue recently as to be honest I don't fully understand the problem enough to be able to give strong opinions as to what to do, and haven't had time to bring myself up to speed.

If we could devise a scheme that was sensible and improved the liklihood of users not hitting the many-threads-in-stack problem that so many seem to do (I suspect it's usually a timer and an application thread that collide) that would be great.

I think part of the problem here is that we try to support quite a range of different applications from those with no OS through to multi-threaded with OS support, and the facilities and requirements of these vary quite significantly.

Kieran Mansley <kieranm>
Group Member
Wed 11 Apr 2007 07:04:25 AM UTC, comment #12: 


>I think you have to assume the OS clock is accurate - what else can you do!


OK, but hooking on the timer tick still can make porting a hard thing.

>I would mention here that the lwip internal thread is not the only one that uses timeouts, other than applications.


Of course, the only problem is, with the current implementation, internal timeouts can occur when the stack is not in a state to handle them. And the easiest solution is to only let timeouts happen when waiting for a message. Although this does not imply the stack will be in a state to handle timeouts with future changes, this is OK for now, as the tcpip_thread only pends on an mbox in its main loop, and the application threads normally don't have timeouts.

With lwip_select(), it's a different case: it passes a timeout value to sys_sem_wait_timeout(), but this has nothing to do with lwip timeouts. lwip_select() does not depend on a function being called while waiting. Instead, event_callback() is used, which is triggered by receive events of the stack.

Sadly, this issue is assigned to noone, yet. Also, it could need a comment from our project leader, since I beleive this to be quit a big step.

Simon Goldschmidt <goldsimon>
Group administrator
Wed 11 Apr 2007 02:05:41 AM UTC, comment #11: 

I think most systems use free-running timers for clocks where possible. I think you have to assume the OS clock is accurate - what else can you do!

I would mention here that the lwip internal thread is not the only one that uses timeouts, other than applications. So does the PPP thread. Also lwip_select() relies on semaphore waits with timeouts.

But there is one thing that from people's comments here I'm not sure is well known..... timeouts are meant to be per-thread. They are not global. If a timeout is set, its callback should only be by that same thread, not any thread. All this talk about thread safety makes me think that some people are allowing their ports to have timeout functions called by any thread.

Jonathan Larmour <jifl>
Group Member
Fri 09 Mar 2007 06:00:37 PM UTC, comment #10: 


> BTW, if the OS tick timer isn't accurate, then the
> timestamps, or mailbox timeouts for that matter,
> won't probably be either :)


Hm, if you use a hardware timer to do the timestamp, then the OS probably still uses the tick for the timeout on mailboxes. BUT: while one call to sys_arch_mbox_fetch() might be a little too late, the overall time (timestamp) stays accurate.

In our application, system timer tick is one of the lowest since other hardware has higher timing accuracy needs. If you only take the system tick as your time, it will always tick a little slower than the real time (unless you have a free-running cyclic timer, which we haven't, we have to re-trigger it in software).

Simon Goldschmidt <goldsimon>
Group administrator
Fri 09 Mar 2007 02:09:20 PM UTC, comment #9: 

I fully agree. Easy porting should definately be one of lwIP's goals. It is kind of nice that just porting semaphores and mailboxes gives timer functionality for free (and you also get this ginsu knife for absolutely no cost at all ;-). Plus the timestamp stuff has less processing overhead than hooking to the tick timer.

BTW, if the OS tick timer isn't accurate, then the timestamps, or mailbox timeouts for that matter, won't probably be either :)

Atte Kojo <kojo>
Fri 09 Mar 2007 01:47:30 PM UTC, comment #8: 

Even IF developing version 2, it would require a system where you can hook on the timer tick and that would have to be accurate.

With the timestamp solution, you only require 2 OS functions: one that can give you the current time and one that can wait on mail using a timeout. It's simpler and easier to port than the tick-hook-thing.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 09 Mar 2007 01:28:42 PM UTC, comment #7: 

Dmitry:

> sending a message per each tick is too expensive...


That isn't of course necessary. The timer with smallest interval is the TCP fast timer with 250 ms resolution and all other timer intervals are multiples of fast timer interval. Hence 250 ms is the minimum interval between two timer messages.

Of course all timer processing could be done in the tick hook, but it would cause all kinds of additional headaches. The timeout list would have to be protected against concurrent accesses and the time spent in the OS tick handler would increase.

But, as I said orignally, this is probably too intrusive, unless you want to start developing version 2 of the stack.

Atte Kojo <kojo>
Fri 09 Mar 2007 12:01:22 PM UTC, comment #6: 


>sending a message per each tick is too expensive...


definitively, yes

> So, perhaps, we should replace sys_mbox_fetch with sys_arch_mbox_fetch in api_lib.c


That's good for me. But how do we get more comments? I'd like to hear from more users than the 4 we've had so far.

Simon Goldschmidt <goldsimon>
Group administrator
Fri 09 Mar 2007 11:15:43 AM UTC, comment #5: 

Atte:

> Create a timeout hook function that is called from OS's tick handler. The hook function sends a timeout message to tcpip_thread


sending a message per each tick is too expensive...

Frédéric:

> it's is really good to use lwip internals features?

You are right, it is not... So, perhaps, we should replace sys_mbox_fetch with sys_arch_mbox_fetch in api_lib.c to make the code more simple, but I would like to know what others think before making any changes here.



Dmitry Potapov <dpotapov>
Fri 09 Mar 2007 09:55:00 AM UTC, comment #4: 

I agree with Frédéric on removing sys_timeout stuff entirely from user-side functions. Simply calling sys_arch_mbox_fetch and sys_sem_wait suffices for user-side timeouts.

In my opinion the stack's internal timeouts could be handled in one of two ways:

1. Make all sys_* functions just thin wrappers around native OS functions and use a special timeout handling wrapper in tcpip_thread á la current sys_mbox_fetch implementation (with timestamping as discussed in bug #19222) around sys_arch_mbox_fetch.

2. Create a timeout hook function that is called from OS's tick handler. The hook function sends a timeout message to tcpip_thread which then calls the appropriate timeout handler. This could be similar to how API calls are currently handled. (I guess that this is what Christiaan was talking about with his SIGALRM stuff).

Both implementations would make timer handling thread-safe because the timers are only called from core stack contenxt. Another plus is that the stack is always in well defined state when calling timeout handlers. Especially when combined with the solution to bug #19206.

Solution 2 would provide pretty accurate timers but since it require re-designing/writing how lwIP handles timeouts, I'd go for solution 1 now. It should provide good enough accuracy for most purposes. I also think that this is what Simon and Frédéric have in mind.

Atte Kojo <kojo>
Fri 09 Mar 2007 09:24:45 AM UTC, comment #3: 

Hi, agree with you.

More informations, based on "problems" I cause to Dmity and others with SO_RCVTIMEO (sorry :( ):

I do lot of tests and measures to evaluate Dimtry's solution (sys_mbox_fetch_timeout based based on sys_mbox_fetch+sys_timeout, like sys_sem_wait_timeout is based on sys_sem_wait+sys_timeout), and in fact, there is not sensible performance difference between his solution and mine (I try on recv and/or write scenarios, with 4Mbps, and with a sys_timeout(10ms) in application to check all the code).

But, you can increase performance when you replace in api_lib.c all sys_mbox_fetch per directly sys_arch_mbox_fetch !! And the code is simpler. In my case, it's work fine and faster (note, I don't have test "select()"), but, if like Dmitry, you have to use internal sys_timeout features in your application level, it's a real problem. But, it's is really good to use lwip internals features? Perhaps it will prevent to do some futur improvements inside lwip core? I don't know. We can talk about that...

I also think that current timeouts implementation in sys.c - even if they help to reduce footprint - are not really efficent (if any internal process take too "long" time, timer would not be accurate), and some timers (arp, dhcp, etc...) would have to be used only in tcpip_thread (the real core stack context when you are in multithread) to be safe-thread.

Perhaps, to kwow what can be done, it will be useful to do a little poll to learn how most of lwip users integrate it (to know what api can be change to avoid extra code, what give more performance for most of users, etc....)


Frédéric Bernon <fbernon>
Group Member
Fri 09 Mar 2007 08:27:16 AM UTC, comment #2: 

I think this is still a major issue. To solve it means quite a big change in implementation of the sys layer. Because of this, I'd like comments from lwip_users, also.

A solution (as discussed e.g. in bug #19167) would be the following:

1. ignore timeouts in semaphore functions since normally, the stack is not in a condition to let timeout callbacks run if it waits for a sem.

2. timeouts for stack-internal functions may only run in stack context. initialization should take care of it.

3. IF timeouts for api-threads (like app threads using a socket) are used, great care must be taken! e.g. timeouts should not run during sys_mbox_fetch() in netconn_bind(). In other words, the timeout api functions should NOT be called by a user application. This could best be achieved by having mboxes allowing timeouts and mboxes not allowing them.

4. To improve timeout accuracy, timestamps should be used. It's still to be determined if 32-bit timestamps are accurate enough.

Please send your comments, I don't want to change it in the wrong direction.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 04 Jul 2006 11:45:22 AM UTC, comment #1: 

A proper portable timer solution would be nice replacement for sys_timout(). sys_timeout() seems to give erratic / random results with the sequential API, I'm only guessing what's going wrong there. I think we should keep timeouts for mailboxes for the purpose of polling a mailbox without blocking the thread.

I've tinkered a SIGALRM based timer as a quick hack into contrib/ports/unix/proj/minimal/timer.c Maybe this can be turned  into a more portable/generic solution. More ideas?

Christiaan Simons <christiaans>
Group Member
Thu 05 Dec 2002 10:33:05 AM UTC, original submission:  

The current implementation of lwIP has implements both of these functions as one.  A number of people have found this to be too restrictive for their needs and would prefer to see a separation of the two.

Kieran Mansley <kieranm>
Group Member

 

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

Attached Files
file #12830:  sys.c.patch added by fbernon (5KiB - text/x-patch)
file #12761:  sys.c added by goldsimon (9KiB - text/x-csrc)
file #12762:  sys.h added by goldsimon (9KiB - text/plain)
file #12539:  newtimers1.patch added by goldsimon (18KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by jifl (Posted a comment)
  • -email is unavailable- added by dpotapov (Posted a comment)
  • -email is unavailable- added by fbernon (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  •  

    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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2007-08-23 goldsimon StatusNone Invalid
        Open/ClosedOpen Closed
    2007-05-22 fbernon Attached File- Added sys.c.patch, #12830
    2007-05-13 goldsimon Attached File- Added sys.c, #12761
        Attached File- Added sys.h, #12762
    2007-04-20 goldsimon Attached File- Added newtimers1.patch, #12539
    2007-03-09 kojo Carbon-CopyRemoved 51921 -
    2007-03-09 kojo Carbon-CopyRemoved 51921 -
    2007-03-09 kojo Carbon-CopyRemoved 51921 -

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code