diff mbox

[net,1/2] xfrm: add rcu grace period in xfrm_policy_destroy()

Message ID 1449588122-13941-1-git-send-email-edumazet@google.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Eric Dumazet Dec. 8, 2015, 3:22 p.m. UTC
We will soon switch sk->sk_policy[] to RCU protection,
as SYNACK packets are sent while listener socket is not locked.

This patch simply adds RCU grace period before struct xfrm_policy
freeing, and the corresponding rcu_head in struct xfrm_policy.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/xfrm.h     |  1 +
 net/xfrm/xfrm_policy.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

David Miller Dec. 9, 2015, 3:58 a.m. UTC | #1
From: Eric Dumazet <edumazet@google.com>
Date: Tue,  8 Dec 2015 07:22:01 -0800

> We will soon switch sk->sk_policy[] to RCU protection,
> as SYNACK packets are sent while listener socket is not locked.
> 
> This patch simply adds RCU grace period before struct xfrm_policy
> freeing, and the corresponding rcu_head in struct xfrm_policy.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

I'll give Steffen an opportunity to review these two patches.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steffen Klassert Dec. 10, 2015, 12:34 p.m. UTC | #2
On Tue, Dec 08, 2015 at 10:58:29PM -0500, David Miller wrote:
> From: Eric Dumazet <edumazet@google.com>
> Date: Tue,  8 Dec 2015 07:22:01 -0800
> 
> > We will soon switch sk->sk_policy[] to RCU protection,
> > as SYNACK packets are sent while listener socket is not locked.
> > 
> > This patch simply adds RCU grace period before struct xfrm_policy
> > freeing, and the corresponding rcu_head in struct xfrm_policy.
> > 
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> I'll give Steffen an opportunity to review these two patches.

Looks ok and survived some tests with socket policies.

If you want to take these direct into the net tree:

Acked-by: Steffen Klassert <steffen.klassert@secunet.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Dec. 12, 2015, 12:22 a.m. UTC | #3
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 10 Dec 2015 13:34:22 +0100

> On Tue, Dec 08, 2015 at 10:58:29PM -0500, David Miller wrote:
>> From: Eric Dumazet <edumazet@google.com>
>> Date: Tue,  8 Dec 2015 07:22:01 -0800
>> 
>> > We will soon switch sk->sk_policy[] to RCU protection,
>> > as SYNACK packets are sent while listener socket is not locked.
>> > 
>> > This patch simply adds RCU grace period before struct xfrm_policy
>> > freeing, and the corresponding rcu_head in struct xfrm_policy.
>> > 
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> 
>> I'll give Steffen an opportunity to review these two patches.
> 
> Looks ok and survived some tests with socket policies.
> 
> If you want to take these direct into the net tree:
> 
> Acked-by: Steffen Klassert <steffen.klassert@secunet.com>

Thanks! I just did exactly that.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 4a9c21f9b4ea..8bae1ef647cd 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -548,6 +548,7 @@  struct xfrm_policy {
 	u16			family;
 	struct xfrm_sec_ctx	*security;
 	struct xfrm_tmpl       	xfrm_vec[XFRM_MAX_DEPTH];
+	struct rcu_head		rcu;
 };
 
 static inline struct net *xp_net(const struct xfrm_policy *xp)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 18276f0cc32b..f57a5712cedd 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -303,6 +303,14 @@  struct xfrm_policy *xfrm_policy_alloc(struct net *net, gfp_t gfp)
 }
 EXPORT_SYMBOL(xfrm_policy_alloc);
 
+static void xfrm_policy_destroy_rcu(struct rcu_head *head)
+{
+	struct xfrm_policy *policy = container_of(head, struct xfrm_policy, rcu);
+
+	security_xfrm_policy_free(policy->security);
+	kfree(policy);
+}
+
 /* Destroy xfrm_policy: descendant resources must be released to this moment. */
 
 void xfrm_policy_destroy(struct xfrm_policy *policy)
@@ -312,8 +320,7 @@  void xfrm_policy_destroy(struct xfrm_policy *policy)
 	if (del_timer(&policy->timer) || del_timer(&policy->polq.hold_timer))
 		BUG();
 
-	security_xfrm_policy_free(policy->security);
-	kfree(policy);
+	call_rcu(&policy->rcu, xfrm_policy_destroy_rcu);
 }
 EXPORT_SYMBOL(xfrm_policy_destroy);