From mboxrd@z Thu Jan 1 00:00:00 1970 From: Song Liu Subject: Re: [PATCH] net/tcp: introduce TRACE_EVENT for TCP/IPv4 state transition Date: Thu, 9 Nov 2017 18:34:08 +0000 Message-ID: <30A95900-7378-439F-954E-BD5487012435@fb.com> References: <1510207298-14828-1-git-send-email-laoar.shao@gmail.com> <20171109064326.skfvvohg7qhgl3lp@ast-mbp> <20171109131808.373c296f@gandalf.local.home> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: Alexei Starovoitov , Yafang Shao , "davem@davemloft.net" , "mingo@redhat.com" , "netdev@vger.kernel.org" , "linux-kernel@vger.kernel.org" To: Steven Rostedt Return-path: In-Reply-To: <20171109131808.373c296f@gandalf.local.home> Content-Language: en-US Content-ID: <152A57E08FB12E4B986365A21168DD3A@namprd15.prod.outlook.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org > On Nov 9, 2017, at 10:18 AM, Steven Rostedt wrote: >=20 > On Thu, 9 Nov 2017 15:43:29 +0900 > Alexei Starovoitov wrote: >=20 >>> +TRACE_EVENT(tcp_set_state, >>> + TP_PROTO(struct sock *sk, int oldstate, int newstate), >>> + TP_ARGS(sk, oldstate, newstate), >>> + >>> + TP_STRUCT__entry( >>> + __field(__be32, dst) >>> + __field(__be32, src) >>> + __field(__u16, dport) >>> + __field(__u16, sport) >>> + __field(int, oldstate) >>> + __field(int, newstate) >>> + ), >>> + >>> + TP_fast_assign( >>> + if (oldstate =3D=3D TCP_TIME_WAIT) { >>> + __entry->dst =3D inet_twsk(sk)->tw_daddr; >>> + __entry->src =3D inet_twsk(sk)->tw_rcv_saddr; >>> + __entry->dport =3D ntohs(inet_twsk(sk)->tw_dport); >>> + __entry->sport =3D ntohs(inet_twsk(sk)->tw_sport); >>> + } else if (oldstate =3D=3D TCP_NEW_SYN_RECV) { >>> + __entry->dst =3D inet_rsk(inet_reqsk(sk))->ir_rmt_addr; >>> + __entry->src =3D inet_rsk(inet_reqsk(sk))->ir_loc_addr; >>> + __entry->dport =3D >>> + ntohs(inet_rsk(inet_reqsk(sk))->ir_rmt_port); >>> + __entry->sport =3D inet_rsk(inet_reqsk(sk))->ir_num; >>> + } else { >>> + __entry->dst =3D inet_sk(sk)->inet_daddr; >>> + __entry->src =3D inet_sk(sk)->inet_rcv_saddr; >>> + __entry->dport =3D ntohs(inet_sk(sk)->inet_dport); >>> + __entry->sport =3D ntohs(inet_sk(sk)->inet_sport); >>> + } >>> + >>> + __entry->oldstate =3D oldstate; >>> + __entry->newstate =3D newstate; >>> + ), >>> + >>> + TP_printk("%08X:%04X %08X:%04X, %02x %02x", >>> + __entry->src, __entry->sport, __entry->dst, __entry->dport, >>> + __entry->oldstate, __entry->newstate) =20 >>=20 >> direct %x of state is not allowed. >> This has to use show_tcp_state_name() like it's done in trace_tcp_set_st= ate >=20 > Hmm, I need to look at trace_tcp_set_state. I'm guessing it is in > net-next too? Yes, in net-next. There are 6 tracepoints under tcp group: tcp_destroy_sock =20 tcp_receive_reset =20 tcp_retransmit_skb =20 tcp_retransmit_synack =20 tcp_send_reset =20 tcp_set_state They are all added recently. =20 >=20 >>=20 >> Also I'm missing the reason to introduce another tracepoint >> that looks just like trace_tcp_set_state. >=20 > I want to make sure that perf and trace-cmd can parse the TP_printk(), > if it is having helper functions like that in the TP_printk() part, > then the libtraceevent needs to be aware of that. >=20 tcp_set_state uses __print_symbolic to show state in text format. I found trace-cmd cannot parse that part: [011] 147338.660560: tcp_set_state: sport=3D16262 dport=3D48346 \ saddr=3D127.0.0.6 daddr=3D127.0.0.6 saddrv6=3D2401:db00:30:317e:face:0:= 1f:0 \ daddrv6=3D2401:db00:30:31e5:face:0:7f:0 oldstate=3D newstate=3D Other parts of the output looks good to me. Thanks, Song=