From mboxrd@z Thu Jan  1 00:00:00 1970
From: Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH] net/tcp: introduce TRACE_EVENT for TCP/IPv4 state
 transition
Date: Thu, 9 Nov 2017 13:18:08 -0500
Message-ID: <20171109131808.373c296f@gandalf.local.home>
References: <1510207298-14828-1-git-send-email-laoar.shao@gmail.com>
        <20171109064326.skfvvohg7qhgl3lp@ast-mbp>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: Yafang Shao <laoar.shao@gmail.com>, davem@davemloft.net,
        mingo@redhat.com, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org, songliubraving@fb.com
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <20171109064326.skfvvohg7qhgl3lp@ast-mbp>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: netdev.vger.kernel.org

On Thu, 9 Nov 2017 15:43:29 +0900
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> > +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 == TCP_TIME_WAIT) {
> > +			__entry->dst = inet_twsk(sk)->tw_daddr;
> > +			__entry->src = inet_twsk(sk)->tw_rcv_saddr;
> > +			__entry->dport = ntohs(inet_twsk(sk)->tw_dport);
> > +			__entry->sport = ntohs(inet_twsk(sk)->tw_sport);
> > +		} else if (oldstate == TCP_NEW_SYN_RECV) {
> > +			__entry->dst = inet_rsk(inet_reqsk(sk))->ir_rmt_addr;
> > +			__entry->src = inet_rsk(inet_reqsk(sk))->ir_loc_addr;
> > +			__entry->dport =
> > +				ntohs(inet_rsk(inet_reqsk(sk))->ir_rmt_port);
> > +			__entry->sport = inet_rsk(inet_reqsk(sk))->ir_num;
> > +		} else {
> > +			__entry->dst = inet_sk(sk)->inet_daddr;
> > +			__entry->src = inet_sk(sk)->inet_rcv_saddr;
> > +			__entry->dport = ntohs(inet_sk(sk)->inet_dport);
> > +			__entry->sport = ntohs(inet_sk(sk)->inet_sport);
> > +		}
> > +
> > +		__entry->oldstate = oldstate;
> > +		__entry->newstate = newstate;
> > +	),
> > +
> > +	TP_printk("%08X:%04X %08X:%04X, %02x %02x",
> > +		__entry->src, __entry->sport, __entry->dst, __entry->dport,
> > +		__entry->oldstate, __entry->newstate)  
> 
> direct %x of state is not allowed.
> This has to use show_tcp_state_name() like it's done in trace_tcp_set_state

Hmm, I need to look at trace_tcp_set_state. I'm guessing it is in
net-next too?

> 
> Also I'm missing the reason to introduce another tracepoint
> that looks just like trace_tcp_set_state.

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.

-- Steve