[PATCH net-next] ip6_tunnel: annotate data-races around t->err_count and t->err_time

[PATCH net-next] ip6_tunnel: annotate data-races around t->err_count and t->err_time

[Eric Dumazet]

ip6_tnl_xmit() and ipip6_tunnel_xmit() run locklessly (dev->lltx == true).

ip6gre_err() and ipip6_err() also run locklessly.

We need to add READ_ONCE() and WRITE_ONCE() annotations
around t->err_count and t->err_time.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv6/ip6_gre.c    |  8 ++++----
 net/ipv6/ip6_tunnel.c | 11 ++++++-----
 net/ipv6/sit.c        | 22 ++++++++++++----------
 3 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 365b4059eb20354c256c491a16db0e606e0a9790..9f767db800ba03f131c3315b8cf808daea50accc 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -445,11 +445,11 @@ static int ip6gre_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		return 0;
 	}
 
-	if (time_before(jiffies, t->err_time + IP6TUNNEL_ERR_TIMEO))
-		t->err_count++;
+	if (time_before(jiffies, READ_ONCE(t->err_time) + IP6TUNNEL_ERR_TIMEO))
+		WRITE_ONCE(t->err_count, t->err_count + 1);
 	else
-		t->err_count = 1;
-	t->err_time = jiffies;
+		WRITE_ONCE(t->err_count, 1);
+	WRITE_ONCE(t->err_time, jiffies);
 
 	return 0;
 }
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 9d1037ac082f6aad36c152ffdbe0f30237b65fc3..3445f81d9957e0ddd3f23d8b550ef5772672dba6 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1104,7 +1104,7 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield,
 	struct ipv6_tel_txoption opt;
 	struct dst_entry *dst = NULL, *ndst = NULL;
 	struct net_device *tdev;
-	int mtu;
+	int err_count, mtu;
 	unsigned int eth_hlen = t->dev->type == ARPHRD_ETHER ? ETH_HLEN : 0;
 	unsigned int psh_hlen = sizeof(struct ipv6hdr) + t->encap_hlen;
 	unsigned int max_headroom = psh_hlen;
@@ -1214,14 +1214,15 @@ int ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev, __u8 dsfield,
 		goto tx_err_dst_release;
 	}
 
-	if (t->err_count > 0) {
+	err_count = READ_ONCE(t->err_count);
+	if (err_count > 0) {
 		if (time_before(jiffies,
-				t->err_time + IP6TUNNEL_ERR_TIMEO)) {
-			t->err_count--;
+				READ_ONCE(t->err_time) + IP6TUNNEL_ERR_TIMEO)) {
+			WRITE_ONCE(t->err_count, err_count - 1);
 
 			dst_link_failure(skb);
 		} else {
-			t->err_count = 0;
+			WRITE_ONCE(t->err_count, 0);
 		}
 	}
 
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 201347b4e12742451de6d6036281d1abaff0dd84..f7a8d1d365e3c41edd9e10854c9009dfdd7e2aae 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -595,11 +595,11 @@ static int ipip6_err(struct sk_buff *skb, u32 info)
 	if (t->parms.iph.ttl == 0 && type == ICMP_TIME_EXCEEDED)
 		goto out;
 
-	if (time_before(jiffies, t->err_time + IPTUNNEL_ERR_TIMEO))
-		t->err_count++;
+	if (time_before(jiffies, READ_ONCE(t->err_time) + IPTUNNEL_ERR_TIMEO))
+		WRITE_ONCE(t->err_count, t->err_count + 1);
 	else
-		t->err_count = 1;
-	t->err_time = jiffies;
+		WRITE_ONCE(t->err_count, 1);
+	WRITE_ONCE(t->err_time, jiffies);
 out:
 	return err;
 }
@@ -909,8 +909,8 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
 	struct net_device *tdev;	/* Device to other host */
 	unsigned int max_headroom;	/* The extra header space needed */
 	__be32 dst = tiph->daddr;
+	int err_count, mtu;
 	struct flowi4 fl4;
-	int    mtu;
 	u8 ttl;
 	u8 protocol = IPPROTO_IPV6;
 	int t_hlen = tunnel->hlen + sizeof(struct iphdr);
@@ -986,13 +986,15 @@ static netdev_tx_t ipip6_tunnel_xmit(struct sk_buff *skb,
 		}
 	}
 
-	if (tunnel->err_count > 0) {
+	err_count = READ_ONCE(tunnel->err_count);
+	if (err_count > 0) {
 		if (time_before(jiffies,
-				tunnel->err_time + IPTUNNEL_ERR_TIMEO)) {
-			tunnel->err_count--;
+				READ_ONCE(tunnel->err_time) + IPTUNNEL_ERR_TIMEO)) {
+			WRITE_ONCE(tunnel->err_count, err_count - 1);
 			dst_link_failure(skb);
-		} else
-			tunnel->err_count = 0;
+		} else {
+			WRITE_ONCE(tunnel->err_count,  0);
+		}
 	}
 
 	/*
-- 
2.54.0.1064.gd145956f57-goog

Re: [PATCH net-next] ip6_tunnel: annotate data-races around t->err_count and t->err_time

[Jakub Kicinski]

On Tue,  9 Jun 2026 09:31:53 +0000 Eric Dumazet wrote:
> -	if (time_before(jiffies, t->err_time + IP6TUNNEL_ERR_TIMEO))
> -		t->err_count++;
> +	if (time_before(jiffies, READ_ONCE(t->err_time) + IP6TUNNEL_ERR_TIMEO))
> +		WRITE_ONCE(t->err_count, t->err_count + 1);

Sashiko complains about the bare t->err_count read. Is there a reason
you added the temp variable in xmit but not on the error side?

Maybe an extra sentence in the commit message on why the approximate
correctness is okay here would also be good?

>  	else
> -		t->err_count = 1;
> -	t->err_time = jiffies;
> +		WRITE_ONCE(t->err_count, 1);
> +	WRITE_ONCE(t->err_time, jiffies);

Re: [PATCH net-next] ip6_tunnel: annotate data-races around t->err_count and t->err_time

[Eric Dumazet]

On Wed, Jun 10, 2026 at 9:05 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue,  9 Jun 2026 09:31:53 +0000 Eric Dumazet wrote:
> > -     if (time_before(jiffies, t->err_time + IP6TUNNEL_ERR_TIMEO))
> > -             t->err_count++;
> > +     if (time_before(jiffies, READ_ONCE(t->err_time) + IP6TUNNEL_ERR_TIMEO))
> > +             WRITE_ONCE(t->err_count, t->err_count + 1);
>
> Sashiko complains about the bare t->err_count read. Is there a reason
> you added the temp variable in xmit but not on the error side?
>
> Maybe an extra sentence in the commit message on why the approximate
> correctness is okay here would also be good?

Sashiko is right; we can have concurrent writers, so I will send a V2.
Thanks.