| Message ID | 1512919904-14166-2-git-send-email-laoar.shao@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | replace tcp_set_state tracepoint with sock_set_state tracepoint | expand |
> 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 >
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 >> >
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)
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(-) -- 1.8.3.1