From mboxrd@z Thu Jan 1 00:00:00 1970 From: Song Liu Subject: Re: [PATCH net-next] net: tracepoint: adding new tracepoint arguments in inet_sock_set_state Date: Sat, 6 Jan 2018 17:46:33 +0000 Message-ID: <544ED2FF-A44E-493D-B84E-46A58EEAB201@fb.com> References: <1515134569-23812-1-git-send-email-laoar.shao@gmail.com> <9A0D3ED9-7FFD-4E96-A42C-90871C2CC0C6@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: David Miller , "brendan.d.gregg@gmail.com" , "marcelo.leitner@gmail.com" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" To: Yafang Shao Return-path: In-Reply-To: Content-Language: en-US Content-ID: Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org > On Jan 5, 2018, at 12:09 AM, Yafang Shao wrote: >=20 > On Fri, Jan 5, 2018 at 3:21 PM, Song Liu wrote: >>=20 >>> On Jan 4, 2018, at 10:42 PM, Yafang Shao wrote: >>>=20 >>> sk->sk_protocol and sk->sk_family are exposed as tracepoint arguments. >>> Then we can conveniently use these two arguments to do the filter. >>>=20 >>> 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(-) >>>=20 >>> 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 >>>=20 >>> -/* 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); >>>=20 >>> +family_names >>> inet_protocol_names >>> tcp_state_names >>>=20 >>> @@ -45,6 +50,9 @@ >>> #define EM(a) { a, #a }, >>> #define EMe(a) { a, #a } >>>=20 >>> +#define show_family_name(val) \ >>> + __print_symbolic(val, family_names) >>> + >>> #define show_inet_protocol_name(val) \ >>> __print_symbolic(val, inet_protocol_names) >>>=20 >>> @@ -108,9 +116,10 @@ >>>=20 >>> TRACE_EVENT(inet_sock_set_state, >>>=20 >>> - TP_PROTO(const struct sock *sk, const int oldstate, const int new= state), >>> + TP_PROTO(const struct sock *sk, const int family, const int proto= col, >>> + const int oldstate, const int newstate), >>=20 >> 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. >>=20 >> Thanks, >> Song >>=20 >=20 > 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. >=20 > Thanks > Yafang How about we do not change the TP_PROTO line? The patch will be like: @@ -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 =3D sk; __entry->oldstate =3D oldstate; __entry->newstate =3D newstate; + __entry->family =3D sk->sk_family; __entry->protocol =3D sk->sk_protocol; __entry->sport =3D ntohs(inet->inet_sport); __entry->dport =3D ntohs(inet->inet_dport); @@ xxxxx - TP_printk("protocol=3D%s sport=3D%hu dport=3D%hu saddr=3D%pI4 daddr= =3D%pI4 saddrv6=3D%pI6c daddrv6=3D%pI6c oldstate=3D%s newstate=3D%s", + TP_printk("family=3D%s protocol=3D%s sport=3D%hu dport=3D%hu saddr=3D= %pI4 daddr=3D%pI4 saddrv6=3D%pI6c daddrv6=3D%pI6c oldstate=3D%s newstate=3D= %s", + show_family_name(__entry->family), Thanks, Song=20 >>>=20 >>> - TP_ARGS(sk, oldstate, newstate), >>> + TP_ARGS(sk, family, protocol, oldstate, newstate), >>>=20 >>> 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 =3D sk; >>> __entry->oldstate =3D oldstate; >>> __entry->newstate =3D newstate; >>> + __entry->family =3D family; >>> + __entry->protocol =3D protocol; >>>=20 >>> - __entry->protocol =3D sk->sk_protocol; >>> __entry->sport =3D ntohs(inet->inet_sport); >>> __entry->dport =3D ntohs(inet->inet_dport); >>>=20 >>> @@ -145,7 +156,7 @@ >>> *p32 =3D inet->inet_daddr; >>>=20 >>> #if IS_ENABLED(CONFIG_IPV6) >>> - if (sk->sk_family =3D=3D AF_INET6) { >>> + if (family =3D=3D AF_INET6) { >>> pin6 =3D (struct in6_addr *)__entry->saddr_v6; >>> *pin6 =3D sk->sk_v6_rcv_saddr; >>> pin6 =3D (struct in6_addr *)__entry->daddr_v6; >>> @@ -160,7 +171,8 @@ >>> } >>> ), >>>=20 >>> - TP_printk("protocol=3D%s sport=3D%hu dport=3D%hu saddr=3D%pI4 dad= dr=3D%pI4 saddrv6=3D%pI6c daddrv6=3D%pI6c oldstate=3D%s newstate=3D%s", >>> + TP_printk("family=3D%s protocol=3D%s sport=3D%hu dport=3D%hu sadd= r=3D%pI4 daddr=3D%pI4 saddrv6=3D%pI6c daddrv6=3D%pI6c oldstate=3D%s newstat= e=3D%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) >>>=20 >>> 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, sta= te); >>> sk->sk_state =3D state; >>> } >>> EXPORT_SYMBOL(inet_sk_set_state); >>>=20 >>> 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, new= state); >>> smp_store_release(&sk->sk_state, newstate); >>> } >>>=20 >>> -- >>> 1.8.3.1