[PATCH net-next 0/3] replace tcp_set_state tracepoint with sock_set_state tracepoint

[PATCH net-next 0/3] replace tcp_set_state tracepoint with sock_set_state tracepoint

[Yafang Shao]

Hi,

According to the discussion in the mail thread
https://patchwork.kernel.org/patch/10099243/,
tcp_set_state tracepoint is renamed to sock_set_state tracepoint and is moved
to include/trace/events/sock.h.

Using this new tracepoint to trace TCP/DCCP/SCTP state transition.

Yafang Shao (3):
  net:tracepoint: replace tcp_set_state tracepoint      with
    sock_set_state tracepoint
  net:tracepoint: using sock_set_state tracepoint      to trace DCCP
    state transition
  net:tracepoint: using sock_set_state tracepoint to trace SCTP state
     transition

 include/net/sock.h              | 15 ++-----
 include/trace/events/sock.h     | 95 +++++++++++++++++++++++++++++++++++++++++
 include/trace/events/tcp.h      | 76 ---------------------------------
 net/core/sock.c                 | 13 ++++++
 net/dccp/proto.c                |  2 +-
 net/ipv4/inet_connection_sock.c |  4 +-
 net/ipv4/inet_hashtables.c      |  2 +-
 net/ipv4/tcp.c                  |  4 --
 net/sctp/endpointola.c          |  2 +-
 net/sctp/sm_sideeffect.c        |  4 +-
 net/sctp/socket.c               | 14 +++---
 11 files changed, 125 insertions(+), 106 deletions(-)

--
1.8.3.1

[PATCH net-next 1/3] net:tracepoint: replace tcp_set_state tracepoint with sock_set_state tracepoint

[Yafang Shao]

As sk_state is a common field for struct sock, so the state
transition should not be a TCP specific feature.
So I rename tcp_set_state tracepoint to sock_set_state tracepoint with
some minor changes and move it into file trace/events/sock.h.

The minor changes against on the original tcp_set_state tracepoint:
- Protocol name is printed to distinguish which protocol it is belonging
  to
- The macros defined in the file are undefed at the end of this file as
  they are only used in this file

Two helpers are introduced to trace sk_state transition
- void sk_state_store(struct sock *sk, int state);
- void sk_set_state(struct sock *sk, int state);

As trace header should not be included in other header files,
so they are defined in sock.c.

The protocol such as SCTP maybe compiled as a ko, hence export
sk_set_state().

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/net/sock.h              | 15 ++-----
 include/trace/events/sock.h     | 95 +++++++++++++++++++++++++++++++++++++++++
 include/trace/events/tcp.h      | 76 ---------------------------------
 net/core/sock.c                 | 13 ++++++
 net/ipv4/inet_connection_sock.c |  4 +-
 net/ipv4/inet_hashtables.c      |  2 +-
 net/ipv4/tcp.c                  |  4 --
 7 files changed, 114 insertions(+), 95 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 79e1a2c..b307b60 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2348,18 +2348,9 @@ static inline int sk_state_load(const struct sock *sk)
 	return smp_load_acquire(&sk->sk_state);
 }

-/**
- * sk_state_store - update sk->sk_state
- * @sk: socket pointer
- * @newstate: new state
- *
- * Paired with sk_state_load(). Should be used in contexts where
- * state change might impact lockless readers.
- */
-static inline void sk_state_store(struct sock *sk, int newstate)
-{
-	smp_store_release(&sk->sk_state, newstate);
-}
+/* For sock_set_state tracepoint */
+void sk_state_store(struct sock *sk, int newstate);
+void sk_set_state(struct sock *sk, int state);

 void sock_enable_timestamp(struct sock *sk, int flag);
 int sock_get_timestamp(struct sock *, struct timeval __user *);
diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
index ec4dade..2728892 100644
--- a/include/trace/events/sock.h
+++ b/include/trace/events/sock.h
@@ -6,7 +6,33 @@
 #define _TRACE_SOCK_H

 #include <net/sock.h>
+#include <net/ipv6.h>
 #include <linux/tracepoint.h>
+#include <linux/ipv6.h>
+#include <linux/tcp.h>
+
+#define inet_protocol_name(protocol) {IPPROTO_##protocol, #protocol}
+#define show_inet_protocol_name(val)	\
+	__print_symbolic(val,	\
+	inet_protocol_name(TCP),	\
+	inet_protocol_name(DCCP),	\
+	inet_protocol_name(SCTP))
+
+#define tcp_state_name(state)   { state, #state }
+#define show_tcp_state_name(val)			\
+		__print_symbolic(val,			\
+		tcp_state_name(TCP_ESTABLISHED),	\
+		tcp_state_name(TCP_SYN_SENT),	\
+		tcp_state_name(TCP_SYN_RECV),	\
+		tcp_state_name(TCP_FIN_WAIT1),	\
+		tcp_state_name(TCP_FIN_WAIT2),	\
+		tcp_state_name(TCP_TIME_WAIT),	\
+		tcp_state_name(TCP_CLOSE),	\
+		tcp_state_name(TCP_CLOSE_WAIT),	\
+		tcp_state_name(TCP_LAST_ACK),	\
+		tcp_state_name(TCP_LISTEN),	\
+		tcp_state_name(TCP_CLOSING),	\
+		tcp_state_name(TCP_NEW_SYN_RECV))

 TRACE_EVENT(sock_rcvqueue_full,

@@ -63,6 +89,75 @@
 		__entry->rmem_alloc)
 );

