| Message ID | 1448932931.25582.2.camel@edumazet-glaptop2.roam.corp.google.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Mon, 30 Nov 2015 17:22:11 -0800 > @@ -2198,6 +2198,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig, > xdst = NULL; > route = NULL; > > + sk = sk_to_full_sk((struct sock *)sk); The war against const... I know this is the only instance where const is input, but you may want to consider adding the const verion of the helper anyways to avoid ugly casts like this. -- 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
On Thu, 2015-12-03 at 11:59 -0500, David Miller wrote: > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Mon, 30 Nov 2015 17:22:11 -0800 > > > @@ -2198,6 +2198,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig, > > xdst = NULL; > > route = NULL; > > > > + sk = sk_to_full_sk((struct sock *)sk); > > The war against const... > > I know this is the only instance where const is input, but you may want to > consider adding the const verion of the helper anyways to avoid ugly casts > like this. Agreed, it is sad C language has no way to 'propagate' the const.. -- 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
On Thu, 2015-12-03 at 09:09 -0800, Eric Dumazet wrote: > On Thu, 2015-12-03 at 11:59 -0500, David Miller wrote: > > From: Eric Dumazet <eric.dumazet@gmail.com> > > Date: Mon, 30 Nov 2015 17:22:11 -0800 > > > > > @@ -2198,6 +2198,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig, > > > xdst = NULL; > > > route = NULL; > > > > > > + sk = sk_to_full_sk((struct sock *)sk); > > > > The war against const... > > > > I know this is the only instance where const is input, but you may want to > > consider adding the const verion of the helper anyways to avoid ugly casts > > like this. > > Agreed, it is sad C language has no way to 'propagate' the const.. I did not send an official patch because we have a problem with xfrm and lockless listeners anyway. Namely, it looks like sk->sk_policy[] should get RCU protection, since another cpu could change sk->sk_policy[] concurrently on a listener, while a cpu is processing a SYN packet (without holding socket lock) Oh well. -- 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
From: Of David Miller > Sent: 03 December 2015 16:59 > From: Eric Dumazet <eric.dumazet@gmail.com> > Date: Mon, 30 Nov 2015 17:22:11 -0800 > > > @@ -2198,6 +2198,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig, > > xdst = NULL; > > route = NULL; > > > > + sk = sk_to_full_sk((struct sock *)sk); > > The war against const... > > I know this is the only instance where const is input, but you may want to > consider adding the const verion of the helper anyways to avoid ugly casts > like this. In that case you could use something like: #define SK_TO_FULL_SK(sk) (typeof (sk))sk_to_full_sk(sk)) With the helper arg and result being 'const struct sock *'. David -- 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 --git a/include/net/inet_sock.h b/include/net/inet_sock.h index 2134e6d815bc..2f2fda879105 100644 --- a/include/net/inet_sock.h +++ b/include/net/inet_sock.h @@ -213,15 +213,20 @@ struct inet_sock { /* SYNACK messages might be attached to request sockets. * Some places want to reach the listener in this case. */ -static inline struct sock *skb_to_full_sk(const struct sk_buff *skb) +static inline struct sock *sk_to_full_sk(struct sock *sk) { - struct sock *sk = skb->sk; - +#ifdef CONFIG_INET if (sk && sk->sk_state == TCP_NEW_SYN_RECV) sk = inet_reqsk(sk)->rsk_listener; +#endif return sk; } +static inline struct sock *skb_to_full_sk(const struct sk_buff *skb) +{ + return sk_to_full_sk(skb->sk); +} + static inline struct inet_sock *inet_sk(const struct sock *sk) { return (struct inet_sock *)sk; diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 09bfcbac63bb..6bfa61d52e3d 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -2198,6 +2198,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig, xdst = NULL; route = NULL; + sk = sk_to_full_sk((struct sock *)sk); if (sk && sk->sk_policy[XFRM_POLICY_OUT]) { num_pols = 1; pols[0] = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl); @@ -2477,6 +2478,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb, } pol = NULL; + sk = sk_to_full_sk(sk); if (sk && sk->sk_policy[dir]) { pol = xfrm_sk_policy_lookup(sk, dir, &fl); if (IS_ERR(pol)) {