Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751444AbeAEIKb (ORCPT + 1 other); Fri, 5 Jan 2018 03:10:31 -0500 Received: from mail-io0-f194.google.com ([209.85.223.194]:33894 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751315AbeAEIKa (ORCPT ); Fri, 5 Jan 2018 03:10:30 -0500 X-Google-Smtp-Source: ACJfBouR64q9DX9Ez+f0ffbzkFBelJRge1N3uVWnx03zNAU5xnmMo9sT1CiMmDrtYqJYzHIU/4E1OkCw6j4cWKKGyPg= MIME-Version: 1.0 In-Reply-To: <9A0D3ED9-7FFD-4E96-A42C-90871C2CC0C6@fb.com> References: <1515134569-23812-1-git-send-email-laoar.shao@gmail.com> <9A0D3ED9-7FFD-4E96-A42C-90871C2CC0C6@fb.com> From: Yafang Shao Date: Fri, 5 Jan 2018 16:09:48 +0800 Message-ID: Subject: Re: [PATCH net-next] net: tracepoint: adding new tracepoint arguments in inet_sock_set_state To: Song Liu Cc: David Miller , "brendan.d.gregg@gmail.com" , "marcelo.leitner@gmail.com" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On Fri, Jan 5, 2018 at 3:21 PM, Song Liu wrote: > >> On Jan 4, 2018, at 10:42 PM, Yafang Shao wrote: >> >> sk->sk_protocol and sk->sk_family are exposed as tracepoint arguments. >> Then we can conveniently use these two arguments to do the filter. >> >> Suggested-by: Brendan Gregg >> Signed-off-by: Yafang Shao >> --- >> include/trace/events/sock.h | 24 ++++++++++++++++++------ >> net/ipv4/af_inet.c | 6 ++++-- >> 2 files changed, 22 insertions(+), 8 deletions(-) >> >> diff --git a/include/trace/events/sock.h b/include/trace/events/sock.h >> index 3537c5f..c7df70f 100644 >> --- a/include/trace/events/sock.h >> +++ b/include/trace/events/sock.h >> @@ -11,7 +11,11 @@ >> #include >> #include >> >> -/* The protocol traced by sock_set_state */ >> +#define family_names \ >> + EM(AF_INET) \ >> + EMe(AF_INET6) >> + >> +/* The protocol traced by inet_sock_set_state */ >> #define inet_protocol_names \ >> EM(IPPROTO_TCP) \ >> EM(IPPROTO_DCCP) \ >> @@ -37,6 +41,7 @@ >> #define EM(a) TRACE_DEFINE_ENUM(a); >> #define EMe(a) TRACE_DEFINE_ENUM(a); >> >> +family_names >> inet_protocol_names >> tcp_state_names >> >> @@ -45,6 +50,9 @@ >> #define EM(a) { a, #a }, >> #define EMe(a) { a, #a } >> >> +#define show_family_name(val) \ >> + __print_symbolic(val, family_names) >> + >> #define show_inet_protocol_name(val) \ >> __print_symbolic(val, inet_protocol_names) >> >> @@ -108,9 +116,10 @@ >> >> TRACE_EVENT(inet_sock_set_state, >> >> - TP_PROTO(const struct sock *sk, const int oldstate, const int newstate), >> + TP_PROTO(const struct sock *sk, const int family, const int protocol, >> + const int oldstate, const int newstate), > > Are there cases we need protocol and/or family that is different to > sk->sk_protocol/sk_family? If not, I think we don't need to change the > TP_PROTO. > > Thanks, > Song > As of now, there're two sk_family, which are AF_INET and AF_INET6, and three sk_protocol which are IPPROTO_TCP, IPPROTO_SCTP and IPPROTO_DCCP. Thanks Yafang >> >> - TP_ARGS(sk, oldstate, newstate), >> + TP_ARGS(sk, family, protocol, oldstate, newstate), >> >> TP_STRUCT__entry( >> __field(const void *, skaddr) >> @@ -118,6 +127,7 @@ >> __field(int, newstate) >> __field(__u16, sport) >> __field(__u16, dport) >> + __field(__u16, family) >> __field(__u8, protocol) >> __array(__u8, saddr, 4) >> __array(__u8, daddr, 4) >> @@ -133,8 +143,9 @@ >> __entry->skaddr = sk; >> __entry->oldstate = oldstate; >> __entry->newstate = newstate; >> + __entry->family = family; >> + __entry->protocol = protocol; >> >> - __entry->protocol = sk->sk_protocol; >> __entry->sport = ntohs(inet->inet_sport); >> __entry->dport = ntohs(inet->inet_dport); >> >> @@ -145,7 +156,7 @@ >> *p32 = inet->inet_daddr; >> >> #if IS_ENABLED(CONFIG_IPV6) >> - if (sk->sk_family == AF_INET6) { >> + if (family == AF_INET6) { >> pin6 = (struct in6_addr *)__entry->saddr_v6; >> *pin6 = sk->sk_v6_rcv_saddr; >> pin6 = (struct in6_addr *)__entry->daddr_v6; >> @@ -160,7 +171,8 @@ >> } >> ), >> >> - TP_printk("protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s", >> + TP_printk("family=%s protocol=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c oldstate=%s newstate=%s", >> + show_family_name(__entry->family), >> show_inet_protocol_name(__entry->protocol), >> __entry->sport, __entry->dport, >> __entry->saddr, __entry->daddr, >> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c >> index bab98a4..1d52796 100644 >> --- a/net/ipv4/af_inet.c >> +++ b/net/ipv4/af_inet.c >> @@ -1223,14 +1223,16 @@ int inet_sk_rebuild_header(struct sock *sk) >> >> void inet_sk_set_state(struct sock *sk, int state) >> { >> - trace_inet_sock_set_state(sk, sk->sk_state, state); >> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol, >> + sk->sk_state, state); >> sk->sk_state = state; >> } >> EXPORT_SYMBOL(inet_sk_set_state); >> >> void inet_sk_state_store(struct sock *sk, int newstate) >> { >> - trace_inet_sock_set_state(sk, sk->sk_state, newstate); >> + trace_inet_sock_set_state(sk, sk->sk_family, sk->sk_protocol, >> + sk->sk_state, newstate); >> smp_store_release(&sk->sk_state, newstate); >> } >> >> -- >> 1.8.3.1 >> >