[PATCHv3 net-next 0/3] xfrm: Refactor xfrm_state timer management

[PATCHv3 net-next 0/3] xfrm: Refactor xfrm_state timer management

[Fan Du]

The first version of "refactor xfrm_state timer management" has been
flushed into toilet since nobody but only me like it.

Anyway new approach here is updating SAs lifetime timeout whenever
clock_was_set is called, iow, system clock changed or host resume from
suspend state. Rule is simple, force soft expire for any SAs which has
not reach their soft expire limit and hard expire for those has experienced
soft expire timeout but wait for hard expire timeout to come.

Locking issue:
 - holding rtnl_lock when iterate on all net namespace.
 - holding xfrm_state_lock when iterate all xfrm_state in this net.
 - holding state->lock when changing xfrm_state.

Any comments would be wellcome! harsh or gentle :)

Thanks!

v3:
  - Fixing locking issue clock_change_callback reported by LOCKDEP.
  - Beautify notifier in clock_was_set as suggested by Daniel Borkmann.

v2: 
  - Instead of getting rid of original xfrm_state timer code almost completely,
    new approach is updating SA lifetime when clock_was_set is called.

v1:
  - The initiative of v1 is making xfrm_state timer independent of wall clock
    changing which could result in sudden SA termination or extremely long SA
    lifetime. So no need to read wall clock in timer handler and also
    unnecessary to turn on the timer for just 1 second as the original
    implementation. Simply start the timer right after SA is create for soft
    timeout, and after soft timeout happened, reload timer for hard timeout.


Fan Du (3):
  hrtimer: Add notifer when clock_was_set was called
  xfrm: Update xfrm_state lifetime expire after clock_was_set
  xfrm: Revert "Fix unexpected SA hard expiration after changing date"

 include/linux/hrtimer.h |    3 +++
 include/net/xfrm.h      |    4 ---
 kernel/hrtimer.c        |   19 ++++++++++++++
 net/xfrm/xfrm_state.c   |   65 ++++++++++++++++++++++++++++++++++-------------
 4 files changed, 70 insertions(+), 21 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] hrtimer: Add notifer when clock_was_set was called

[Fan Du]

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

[PATCH 2/3] xfrm: Update xfrm_state lifetime expire after clock_was_set

[Fan Du]

After clock_was_set called to set new time or host resume from suspend
state. Notify IKED with soft timeout for SAs which haven't reach its
soft timeout limit. For those dying SAs, arrange them to hard expire.

This modification is characterized by SA is sensible to any degree of
clock changes while as SA lifetime is marked by second.

Another point is clock_was_set is traversing all net name space to
update SA time while holding rtnl_lock, it may not scale very well.

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

v3:
  - Fix lockdep complaint about circular locking with trying to acquire
    state->clock while holding xfrm_state_lock.
v2:
  - Use notifier when clock was set, and then update SA lifetime accordingly.
---
 net/xfrm/xfrm_state.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 78f66fa..dcfcd98 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2002,6 +2002,48 @@ int xfrm_init_state(struct xfrm_state *x)
 
 EXPORT_SYMBOL(xfrm_init_state);
 