+TRACE_EVENT(sock_set_state,
+
+	TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
+
+	TP_ARGS(sk, oldstate, newstate),
+
+	TP_STRUCT__entry(
+		__field(const void *, skaddr)
+		__field(int, oldstate)
+		__field(int, newstate)
+		__field(__u16, sport)
+		__field(__u16, dport)
+		__field(__u8, protocol);
+		__array(__u8, saddr, 4)
+		__array(__u8, daddr, 4)
+		__array(__u8, saddr_v6, 16)
+		__array(__u8, daddr_v6, 16)
+	),
+
+	TP_fast_assign(
+		struct inet_sock *inet = inet_sk(sk);
+		struct in6_addr *pin6;
+		__be32 *p32;
+
+		__entry->skaddr = sk;
+		__entry->oldstate = oldstate;
+		__entry->newstate = newstate;
+
+		__entry->protocol = sk->sk_protocol;
+		__entry->sport = ntohs(inet->inet_sport);
+		__entry->dport = ntohs(inet->inet_dport);
+
+		p32 = (__be32 *) __entry->saddr;
+		*p32 = inet->inet_saddr;
+
+		p32 = (__be32 *) __entry->daddr;
+		*p32 =  inet->inet_daddr;
+
+#if IS_ENABLED(CONFIG_IPV6)
+		if (sk->sk_family == AF_INET6) {
+			pin6 = (struct in6_addr *)__entry->saddr_v6;
+			*pin6 = sk->sk_v6_rcv_saddr;
+			pin6 = (struct in6_addr *)__entry->daddr_v6;
+			*pin6 = sk->sk_v6_daddr;
+		} else
+#endif
+		{
+			pin6 = (struct in6_addr *)__entry->saddr_v6;
+			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
+			pin6 = (struct in6_addr *)__entry->daddr_v6;
+			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
+		}
+	),
+
+	TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4"
+				"saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
+		show_inet_protocol_name(__entry->protocol),
+		__entry->sport, __entry->dport,
+		__entry->saddr, __entry->daddr,
+		__entry->saddr_v6, __entry->daddr_v6,
+		show_tcp_state_name(__entry->oldstate),
+		show_tcp_state_name(__entry->newstate))
+);
+
+#undef show_tcp_state_name
+#undef tcp_state_name
+#undef show_inet_protocol_name
+#undef inet_protocol_name
+
 #endif /* _TRACE_SOCK_H */

 /* This part must be outside protection */
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 07cccca..7399399 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -9,22 +9,6 @@
 #include <linux/tracepoint.h>
 #include <net/ipv6.h>

