Message ID | 20240409100934.37725-7-kerneljasonxing@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | Implement reset reason mechanism to detect | expand |
On Tue, 9 Apr 2024 18:09:34 +0800 Jason Xing <kerneljasonxing@gmail.com> wrote: > /* > * tcp event with arguments sk and skb > @@ -74,20 +75,38 @@ DEFINE_EVENT(tcp_event_sk_skb, tcp_retransmit_skb, > TP_ARGS(sk, skb) > ); > > +#undef FN1 > +#define FN1(reason) TRACE_DEFINE_ENUM(SK_RST_REASON_##reason); > +#undef FN2 > +#define FN2(reason) TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason); > +DEFINE_RST_REASON(FN1, FN1) Interesting. I've never seen the passing of the internal macros to the main macro before. I see that you are using it for handling both the SK_RST_REASON and the SK_DROP_REASON. > + > +#undef FN1 > +#undef FNe1 > +#define FN1(reason) { SK_RST_REASON_##reason, #reason }, > +#define FNe1(reason) { SK_RST_REASON_##reason, #reason } > + > +#undef FN2 > +#undef FNe2 > +#define FN2(reason) { SKB_DROP_REASON_##reason, #reason }, > +#define FNe2(reason) { SKB_DROP_REASON_##reason, #reason } Anyway, from a tracing point of view, as it looks like it would work (I haven't tested it). Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org> -- Steve
Hi Steven, On Tue, Apr 9, 2024 at 11:36 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Tue, 9 Apr 2024 18:09:34 +0800 > Jason Xing <kerneljasonxing@gmail.com> wrote: > > > /* > > * tcp event with arguments sk and skb > > @@ -74,20 +75,38 @@ DEFINE_EVENT(tcp_event_sk_skb, tcp_retransmit_skb, > > TP_ARGS(sk, skb) > > ); > > > > +#undef FN1 > > +#define FN1(reason) TRACE_DEFINE_ENUM(SK_RST_REASON_##reason); > > +#undef FN2 > > +#define FN2(reason) TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason); > > +DEFINE_RST_REASON(FN1, FN1) > > Interesting. I've never seen the passing of the internal macros to the main > macro before. I see that you are using it for handling both the > SK_RST_REASON and the SK_DROP_REASON. Yes, I want to cover two kinds of reasons and then strip them of prefixes which can be reported to userspace. > > > + > > +#undef FN1 > > +#undef FNe1 > > +#define FN1(reason) { SK_RST_REASON_##reason, #reason }, > > +#define FNe1(reason) { SK_RST_REASON_##reason, #reason } > > + > > +#undef FN2 > > +#undef FNe2 > > +#define FN2(reason) { SKB_DROP_REASON_##reason, #reason }, > > +#define FNe2(reason) { SKB_DROP_REASON_##reason, #reason } > > Anyway, from a tracing point of view, as it looks like it would work > (I haven't tested it). Sure, it works. One simple test if you're interested: 1) Apply this patchset locally 2) add 'trace_tcp_send_reset(sk, skb, [one reason])' in the receive path, say, somewhere in the tcp_v4_rcv() The possible result can be seen in the cover letter. I list here: <idle>-0 [002] ..s1. 1830.262425: tcp_send_reset: skbaddr=x skaddr=x src=x dest=x state=x reason=NOT_SPECIFIED <idle>-0 [002] ..s1. 1830.262425: tcp_send_reset: skbaddr=x skaddr=x src=x dest=x state=x reason=NO_SOCKET > > Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org> Thanks! > > -- Steve
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h index 5c04a61a11c2..9bed9e63c9c5 100644 --- a/include/trace/events/tcp.h +++ b/include/trace/events/tcp.h @@ -11,6 +11,7 @@ #include <net/ipv6.h> #include <net/tcp.h> #include <linux/sock_diag.h> +#include <net/rstreason.h> /* * tcp event with arguments sk and skb @@ -74,20 +75,38 @@ DEFINE_EVENT(tcp_event_sk_skb, tcp_retransmit_skb, TP_ARGS(sk, skb) ); +#undef FN1 +#define FN1(reason) TRACE_DEFINE_ENUM(SK_RST_REASON_##reason); +#undef FN2 +#define FN2(reason) TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason); +DEFINE_RST_REASON(FN1, FN1) + +#undef FN1 +#undef FNe1 +#define FN1(reason) { SK_RST_REASON_##reason, #reason }, +#define FNe1(reason) { SK_RST_REASON_##reason, #reason } + +#undef FN2 +#undef FNe2 +#define FN2(reason) { SKB_DROP_REASON_##reason, #reason }, +#define FNe2(reason) { SKB_DROP_REASON_##reason, #reason } /* * skb of trace_tcp_send_reset is the skb that caused RST. In case of * active reset, skb should be NULL */ TRACE_EVENT(tcp_send_reset, - TP_PROTO(const struct sock *sk, const struct sk_buff *skb), + TP_PROTO(const struct sock *sk, + const struct sk_buff *skb, + const int reason), - TP_ARGS(sk, skb), + TP_ARGS(sk, skb, reason), TP_STRUCT__entry( __field(const void *, skbaddr) __field(const void *, skaddr) __field(int, state) + __field(int, reason) __array(__u8, saddr, sizeof(struct sockaddr_in6)) __array(__u8, daddr, sizeof(struct sockaddr_in6)) ), @@ -113,14 +132,24 @@ TRACE_EVENT(tcp_send_reset, */ TP_STORE_ADDR_PORTS_SKB(skb, th, entry->daddr, entry->saddr); } + __entry->reason = reason; ), - TP_printk("skbaddr=%p skaddr=%p src=%pISpc dest=%pISpc state=%s", + TP_printk("skbaddr=%p skaddr=%p src=%pISpc dest=%pISpc state=%s reason=%s", __entry->skbaddr, __entry->skaddr, __entry->saddr, __entry->daddr, - __entry->state ? show_tcp_state_name(__entry->state) : "UNKNOWN") + __entry->state ? show_tcp_state_name(__entry->state) : "UNKNOWN", + __entry->reason < RST_REASON_START ? + __print_symbolic(__entry->reason, DEFINE_DROP_REASON(FN2, FNe2)) : + __print_symbolic(__entry->reason, DEFINE_RST_REASON(FN1, FNe1))) ); +#undef FN1 +#undef FNe1 + +#undef FN2 +#undef FNe2 + /* * tcp event with arguments sk * diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 03c5af9decbf..4889fccbf754 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -871,7 +871,7 @@ static void tcp_v4_send_reset(const struct sock *sk, struct sk_buff *skb, if (sk) arg.bound_dev_if = sk->sk_bound_dev_if; - trace_tcp_send_reset(sk, skb); + trace_tcp_send_reset(sk, skb, reason); BUILD_BUG_ON(offsetof(struct sock, sk_bound_dev_if) != offsetof(struct inet_timewait_sock, tw_bound_dev_if)); diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 6d807b5c1b9c..710922f7d4d6 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -3610,7 +3610,7 @@ void tcp_send_active_reset(struct sock *sk, gfp_t priority, int reason) /* skb of trace_tcp_send_reset() keeps the skb that caused RST, * skb here is different to the troublesome skb, so use NULL */ - trace_tcp_send_reset(sk, NULL); + trace_tcp_send_reset(sk, NULL, SK_RST_REASON_NOT_SPECIFIED); } /* Send a crossed SYN-ACK during socket establishment. diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 6889ea70c760..3c995eff6e52 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1131,7 +1131,7 @@ static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb, label = ip6_flowlabel(ipv6h); } - trace_tcp_send_reset(sk, skb); + trace_tcp_send_reset(sk, skb, reason); tcp_v6_send_response(sk, skb, seq, ack_seq, 0, 0, 0, oif, 1, ipv6_get_dsfield(ipv6h), label, priority, txhash,