* [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() @ 2015-12-08 15:22 Eric Dumazet 2015-12-08 15:22 ` [PATCH net 2/2] xfrm: add rcu protection to sk->sk_policy[] Eric Dumazet 2015-12-09 3:58 ` [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() David Miller 0 siblings, 2 replies; 5+ messages in thread From: Eric Dumazet @ 2015-12-08 15:22 UTC (permalink / raw) To: David S . Miller; +Cc: netdev, Eric Dumazet, Steffen Klassert 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(-) 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); -- 2.6.0.rc2.230.g3dd15c0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH net 2/2] xfrm: add rcu protection to sk->sk_policy[] 2015-12-08 15:22 [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() Eric Dumazet @ 2015-12-08 15:22 ` Eric Dumazet 2015-12-09 3:58 ` [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() David Miller 1 sibling, 0 replies; 5+ messages in thread From: Eric Dumazet @ 2015-12-08 15:22 UTC (permalink / raw) To: David S . Miller; +Cc: netdev, Eric Dumazet, Steffen Klassert XFRM can deal with SYNACK messages, sent while listener socket is not locked. We add proper rcu protection to __xfrm_sk_clone_policy() and xfrm_sk_policy_lookup() This might serve as the first step to remove xfrm.xfrm_policy_lock use in fast path. Fixes: fa76ce7328b2 ("inet: get rid of central tcp/dccp listener timer") Signed-off-by: Eric Dumazet <edumazet@google.com> --- include/net/sock.h | 2 +- include/net/xfrm.h | 24 +++++++++++++++--------- net/core/sock.c | 2 +- net/xfrm/xfrm_policy.c | 37 +++++++++++++++++++++++++------------ 4 files changed, 42 insertions(+), 23 deletions(-) diff --git a/include/net/sock.h b/include/net/sock.h index b1d475b5db68..eaef41433d7a 100644 --- a/include/net/sock.h +++ b/include/net/sock.h @@ -388,7 +388,7 @@ struct sock { struct socket_wq *sk_wq_raw; }; #ifdef CONFIG_XFRM - struct xfrm_policy *sk_policy[2]; + struct xfrm_policy __rcu *sk_policy[2]; #endif struct dst_entry *sk_rx_dst; struct dst_entry __rcu *sk_dst_cache; diff --git a/include/net/xfrm.h b/include/net/xfrm.h index 8bae1ef647cd..d6f6e5006ee9 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -1142,12 +1142,14 @@ static inline int xfrm6_route_forward(struct sk_buff *skb) return xfrm_route_forward(skb, AF_INET6); } -int __xfrm_sk_clone_policy(struct sock *sk); +int __xfrm_sk_clone_policy(struct sock *sk, const struct sock *osk); -static inline int xfrm_sk_clone_policy(struct sock *sk) +static inline int xfrm_sk_clone_policy(struct sock *sk, const struct sock *osk) { - if (unlikely(sk->sk_policy[0] || sk->sk_policy[1])) - return __xfrm_sk_clone_policy(sk); + sk->sk_policy[0] = NULL; + sk->sk_policy[1] = NULL; + if (unlikely(osk->sk_policy[0] || osk->sk_policy[1])) + return __xfrm_sk_clone_policy(sk, osk); return 0; } @@ -1155,12 +1157,16 @@ int xfrm_policy_delete(struct xfrm_policy *pol, int dir); static inline void xfrm_sk_free_policy(struct sock *sk) { - if (unlikely(sk->sk_policy[0] != NULL)) { - xfrm_policy_delete(sk->sk_policy[0], XFRM_POLICY_MAX); + struct xfrm_policy *pol; + + pol = rcu_dereference_protected(sk->sk_policy[0], 1); + if (unlikely(pol != NULL)) { + xfrm_policy_delete(pol, XFRM_POLICY_MAX); sk->sk_policy[0] = NULL; } - if (unlikely(sk->sk_policy[1] != NULL)) { - xfrm_policy_delete(sk->sk_policy[1], XFRM_POLICY_MAX+1); + pol = rcu_dereference_protected(sk->sk_policy[1], 1); + if (unlikely(pol != NULL)) { + xfrm_policy_delete(pol, XFRM_POLICY_MAX+1); sk->sk_policy[1] = NULL; } } @@ -1170,7 +1176,7 @@ void xfrm_garbage_collect(struct net *net); #else static inline void xfrm_sk_free_policy(struct sock *sk) {} -static inline int xfrm_sk_clone_policy(struct sock *sk) { return 0; } +static inline int xfrm_sk_clone_policy(struct sock *sk, const struct sock *osk) { return 0; } static inline int xfrm6_route_forward(struct sk_buff *skb) { return 1; } static inline int xfrm4_route_forward(struct sk_buff *skb) { return 1; } static inline int xfrm6_policy_check(struct sock *sk, int dir, struct sk_buff *skb) diff --git a/net/core/sock.c b/net/core/sock.c index d01c8f42dbb2..765be835b06c 100644 --- a/net/core/sock.c +++ b/net/core/sock.c @@ -1550,7 +1550,7 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority) */ is_charged = sk_filter_charge(newsk, filter); - if (unlikely(!is_charged || xfrm_sk_clone_policy(newsk))) { + if (unlikely(!is_charged || xfrm_sk_clone_policy(newsk, sk))) { /* It is still raw copy of parent, so invalidate * destructor and make plain sk_free() */ newsk->sk_destruct = NULL; diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index f57a5712cedd..948fa5560de5 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -1221,8 +1221,10 @@ static struct xfrm_policy *xfrm_sk_policy_lookup(const struct sock *sk, int dir, struct xfrm_policy *pol; struct net *net = sock_net(sk); + rcu_read_lock(); read_lock_bh(&net->xfrm.xfrm_policy_lock); - if ((pol = sk->sk_policy[dir]) != NULL) { + pol = rcu_dereference(sk->sk_policy[dir]); + if (pol != NULL) { bool match = xfrm_selector_match(&pol->selector, fl, sk->sk_family); int err = 0; @@ -1246,6 +1248,7 @@ static struct xfrm_policy *xfrm_sk_policy_lookup(const struct sock *sk, int dir, } out: read_unlock_bh(&net->xfrm.xfrm_policy_lock); + rcu_read_unlock(); return pol; } @@ -1314,13 +1317,14 @@ int xfrm_sk_policy_insert(struct sock *sk, int dir, struct xfrm_policy *pol) #endif write_lock_bh(&net->xfrm.xfrm_policy_lock); - old_pol = sk->sk_policy[dir]; - sk->sk_policy[dir] = pol; + old_pol = rcu_dereference_protected(sk->sk_policy[dir], + lockdep_is_held(&net->xfrm.xfrm_policy_lock)); if (pol) { pol->curlft.add_time = get_seconds(); pol->index = xfrm_gen_index(net, XFRM_POLICY_MAX+dir, 0); xfrm_sk_policy_link(pol, dir); } + rcu_assign_pointer(sk->sk_policy[dir], pol); if (old_pol) { if (pol) xfrm_policy_requeue(old_pol, pol); @@ -1368,17 +1372,26 @@ static struct xfrm_policy *clone_policy(const struct xfrm_policy *old, int dir) return newp; } -int __xfrm_sk_clone_policy(struct sock *sk) +int __xfrm_sk_clone_policy(struct sock *sk, const struct sock *osk) { - struct xfrm_policy *p0 = sk->sk_policy[0], - *p1 = sk->sk_policy[1]; + const struct xfrm_policy *p; + struct xfrm_policy *np; + int i, ret = 0; - sk->sk_policy[0] = sk->sk_policy[1] = NULL; - if (p0 && (sk->sk_policy[0] = clone_policy(p0, 0)) == NULL) - return -ENOMEM; - if (p1 && (sk->sk_policy[1] = clone_policy(p1, 1)) == NULL) - return -ENOMEM; - return 0; + rcu_read_lock(); + for (i = 0; i < 2; i++) { + p = rcu_dereference(osk->sk_policy[i]); + if (p) { + np = clone_policy(p, i); + if (unlikely(!np)) { + ret = -ENOMEM; + break; + } + rcu_assign_pointer(sk->sk_policy[i], np); + } + } + rcu_read_unlock(); + return ret; } static int -- 2.6.0.rc2.230.g3dd15c0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() 2015-12-08 15:22 [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() Eric Dumazet 2015-12-08 15:22 ` [PATCH net 2/2] xfrm: add rcu protection to sk->sk_policy[] Eric Dumazet @ 2015-12-09 3:58 ` David Miller 2015-12-10 12:34 ` Steffen Klassert 1 sibling, 1 reply; 5+ messages in thread From: David Miller @ 2015-12-09 3:58 UTC (permalink / raw) To: edumazet; +Cc: netdev, steffen.klassert 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() 2015-12-09 3:58 ` [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() David Miller @ 2015-12-10 12:34 ` Steffen Klassert 2015-12-12 0:22 ` David Miller 0 siblings, 1 reply; 5+ messages in thread From: Steffen Klassert @ 2015-12-10 12:34 UTC (permalink / raw) To: David Miller; +Cc: edumazet, netdev 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> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() 2015-12-10 12:34 ` Steffen Klassert @ 2015-12-12 0:22 ` David Miller 0 siblings, 0 replies; 5+ messages in thread From: David Miller @ 2015-12-12 0:22 UTC (permalink / raw) To: steffen.klassert; +Cc: edumazet, netdev 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-12 0:22 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-08 15:22 [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() Eric Dumazet 2015-12-08 15:22 ` [PATCH net 2/2] xfrm: add rcu protection to sk->sk_policy[] Eric Dumazet 2015-12-09 3:58 ` [PATCH net 1/2] xfrm: add rcu grace period in xfrm_policy_destroy() David Miller 2015-12-10 12:34 ` Steffen Klassert 2015-12-12 0:22 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).