All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next] ipv6: lockless IPV6_ADDR_PREFERENCES implementation
@ 2023-09-18 14:23 Eric Dumazet
  2023-09-19  2:48 ` David Ahern
  2023-09-19 16:30 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Eric Dumazet @ 2023-09-18 14:23 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, David Ahern, eric.dumazet, Eric Dumazet

We have data-races while reading np->srcprefs

Switch the field to a plain byte, add READ_ONCE()
and WRITE_ONCE() annotations where needed,
and IPV6_ADDR_PREFERENCES setsockopt() can now be lockless.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/linux/ipv6.h     |  2 +-
 include/net/ip6_route.h  |  5 ++---
 include/net/ipv6.h       | 20 +++++++-------------
 net/ipv6/ip6_output.c    |  2 +-
 net/ipv6/ipv6_sockglue.c | 19 ++++++++++---------
 net/ipv6/route.c         |  2 +-
 6 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 09253825c99c7a94c4c8a3f176f0ceecd0b166bc..e400ff757f136e72e81277d48063551e445b4970 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -243,7 +243,7 @@ struct ipv6_pinfo {
 	} rxopt;
 
 	/* sockopt flags */
-	__u8			srcprefs:3;	/* 001: prefer temporary address
+	__u8			srcprefs;	/* 001: prefer temporary address
 						 * 010: prefer public address
 						 * 100: prefer care-of address
 						 */
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index b1ea49900b4ae17cb3436f884e26f5ae3a7a761c..28b0657902615157c4cbd836cc70e0767cf49a4d 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -53,13 +53,12 @@ struct route_info {
  */
 static inline int rt6_srcprefs2flags(unsigned int srcprefs)
 {
-	/* No need to bitmask because srcprefs have only 3 bits. */
-	return srcprefs << 3;
+	return (srcprefs & IPV6_PREFER_SRC_MASK) << 3;
 }
 
 static inline unsigned int rt6_flags2srcprefs(int flags)
 {
-	return (flags >> 3) & 7;
+	return (flags >> 3) & IPV6_PREFER_SRC_MASK;
 }
 
 static inline bool rt6_need_strict(const struct in6_addr *daddr)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index bd115980809f386a7d38a5471d8d636f25ce1eba..b3444c8a6f744c17052a9fa1c85d54c6b08a1889 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -1306,10 +1306,13 @@ static inline void ip6_sock_set_recverr(struct sock *sk)
 	inet6_set_bit(RECVERR6, sk);
 }
 
-static inline int __ip6_sock_set_addr_preferences(struct sock *sk, int val)
+#define IPV6_PREFER_SRC_MASK (IPV6_PREFER_SRC_TMP | IPV6_PREFER_SRC_PUBLIC | \
+			      IPV6_PREFER_SRC_COA)
+
+static inline int ip6_sock_set_addr_preferences(struct sock *sk, int val)
 {
+	unsigned int prefmask = ~IPV6_PREFER_SRC_MASK;
 	unsigned int pref = 0;
-	unsigned int prefmask = ~0;
 
 	/* check PUBLIC/TMP/PUBTMP_DEFAULT conflicts */
 	switch (val & (IPV6_PREFER_SRC_PUBLIC |
@@ -1359,20 +1362,11 @@ static inline int __ip6_sock_set_addr_preferences(struct sock *sk, int val)
 		return -EINVAL;
 	}
 
-	inet6_sk(sk)->srcprefs = (inet6_sk(sk)->srcprefs & prefmask) | pref;
+	WRITE_ONCE(inet6_sk(sk)->srcprefs,
+		   (READ_ONCE(inet6_sk(sk)->srcprefs) & prefmask) | pref);
 	return 0;
 }
 
-static inline int ip6_sock_set_addr_preferences(struct sock *sk, int val)
-{
-	int ret;
-
-	lock_sock(sk);
-	ret = __ip6_sock_set_addr_preferences(sk, val);
-	release_sock(sk);
-	return ret;
-}
-
 static inline void ip6_sock_set_recvpktinfo(struct sock *sk)
 {
 	lock_sock(sk);
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 7e5d9eeb990fd4549be753fdaaf1e6c6c21d3f8d..951ba8089b5b44c589f1b497e645ffc15a86c7c8 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1113,7 +1113,7 @@ static int ip6_dst_lookup_tail(struct net *net, const struct sock *sk,
 		rcu_read_lock();
 		from = rt ? rcu_dereference(rt->from) : NULL;
 		err = ip6_route_get_saddr(net, from, &fl6->daddr,
-					  sk ? inet6_sk(sk)->srcprefs : 0,
+					  sk ? READ_ONCE(inet6_sk(sk)->srcprefs) : 0,
 					  &fl6->saddr);
 		rcu_read_unlock();
 
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index e9dc6f881bb92db267903a71f3f3e4de4c557819..7d661735cb9d519ab4691979f30365acda0a28c3 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -505,6 +505,10 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 			return -EINVAL;
 		inet6_assign_bit(SNDFLOW, sk, valbool);
 		return 0;
+	case IPV6_ADDR_PREFERENCES:
+		if (optlen < sizeof(int))
+			return -EINVAL;
+		return ip6_sock_set_addr_preferences(sk, val);
 	}
 	if (needs_rtnl)
 		rtnl_lock();
@@ -964,11 +968,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 		retv = xfrm_user_policy(sk, optname, optval, optlen);
 		break;
 
-	case IPV6_ADDR_PREFERENCES:
-		if (optlen < sizeof(int))
-			goto e_inval;
-		retv = __ip6_sock_set_addr_preferences(sk, val);
-		break;
 	case IPV6_RECVFRAGSIZE:
 		np->rxopt.bits.recvfragsize = valbool;
 		retv = 0;
@@ -1415,23 +1414,25 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 	}
 
 	case IPV6_ADDR_PREFERENCES:
+		{
+		u8 srcprefs = READ_ONCE(np->srcprefs);
 		val = 0;
 
-		if (np->srcprefs & IPV6_PREFER_SRC_TMP)
+		if (srcprefs & IPV6_PREFER_SRC_TMP)
 			val |= IPV6_PREFER_SRC_TMP;
-		else if (np->srcprefs & IPV6_PREFER_SRC_PUBLIC)
+		else if (srcprefs & IPV6_PREFER_SRC_PUBLIC)
 			val |= IPV6_PREFER_SRC_PUBLIC;
 		else {
 			/* XXX: should we return system default? */
 			val |= IPV6_PREFER_SRC_PUBTMP_DEFAULT;
 		}
 
-		if (np->srcprefs & IPV6_PREFER_SRC_COA)
+		if (srcprefs & IPV6_PREFER_SRC_COA)
 			val |= IPV6_PREFER_SRC_COA;
 		else
 			val |= IPV6_PREFER_SRC_HOME;
 		break;
-
+		}
 	case IPV6_MINHOPCOUNT:
 		val = READ_ONCE(np->min_hopcount);
 		break;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9d8dfc7423e49af6df6ddc95ddf235b0b2b758ef..b132feae3393f313b48fb84fc56e2c2aad37608a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2622,7 +2622,7 @@ static struct dst_entry *ip6_route_output_flags_noref(struct net *net,
 	if (!any_src)
 		flags |= RT6_LOOKUP_F_HAS_SADDR;
 	else if (sk)
-		flags |= rt6_srcprefs2flags(inet6_sk(sk)->srcprefs);
+		flags |= rt6_srcprefs2flags(READ_ONCE(inet6_sk(sk)->srcprefs));
 
 	return fib6_rule_lookup(net, fl6, NULL, flags, ip6_pol_route_output);
 }
-- 
2.42.0.459.ge4e396fd5e-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH net-next] ipv6: lockless IPV6_ADDR_PREFERENCES implementation
  2023-09-18 14:23 [PATCH net-next] ipv6: lockless IPV6_ADDR_PREFERENCES implementation Eric Dumazet
@ 2023-09-19  2:48 ` David Ahern
  2023-09-19 16:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: David Ahern @ 2023-09-19  2:48 UTC (permalink / raw)
  To: Eric Dumazet, David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: netdev, eric.dumazet