+static int clock_change_callback(struct notifier_block *nb,
+				unsigned long reason, void *arg)
+{
+	struct xfrm_state_walk *walk;
+	struct xfrm_state *state;
+	struct net *net;
+	long next;
+
+	rtnl_lock();
+	for_each_net(net) {
+		spin_lock_bh(&xfrm_state_lock);
+		list_for_each_entry(walk, &net->xfrm.state_all, all) {
+			state = container_of(walk, struct xfrm_state, km);
+			xfrm_state_hold(state);
+			spin_unlock_bh(&xfrm_state_lock);
+
+			spin_lock_bh(&state->lock);
+			if (state->km.dying) {
+				next = 0;
+			} else {
+				state->km.dying = 1;
+				km_state_expired(state, 0, 0);
+				next = state->lft.hard_add_expires_seconds -
+					state->lft.soft_add_expires_seconds;
+			}
+			state->km.state = XFRM_STATE_EXPIRED;
+			tasklet_hrtimer_start(&state->mtimer, ktime_set(next, 0), HRTIMER_MODE_REL);
+			spin_unlock_bh(&state->lock);
+			xfrm_state_put(state);
+			spin_lock_bh(&xfrm_state_lock);
+		}
+		spin_unlock_bh(&xfrm_state_lock);
+	}
+	rtnl_unlock();
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block clock_change_notifier = {
+	.notifier_call = clock_change_callback,
+};
+
 int __net_init xfrm_state_init(struct net *net)
 {
 	unsigned int sz;
@@ -2026,6 +2068,7 @@ int __net_init xfrm_state_init(struct net *net)
 	INIT_HLIST_HEAD(&net->xfrm.state_gc_list);
 	INIT_WORK(&net->xfrm.state_gc_work, xfrm_state_gc_task);
 	init_waitqueue_head(&net->xfrm.km_waitq);
+	register_clock_change_notifier(&clock_change_notifier);
 	return 0;
 
 out_byspi:
@@ -2057,6 +2100,7 @@ void xfrm_state_fini(struct net *net)
 	xfrm_hash_free(net->xfrm.state_bysrc, sz);
 	WARN_ON(!hlist_empty(net->xfrm.state_bydst));
 	xfrm_hash_free(net->xfrm.state_bydst, sz);
+	unregister_clock_change_notifier(&clock_change_notifier);
 }
 
 #ifdef CONFIG_AUDITSYSCALL
-- 
1.7.9.5

[PATCH 3/3] xfrm: Revert "Fix unexpected SA hard expiration after changing date"

[Fan Du]

Since SAs lifetime timeout has been updated whenever clock_was_set is
called. So commit: e3c0d04750751389d5116267f8cf4687444d9a50
("Fix unexpected SA hard expiration after changing date") is not needed
anymore.

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 include/net/xfrm.h    |    4 ----
 net/xfrm/xfrm_state.c |   21 ++++-----------------
 2 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 94ce082..b9df23f 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -214,9 +214,6 @@ struct xfrm_state {
 	struct xfrm_lifetime_cur curlft;
 	struct tasklet_hrtimer	mtimer;
 
-	/* used to fix curlft->add_time when changing date */
-	long		saved_tmo;
-
 	/* Last used time */
 	unsigned long		lastused;
 
@@ -242,7 +239,6 @@ static inline struct net *xs_net(struct xfrm_state *x)
 
 /* xflags - make enum if more show up */
 #define XFRM_TIME_DEFER	1
-#define XFRM_SOFT_EXPIRE 2
 
 enum {
 	XFRM_STATE_VOID,
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index dcfcd98..79bf9a0 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -407,17 +407,8 @@ static enum hrtimer_restart xfrm_timer_handler(struct hrtimer * me)
 	if (x->lft.hard_add_expires_seconds) {
 		long tmo = x->lft.hard_add_expires_seconds +
 			x->curlft.add_time - now;
-		if (tmo <= 0) {
-			if (x->xflags & XFRM_SOFT_EXPIRE) {
-				/* enter hard expire without soft expire first?!
-				 * setting a new date could trigger this.
-				 * workarbound: fix x->curflt.add_time by below:
-				 */
-				x->curlft.add_time = now - x->saved_tmo - 1;
-				tmo = x->lft.hard_add_expires_seconds - x->saved_tmo;
-			} else
-				goto expired;
-		}
+		if (tmo <= 0)
+			goto expired;
 		if (tmo < next)
 			next = tmo;
 	}
@@ -434,14 +425,10 @@ static enum hrtimer_restart xfrm_timer_handler(struct hrtimer * me)
 	if (x->lft.soft_add_expires_seconds) {
 		long tmo = x->lft.soft_add_expires_seconds +
 			x->curlft.add_time - now;
-		if (tmo <= 0) {
+		if (tmo <= 0)
 			warn = 1;
-			x->xflags &= ~XFRM_SOFT_EXPIRE;
-		} else if (tmo < next) {
+		else if (tmo < next)
 			next = tmo;
-			x->xflags |= XFRM_SOFT_EXPIRE;
-			x->saved_tmo = tmo;
-		}
 	}
 	if (x->lft.soft_use_expires_seconds) {
 		long tmo = x->lft.soft_use_expires_seconds +
-- 
1.7.9.5