From 3e5cbbb1fb9a64588a2c6ddc5e432a303d36a488 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Tue, 27 Aug 2024 01:52:49 +0000 Subject: [PATCH] tcp: remove volatile qualifier on tw_substate Using a volatile qualifier for a specific struct field is unusual. Use instead READ_ONCE()/WRITE_ONCE() where necessary. tcp_timewait_state_process() can change tw_substate while other threads are reading this field. Signed-off-by: Eric Dumazet Reviewed-by: Jason Xing Link: https://patch.msgid.link/20240827015250.3509197-2-edumazet@google.com Signed-off-by: Jakub Kicinski --- include/net/inet_timewait_sock.h | 2 +- net/ipv4/inet_diag.c | 4 ++-- net/ipv4/tcp_ipv4.c | 4 ++-- net/ipv4/tcp_minisocks.c | 4 ++-- net/ipv6/tcp_ipv6.c | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h index f88b68269012..beb533a0e880 100644 --- a/include/net/inet_timewait_sock.h +++ b/include/net/inet_timewait_sock.h @@ -58,7 +58,7 @@ struct inet_timewait_sock { #define tw_dr __tw_common.skc_tw_dr __u32 tw_mark; - volatile unsigned char tw_substate; + unsigned char tw_substate; unsigned char tw_rcv_wscale; /* Socket demultiplex comparisons on incoming packets. */ diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c index 9712cdb8087c..67639309163d 100644 --- a/net/ipv4/inet_diag.c +++ b/net/ipv4/inet_diag.c @@ -442,7 +442,7 @@ static int inet_twsk_diag_fill(struct sock *sk, inet_diag_msg_common_fill(r, sk); r->idiag_retrans = 0; - r->idiag_state = tw->tw_substate; + r->idiag_state = READ_ONCE(tw->tw_substate); r->idiag_timer = 3; tmo = tw->tw_timer.expires - jiffies; r->idiag_expires = jiffies_delta_to_msecs(tmo); @@ -1209,7 +1209,7 @@ next_chunk: if (num < s_num) goto next_normal; state = (sk->sk_state == TCP_TIME_WAIT) ? - inet_twsk(sk)->tw_substate : sk->sk_state; + READ_ONCE(inet_twsk(sk)->tw_substate) : sk->sk_state; if (!(idiag_states & (1 << state))) goto next_normal; if (r->sdiag_family != AF_UNSPEC && diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 5087e12209a1..7c29158e1abc 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -120,7 +120,7 @@ int tcp_twsk_unique(struct sock *sk, struct sock *sktw, void *twp) struct tcp_sock *tp = tcp_sk(sk); int ts_recent_stamp; - if (tw->tw_substate == TCP_FIN_WAIT2) + if (READ_ONCE(tw->tw_substate) == TCP_FIN_WAIT2) reuse = 0; if (reuse == 2) { @@ -2948,7 +2948,7 @@ static void get_timewait4_sock(const struct inet_timewait_sock *tw, seq_printf(f, "%4d: %08X:%04X %08X:%04X" " %02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %pK", - i, src, srcp, dest, destp, tw->tw_substate, 0, 0, + i, src, srcp, dest, destp, READ_ONCE(tw->tw_substate), 0, 0, 3, jiffies_delta_to_clock_t(delta), 0, 0, 0, 0, refcount_read(&tw->tw_refcnt), tw); } diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c index a19a9dbd3409..b6d547d29f9a 100644 --- a/net/ipv4/tcp_minisocks.c +++ b/net/ipv4/tcp_minisocks.c @@ -117,7 +117,7 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb, } } - if (tw->tw_substate == TCP_FIN_WAIT2) { + if (READ_ONCE(tw->tw_substate) == TCP_FIN_WAIT2) { /* Just repeat all the checks of tcp_rcv_state_process() */ /* Out of window, send ACK */ @@ -150,7 +150,7 @@ tcp_timewait_state_process(struct inet_timewait_sock *tw, struct sk_buff *skb, return TCP_TW_RST; /* FIN arrived, enter true time-wait state. */ - tw->tw_substate = TCP_TIME_WAIT; + WRITE_ONCE(tw->tw_substate, TCP_TIME_WAIT); twsk_rcv_nxt_update(tcptw, TCP_SKB_CB(skb)->end_seq); if (tmp_opt.saw_tstamp) { diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 200fea92f12f..fb2e64ce660f 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -2258,7 +2258,7 @@ static void get_timewait6_sock(struct seq_file *seq, src->s6_addr32[2], src->s6_addr32[3], srcp, dest->s6_addr32[0], dest->s6_addr32[1], dest->s6_addr32[2], dest->s6_addr32[3], destp, - tw->tw_substate, 0, 0, + READ_ONCE(tw->tw_substate), 0, 0, 3, jiffies_delta_to_clock_t(delta), 0, 0, 0, 0, refcount_read(&tw->tw_refcnt), tw); }