-#define tcp_state_name(state)	{ state, #state }
-#define show_tcp_state_name(val)			\
-	__print_symbolic(val,				\
-		tcp_state_name(TCP_ESTABLISHED),	\
-		tcp_state_name(TCP_SYN_SENT),		\
-		tcp_state_name(TCP_SYN_RECV),		\
-		tcp_state_name(TCP_FIN_WAIT1),		\
-		tcp_state_name(TCP_FIN_WAIT2),		\
-		tcp_state_name(TCP_TIME_WAIT),		\
-		tcp_state_name(TCP_CLOSE),		\
-		tcp_state_name(TCP_CLOSE_WAIT),		\
-		tcp_state_name(TCP_LAST_ACK),		\
-		tcp_state_name(TCP_LISTEN),		\
-		tcp_state_name(TCP_CLOSING),		\
-		tcp_state_name(TCP_NEW_SYN_RECV))
-
 /*
  * tcp event with arguments sk and skb
  *
@@ -177,66 +161,6 @@
 	TP_ARGS(sk)
 );

-TRACE_EVENT(tcp_set_state,
-
-	TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
-
-	TP_ARGS(sk, oldstate, newstate),
-
-	TP_STRUCT__entry(
-		__field(const void *, skaddr)
-		__field(int, oldstate)
-		__field(int, newstate)
-		__field(__u16, sport)
-		__field(__u16, dport)
-		__array(__u8, saddr, 4)
-		__array(__u8, daddr, 4)
-		__array(__u8, saddr_v6, 16)
-		__array(__u8, daddr_v6, 16)
-	),
-
-	TP_fast_assign(
-		struct inet_sock *inet = inet_sk(sk);
-		struct in6_addr *pin6;
-		__be32 *p32;
-
-		__entry->skaddr = sk;
-		__entry->oldstate = oldstate;
-		__entry->newstate = newstate;
-
-		__entry->sport = ntohs(inet->inet_sport);
-		__entry->dport = ntohs(inet->inet_dport);
-
-		p32 = (__be32 *) __entry->saddr;
-		*p32 = inet->inet_saddr;
-
-		p32 = (__be32 *) __entry->daddr;
-		*p32 =  inet->inet_daddr;
-
-#if IS_ENABLED(CONFIG_IPV6)
-		if (sk->sk_family == AF_INET6) {
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			*pin6 = sk->sk_v6_rcv_saddr;
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			*pin6 = sk->sk_v6_daddr;
-		} else
-#endif
-		{
-			pin6 = (struct in6_addr *)__entry->saddr_v6;
-			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
-			pin6 = (struct in6_addr *)__entry->daddr_v6;
-			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
-		}
-	),
-
-	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
-		  __entry->sport, __entry->dport,
-		  __entry->saddr, __entry->daddr,
-		  __entry->saddr_v6, __entry->daddr_v6,
-		  show_tcp_state_name(__entry->oldstate),
-		  show_tcp_state_name(__entry->newstate))
-);
-
 TRACE_EVENT(tcp_retransmit_synack,

 	TP_PROTO(const struct sock *sk, const struct request_sock *req),
diff --git a/net/core/sock.c b/net/core/sock.c
index c0b5b2f..ee0c1bc 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2859,6 +2859,19 @@ int sock_get_timestampns(struct sock *sk, struct timespec __user *userstamp)
 }
 EXPORT_SYMBOL(sock_get_timestampns);

+void sk_state_store(struct sock *sk, int state)
+{
+	trace_sock_set_state(sk, sk->sk_state, state);
+	smp_store_release(&sk->sk_state, state);
+}
+
+void sk_set_state(struct sock *sk, int state)
+{
+	trace_sock_set_state(sk, sk->sk_state, state);
+	sk->sk_state = state;
+}
+EXPORT_SYMBOL(sk_set_state);
+
 void sock_enable_timestamp(struct sock *sk, int flag)
 {
 	if (!sock_flag(sk, flag)) {
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 4ca46dc..001f7b0 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -783,7 +783,7 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
 	if (newsk) {
 		struct inet_connection_sock *newicsk = inet_csk(newsk);

-		newsk->sk_state = TCP_SYN_RECV;
+		sk_set_state(newsk, TCP_SYN_RECV);
 		newicsk->icsk_bind_hash = NULL;

 		inet_sk(newsk)->inet_dport = inet_rsk(req)->ir_rmt_port;
@@ -888,7 +888,7 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
 			return 0;
 	}

-	sk->sk_state = TCP_CLOSE;
+	sk_set_state(sk, TCP_CLOSE);
 	return err;
 }
 EXPORT_SYMBOL_GPL(inet_csk_listen_start);
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index f6f5810..5973693 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -544,7 +544,7 @@ bool inet_ehash_nolisten(struct sock *sk, struct sock *osk)
 		sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
 	} else {
 		percpu_counter_inc(sk->sk_prot->orphan_count);
-		sk->sk_state = TCP_CLOSE;
+		sk_set_state(sk, TCP_CLOSE);
 		sock_set_flag(sk, SOCK_DEAD);
 		inet_csk_destroy_sock(sk);
 	}
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 1803116..000504f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -283,8 +283,6 @@
 #include <asm/ioctls.h>
 #include <net/busy_poll.h>

-#include <trace/events/tcp.h>
-
 struct percpu_counter tcp_orphan_count;
 EXPORT_SYMBOL_GPL(tcp_orphan_count);

@@ -2040,8 +2038,6 @@ void tcp_set_state(struct sock *sk, int state)
 {
 	int oldstate = sk->sk_state;

-	trace_tcp_set_state(sk, oldstate, state);
-
 	switch (state) {
 	case TCP_ESTABLISHED:
 		if (oldstate != TCP_ESTABLISHED)
--
1.8.3.1

Re: [PATCH net-next 1/3] net:tracepoint: replace tcp_set_state tracepoint with sock_set_state tracepoint

[Song Liu]

> On Dec 10, 2017, at 7:31 AM, Yafang Shao <laoar.shao@gmail.com> wrote:
> 
> As sk_state is a common field for struct sock, so the state
> transition should not be a TCP specific feature.
> So I rename tcp_set_state tracepoint to sock_set_state tracepoint with
> some minor changes and move it into file trace/events/sock.h.
> 
> The minor changes against on the original tcp_set_state tracepoint:
> - Protocol name is printed to distinguish which protocol it is belonging
>  to
> - The macros defined in the file are undefed at the end of this file as
>  they are only used in this file
> 
> Two helpers are introduced to trace sk_state transition
> - void sk_state_store(struct sock *sk, int state);
> - void sk_set_state(struct sock *sk, int state);
> 
> As trace header should not be included in other header files,
> so they are defined in sock.c.
> 
> The protocol such as SCTP maybe compiled as a ko, hence export
> sk_set_state().
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> include/net/sock.h              | 15 ++-----
> include/trace/events/sock.h     | 95 +++++++++++++++++++++++++++++++++++++++++
> include/trace/events/tcp.h      | 76 ---------------------------------
> net/core/sock.c                 | 13 ++++++
> net/ipv4/inet_connection_sock.c |  4 +-
> net/ipv4/inet_hashtables.c      |  2 +-
> net/ipv4/tcp.c                  |  4 --
> 7 files changed, 114 insertions(+), 95 deletions(-)
> 
> diff --git a/include/net/sock.h b/include/net/sock.h
> index 79e1a2c..b307b60 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -2348,18 +2348,9 @@ static inline int sk_state_load(const struct sock *sk)
> 	return smp_load_acquire(&sk->sk_state);
> }
> 
> -/**
> - * sk_state_store - update sk->sk_state
> - * @sk: socket pointer
> - * @newstate: new state
> - *
> - * Paired with sk_state_load(). Should be used in contexts where
> - * state change might impact lockless readers.
> - */
> -static inline void sk_state_store(struct sock *sk, int newstate)
> -{
> -	smp_store_release(&sk->sk_state, newstate);
> -}
> +/* For sock_set_state tracepoint */
> +void sk_state_store(struct sock *sk, int newstate);
> +void sk_set_state(struct sock *sk, int state);
> 
> void sock_enable_timestamp(struct sock *sk, int flag);
> int sock_get_timestamp(struct sock *, struct timeval __user *);
> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
> index ec4dade..2728892 100644
> --- a/include/trace/events/sock.h
> +++ b/include/trace/events/sock.h
> @@ -6,7 +6,33 @@
> #define _TRACE_SOCK_H
> 
> #include <net/sock.h>
> +#include <net/ipv6.h>
> #include <linux/tracepoint.h>
> +#include <linux/ipv6.h>
> +#include <linux/tcp.h>
> +
> +#define inet_protocol_name(protocol) {IPPROTO_##protocol, #protocol}
> +#define show_inet_protocol_name(val)	\
> +	__print_symbolic(val,	\
> +	inet_protocol_name(TCP),	\
> +	inet_protocol_name(DCCP),	\
> +	inet_protocol_name(SCTP))
> +
> +#define tcp_state_name(state)   { state, #state }
> +#define show_tcp_state_name(val)			\
> +		__print_symbolic(val,			\
> +		tcp_state_name(TCP_ESTABLISHED),	\
> +		tcp_state_name(TCP_SYN_SENT),	\
> +		tcp_state_name(TCP_SYN_RECV),	\
> +		tcp_state_name(TCP_FIN_WAIT1),	\
> +		tcp_state_name(TCP_FIN_WAIT2),	\
> +		tcp_state_name(TCP_TIME_WAIT),	\
> +		tcp_state_name(TCP_CLOSE),	\
> +		tcp_state_name(TCP_CLOSE_WAIT),	\
> +		tcp_state_name(TCP_LAST_ACK),	\
> +		tcp_state_name(TCP_LISTEN),	\
> +		tcp_state_name(TCP_CLOSING),	\
> +		tcp_state_name(TCP_NEW_SYN_RECV))

