From mboxrd@z Thu Jan  1 00:00:00 1970
From: David Miller <davem@davemloft.net>
Subject: Re: [PATCH] net/tcp: track all ipv4/tcp state transition in
 tcp_set_state
Date: Wed, 08 Nov 2017 15:51:12 +0900 (KST)
Message-ID: <20171108.155112.1483404763325763621.davem@davemloft.net>
References: <1510050988-17688-1-git-send-email-laoar.shao@gmail.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Cc: kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
To: laoar.shao@gmail.com
Return-path: <linux-kernel-owner@vger.kernel.org>
In-Reply-To: <1510050988-17688-1-git-send-email-laoar.shao@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: netdev.vger.kernel.org

From: Yafang Shao <laoar.shao@gmail.com>
Date: Tue,  7 Nov 2017 18:36:28 +0800

> When I hooked the function tcp_set_state with kprobe to track the ipv4/tcp
> state transistion, I found state transition from TCP_LISTEN to TCP_SYN_RECV
> is missed.
> 
> I think it is better to use the helper to do state transition instead of
> assigning the state to sk_state directly.
> Then we can monitor the whole tcp lifespans with kprobe or ftrace easily.
> 
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

This is really heavy handed and excessive for these cases.

They don't have to handle any of the issues dealt with in
tcp_set_state().

I would prefer if you made a special helper to net/tcp.h which did:

static inline void __tcp_set_state(struct sock *sk, int state)
{
	trace_tcp_set_state(sk, sk->sk_state, state);
	sk->sk_state = state;
}