Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754574AbdLHPve (ORCPT ); Fri, 8 Dec 2017 10:51:34 -0500 Received: from mail-it0-f43.google.com ([209.85.214.43]:35442 "EHLO mail-it0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754451AbdLHPv0 (ORCPT ); Fri, 8 Dec 2017 10:51:26 -0500 X-Google-Smtp-Source: AGs4zMYBHMHzGHdDWOuOpZUXtmY2Sm6WItN4gAgUjzDNtRoZ6tX1x5kVLbHXccJsB01Vy00thMeysecUfoj6dqEG9OY= MIME-Version: 1.0 In-Reply-To: <20171208.104256.1026704308758536035.davem@davemloft.net> References: <20171207200245.GQ13341@localhost.localdomain> <20171208.104256.1026704308758536035.davem@davemloft.net> From: Yafang Shao Date: Fri, 8 Dec 2017 23:50:44 +0800 Message-ID: Subject: Re: [PATCH v5 net-next] net/tcp: trace all TCP/IP state transition with tcp_set_state tracepoint To: David Miller Cc: Marcelo Ricardo Leitner , Song Liu , Alexey Kuznetsov , yoshfuji@linux-ipv6.org, Steven Rostedt , Brendan Gregg , netdev@vger.kernel.org, LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1021 Lines: 37 2017-12-08 23:42 GMT+08:00 David Miller : > From: Yafang Shao > Date: Fri, 8 Dec 2017 11:40:23 +0800 > >> It will looks like these, >> >> if (sk->sk_protocol == IPPROTO_TCP) >> __tcp_set_state(newsk, TCP_SYN_RECV); >> else >> newsk->sk_state = TCP_SYN_RECV; >> >> >> if (sk->sk_protocol == IPPROTO_TCP) >> __tcp_set_state(sk, TCP_CLOSE); >> else >> sk->sk_state = TCP_CLOSE; >> >> if (sk->sk_protocol == IPPROTO_TCP) >> tcp_state_store(sk, state); >> else >> sk_state_store(sk, state); >> >> >> Some redundant code. >> >> IMO, put these similar code into a wrapper is more nice. > > I think this discussion and how ugly this is getting shows that > tracing the state transitions of a socket is perhaps not best as a TCP > specific feature. Do you mean that tcp_set_state tracepoint should be replaced with sk_set_state tracepoint and move that tracepoint to trace/events/sock.h ? Thanks Yafang