Please include Steven's fix to export names to user space: 

https://patchwork.kernel.org/patch/10052201/


> TRACE_EVENT(sock_rcvqueue_full,
> 
> @@ -63,6 +89,75 @@
> 		__entry->rmem_alloc)
> );
> 
> +TRACE_EVENT(sock_set_state,
> +
> +	TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
> +
> +	TP_ARGS(sk, oldstate, newstate),
> +
> +	TP_STRUCT__entry(
> +		__field(const void *, skaddr)
> +		__field(int, oldstate)
> +		__field(int, newstate)
> +		__field(__u16, sport)
> +		__field(__u16, dport)
> +		__field(__u8, protocol);
> +		__array(__u8, saddr, 4)
> +		__array(__u8, daddr, 4)
> +		__array(__u8, saddr_v6, 16)
> +		__array(__u8, daddr_v6, 16)
> +	),
> +
> +	TP_fast_assign(
> +		struct inet_sock *inet = inet_sk(sk);
> +		struct in6_addr *pin6;
> +		__be32 *p32;
> +
> +		__entry->skaddr = sk;
> +		__entry->oldstate = oldstate;
> +		__entry->newstate = newstate;
> +
> +		__entry->protocol = sk->sk_protocol;
> +		__entry->sport = ntohs(inet->inet_sport);
> +		__entry->dport = ntohs(inet->inet_dport);
> +
> +		p32 = (__be32 *) __entry->saddr;
> +		*p32 = inet->inet_saddr;
> +
> +		p32 = (__be32 *) __entry->daddr;
> +		*p32 =  inet->inet_daddr;
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +		if (sk->sk_family == AF_INET6) {
> +			pin6 = (struct in6_addr *)__entry->saddr_v6;
> +			*pin6 = sk->sk_v6_rcv_saddr;
> +			pin6 = (struct in6_addr *)__entry->daddr_v6;
> +			*pin6 = sk->sk_v6_daddr;
> +		} else
> +#endif
> +		{
> +			pin6 = (struct in6_addr *)__entry->saddr_v6;
> +			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
> +			pin6 = (struct in6_addr *)__entry->daddr_v6;
> +			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
> +		}
> +	),
> +
> +	TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4"
> +				"saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> +		show_inet_protocol_name(__entry->protocol),
> +		__entry->sport, __entry->dport,
> +		__entry->saddr, __entry->daddr,
> +		__entry->saddr_v6, __entry->daddr_v6,
> +		show_tcp_state_name(__entry->oldstate),
> +		show_tcp_state_name(__entry->newstate))
> +);
> +
> +#undef show_tcp_state_name
> +#undef tcp_state_name
> +#undef show_inet_protocol_name
> +#undef inet_protocol_name
> +
> #endif /* _TRACE_SOCK_H */
> 
> /* This part must be outside protection */
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> index 07cccca..7399399 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -9,22 +9,6 @@
> #include <linux/tracepoint.h>
> #include <net/ipv6.h>
> 
> -#define tcp_state_name(state)	{ state, #state }
> -#define show_tcp_state_name(val)			\
> -	__print_symbolic(val,				\
> -		tcp_state_name(TCP_ESTABLISHED),	\
> -		tcp_state_name(TCP_SYN_SENT),		\
> -		tcp_state_name(TCP_SYN_RECV),		\
> -		tcp_state_name(TCP_FIN_WAIT1),		\
> -		tcp_state_name(TCP_FIN_WAIT2),		\
> -		tcp_state_name(TCP_TIME_WAIT),		\
> -		tcp_state_name(TCP_CLOSE),		\
> -		tcp_state_name(TCP_CLOSE_WAIT),		\
> -		tcp_state_name(TCP_LAST_ACK),		\
> -		tcp_state_name(TCP_LISTEN),		\
> -		tcp_state_name(TCP_CLOSING),		\
> -		tcp_state_name(TCP_NEW_SYN_RECV))
> -
> /*
>  * tcp event with arguments sk and skb
>  *
> @@ -177,66 +161,6 @@
> 	TP_ARGS(sk)
> );
> 
> -TRACE_EVENT(tcp_set_state,
> -
> -	TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
> -
> -	TP_ARGS(sk, oldstate, newstate),
> -
> -	TP_STRUCT__entry(
> -		__field(const void *, skaddr)
> -		__field(int, oldstate)
> -		__field(int, newstate)
> -		__field(__u16, sport)
> -		__field(__u16, dport)
> -		__array(__u8, saddr, 4)
> -		__array(__u8, daddr, 4)
> -		__array(__u8, saddr_v6, 16)
> -		__array(__u8, daddr_v6, 16)
> -	),
> -
> -	TP_fast_assign(
> -		struct inet_sock *inet = inet_sk(sk);
> -		struct in6_addr *pin6;
> -		__be32 *p32;
> -
> -		__entry->skaddr = sk;
> -		__entry->oldstate = oldstate;
> -		__entry->newstate = newstate;
> -
> -		__entry->sport = ntohs(inet->inet_sport);
> -		__entry->dport = ntohs(inet->inet_dport);
> -
> -		p32 = (__be32 *) __entry->saddr;
> -		*p32 = inet->inet_saddr;
> -
> -		p32 = (__be32 *) __entry->daddr;
> -		*p32 =  inet->inet_daddr;
> -
> -#if IS_ENABLED(CONFIG_IPV6)
> -		if (sk->sk_family == AF_INET6) {
> -			pin6 = (struct in6_addr *)__entry->saddr_v6;
> -			*pin6 = sk->sk_v6_rcv_saddr;
> -			pin6 = (struct in6_addr *)__entry->daddr_v6;
> -			*pin6 = sk->sk_v6_daddr;
> -		} else
> -#endif
> -		{
> -			pin6 = (struct in6_addr *)__entry->saddr_v6;
> -			ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
> -			pin6 = (struct in6_addr *)__entry->daddr_v6;
> -			ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
> -		}
> -	),
> -
> -	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
> -		  __entry->sport, __entry->dport,
> -		  __entry->saddr, __entry->daddr,
> -		  __entry->saddr_v6, __entry->daddr_v6,
> -		  show_tcp_state_name(__entry->oldstate),
> -		  show_tcp_state_name(__entry->newstate))
> -);
> -
> TRACE_EVENT(tcp_retransmit_synack,
> 
> 	TP_PROTO(const struct sock *sk, const struct request_sock *req),
> diff --git a/net/core/sock.c b/net/core/sock.c
> index c0b5b2f..ee0c1bc 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2859,6 +2859,19 @@ int sock_get_timestampns(struct sock *sk, struct timespec __user *userstamp)
> }
> EXPORT_SYMBOL(sock_get_timestampns);
> 
> +void sk_state_store(struct sock *sk, int state)
> +{
> +	trace_sock_set_state(sk, sk->sk_state, state);
> +	smp_store_release(&sk->sk_state, state);
> +}
> +
> +void sk_set_state(struct sock *sk, int state)
> +{
> +	trace_sock_set_state(sk, sk->sk_state, state);
> +	sk->sk_state = state;
> +}
> +EXPORT_SYMBOL(sk_set_state);

These two functions are short. Can we keep this in sock.h?

Thanks,
Song

> +
> void sock_enable_timestamp(struct sock *sk, int flag)
> {
> 	if (!sock_flag(sk, flag)) {
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 4ca46dc..001f7b0 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -783,7 +783,7 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
> 	if (newsk) {
> 		struct inet_connection_sock *newicsk = inet_csk(newsk);
> 
> -		newsk->sk_state = TCP_SYN_RECV;
> +		sk_set_state(newsk, TCP_SYN_RECV);
> 		newicsk->icsk_bind_hash = NULL;
> 
> 		inet_sk(newsk)->inet_dport = inet_rsk(req)->ir_rmt_port;
> @@ -888,7 +888,7 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
> 			return 0;
> 	}
> 
> -	sk->sk_state = TCP_CLOSE;
> +	sk_set_state(sk, TCP_CLOSE);
> 	return err;
> }
> EXPORT_SYMBOL_GPL(inet_csk_listen_start);
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index f6f5810..5973693 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -544,7 +544,7 @@ bool inet_ehash_nolisten(struct sock *sk, struct sock *osk)
> 		sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
> 	} else {
> 		percpu_counter_inc(sk->sk_prot->orphan_count);
> -		sk->sk_state = TCP_CLOSE;
> +		sk_set_state(sk, TCP_CLOSE);
> 		sock_set_flag(sk, SOCK_DEAD);
> 		inet_csk_destroy_sock(sk);
> 	}
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 1803116..000504f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -283,8 +283,6 @@
> #include <asm/ioctls.h>
> #include <net/busy_poll.h>
> 
> -#include <trace/events/tcp.h>
> -
> struct percpu_counter tcp_orphan_count;
> EXPORT_SYMBOL_GPL(tcp_orphan_count);
> 
> @@ -2040,8 +2038,6 @@ void tcp_set_state(struct sock *sk, int state)
> {
> 	int oldstate = sk->sk_state;
> 
> -	trace_tcp_set_state(sk, oldstate, state);
> -
> 	switch (state) {
> 	case TCP_ESTABLISHED:
> 		if (oldstate != TCP_ESTABLISHED)
> --
> 1.8.3.1
> 

Re: [PATCH net-next 1/3] net:tracepoint: replace tcp_set_state tracepoint with sock_set_state tracepoint

[Yafang Shao]

2017-12-13 9:19 GMT+08:00 Song Liu <songliubraving@fb.com>:
>
>> On Dec 10, 2017, at 7:31 AM, Yafang Shao <laoar.shao@gmail.com> wrote:
>>
>> As sk_state is a common field for struct sock, so the state
>> transition should not be a TCP specific feature.
>> So I rename tcp_set_state tracepoint to sock_set_state tracepoint with
>> some minor changes and move it into file trace/events/sock.h.
>>
>> The minor changes against on the original tcp_set_state tracepoint:
>> - Protocol name is printed to distinguish which protocol it is belonging
>>  to
>> - The macros defined in the file are undefed at the end of this file as
>>  they are only used in this file
>>
>> Two helpers are introduced to trace sk_state transition
>> - void sk_state_store(struct sock *sk, int state);
>> - void sk_set_state(struct sock *sk, int state);
>>
>> As trace header should not be included in other header files,
>> so they are defined in sock.c.
>>
>> The protocol such as SCTP maybe compiled as a ko, hence export
>> sk_set_state().
>>
>> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
>> ---
>> include/net/sock.h              | 15 ++-----
>> include/trace/events/sock.h     | 95 +++++++++++++++++++++++++++++++++++++++++
>> include/trace/events/tcp.h      | 76 ---------------------------------
>> net/core/sock.c                 | 13 ++++++
>> net/ipv4/inet_connection_sock.c |  4 +-
>> net/ipv4/inet_hashtables.c      |  2 +-
>> net/ipv4/tcp.c                  |  4 --
>> 7 files changed, 114 insertions(+), 95 deletions(-)
>>
>> diff --git a/include/net/sock.h b/include/net/sock.h
>> index 79e1a2c..b307b60 100644
>> --- a/include/net/sock.h
>> +++ b/include/net/sock.h
>> @@ -2348,18 +2348,9 @@ static inline int sk_state_load(const struct sock *sk)
>>       return smp_load_acquire(&sk->sk_state);
>> }
>>
>> -/**
>> - * sk_state_store - update sk->sk_state
>> - * @sk: socket pointer
>> - * @newstate: new state
>> - *
>> - * Paired with sk_state_load(). Should be used in contexts where
>> - * state change might impact lockless readers.
>> - */
>> -static inline void sk_state_store(struct sock *sk, int newstate)
>> -{
>> -     smp_store_release(&sk->sk_state, newstate);
>> -}
>> +/* For sock_set_state tracepoint */
>> +void sk_state_store(struct sock *sk, int newstate);
>> +void sk_set_state(struct sock *sk, int state);
>>
>> void sock_enable_timestamp(struct sock *sk, int flag);
>> int sock_get_timestamp(struct sock *, struct timeval __user *);
>> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h
>> index ec4dade..2728892 100644
>> --- a/include/trace/events/sock.h
>> +++ b/include/trace/events/sock.h
>> @@ -6,7 +6,33 @@
>> #define _TRACE_SOCK_H
>>
>> #include <net/sock.h>
>> +#include <net/ipv6.h>
>> #include <linux/tracepoint.h>
>> +#include <linux/ipv6.h>
>> +#include <linux/tcp.h>
>> +
>> +#define inet_protocol_name(protocol) {IPPROTO_##protocol, #protocol}
>> +#define show_inet_protocol_name(val) \
>> +     __print_symbolic(val,   \
>> +     inet_protocol_name(TCP),        \
>> +     inet_protocol_name(DCCP),       \
>> +     inet_protocol_name(SCTP))
>> +
>> +#define tcp_state_name(state)   { state, #state }
>> +#define show_tcp_state_name(val)                     \
>> +             __print_symbolic(val,                   \
>> +             tcp_state_name(TCP_ESTABLISHED),        \
>> +             tcp_state_name(TCP_SYN_SENT),   \
>> +             tcp_state_name(TCP_SYN_RECV),   \
>> +             tcp_state_name(TCP_FIN_WAIT1),  \
>> +             tcp_state_name(TCP_FIN_WAIT2),  \
>> +             tcp_state_name(TCP_TIME_WAIT),  \
>> +             tcp_state_name(TCP_CLOSE),      \
>> +             tcp_state_name(TCP_CLOSE_WAIT), \
>> +             tcp_state_name(TCP_LAST_ACK),   \
>> +             tcp_state_name(TCP_LISTEN),     \
>> +             tcp_state_name(TCP_CLOSING),    \
>> +             tcp_state_name(TCP_NEW_SYN_RECV))
>
> Please include Steven's fix to export names to user space:
>
> https://patchwork.kernel.org/patch/10052201/
>

