On 2023/8/13 04:12, Zheao Li wrote:
> In normal use case, the tcp_ca_event would be changed in high frequency.
>
> The developer can monitor the network quality more easier by tracing
> TCP stack with this TP event.
>
> 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: Zheao Li <me@xxxxxxxxxxxx>
> ---
> include/net/tcp.h | 9 ++----
> include/trace/events/tcp.h | 60 ++++++++++++++++++++++++++++++++++++++
> net/ipv4/tcp_cong.c | 10 +++++++
> 3 files changed, 72 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 7b1ddffa3dfc..993eb00403ea 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -41,6 +41,18 @@
> TP_STORE_V4MAPPED(__entry, saddr, daddr)
> #endif
>
> +/* The TCP CA event traced by tcp_ca_event*/
> +#define tcp_ca_event_names \
> + EM(CA_EVENT_TX_START) \
> + EM(CA_EVENT_CWND_RESTART) \
> + EM(CA_EVENT_COMPLETE_CWR) \
> + EM(CA_EVENT_LOSS) \
> + EM(CA_EVENT_ECN_NO_CE) \
> + EMe(CA_EVENT_ECN_IS_CE)
> +
> +#define show_tcp_ca_event_names(val) \
> + __print_symbolic(val, tcp_ca_event_names)
> +
> /*
> * tcp event with arguments sk and skb
> *
> @@ -419,6 +431,54 @@ 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)
> + __field(__u16, family)
> + __array(__u8, saddr, 4)
> + __array(__u8, daddr, 4)
> + __array(__u8, saddr_v6, 16)
> + __array(__u8, daddr_v6, 16)
> + __field(__u8, ca_event)
> + ),
> +
> + 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);
> + __entry->family = sk->sk_family;
> +
> + p32 = (__be32 *) __entry->saddr;
> + *p32 = inet->inet_saddr;
> +
> + p32 = (__be32 *) __entry->daddr;
> + *p32 = inet->inet_daddr;
> +
> + TP_STORE_ADDRS(__entry, inet->inet_saddr, inet->inet_daddr,
> + sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
> +
> + __entry->ca_event = ca_event;
> + ),
> +
> + TP_printk("family=%s sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c ca_event=%s",
> + show_family_name(__entry->family),
> + __entry->sport, __entry->dport,
> + __entry->saddr, __entry->daddr,
> + __entry->saddr_v6, __entry->daddr_v6,
> + show_tcp_ca_event_names(__entry->ca_event))
> +);
> +
> #endif /* _TRACE_TCP_H */
>
> /* This part must be outside protection */
> diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
> index 1b34050a7538..fb7ec6ebbbd0 100644
> --- a/net/ipv4/tcp_cong.c
> +++ b/net/ipv4/tcp_cong.c
> @@ -34,6 +34,16 @@ struct tcp_congestion_ops *tcp_ca_find(const char *name)
> return NULL;
> }
>
> +void tcp_ca_event(struct sock *sk, const enum tcp_ca_event event)
> +{
> + const struct inet_connection_sock *icsk = inet_csk(sk);
> +
> + trace_tcp_ca_event(sk, (u8)event);
> +
> + if (icsk->icsk_ca_ops->cwnd_event)
> + icsk->icsk_ca_ops->cwnd_event(sk, event);
> +}
> +
> void tcp_set_ca_state(struct sock *sk, const u8 ca_state)
> {
> struct inet_connection_sock *icsk = inet_csk(sk);
For more information, this patch is not passthrough the `./scripts/checkpatch.pl` check
with the following error message `Macros with complex values should be enclosed in parentheses`.
I have no idea because there is no complex expression and the `include/trace/events/sock.h` files
also failed in the style check.
