Oops, must be the multitasking problem. This should work -
--- snmp_alarm.c.old 2005-09-28 11:03:28.000000000 -0700
+++ snmp_alarm.c 2005-09-30 09:20:30.000000000 -0700
@@ -223,6 +223,7 @@
return NULL;
}
+static timeval t_last_run_alarms={ 0, 0 };
void
run_alarms(void)
{
@@ -231,6 +232,16 @@
unsigned int clientreg;
struct timeval t_now;
+ int timeBackFlag = 0;
+ gettimeofday(&t_now, NULL);
+ if( (t_now.tv_sec < t_last_run_alarms.tv_sec) ||
+ ((t_now.tv_sec == t_last_run_alarms.tv_sec) && (t_now.tv_usec <
t_last_run_alarms.tv_usec) )
+ ) { // we have a clock going back reset the timers
+ timeBackFlag =1;
+ }
+ t_last_run_alarms.tv_sec = t_now.tv_sec;
+ t_last_run_alarms.tv_usec = t_now.tv_usec;
+
/*
* Loop through everything we have repeatedly looking for the next thing to
* call until all events are finally in the future again.
@@ -243,13 +254,14 @@
gettimeofday(&t_now, NULL);
- if ((a->t_next.tv_sec < t_now.tv_sec) ||
+ if ( timeBackFlag || (a->t_next.tv_sec < t_now.tv_sec) ||
((a->t_next.tv_sec == t_now.tv_sec) &&
(a->t_next.tv_usec < t_now.tv_usec))) {
clientreg = a->clientreg;
DEBUGMSGTL(("snmp_alarm", "run alarm %d\n", clientreg));
(*(a->thecallback)) (clientreg, a->clientarg);
DEBUGMSGTL(("snmp_alarm", "alarm %d completed\n", clientreg));
+ timeBackFlag = 0;
if ((a = sa_find_specific(clientreg)) != NULL) {
a->t_last.tv_sec = t_now.tv_sec;
-----Original Message-----
From: Magnus Fromreide [mailto:[EMAIL PROTECTED]
Sent: Thursday, September 29, 2005 11:56 PM
To: Juan Betty-MGIA2720
Cc: [email protected]
Subject: Re: Patch for delaying of tasks attached to snmp_alarm_register
First off, please keep the discussion on the mailing list.
On Thu, Sep 29, 2005 at 04:12:38PM -0700, Juan Betty-MGIA2720 wrote:
> Hi MF,
> Resetting the system clock is common when an equipment is freshly installed
> or reinstalled. In most cases, the system clock is set forward and the agent
> is happy to continue running off this clock.
Yes, I agree with this, as long as the time doesn't go backwards that is
handled, but we could end up running things earlier than expected.
> This problem happened on us when someone fixes an incorrect system time
> (unfortunately they the incorrect time is set to the future), and the traps
> are all muted. The patch didn't attempt to mess with the system clock, it
> simply allows the tasks to be spawned once when this happen. I think the
> better fix is Net-Snmp being able to always sync up with system time.
Yes, I agree that you didn't change the system clock from the patch.
I also assume that you would get trouble in many programs, not just net-snmp,
from changing the clock. Normally the clock is set upon system installaton and
then it is slowed down or speeded up to adjust.
Now to the problems with the patch:
0) It doesn't apply since it is malformed (Chunk 1 and 2 lacks one line each)
This I am certain is just a cut'n-paste mistake or some such :-)
1) It might end up running things early if the time went backwards. Now I do
not say that early is worse than late, I just say that it is't much better
either.
2) It handles only one outstanding timeout, if there are two in the queue that
should happen in the next minute and you set back the clock an hour your
patch will run the first one and the second one will be delayed.
Now, to run every timeout is also wrong, what is needed is a way to tell
how big the time warp was and then adjust timeouts that should wait for
N time units but leave those that should run at a certain time alone.
The size of the timewarp could probably be found using something like
clock_gettime(CLOCK_MONOTONIC, ...) but that is not a required interface.
Also note that this could be used to detect forward warps.
/MF
> -----Original Message-----
> From: Magnus Fromreide [mailto:[EMAIL PROTECTED]
> Sent: Thursday, September 29, 2005 11:48 AM
> To: Juan Betty-MGIA2720
> Cc: [email protected]
> Subject: Re: Patch for delaying of tasks attached to
> snmp_alarm_register
>
> On Thu, Sep 29, 2005 at 10:47:02AM -0700, Juan Betty-MGIA2720 wrote:
> > Wes,
> > I modified and tested this patch a bit. This should be a better fix, what
> > you think ?
>
> I still have a bad feeling about this, but I can't put the finger on it.
>
> Basically timeouts are used for two purposes:
>
> a) To make something happen at wall clock time T
> b) To make something happen N time units into the future
>
> Now, your patch seems to ensure that if the wall time ever goes backwards
> then the first queued entry is run, regardless of when it should be run, so
> if it really should be run sometime next month it will be run now because the
> wall time went backwards a few seconds, and that feels very wrong.
>
> The conclusion probably is 'do not mess with the system clock' but sometimes
> that is hard to avoid.
>
> /MF
>
> > --- snmp_alarm.c.old 2005-09-28 11:03:28.000000000 -0700
> > +++ snmp_alarm.c 2005-09-29 09:24:14.000000000 -0700
> > @@ -223,6 +223,8 @@
> > return NULL;
> > }
> >
> > +static timeval t_last_run_alarms={ 0, 0 };
> > void
> > run_alarms(void)
> > {
> > @@ -231,6 +233,17 @@
> > unsigned int clientreg;
> > struct timeval t_now;
> >
> > + int timeBackFlag = 0;
> > + gettimeofday(&t_now, NULL);
> > + if( (t_now.tv_sec < t_last_run_alarms.tv_sec) ||
> > + ((t_now.tv_sec == t_last_run_alarms.tv_sec) && (t_now.tv_usec <
> > t_last_run_alarms.tv_usec) )
> > + ) { // we have a clock going back reset the timers
> > + timeBackFlag =1;
> > + }
> > + t_last_run_alarms.tv_sec = t_now.tv_sec;
> > + t_last_run_alarms.tv_usec = t_now.tv_usec;
> > +
> > /*
> > * Loop through everything we have repeatedly looking for the next
> > thing to
> > * call until all events are finally in the future again.
> > @@ -243,13 +256,14 @@
> >
> > gettimeofday(&t_now, NULL);
> >
> > - if ((a->t_next.tv_sec < t_now.tv_sec) ||
> > + if ( timeBackFlag || (a->t_next.tv_sec < t_now.tv_sec) ||
> > ((a->t_next.tv_sec == t_now.tv_sec) &&
> > (a->t_next.tv_usec < t_now.tv_usec))) {
> > clientreg = a->clientreg;
> > DEBUGMSGTL(("snmp_alarm", "run alarm %d\n", clientreg));
> > (*(a->thecallback)) (clientreg, a->clientarg);
> > DEBUGMSGTL(("snmp_alarm", "alarm %d completed\n",
> > clientreg));
> > + timeBackFlag = 0;
> >
> >
> >
> > -----Original Message-----
> > From: Juan Betty-MGIA2720
> > Sent: Wednesday, September 28, 2005 2:13 PM
> > To: 'Wes Hardaker'
> > Cc: [email protected]
> > Subject: RE: Patch for delaying of tasks attached to
> > snmp_alarm_register
> >
> > I agreed with you. I thought about it when doing the fix, that's why
> > the 1000 was picked as a trick since the struct only addresses the
> > sec and usec. (I think the factor of 100 could also work, but
> > anyway.)
> >
> > It just allows the alarms (or tasks attach to the alarm) to run at least
> > once. I think, according to the logic, the tasks time will re-sync again.
> >
> > -----Original Message-----
> > From: Wes Hardaker [mailto:[EMAIL PROTECTED]
> > Sent: Wednesday, September 28, 2005 1:52 PM
> > To: Juan Betty-MGIA2720
> > Cc: [email protected]
> > Subject: Re: Patch for delaying of tasks attached to
> > snmp_alarm_register
> >
> > >>>>> On Wed, 28 Sep 2005 12:48:34 -0700, Juan Betty-MGIA2720 <[EMAIL
> > >>>>> PROTECTED]> said:
> >
> > Juan> I found out when setting the system clock backwards (not
> > Juan> forwards), delaying of tasks that are registered with
> > Juan> snmp_alarm_register. The tasks which are scheduled to run at
> > Juan> certain time seem to be held back until the new time catches
> > Juan> up with old time again. Didn't see anyone post a patch for
> > Juan> this problem, so shared here is a quick patch implemented.
> > Juan> There is probably a better solution, but this solved my problem.
> >
> > Looks like a good patch, but there are a couple of minor issues with
> > it:
> >
> > 1) tv_usec is actualyl 1/1000000th of a tv_sec not 1/1000
> > 2) doing math the way you are would require 1000000 * tv_sec, which
> > will wrap a 32 bit integer. You can't do it that way unfortunately
> > and have to treat the tests individually (hence the reason all the
> > other code does a bunch of nasty if checks to verify one stamp is
> > less than the other).
> > 3) I didn't check the logic carefully so this may be wrong, but I
> > think you're running every alarm if the time goes backward at all?
> >
> > --
> > Wes Hardaker
> > Sparta, Inc.
> >
> >
> > -------------------------------------------------------
> > This SF.Net email is sponsored by:
> > Power Architecture Resource Center: Free content, downloads,
> > discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
> > _______________________________________________
> > Net-snmp-coders mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
> >
> >
> > -------------------------------------------------------
> > This SF.Net email is sponsored by:
> > Power Architecture Resource Center: Free content, downloads,
> > discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
> > _______________________________________________
> > Net-snmp-coders mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
--
Magnus Fromreide +46-13 17 68 48
Mårdtorpsgatan 21, 2tr [EMAIL PROTECTED]
SE-584 34 LINKÖPING
-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Net-snmp-coders mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
