[XFRM][RFC v2] Fix unexpected SA hard expiration after setting new date

[XFRM][RFC v2] Fix unexpected SA hard expiration after setting new date

[fan.du]

Hi, Dave and Herbert

Could you give a try to review this?
thanks :)

Changelog:
v1->v2
1) use xflags instead of creating new flags(suggested by Steffen Klassert)


*Background*:
Once IPsec SAs are created between two peers, kernel setup a timer to monitor
two events: soft/hard expiration. However the timer handler use xtime to
caculate whether it's soft or hard expiration event.

normal code flow(hard expire time:100s, soft expire time:82s)

a) When new SAs created, xfrm_timer_handler is called one second
after its creation. At this point, calculate soft expire
interval(81s), setup the timer;

b) soft expire occur, rearm the timer with hard expire interval(18s)
then notify racoon2 about soft expire event. racoon2 will create
new SAs.

c) hard expire happen, notify racoon2 about it. racoon2 will delete
the old SAs.

*Scenario*:
Setting a new date before b),and after a) could result c) happens first,
As a result, old SAs is deleted before new ones are created. Normally
new SAs will be created by the next time networking traffic, but there
is a small time being when networking connection is down, this could
result in upper layer connections failed in tel comm area, thus it's
better to keep it strict in sequence.

*Workaround*:
set new time could happen:
1) before a), then SAs is updated with new time.
2) before b),and after a)
2a) When new SAs created, xfrm_timer_handler is called one second
after its creation. At this point, calculate soft expire
interval(81s), setup the timer;(set flag to mark next time should
be soft time expire)

<<---- new date comes

2b) soft expire occur, the calculation results in a hard time expire
event, but flag is set, so catch ya. Sync the addtime, and rearm
the timer with hard expire interval(18s), then notify racoon2
about soft expire event;

2c) hard expire happen, notify racoon2 about it;
so everything is in order.

3) after b), hard expire always happened anyway.

[PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[fan.du]

From: "fan.du" <fan.du@windriver.com>

After SA is setup, one timer is armed to detect soft/hard expiration,
however the timer handler uses xtime to do the math. This makes hard
expiration occurs first before soft expiration after setting new date
with big interval. As a result new child SA is deleted before rekeying
the new one.

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

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 2933d74..8d16777 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -197,6 +197,8 @@ struct xfrm_state
 
 	struct xfrm_lifetime_cur curlft;
 	struct timer_list	timer;
+	/* used to fix curlft->add_time when changing date */
+	long		saved_tmo;
 
 	/* Last used time */
 	unsigned long		lastused;
@@ -218,6 +220,7 @@ struct xfrm_state
 
 /* 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 fd77cf0..ab4aa0f 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -442,8 +442,17 @@ static void xfrm_timer_handler(unsigned long data)
 	if (x->lft.hard_add_expires_seconds) {
 		long tmo = x->lft.hard_add_expires_seconds +
 			x->curlft.add_time - now;
-		if (tmo <= 0)
-			goto expired;
+		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 < next)
 			next = tmo;
 	}
@@ -460,10 +469,14 @@ static void xfrm_timer_handler(unsigned long data)
 	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;
-		else if (tmo < next)
+			x->xflags |= ~XFRM_SOFT_EXPIRE;
+		} 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.11

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[David Miller]

From: "fan.du" <fan.du@windriver.com>
Date: Tue, 19 Jun 2012 15:51:09 +0800

> From: "fan.du" <fan.du@windriver.com>

Please don't put your email user name instead of real name in quotes
there.  Thank you.

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Tue, 19 Jun 2012 02:01:11 -0700 (PDT)

> From: "fan.du" <fan.du@windriver.com>
> Date: Tue, 19 Jun 2012 15:51:09 +0800
> 
>> From: "fan.du" <fan.du@windriver.com>
> 
> Please don't put your email user name instead of real name in quotes
> there.  Thank you.

Also fdu@windriver.com bounces, do not put it in the CC: list.

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[Fan Du]

Hi David

On 2012年06月19日 17:05, David Miller wrote:
> From: David Miller<davem@davemloft.net>
> Date: Tue, 19 Jun 2012 02:01:11 -0700 (PDT)
>
>> From: "fan.du"<fan.du@windriver.com>
>> Date: Tue, 19 Jun 2012 15:51:09 +0800
>>
>>> From: "fan.du"<fan.du@windriver.com>
>>
>> Please don't put your email user name instead of real name in quotes
>> there.  Thank you.
>
> Also fdu@windriver.com bounces, do not put it in the CC: list.

I have send the V3 which made modifications requested by your advice at
here:
http://marc.info/?l=linux-netdev&m=134009837402499&w=2
http://marc.info/?l=linux-netdev&m=134009837402498&w=2

Could you please take a look at it if you have time?

thanks :)

-- 

Love each day!
--fan

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[David Miller]

From: Fan Du <fan.du@windriver.com>
Date: Thu, 21 Jun 2012 10:11:51 +0800

> Could you please take a look at it if you have time?

Everyone saw it, you need to be patient.

And you still have fdu@windriver.com in the CC: list, which I told you
bounces.

I asked you explicitly to remove this email address from the CC: list,
because it bounces.

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[Fan Du]

Hi, David

On 2012年06月21日 11:43, David Miller wrote:
> From: Fan Du<fan.du@windriver.com>
> Date: Thu, 21 Jun 2012 10:11:51 +0800
>
>> Could you please take a look at it if you have time?
>
> Everyone saw it, you need to be patient.
>

Apologize for this noise.
Yes, I'm young, and a bit of rush :)


> And you still have fdu@windriver.com in the CC: list, which I told you
> bounces.
>
> I asked you explicitly to remove this email address from the CC: list,
> because it bounces.

I didn't do it deliberately.
Damn, must be my afternoon buzzy head!

I have sent out V4, I hope I will not make any dummy mistake.
Anyway, please review it if you have time, and I will wait for your
response *PATIENTLY*.

thanks

-- 

Love each day!
--fan

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[David Miller]

From: "fan.du" <fan.du@windriver.com>
Date: Tue, 19 Jun 2012 15:51:09 +0800

> +			x->xflags |= ~XFRM_SOFT_EXPIRE;

This is not how you clear a bit in a bitmask.