Re: [PATCH v2] tracepoint: add new `tcp:tcp_ca_event` trace event
From: Eric Dumazet <edumazet@google.com>
Date: 2023-08-08 17:32:43
Also in:
bpf, linux-trace-kernel, lkml
On Tue, Aug 8, 2023 at 7:59 AM Manjusaka [off-list ref] wrote:
In normal use case, the tcp_ca_event would be changed in high frequency. It's a good indicator to represent the network quanlity.
quality ? Honestly, it is more about TCP stack tracing than 'network quality'
quoted hunk ↗ jump to hunk
So I propose to add a `tcp:tcp_ca_event` trace event like `tcp:tcp_cong_state_set` to help the people to trace the TCP connection status Signed-off-by: Manjusaka <redacted> --- include/net/tcp.h | 9 ++------ include/trace/events/tcp.h | 45 ++++++++++++++++++++++++++++++++++++++ net/ipv4/tcp_cong.c | 10 +++++++++ 3 files changed, 57 insertions(+), 7 deletions(-)diff --git a/include/net/tcp.h b/include/net/tcp.h index 0ca972ebd3dd..a68c5b61889c 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h@@ -1154,13 +1154,8 @@ static inline bool tcp_ca_needs_ecn(const struct sock *sk) return icsk->icsk_ca_ops->flags & TCP_CONG_NEEDS_ECN; } -static inline void tcp_ca_event(struct sock *sk, const enum tcp_ca_event event) -{ - const struct inet_connection_sock *icsk = inet_csk(sk); - - if (icsk->icsk_ca_ops->cwnd_event) - icsk->icsk_ca_ops->cwnd_event(sk, event); -} +/* from tcp_cong.c */ +void tcp_ca_event(struct sock *sk, const enum tcp_ca_event event); /* From tcp_cong.c */ void tcp_set_ca_state(struct sock *sk, const u8 ca_state);diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h index bf06db8d2046..b374eb636af9 100644 --- a/include/trace/events/tcp.h +++ b/include/trace/events/tcp.h@@ -416,6 +416,51 @@ TRACE_EVENT(tcp_cong_state_set, __entry->cong_state) ); +TRACE_EVENT(tcp_ca_event, + + TP_PROTO(struct sock *sk, const u8 ca_event), + + TP_ARGS(sk, ca_event), + + TP_STRUCT__entry( + __field(const void *, skaddr) + __field(__u16, sport) + __field(__u16, dport) + __array(__u8, saddr, 4) + __array(__u8, daddr, 4) + __array(__u8, saddr_v6, 16) + __array(__u8, daddr_v6, 16) + __field(__u8, ca_event) + ), +
Please add the family (look at commit 3dd344ea84e1 ("net: tracepoint:
exposing sk_family in all tcp:tracepoints"))
+ TP_fast_assign( + struct inet_sock *inet = inet_sk(sk); + __be32 *p32; + + __entry->skaddr = sk; + + __entry->sport = ntohs(inet->inet_sport); + __entry->dport = ntohs(inet->inet_dport); + + p32 = (__be32 *) __entry->saddr; + *p32 = inet->inet_saddr; + + p32 = (__be32 *) __entry->daddr; + *p32 = inet->inet_daddr;
We keep copying IPv4 addresses that might contain garbage for IPv6 sockets :/
+ + TP_STORE_ADDRS(__entry, inet->inet_saddr, inet->inet_daddr, + sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
I will send a cleanup, because IP_STORE_ADDRS() should really take care of all details.
+
+ __entry->ca_event = ca_event;
+ ),
+
+ TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c ca_event=%u",
+ __entry->sport, __entry->dport,
+ __entry->saddr, __entry->daddr,
+ __entry->saddr_v6, __entry->daddr_v6,
+ __entry->ca_event)Please print the symbol instead of numeric ca_event. Look at show_tcp_state_name() for instance.