On 9/18/23 8:23 AM, Eric Dumazet wrote:
> We have data-races while reading np->srcprefs
> 
> Switch the field to a plain byte, add READ_ONCE()
> and WRITE_ONCE() annotations where needed,
> and IPV6_ADDR_PREFERENCES setsockopt() can now be lockless.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  include/linux/ipv6.h     |  2 +-
>  include/net/ip6_route.h  |  5 ++---
>  include/net/ipv6.h       | 20 +++++++-------------
>  net/ipv6/ip6_output.c    |  2 +-
>  net/ipv6/ipv6_sockglue.c | 19 ++++++++++---------
>  net/ipv6/route.c         |  2 +-
>  6 files changed, 22 insertions(+), 28 deletions(-)
> 
> diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
> index 09253825c99c7a94c4c8a3f176f0ceecd0b166bc..e400ff757f136e72e81277d48063551e445b4970 100644
> --- a/include/linux/ipv6.h
> +++ b/include/linux/ipv6.h
> @@ -243,7 +243,7 @@ struct ipv6_pinfo {
>  	} rxopt;
>  
>  	/* sockopt flags */
> -	__u8			srcprefs:3;	/* 001: prefer temporary address
> +	__u8			srcprefs;	/* 001: prefer temporary address
>  						 * 010: prefer public address
>  						 * 100: prefer care-of address
>  						 */
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index b1ea49900b4ae17cb3436f884e26f5ae3a7a761c..28b0657902615157c4cbd836cc70e0767cf49a4d 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -53,13 +53,12 @@ struct route_info {
>   */
>  static inline int rt6_srcprefs2flags(unsigned int srcprefs)
>  {
> -	/* No need to bitmask because srcprefs have only 3 bits. */
> -	return srcprefs << 3;
> +	return (srcprefs & IPV6_PREFER_SRC_MASK) << 3;
>  }
>  
>  static inline unsigned int rt6_flags2srcprefs(int flags)
>  {
> -	return (flags >> 3) & 7;
> +	return (flags >> 3) & IPV6_PREFER_SRC_MASK;
>  }
>  
>  static inline bool rt6_need_strict(const struct in6_addr *daddr)
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index bd115980809f386a7d38a5471d8d636f25ce1eba..b3444c8a6f744c17052a9fa1c85d54c6b08a1889 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -1306,10 +1306,13 @@ static inline void ip6_sock_set_recverr(struct sock *sk)
>  	inet6_set_bit(RECVERR6, sk);
>  }
>  
> -static inline int __ip6_sock_set_addr_preferences(struct sock *sk, int val)
> +#define IPV6_PREFER_SRC_MASK (IPV6_PREFER_SRC_TMP | IPV6_PREFER_SRC_PUBLIC | \
> +			      IPV6_PREFER_SRC_COA)
> +
> +static inline int ip6_sock_set_addr_preferences(struct sock *sk, int val)
>  {
> +	unsigned int prefmask = ~IPV6_PREFER_SRC_MASK;
>  	unsigned int pref = 0;
> -	unsigned int prefmask = ~0;
>  
>  	/* check PUBLIC/TMP/PUBTMP_DEFAULT conflicts */
>  	switch (val & (IPV6_PREFER_SRC_PUBLIC |

Unfortunate that address preference details are spread across 3 non-uapi
header files, but that is a change for a different patch.

Reviewed-by: David Ahern <dsahern@kernel.org>



^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH net-next] ipv6: lockless IPV6_ADDR_PREFERENCES implementation
  2023-09-18 14:23 [PATCH net-next] ipv6: lockless IPV6_ADDR_PREFERENCES implementation Eric Dumazet
  2023-09-19  2:48 ` David Ahern
@ 2023-09-19 16:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-09-19 16:30 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, kuba, pabeni, netdev, dsahern, eric.dumazet

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 18 Sep 2023 14:23:21 +0000 you wrote:
> We have data-races while reading np->srcprefs
> 
> Switch the field to a plain byte, add READ_ONCE()
> and WRITE_ONCE() annotations where needed,
> and IPV6_ADDR_PREFERENCES setsockopt() can now be lockless.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> [...]

Here is the summary with links:
  - [net-next] ipv6: lockless IPV6_ADDR_PREFERENCES implementation
    https://git.kernel.org/netdev/net-next/c/fa17a6d8a5bd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-19 16:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-18 14:23 [PATCH net-next] ipv6: lockless IPV6_ADDR_PREFERENCES implementation Eric Dumazet
2023-09-19  2:48 ` David Ahern
2023-09-19 16:30 ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.