[PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Fan Du]

 From e3929d4fdfad5b40fd8cad0e217597670d1aef54 Mon Sep 17 00:00:00 2001
From: Fan Du <fan.du@windriver.com>
Date: Wed, 14 Aug 2013 16:39:23 +0800
Subject: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

When clock_was_set is called in case of system wall time change
or host resume from suspend state, use this notifier for places
where interested in this action, e.g Ipsec SA lifetime management.

Signed-off-by: Fan Du <fan.du@windriver.com>

v3:
   -Beautify notifier with register/unregister API exported for other subsystem.

---
  include/linux/hrtimer.h |    3 +++
  kernel/hrtimer.c        |   19 +++++++++++++++++++
  2 files changed, 22 insertions(+)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index d19a5c2..f0404e4 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -461,4 +461,7 @@ extern u64 ktime_divns(const ktime_t kt, s64 div);
  /* Show pending timers: */
  extern void sysrq_timer_list_show(void);

+extern int register_clock_change_notifier(struct notifier_block *nb);
+extern int unregister_clock_change_notifier(struct notifier_block *nb);
+
  #endif
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 383319b..c6e6405 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -755,6 +755,24 @@ static inline void retrigger_next_event(void *arg) { }

  #endif /* CONFIG_HIGH_RES_TIMERS */

