[IPSEC] Kill spurious hard expire messages

This patch ensures that the hard state/policy expire notifications are
only sent when the state/policy is successfully removed from their
respective tables.

As it is, it's possible for a state/policy to both expire through
reaching a hard limit, as well as being deleted by the user.

Note that this behaviour isn't actually forbidden by RFC 2367.
However, it is a quality of implementation issue.

As an added bonus, the restructuring in this patch will help
eventually in moving the expire notifications from softirq
context into process context, thus improving their reliability.

One important side-effect from this change is that SAs reaching
their hard byte/packet limits are now deleted immediately, just
like SAs that have reached their hard time limits.

Previously they were announced immediately but only deleted after
30 seconds.

This is bad because it prevents the system from issuing an ACQUIRE
command until the existing state was deleted by the user or expires
after the time is up.

In the scenario where the expire notification was lost this introduces
a 30 second delay into the system for no good reason.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index a159655ebedee033b4d32db388da7ce21babded2..aaa0f5f330e2395cca09123980d85421f53f1729 100644 (file)
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -679,7 +679,7 @@ static inline int xfrm_sk_clone_policy(struct sock *sk)
        return 0;
 }
 
-extern void xfrm_policy_delete(struct xfrm_policy *pol, int dir);
+extern int xfrm_policy_delete(struct xfrm_policy *pol, int dir);
 
 static inline void xfrm_sk_free_policy(struct sock *sk)
 {

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index d07f5ce31824f0f887a737cd544c4dbbb035344b..0a4260719a123113b03e0e7bfed04ea363ca655f 100644 (file)
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -216,8 +216,8 @@ out:
 
 expired:
        read_unlock(&xp->lock);
-       km_policy_expired(xp, dir, 1);
-       xfrm_policy_delete(xp, dir);
+       if (!xfrm_policy_delete(xp, dir))
+               km_policy_expired(xp, dir, 1);
        xfrm_pol_put(xp);
 }
 
@@ -555,7 +555,7 @@ static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol,
        return NULL;
 }
 
-void xfrm_policy_delete(struct xfrm_policy *pol, int dir)
+int xfrm_policy_delete(struct xfrm_policy *pol, int dir)
 {
        write_lock_bh(&xfrm_policy_lock);
        pol = __xfrm_policy_unlink(pol, dir);
@@ -564,7 +564,9 @@ void xfrm_policy_delete(struct xfrm_policy *pol, int dir)
                if (dir < XFRM_POLICY_MAX)
                        atomic_inc(&flow_cache_genid);
                xfrm_policy_kill(pol);
+               return 0;
        }
+       return -ENOENT;
 }
 
 int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 918a94c552a50f16fa32b811672532908caca5f8..94f7416a4ab0c634651dd402b8be62f5b46733f1 100644 (file)
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -154,6 +154,7 @@ static void xfrm_timer_handler(unsigned long data)
                        next = tmo;
        }
 
+       x->km.dying = warn;
        if (warn)
                km_state_expired(x, 0);
 resched:
@@ -169,9 +170,8 @@ expired:
                next = 2;
                goto resched;
        }
-       if (x->id.spi != 0)
+       if (!__xfrm_state_delete(x) && x->id.spi)
                km_state_expired(x, 1);
-       __xfrm_state_delete(x);
 
 out:
        spin_unlock(&x->lock);
@@ -566,16 +566,18 @@ int xfrm_state_check_expire(struct xfrm_state *x)
 
        if (x->curlft.bytes >= x->lft.hard_byte_limit ||
            x->curlft.packets >= x->lft.hard_packet_limit) {
-               km_state_expired(x, 1);
-               if (!mod_timer(&x->timer, jiffies + XFRM_ACQ_EXPIRES*HZ))
+               x->km.state = XFRM_STATE_EXPIRED;
+               if (!mod_timer(&x->timer, jiffies))
                        xfrm_state_hold(x);
                return -EINVAL;
        }
 
        if (!x->km.dying &&
            (x->curlft.bytes >= x->lft.soft_byte_limit ||
-            x->curlft.packets >= x->lft.soft_packet_limit))
+            x->curlft.packets >= x->lft.soft_packet_limit)) {
+               x->km.dying = 1;
                km_state_expired(x, 0);
+       }
        return 0;
 }
 EXPORT_SYMBOL(xfrm_state_check_expire);
@@ -833,10 +835,6 @@ static void km_state_expired(struct xfrm_state *x, int hard)
 {
        struct km_event c;
 
-       if (hard)
-               x->km.state = XFRM_STATE_EXPIRED;
-       else
-               x->km.dying = 1;
        c.data = hard;
        c.event = XFRM_SAP_EXPIRED;
        km_state_notify(x, &c);