* [PATCH] net/tcp: track all ipv4/tcp state transition in tcp_set_state
@ 2017-11-07 10:36 Yafang Shao
2017-11-08 6:51 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Yafang Shao @ 2017-11-07 10:36 UTC (permalink / raw)
To: davem, kuznet, yoshfuji; +Cc: netdev, linux-kernel, Yafang Shao
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>
---
net/ipv4/inet_connection_sock.c | 6 +++---
net/ipv4/inet_hashtables.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index c039c93..7c987c5 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -786,7 +786,7 @@ struct sock *inet_csk_clone_lock(const struct sock *sk,
if (newsk) {
struct inet_connection_sock *newicsk = inet_csk(newsk);
- newsk->sk_state = TCP_SYN_RECV;
+ tcp_set_state(newsk, TCP_SYN_RECV);
newicsk->icsk_bind_hash = NULL;
inet_sk(newsk)->inet_dport = inet_rsk(req)->ir_rmt_port;
@@ -880,7 +880,7 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
* It is OK, because this socket enters to hash table only
* after validation is complete.
*/
- sk_state_store(sk, TCP_LISTEN);
+ tcp_set_state(sk, TCP_LISTEN);
if (!sk->sk_prot->get_port(sk, inet->inet_num)) {
inet->inet_sport = htons(inet->inet_num);
@@ -891,7 +891,7 @@ int inet_csk_listen_start(struct sock *sk, int backlog)
return 0;
}
- sk->sk_state = TCP_CLOSE;
+ tcp_set_state(sk, TCP_CLOSE);
return err;
}
EXPORT_SYMBOL_GPL(inet_csk_listen_start);
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 597bb4c..bea8318 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -430,7 +430,7 @@ bool inet_ehash_nolisten(struct sock *sk, struct sock *osk)
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
} else {
percpu_counter_inc(sk->sk_prot->orphan_count);
- sk->sk_state = TCP_CLOSE;
+ tcp_set_state(sk, TCP_CLOSE);
sock_set_flag(sk, SOCK_DEAD);
inet_csk_destroy_sock(sk);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net/tcp: track all ipv4/tcp state transition in tcp_set_state
2017-11-07 10:36 [PATCH] net/tcp: track all ipv4/tcp state transition in tcp_set_state Yafang Shao
@ 2017-11-08 6:51 ` David Miller
2017-11-08 10:48 ` Yafang Shao
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2017-11-08 6:51 UTC (permalink / raw)
To: laoar.shao; +Cc: kuznet, yoshfuji, netdev, linux-kernel
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;
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net/tcp: track all ipv4/tcp state transition in tcp_set_state
2017-11-08 6:51 ` David Miller
@ 2017-11-08 10:48 ` Yafang Shao
0 siblings, 0 replies; 3+ messages in thread
From: Yafang Shao @ 2017-11-08 10:48 UTC (permalink / raw)
To: David Miller; +Cc: Alexey Kuznetsov, yoshfuji, netdev, linux-kernel
2017-11-08 14:51 GMT+08:00 David Miller <davem@davemloft.net>:
> 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;
> }
Good idea!
I will try to implement it.
Thanks
Yafang
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-11-08 10:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-07 10:36 [PATCH] net/tcp: track all ipv4/tcp state transition in tcp_set_state Yafang Shao
2017-11-08 6:51 ` David Miller
2017-11-08 10:48 ` Yafang Shao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).