+static ATOMIC_NOTIFIER_HEAD(clock_change_notifier_list);
+static int call_clock_change_notifiers(void)
+{
+	return atomic_notifier_call_chain(&clock_change_notifier_list, 0, 0);
+}
+
+int register_clock_change_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_register(&clock_change_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_clock_change_notifier);
+
+int unregister_clock_change_notifier(struct notifier_block *nb)
+{
+	return atomic_notifier_chain_unregister(&clock_change_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_clock_change_notifier);
+
  /*
   * Clock realtime was set
   *
@@ -773,6 +791,7 @@ void clock_was_set(void)
  	on_each_cpu(retrigger_next_event, NULL, 1);
  #endif
  	timerfd_clock_was_set();
+	call_clock_change_notifiers();
  }

  /*
-- 
1.7.9.5


-- 
浮沉随浪只记今朝笑

--fan

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Sergei Shtylyov]

Hello.

On 08/14/2013 12:52 PM, Fan Du wrote:

>  From e3929d4fdfad5b40fd8cad0e217597670d1aef54 Mon Sep 17 00:00:00 2001
> From: Fan Du <fan.du@windriver.com>
> Date: Wed, 14 Aug 2013 16:39:23 +0800
> Subject: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

    This header is not needed.

> When clock_was_set is called in case of system wall time change
> or host resume from suspend state, use this notifier for places
> where interested in this action, e.g Ipsec SA lifetime management.

> Signed-off-by: Fan Du <fan.du@windriver.com>

> v3:
>    -Beautify notifier with register/unregister API exported for other subsystem.

[...]

> diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
> index d19a5c2..f0404e4 100644
> --- a/include/linux/hrtimer.h
> +++ b/include/linux/hrtimer.h
> @@ -461,4 +461,7 @@ extern u64 ktime_divns(const ktime_t kt, s64 div);
>   /* Show pending timers: */
>   extern void sysrq_timer_list_show(void);
>
> +extern int register_clock_change_notifier(struct notifier_block *nb);
> +extern int unregister_clock_change_notifier(struct notifier_block *nb);
> +
>   #endif
> diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
> index 383319b..c6e6405 100644
> --- a/kernel/hrtimer.c
> +++ b/kernel/hrtimer.c
> @@ -755,6 +755,24 @@ static inline void retrigger_next_event(void *arg) { }
>
>   #endif /* CONFIG_HIGH_RES_TIMERS */
>
> +static ATOMIC_NOTIFIER_HEAD(clock_change_notifier_list);
> +static int call_clock_change_notifiers(void)
> +{
> +    return atomic_notifier_call_chain(&clock_change_notifier_list, 0, 0);
> +}
> +
> +int register_clock_change_notifier(struct notifier_block *nb)
> +{
> +    return atomic_notifier_chain_register(&clock_change_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_clock_change_notifier);
> +
> +int unregister_clock_change_notifier(struct notifier_block *nb)
> +{
> +    return atomic_notifier_chain_unregister(&clock_change_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_clock_change_notifier);
> +
>   /*
>    * Clock realtime was set
>    *
> @@ -773,6 +791,7 @@ void clock_was_set(void)
>       on_each_cpu(retrigger_next_event, NULL, 1);
>   #endif
>       timerfd_clock_was_set();
> +    call_clock_change_notifiers();
>   }

    Your patch seems whitepsace damaged, i.e. a space is added before each 
line starting with space.

WBR, Sergei

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Thomas Gleixner]

On Wed, 14 Aug 2013, Fan Du wrote:

> From e3929d4fdfad5b40fd8cad0e217597670d1aef54 Mon Sep 17 00:00:00 2001
> From: Fan Du <fan.du@windriver.com>
> Date: Wed, 14 Aug 2013 16:39:23 +0800
> Subject: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was
> called
> 
> When clock_was_set is called in case of system wall time change
> or host resume from suspend state, use this notifier for places
> where interested in this action, e.g Ipsec SA lifetime management.

Sigh. These notifiers have been proposed in the past and we always
rejected them. Why do you need this? There should be nothing except
the core timekeeping code which needs to know about clock_was_set.

Can you please explain what kind of users you have in mind and WHY
they need to know about it.

Thanks,

	tglx

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Fan Du]

On 2013年08月18日 17:05, Thomas Gleixner wrote:
> On Wed, 14 Aug 2013, Fan Du wrote:
>
>>  From e3929d4fdfad5b40fd8cad0e217597670d1aef54 Mon Sep 17 00:00:00 2001
>> From: Fan Du<fan.du@windriver.com>
>> Date: Wed, 14 Aug 2013 16:39:23 +0800
>> Subject: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was
>> called
>>
>> When clock_was_set is called in case of system wall time change
>> or host resume from suspend state, use this notifier for places
>> where interested in this action, e.g Ipsec SA lifetime management.
>
> Sigh. These notifiers have been proposed in the past and we always
> rejected them. Why do you need this? There should be nothing except
> the core timekeeping code which needs to know about clock_was_set.
>
> Can you please explain what kind of users you have in mind and WHY
> they need to know about it.

Hi, Thomas


Thanks for your patience. Please let me take a few seconds try to
explain this.

Current xfrm layers has *one* hrtimer to guard Ipsec keys timeout,
The timeout could be measured in either of below two ways:

  (1) The timer is started once the keys is created, but this
      key is not necessary actually used right now. In detail,
      record the get_seconds() when this key is created.

  (2) Starting the timer when this key is actually used, e.g when
      an IP packet need to be encrypted. In details, recored the
      get_seconds() when this key is first used.

So in the hrtimer handler, the code get current get_seconds, and
check against with what saved in (1)or(2), and notify the timeout
up to user land.

So the pitfall is using one hrtimer for two timeout events,
most importantly using get_seconds to check timeout, once system
clock is changed by user intentionally, the key timeout could
misbehave wildly.

A refractor has been proposed to get rid of depending on system wall
clock by cleaning up the hrtimer handler. Unfortunately David frowned
on it in (3), and suggest once system clock is changed, adjust the
timeout of the key.


(3): http://www.spinics.net/lists/netdev/msg245169.html


> Thanks,
>
> 	tglx
>

-- 
浮沉随浪只记今朝笑

--fan

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Thomas Gleixner]

On Tue, 20 Aug 2013, Fan Du wrote:
> Thanks for your patience. Please let me take a few seconds try to
> explain this.

Sorry for the late reply.
 
> Current xfrm layers has *one* hrtimer to guard Ipsec keys timeout,
> The timeout could be measured in either of below two ways:
> 
>  (1) The timer is started once the keys is created, but this
>      key is not necessary actually used right now. In detail,
>      record the get_seconds() when this key is created.
> 
>  (2) Starting the timer when this key is actually used, e.g when
>      an IP packet need to be encrypted. In details, recored the
>      get_seconds() when this key is first used.
> 
> So in the hrtimer handler, the code get current get_seconds, and
> check against with what saved in (1)or(2), and notify the timeout
> up to user land.
> 
> So the pitfall is using one hrtimer for two timeout events,
> most importantly using get_seconds to check timeout, once system
> clock is changed by user intentionally, the key timeout could
> misbehave wildly.
> 
> A refractor has been proposed to get rid of depending on system wall
> clock by cleaning up the hrtimer handler. Unfortunately David frowned
> on it in (3), and suggest once system clock is changed, adjust the
> timeout of the key.
> 
> 
> (3): http://www.spinics.net/lists/netdev/msg245169.html

Thanks for the explanation so far.

What's still unclear to me is why these timeouts are bound to wall
time in the first place.

Is there any real reason why the key life time can't simply be
expressed in monotonic time, e.g. N seconds after creation or M
seconds after usage? Looking at the relevant RFCs I can't find any
requirement for binding the life time to wall time. 

A life time of 10 minutes does not change when the wall clock is
adjusted for whatever reasons. It's still 10 minutes and not some
random result of the wall clock adjustments. But I might be wrong as
usual :)

Thanks,

	tglx

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Herbert Xu]

On Thu, Sep 12, 2013 at 03:21:24PM +0200, Thomas Gleixner wrote:
>
> > (3): http://www.spinics.net/lists/netdev/msg245169.html
> 
> Thanks for the explanation so far.
> 
> What's still unclear to me is why these timeouts are bound to wall
> time in the first place.
> 
> Is there any real reason why the key life time can't simply be
> expressed in monotonic time, e.g. N seconds after creation or M
> seconds after usage? Looking at the relevant RFCs I can't find any
> requirement for binding the life time to wall time. 
> 
> A life time of 10 minutes does not change when the wall clock is
> adjusted for whatever reasons. It's still 10 minutes and not some
> random result of the wall clock adjustments. But I might be wrong as
> usual :)

Well we started out with straight timers.  It was changed because
people wanted IPsec SAs to expire after a suspect/resume which
AFAIK does not touch normal timers.

Of course, this brought with it a new set of problems when the
system time is stepped which now cause SAs to expire even though
they probably shouldn't.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Thomas Gleixner]

On Thu, 12 Sep 2013, Herbert Xu wrote:
> On Thu, Sep 12, 2013 at 03:21:24PM +0200, Thomas Gleixner wrote:
> >
> > > (3): http://www.spinics.net/lists/netdev/msg245169.html
> > 
> > Thanks for the explanation so far.
> > 
> > What's still unclear to me is why these timeouts are bound to wall
> > time in the first place.
> > 
> > Is there any real reason why the key life time can't simply be
> > expressed in monotonic time, e.g. N seconds after creation or M
> > seconds after usage? Looking at the relevant RFCs I can't find any
> > requirement for binding the life time to wall time. 
> > 
> > A life time of 10 minutes does not change when the wall clock is
> > adjusted for whatever reasons. It's still 10 minutes and not some
> > random result of the wall clock adjustments. But I might be wrong as
> > usual :)
> 
> Well we started out with straight timers.  It was changed because
> people wanted IPsec SAs to expire after a suspect/resume which

Right suspend is the usual suspect :)

> AFAIK does not touch normal timers.
> 
> Of course, this brought with it a new set of problems when the
> system time is stepped which now cause SAs to expire even though
> they probably shouldn't.

Right. That's what I guessed. So your problem is that the timer_list
timers which are the proper mechanism for this (the life time has a 1
second granularity, so hrtimers are complete overkill) are not
expiring after a suspend/resume cycle.

So what about going back to timer_list timers and simply utilize
register_pm_notifier(), which will tell you that the system resumed?

Thanks,

	tglx

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[David Miller]

From: Thomas Gleixner <tglx@linutronix.de>
Date: Thu, 12 Sep 2013 16:43:37 +0200 (CEST)

> So what about going back to timer_list timers and simply utilize
> register_pm_notifier(), which will tell you that the system resumed?

The thing to understand is that there are two timeouts for an IPSEC
rule, a soft and a hard timeout.

There is a gap between these two exactly so that we can negotiate a
new encapsulation with the IPSEC gateway before communication ceases
to be possible over the IPSEC protected path.

So the idea is that the soft timeout triggers the re-negotiation,
and after a hard timeout the IPSEC path is no longer usable and
all communication will fail.

Simply triggering a re-negoation after every suspend/resume makes
no sense at all.  Spurious re-negotiations are undesirable.

What we want are real timers.  We want that rather than a "we
suspended so just assume all timers expired" event which is not very
useful for this kind of application.

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Thomas Gleixner]

On Thu, 12 Sep 2013, David Miller wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Thu, 12 Sep 2013 16:43:37 +0200 (CEST)
> 
> > So what about going back to timer_list timers and simply utilize
> > register_pm_notifier(), which will tell you that the system resumed?
> 
> The thing to understand is that there are two timeouts for an IPSEC
> rule, a soft and a hard timeout.
> 
> There is a gap between these two exactly so that we can negotiate a
> new encapsulation with the IPSEC gateway before communication ceases
> to be possible over the IPSEC protected path.
> 
> So the idea is that the soft timeout triggers the re-negotiation,
> and after a hard timeout the IPSEC path is no longer usable and
> all communication will fail.
> 
> Simply triggering a re-negoation after every suspend/resume makes
> no sense at all.  Spurious re-negotiations are undesirable.
> 
> What we want are real timers.  We want that rather than a "we
> suspended so just assume all timers expired" event which is not very
> useful for this kind of application.

Your argumentation makes no sense at all. Where is the difference
between the "real timer" plus a clock was set notification and a timer
list timer and a resume notification?

In both cases you need to walk through the timers and reevaluate
them. Just in the clock was set notification case you need to deal
with NTP/settimeofday/PPS and whatever cases which are completely
irrelevant to the life time management.

So what's wrong with:

   now = get_seconds();
   x->timeout = now + x->soft_timeout;
   x->timeout_active = SOFT;
   start_timer(x->timer, jiffies + sec_to_jiffies(x->soft_timeout));

In the timer handler:

   switch (x->timeout_active) {
      case SOFT:
            trigger_renegotiation();
	    hts = x->hard_timeout - x->soft_timeout;
	    x->timeout += hts;
	    x->timeout_active = HARD;
      	    start_timer(x->timer, jiffies + sec_to_jiffies(hts));
	    break;
      case HARD:
      	   stop_connection();
	   break:
   }
   
If the negotiation succeeds:

   now = get_seconds();
   x->t_timeout = now + x->soft_timeout;
   x->timeout_active = SOFT;
   mod_timer(x->timer, jiffies + sec_to_jiffies(x->soft_timeout));

Now in the resume notification you walk the active timers and do for
each timer:

    now = get_seconds();

    switch (x->timeout_active) {
    case SOFT:
    	 if (now >= x->timeout) {
	    hts = x->hard_timeout - x->soft_timeout;
	    x->timeout += hts;
	    if (now >= x->timeout) {
	       del_timer(x->timeout);
	       stop_connection();
	       break:
	    }
	    trigger_renegotiation();
	    x->timeout_active = HARD;
      	    mod_timer(x->timer, jiffies + sec_to_jiffies(hts));
	    break;
   	 }
	 delta = x->timeout - now;
      	 mod_timer(x->timer, jiffies + sec_to_jiffies(delta));
	 break;

    case HARD:	    
    	 if (now >= x->timeout) {
	       del_timer(x->timeout);
	       stop_connection();
	 }    	 
	 delta = x->timeout - now;
      	 mod_timer(x->timer, jiffies + sec_to_jiffies(delta));
	 break;
    }

That's what you have to do with a clock was set notification as
well. And what's worse is that you need to figure out whether the
clock change was due to a suspend/resume cycle or just because
NTP/settimeofday/PPS or whatever decided to fiddle with the wall
time. And you need to do that to avoid the spurious renegotiations
which you are afraid of.


Thanks,

	tglx

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Fan Du]

Hi Dave/Thomas

On 2013年09月13日 01:32, David Miller wrote:
> From: Thomas Gleixner<tglx@linutronix.de>
> Date: Thu, 12 Sep 2013 16:43:37 +0200 (CEST)
>
>> So what about going back to timer_list timers and simply utilize
>> register_pm_notifier(), which will tell you that the system resumed?
>
> The thing to understand is that there are two timeouts for an IPSEC
> rule, a soft and a hard timeout.
>
> There is a gap between these two exactly so that we can negotiate a
> new encapsulation with the IPSEC gateway before communication ceases
> to be possible over the IPSEC protected path.
>
> So the idea is that the soft timeout triggers the re-negotiation,
> and after a hard timeout the IPSEC path is no longer usable and
> all communication will fail.
>
> Simply triggering a re-negoation after every suspend/resume makes
> no sense at all.  Spurious re-negotiations are undesirable.
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (*a*)

What's the differences between this with re-negotiation after every
system wall clock changing by using clock_was_set notifier?


 > On 2013年08月02日 06:35, David Miller wrote:
 >
 > I suspect the thing to do is to have system time changes generate a
 > notifier when clock_was_set() is called.
 >
 > The XFRM code would walk the rules and pretend that we hit the soft
 > timeout for every rule that we haven't hit the soft timeout yet
 > already.
 >
 > If a rule hit the soft timeout, force a hard timeout.
 >
 > When forcing a soft timeout, adjust the hard timeout to be
 > (hard_timeout - soft_timeout) into the future.



> What we want are real timers.  We want that rather than a "we
> suspended so just assume all timers expired" event which is not very
> useful for this kind of application.
>

Here we are facing two problems:)

(1) what kind timer should xfrm_state should employ, Two requirements here:
     First one, KEY lifetime should include suspend/resume time. Second one,
     system wall clock time changing(backward/forward) should *not* impact
     *timer* timeout event(not the soft/hard IPsec events fired to user space!)

     net-next commit 99565a6c471cbb66caa68347c195133017559943 ("xfrm: Make
     xfrm_state timer monotonic") by utilizing *CLOCK_BOOTTIME* has solved this problem.

(2) What I have been bugging you around here for this long time is really the second
     problem, I'm sorry I didn't make it clearly to you and others, which is below:

     Why using wall clock time to calculate soft/hard IPsec events when xfrm_state timer
     out happens in its timeout handler? Because even if xfrm_state using CLOCK_BOOTTIME,
     system wall clock time changing will surely disturb soft/hard IPsec events, which
     you raised your concern about in (*a*).

     The initial approach( http://marc.info/?l=linux-netdev&m=137534280429187&w=2) has
     tried to solve this second problem by eliminating depending system wall clock in
     xfrm_state timer timeout handler.

I think this time, I have made this situation crystal clear.

-- 
浮沉随浪只记今朝笑

--fan

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Thomas Gleixner]

On Fri, 13 Sep 2013, Fan Du wrote:
> (2) What I have been bugging you around here for this long time is really the
> second
>     problem, I'm sorry I didn't make it clearly to you and others, which is
> below:
> 
>     Why using wall clock time to calculate soft/hard IPsec events when
> xfrm_state timer
>     out happens in its timeout handler? Because even if xfrm_state using
> CLOCK_BOOTTIME,
>     system wall clock time changing will surely disturb soft/hard IPsec
> events, which
>     you raised your concern about in (*a*).

No CLOCK_BOOTTIME is not affected by wall clock time changes. It's
basically CLOCK_MONOTONIC, it just keeps counting the suspend time as
well. So without a suspend/resume cycle CLOCK_MONOTONIC and
CLOCK_BOOTTIME are the same. After a suspend/resume cycle
CLOCK_BOOTTIME will be N seconds ahead of CLOCK_MONOTONIC, where N is
the number of seconds spent in suspend.

Thanks,

	tglx

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Fan Du]

Hi, Thomas

On 2013年09月13日 22:32, Thomas Gleixner wrote:
> On Fri, 13 Sep 2013, Fan Du wrote:
>> (2) What I have been bugging you around here for this long time is really the
>> second
>>      problem, I'm sorry I didn't make it clearly to you and others, which is
>> below:
>>
>>      Why using wall clock time to calculate soft/hard IPsec events when
>> xfrm_state timer
>>      out happens in its timeout handler? Because even if xfrm_state using
>> CLOCK_BOOTTIME,
>>      system wall clock time changing will surely disturb soft/hard IPsec
>> events, which
>>      you raised your concern about in (*a*).
>
> No CLOCK_BOOTTIME is not affected by wall clock time changes. It's
> basically CLOCK_MONOTONIC, it just keeps counting the suspend time as
> well. So without a suspend/resume cycle CLOCK_MONOTONIC and
> CLOCK_BOOTTIME are the same. After a suspend/resume cycle
> CLOCK_BOOTTIME will be N seconds ahead of CLOCK_MONOTONIC, where N is
> the number of seconds spent in suspend.

Sorry for the ambiguity. Yes, CLOCK_BOOTTIME is monotonic *and* counting
suspend/resume time as well as not affected by wall clock time change.

Using CLOCK_BOOTTIME guarantees b- will timeout in a right manner, i.e., not
affected by wall clock time change, as well as keep the timer valid when
suspend/resume.

A classic way of using timer is:
  a- Arm a timer with specified interval
  b- Wait for the timer to timeout
  c- After the timeout, notify the event to other place in the timer handler.

IPsec key lifetime timer does its this way:
  a- Arm a timer with specified interval
  b- Wait for the timer to timeout
  c- After timeout, in the timer handler, using wall clock time to calculate
    which kind of event to notify user(soft or hard for both key use time,
    and key created time). So the problem arises at this stage if wall clock
    time changed.



Thanks

> Thanks,
>
> 	tglx
>

-- 
浮沉随浪只记今朝笑

--fan

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Thomas Gleixner]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2076 bytes --]

On Mon, 16 Sep 2013, Fan Du wrote:
> On 2013年09月13日 22:32, Thomas Gleixner wrote:
> > On Fri, 13 Sep 2013, Fan Du wrote:
> > > (2) What I have been bugging you around here for this long time is really
> > > the
> > > second
> > >      problem, I'm sorry I didn't make it clearly to you and others, which
> > > is
> > > below:
> > > 
> > >      Why using wall clock time to calculate soft/hard IPsec events when
> > > xfrm_state timer
> > >      out happens in its timeout handler? Because even if xfrm_state using
> > > CLOCK_BOOTTIME,
> > >      system wall clock time changing will surely disturb soft/hard IPsec
> > > events, which
> > >      you raised your concern about in (*a*).
> > 
> > No CLOCK_BOOTTIME is not affected by wall clock time changes. It's
> > basically CLOCK_MONOTONIC, it just keeps counting the suspend time as
> > well. So without a suspend/resume cycle CLOCK_MONOTONIC and
> > CLOCK_BOOTTIME are the same. After a suspend/resume cycle
> > CLOCK_BOOTTIME will be N seconds ahead of CLOCK_MONOTONIC, where N is
> > the number of seconds spent in suspend.
> 
> Sorry for the ambiguity. Yes, CLOCK_BOOTTIME is monotonic *and* counting
> suspend/resume time as well as not affected by wall clock time change.
> 
> Using CLOCK_BOOTTIME guarantees b- will timeout in a right manner, i.e., not
> affected by wall clock time change, as well as keep the timer valid when
> suspend/resume.
> 
> A classic way of using timer is:
>  a- Arm a timer with specified interval
>  b- Wait for the timer to timeout
>  c- After the timeout, notify the event to other place in the timer handler.
> 
> IPsec key lifetime timer does its this way:
>  a- Arm a timer with specified interval
>  b- Wait for the timer to timeout
>  c- After timeout, in the timer handler, using wall clock time to calculate
>    which kind of event to notify user(soft or hard for both key use time,
>    and key created time). So the problem arises at this stage if wall clock
>    time changed.

And why on earth must it use wall clock time for selecting the event
type?

Thanks,

	tglx

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Fan Du]

On 2013年09月16日 17:01, Thomas Gleixner wrote:
> On Mon, 16 Sep 2013, Fan Du wrote:
>> On 2013年09月13日 22:32, Thomas Gleixner wrote:
>>> On Fri, 13 Sep 2013, Fan Du wrote:
>>>> (2) What I have been bugging you around here for this long time is really
>>>> the
>>>> second
>>>>       problem, I'm sorry I didn't make it clearly to you and others, which
>>>> is
>>>> below:
>>>>
>>>>       Why using wall clock time to calculate soft/hard IPsec events when
>>>> xfrm_state timer
>>>>       out happens in its timeout handler? Because even if xfrm_state using
>>>> CLOCK_BOOTTIME,
>>>>       system wall clock time changing will surely disturb soft/hard IPsec
>>>> events, which
>>>>       you raised your concern about in (*a*).
>>>
>>> No CLOCK_BOOTTIME is not affected by wall clock time changes. It's
>>> basically CLOCK_MONOTONIC, it just keeps counting the suspend time as
>>> well. So without a suspend/resume cycle CLOCK_MONOTONIC and
>>> CLOCK_BOOTTIME are the same. After a suspend/resume cycle
>>> CLOCK_BOOTTIME will be N seconds ahead of CLOCK_MONOTONIC, where N is
>>> the number of seconds spent in suspend.
>>
>> Sorry for the ambiguity. Yes, CLOCK_BOOTTIME is monotonic *and* counting
>> suspend/resume time as well as not affected by wall clock time change.
>>
>> Using CLOCK_BOOTTIME guarantees b- will timeout in a right manner, i.e., not
>> affected by wall clock time change, as well as keep the timer valid when
>> suspend/resume.
>>
>> A classic way of using timer is:
>>   a- Arm a timer with specified interval
>>   b- Wait for the timer to timeout
>>   c- After the timeout, notify the event to other place in the timer handler.
>>
>> IPsec key lifetime timer does its this way:
>>   a- Arm a timer with specified interval
>>   b- Wait for the timer to timeout
>>   c- After timeout, in the timer handler, using wall clock time to calculate
>>     which kind of event to notify user(soft or hard for both key use time,
>>     and key created time). So the problem arises at this stage if wall clock
>>     time changed.
>
> And why on earth must it use wall clock time for selecting the event
> type?

/*shivering... sorry to bother you again.*/

Yep, it's a bit twisted. This parts of codes come a long way from v2.5.67
Beyond this fact, I barely know its original design goal by doing so.
The first version of this patch is to remove the wall clock time things
by myself, it's a pity that the feedback is not very welcome at the end.
So later on adding notifier at clock_was_set is suggested by David.

> Thanks,
>
> 	tglx

-- 
浮沉随浪只记今朝笑

--fan

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[John Stultz]

On Thu, Sep 12, 2013 at 6:44 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Thu, Sep 12, 2013 at 03:21:24PM +0200, Thomas Gleixner wrote:
>>
>> > (3): http://www.spinics.net/lists/netdev/msg245169.html
>>
>> Thanks for the explanation so far.
>>
>> What's still unclear to me is why these timeouts are bound to wall
>> time in the first place.
>>
>> Is there any real reason why the key life time can't simply be
>> expressed in monotonic time, e.g. N seconds after creation or M
>> seconds after usage? Looking at the relevant RFCs I can't find any
>> requirement for binding the life time to wall time.
>>
>> A life time of 10 minutes does not change when the wall clock is
>> adjusted for whatever reasons. It's still 10 minutes and not some
>> random result of the wall clock adjustments. But I might be wrong as
>> usual :)
>
> Well we started out with straight timers.  It was changed because
> people wanted IPsec SAs to expire after a suspect/resume which
> AFAIK does not touch normal timers.

I'm not sure I've totally groked the specific need here, but if you're
wanting a monotonic clockbase that includes suspend time, then you
might checkout CLOCK_BOOTTIME.

thanks
-john

Re: [PATCHv3 linux-next] hrtimer: Add notifier when clock_was_set was called

[Thomas Gleixner]

On Thu, 12 Sep 2013, John Stultz wrote:
> On Thu, Sep 12, 2013 at 6:44 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > On Thu, Sep 12, 2013 at 03:21:24PM +0200, Thomas Gleixner wrote:
> >>
> >> > (3): http://www.spinics.net/lists/netdev/msg245169.html
> >>
> >> Thanks for the explanation so far.
> >>
> >> What's still unclear to me is why these timeouts are bound to wall
> >> time in the first place.
> >>
> >> Is there any real reason why the key life time can't simply be
> >> expressed in monotonic time, e.g. N seconds after creation or M
> >> seconds after usage? Looking at the relevant RFCs I can't find any
> >> requirement for binding the life time to wall time.
> >>
> >> A life time of 10 minutes does not change when the wall clock is
> >> adjusted for whatever reasons. It's still 10 minutes and not some
> >> random result of the wall clock adjustments. But I might be wrong as
> >> usual :)
> >
> > Well we started out with straight timers.  It was changed because
> > people wanted IPsec SAs to expire after a suspect/resume which
> > AFAIK does not touch normal timers.
> 
> I'm not sure I've totally groked the specific need here, but if you're
> wanting a monotonic clockbase that includes suspend time, then you
> might checkout CLOCK_BOOTTIME.

Duh, completely forgot about that one. Sure that would avoid the whole
business.

Thanks,

	tglx