| Message ID | 20230825133246.344364-1-me@manjusaka.me (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v4] tracepoint: add new `tcp:tcp_ca_event` trace event | expand |
On Fri, 25 Aug 2023 13:32:47 +0000 Zheao Li wrote: > This the 4th version of the patch, the previous discusstion is here > > https://lore.kernel.org/linux-trace-kernel/20230807183308.9015-1-me@manjusaka.me/ > > In this version of the code, here's some different: > > 1. The event name has been changed from `tcp_ca_event_set` to > `tcp_ca_event` > > 2. Output the current protocol family in TP_printk > > 3. Show the ca_event symbol instead of the original number > > But the `./scripts/checkpatch.pl` has been failed to check this patch, > because we sill have some code error in ./scripts/checkpatch.pl(in > another world, the test would be failed when we use the > scripts/checkpatch.pl to check the events/tcp.h > > Feel free to ask me if you have have any issues and ideas. ## Form letter - net-next-closed The merge window for v6.6 has begun and therefore net-next is closed for new drivers, features, code refactoring and optimizations. We are currently accepting bug fixes only. Please repost when net-next reopens after Sept 11th. RFC patches sent for review only are obviously welcome at any time. See: https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#development-cycle
On 2023/8/25 21:32, Zheao Li wrote: > Hello > > This the 4th version of the patch, the previous discusstion is here > > https://lore.kernel.org/linux-trace-kernel/20230807183308.9015-1-me@manjusaka.me/ > > In this version of the code, here's some different: > > 1. The event name has been changed from `tcp_ca_event_set` to > `tcp_ca_event` > > 2. Output the current protocol family in TP_printk > > 3. Show the ca_event symbol instead of the original number > > But the `./scripts/checkpatch.pl` has been failed to check this patch, > because we sill have some code error in ./scripts/checkpatch.pl(in > another world, the test would be failed when we use the > scripts/checkpatch.pl to check the events/tcp.h > > Feel free to ask me if you have have any issues and ideas. > > Thanks > > --- > > 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@manjusaka.me> > --- > 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); Ping to review(
On Sun, 17 Sep 2023 22:06:45 +0800
Manjusaka <me@manjusaka.me> wrote:
> Ping to review(
The ask was to resend the patches after the merge window.
-- Steve
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);
Hello This the 4th version of the patch, the previous discusstion is here https://lore.kernel.org/linux-trace-kernel/20230807183308.9015-1-me@manjusaka.me/ In this version of the code, here's some different: 1. The event name has been changed from `tcp_ca_event_set` to `tcp_ca_event` 2. Output the current protocol family in TP_printk 3. Show the ca_event symbol instead of the original number But the `./scripts/checkpatch.pl` has been failed to check this patch, because we sill have some code error in ./scripts/checkpatch.pl(in another world, the test would be failed when we use the scripts/checkpatch.pl to check the events/tcp.h Feel free to ask me if you have have any issues and ideas. Thanks --- 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@manjusaka.me> --- include/net/tcp.h | 9 ++---- include/trace/events/tcp.h | 60 ++++++++++++++++++++++++++++++++++++++ net/ipv4/tcp_cong.c | 10 +++++++ 3 files changed, 72 insertions(+), 7 deletions(-)