-
Notifications
You must be signed in to change notification settings - Fork 342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use monotonic clock. #281
base: master
Are you sure you want to change the base?
Use monotonic clock. #281
Conversation
I seriously doubt that all systems that we support will have the function that you use to get the monotonic clock. So, you've done things nicely that there is now one "getmonotime" function whose implementation could be what you've written, OR just "gettimeofday" when clock_gettime or TIMESPEC_TO_TIMEVAL does not exist. Ideally a compile time check would determine if clock_gettime is available and use the "better" interface when available. If you can't hack autoconf to make it that way I need proof that this causes problems in practice before I commit such a change that is sure to cause problems on systems that don't have clock_gettime in the way that you expect.... |
This avoids a jumping system clock causing issues.
gettimeofday(&lasttime, NULL); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use getmonotime
here?
gettimeofday(&thistime, NULL); | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use getmonotime
here?
Look, I don't understand what the problem is that this solves. The systemtime is normally monotonic, it doesn't jump around when it wants to. Or say, "well, on tuesdays it is often really bad". No, time just goes forward all the time. So while this is running, ntp finally trusts the measurements enough to jump the system time. from 10:05:23 time suddenly jumps to 10:05:28. Now packets sent at 10:05:21, suddenly arrive only at 10:05:36. getmonotime does nothing to correct this. If ntp jumps the time the other way, there might be some "faster" packets that get a corrected, but still wrong timestamp when they get back. On the long packets, you simply get wrong measurements again. If mtr does see and display these absurd values, the user can clearly see that there are measurement errors. Suppose this jump happens when I'm not looking. When I get back, I see a nicely progressing "minimum time" going from 0 to 1 second. That minimum time is what I often use to guess the "no congestion" delay of a path. When there are negative numbers in the list I know there is a measurement error. If all looks seemingly OK, I might be convinced that those numbers are real. getmonotime is meant for situations where time going backwards is really bad, Or when two consecutive timestamps should in no circumstance be the same, because the order in which things happened is very important for you. So..... I'm afraid such a change will break compatibility with some existing platforms that we support. Secondly I don't see the problem that it is supposed to fix. |
#ifdef HAVE_CLOCK_GETTIME | ||
#include <time.h> | ||
#endif | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to add #include <stddef.h>
otherwise NULL
is undefined.
This avoids a jumping system clock causing issues.