OK.


>
>> TRACE_EVENT(sock_rcvqueue_full,
>>
>> @@ -63,6 +89,75 @@
>>               __entry->rmem_alloc)
>> );
>>
>> +TRACE_EVENT(sock_set_state,
>> +
>> +     TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
>> +
>> +     TP_ARGS(sk, oldstate, newstate),
>> +
>> +     TP_STRUCT__entry(
>> +             __field(const void *, skaddr)
>> +             __field(int, oldstate)
>> +             __field(int, newstate)
>> +             __field(__u16, sport)
>> +             __field(__u16, dport)
>> +             __field(__u8, protocol);
>> +             __array(__u8, saddr, 4)
>> +             __array(__u8, daddr, 4)
>> +             __array(__u8, saddr_v6, 16)
>> +             __array(__u8, daddr_v6, 16)
>> +     ),
>> +
>> +     TP_fast_assign(
>> +             struct inet_sock *inet = inet_sk(sk);
>> +             struct in6_addr *pin6;
>> +             __be32 *p32;
>> +
>> +             __entry->skaddr = sk;
>> +             __entry->oldstate = oldstate;
>> +             __entry->newstate = newstate;
>> +
>> +             __entry->protocol = sk->sk_protocol;
>> +             __entry->sport = ntohs(inet->inet_sport);
>> +             __entry->dport = ntohs(inet->inet_dport);
>> +
>> +             p32 = (__be32 *) __entry->saddr;
>> +             *p32 = inet->inet_saddr;
>> +
>> +             p32 = (__be32 *) __entry->daddr;
>> +             *p32 =  inet->inet_daddr;
>> +
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +             if (sk->sk_family == AF_INET6) {
>> +                     pin6 = (struct in6_addr *)__entry->saddr_v6;
>> +                     *pin6 = sk->sk_v6_rcv_saddr;
>> +                     pin6 = (struct in6_addr *)__entry->daddr_v6;
>> +                     *pin6 = sk->sk_v6_daddr;
>> +             } else
>> +#endif
>> +             {
>> +                     pin6 = (struct in6_addr *)__entry->saddr_v6;
>> +                     ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
>> +                     pin6 = (struct in6_addr *)__entry->daddr_v6;
>> +                     ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
>> +             }
>> +     ),
>> +
>> +     TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4"
>> +                             "saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>> +             show_inet_protocol_name(__entry->protocol),
>> +             __entry->sport, __entry->dport,
>> +             __entry->saddr, __entry->daddr,
>> +             __entry->saddr_v6, __entry->daddr_v6,
>> +             show_tcp_state_name(__entry->oldstate),
>> +             show_tcp_state_name(__entry->newstate))
>> +);
>> +
>> +#undef show_tcp_state_name
>> +#undef tcp_state_name
>> +#undef show_inet_protocol_name
>> +#undef inet_protocol_name
>> +
>> #endif /* _TRACE_SOCK_H */
>>
>> /* This part must be outside protection */
>> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
>> index 07cccca..7399399 100644
>> --- a/include/trace/events/tcp.h
>> +++ b/include/trace/events/tcp.h
>> @@ -9,22 +9,6 @@
>> #include <linux/tracepoint.h>
>> #include <net/ipv6.h>
>>
>> -#define tcp_state_name(state)        { state, #state }
>> -#define show_tcp_state_name(val)                     \
>> -     __print_symbolic(val,                           \
>> -             tcp_state_name(TCP_ESTABLISHED),        \
>> -             tcp_state_name(TCP_SYN_SENT),           \
>> -             tcp_state_name(TCP_SYN_RECV),           \
>> -             tcp_state_name(TCP_FIN_WAIT1),          \
>> -             tcp_state_name(TCP_FIN_WAIT2),          \
>> -             tcp_state_name(TCP_TIME_WAIT),          \
>> -             tcp_state_name(TCP_CLOSE),              \
>> -             tcp_state_name(TCP_CLOSE_WAIT),         \
>> -             tcp_state_name(TCP_LAST_ACK),           \
>> -             tcp_state_name(TCP_LISTEN),             \
>> -             tcp_state_name(TCP_CLOSING),            \
>> -             tcp_state_name(TCP_NEW_SYN_RECV))
>> -
>> /*
>>  * tcp event with arguments sk and skb
>>  *
>> @@ -177,66 +161,6 @@
>>       TP_ARGS(sk)
>> );
>>
>> -TRACE_EVENT(tcp_set_state,
>> -
>> -     TP_PROTO(const struct sock *sk, const int oldstate, const int newstate),
>> -
>> -     TP_ARGS(sk, oldstate, newstate),
>> -
>> -     TP_STRUCT__entry(
>> -             __field(const void *, skaddr)
>> -             __field(int, oldstate)
>> -             __field(int, newstate)
>> -             __field(__u16, sport)
>> -             __field(__u16, dport)
>> -             __array(__u8, saddr, 4)
>> -             __array(__u8, daddr, 4)
>> -             __array(__u8, saddr_v6, 16)
>> -             __array(__u8, daddr_v6, 16)
>> -     ),
>> -
>> -     TP_fast_assign(
>> -             struct inet_sock *inet = inet_sk(sk);
>> -             struct in6_addr *pin6;
>> -             __be32 *p32;
>> -
>> -             __entry->skaddr = sk;
>> -             __entry->oldstate = oldstate;
>> -             __entry->newstate = newstate;
>> -
>> -             __entry->sport = ntohs(inet->inet_sport);
>> -             __entry->dport = ntohs(inet->inet_dport);
>> -
>> -             p32 = (__be32 *) __entry->saddr;
>> -             *p32 = inet->inet_saddr;
>> -
>> -             p32 = (__be32 *) __entry->daddr;
>> -             *p32 =  inet->inet_daddr;
>> -
>> -#if IS_ENABLED(CONFIG_IPV6)
>> -             if (sk->sk_family == AF_INET6) {
>> -                     pin6 = (struct in6_addr *)__entry->saddr_v6;
>> -                     *pin6 = sk->sk_v6_rcv_saddr;
>> -                     pin6 = (struct in6_addr *)__entry->daddr_v6;
>> -                     *pin6 = sk->sk_v6_daddr;
>> -             } else
>> -#endif
>> -             {
>> -                     pin6 = (struct in6_addr *)__entry->saddr_v6;
>> -                     ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
>> -                     pin6 = (struct in6_addr *)__entry->daddr_v6;
>> -                     ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
>> -             }
>> -     ),
>> -
>> -     TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s",
>> -               __entry->sport, __entry->dport,
>> -               __entry->saddr, __entry->daddr,
>> -               __entry->saddr_v6, __entry->daddr_v6,
>> -               show_tcp_state_name(__entry->oldstate),
>> -               show_tcp_state_name(__entry->newstate))
>> -);
>> -
>> TRACE_EVENT(tcp_retransmit_synack,
>>
>>       TP_PROTO(const struct sock *sk, const struct request_sock *req),
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index c0b5b2f..ee0c1bc 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -2859,6 +2859,19 @@ int sock_get_timestampns(struct sock *sk, struct timespec __user *userstamp)
>> }
>> EXPORT_SYMBOL(sock_get_timestampns);
>>
>> +void sk_state_store(struct sock *sk, int state)
>> +{
>> +     trace_sock_set_state(sk, sk->sk_state, state);
>> +     smp_store_release(&sk->sk_state, state);
>> +}
>> +
>> +void sk_set_state(struct sock *sk, int state)
>> +{
>> +     trace_sock_set_state(sk, sk->sk_state, state);
>> +     sk->sk_state = state;
>> +}
>> +EXPORT_SYMBOL(sk_set_state);
>
> These two functions are short. Can we keep this in sock.h?

sock.h is included in trace/events/sock.h, so it will cause compile
error if hese two functions are defined in sock.h.

>
> Thanks,
> Song
>
>> +
>> void sock_enable_timestamp(struct sock *sk, int flag)
>> {
>>       if (!sock_flag(sk, flag)) {
>> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
>> index 4ca46dc..001f7b0 100644
>> --- a/net/ipv4/inet_connection_sock.c
>> +++ b/net/ipv4/inet_connection_sock.c
>> @@ -783,7 +783,7 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
>>       if (newsk) {
>>               struct inet_connection_sock *newicsk = inet_csk(newsk);
>>
>> -             newsk->sk_state = TCP_SYN_RECV;
>> +             sk_set_state(newsk, TCP_SYN_RECV);
>>               newicsk->icsk_bind_hash = NULL;
>>
>>               inet_sk(newsk)->inet_dport = inet_rsk(req)->ir_rmt_port;
>> @@ -888,7 +888,7 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
>>                       return 0;
>>       }
>>
>> -     sk->sk_state = TCP_CLOSE;
>> +     sk_set_state(sk, TCP_CLOSE);
>>       return err;
>> }
>> EXPORT_SYMBOL_GPL(inet_csk_listen_start);
>> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
>> index f6f5810..5973693 100644
>> --- a/net/ipv4/inet_hashtables.c
>> +++ b/net/ipv4/inet_hashtables.c
>> @@ -544,7 +544,7 @@ bool inet_ehash_nolisten(struct sock *sk, struct sock *osk)
>>               sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
>>       } else {
>>               percpu_counter_inc(sk->sk_prot->orphan_count);
>> -             sk->sk_state = TCP_CLOSE;
>> +             sk_set_state(sk, TCP_CLOSE);
>>               sock_set_flag(sk, SOCK_DEAD);
>>               inet_csk_destroy_sock(sk);
>>       }
>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>> index 1803116..000504f 100644
>> --- a/net/ipv4/tcp.c
>> +++ b/net/ipv4/tcp.c
>> @@ -283,8 +283,6 @@
>> #include <asm/ioctls.h>
>> #include <net/busy_poll.h>
>>
>> -#include <trace/events/tcp.h>
>> -
>> struct percpu_counter tcp_orphan_count;
>> EXPORT_SYMBOL_GPL(tcp_orphan_count);
>>
>> @@ -2040,8 +2038,6 @@ void tcp_set_state(struct sock *sk, int state)
>> {
>>       int oldstate = sk->sk_state;
>>
>> -     trace_tcp_set_state(sk, oldstate, state);
>> -
>>       switch (state) {
>>       case TCP_ESTABLISHED:
>>               if (oldstate != TCP_ESTABLISHED)
>> --
>> 1.8.3.1
>>
>

[PATCH net-next 2/3] net:tracepoint: using sock_set_state tracepoint to trace DCCP state transition

[Yafang Shao]

With changes in inet_ files, DCCP state transitions are traced with
sock_set_state tracepoint.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/dccp/proto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index b68168f..92b9613 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -110,7 +110,7 @@ void dccp_set_state(struct sock *sk, const int state)
 	/* Change state AFTER socket is unhashed to avoid closed
 	 * socket sitting in hash tables.
 	 */
-	sk->sk_state = state;
+	sk_set_state(sk, state);
 }

 EXPORT_SYMBOL_GPL(dccp_set_state);
--
1.8.3.1

[PATCH net-next 3/3] net:tracepoint: using sock_set_state tracepoint to trace SCTP state transition

[Yafang Shao]

With changes in inet_ files, SCTP state transitions are traced with
sockt_set_state tracepoint.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/sctp/endpointola.c   |  2 +-
 net/sctp/sm_sideeffect.c |  4 ++--
 net/sctp/socket.c        | 14 +++++++-------
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index ee1e601..52d2bb3 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -232,7 +232,7 @@ void sctp_endpoint_free(struct sctp_endpoint *ep)
 {
 	ep->base.dead = true;

-	ep->base.sk->sk_state = SCTP_SS_CLOSED;
+	sk_set_state(ep->base.sk, SCTP_SS_CLOSED);

 	/* Unlink this endpoint, so we can't find it again! */
 	sctp_unhash_endpoint(ep);
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index df94d77..dd2d7f8 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -878,12 +878,12 @@ static void sctp_cmd_new_state(struct sctp_cmd_seq *cmds,
 		 * successfully completed a connect() call.
 		 */
 		if (sctp_state(asoc, ESTABLISHED) && sctp_sstate(sk, CLOSED))
-			sk->sk_state = SCTP_SS_ESTABLISHED;
+			sk_set_state(sk, SCTP_SS_ESTABLISHED);

 		/* Set the RCV_SHUTDOWN flag when a SHUTDOWN is received. */
 		if (sctp_state(asoc, SHUTDOWN_RECEIVED) &&
 		    sctp_sstate(sk, ESTABLISHED)) {
-			sk->sk_state = SCTP_SS_CLOSING;
+			sk_set_state(sk, SCTP_SS_CLOSING);
 			sk->sk_shutdown |= RCV_SHUTDOWN;
 		}
 	}
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 014847e..51ebb38 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1528,7 +1528,7 @@ static void sctp_close(struct sock *sk, long timeout)

 	lock_sock_nested(sk, SINGLE_DEPTH_NESTING);
 	sk->sk_shutdown = SHUTDOWN_MASK;
-	sk->sk_state = SCTP_SS_CLOSING;
+	sk_set_state(sk, SCTP_SS_CLOSING);

 	ep = sctp_sk(sk)->ep;

@@ -4582,7 +4582,7 @@ static void sctp_shutdown(struct sock *sk, int how)
 	if (how & SEND_SHUTDOWN && !list_empty(&ep->asocs)) {
 		struct sctp_association *asoc;

-		sk->sk_state = SCTP_SS_CLOSING;
+		sk_set_state(sk, SCTP_SS_CLOSING);
 		asoc = list_entry(ep->asocs.next,
 				  struct sctp_association, asocs);
 		sctp_primitive_SHUTDOWN(net, asoc, NULL);
@@ -7405,13 +7405,13 @@ static int sctp_listen_start(struct sock *sk, int backlog)
 	 * sockets.
 	 *
 	 */
-	sk->sk_state = SCTP_SS_LISTENING;
+	sk_set_state(sk, SCTP_SS_LISTENING);
 	if (!ep->base.bind_addr.port) {
 		if (sctp_autobind(sk))
 			return -EAGAIN;
 	} else {
 		if (sctp_get_port(sk, inet_sk(sk)->inet_num)) {
-			sk->sk_state = SCTP_SS_CLOSED;
+			sk_set_state(sk, SCTP_SS_CLOSED);
 			return -EADDRINUSE;
 		}
 	}
@@ -7463,7 +7463,7 @@ int sctp_inet_listen(struct socket *sock, int backlog)

 		err = 0;
 		sctp_unhash_endpoint(ep);
-		sk->sk_state = SCTP_SS_CLOSED;
+		sk_set_state(sk, SCTP_SS_CLOSED);
 		if (sk->sk_reuse)
 			sctp_sk(sk)->bind_hash->fastreuse = 1;
 		goto out;
@@ -8438,10 +8438,10 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 	 * is called, set RCV_SHUTDOWN flag.
 	 */
 	if (sctp_state(assoc, CLOSED) && sctp_style(newsk, TCP)) {
-		newsk->sk_state = SCTP_SS_CLOSED;
+		sk_set_state(newsk, SCTP_SS_CLOSED);
 		newsk->sk_shutdown |= RCV_SHUTDOWN;
 	} else {
-		newsk->sk_state = SCTP_SS_ESTABLISHED;
+		sk_set_state(newsk, SCTP_SS_ESTABLISHED);
 	}

 	release_sock(newsk);
--
1.8.3.1