[PATCH net-next 0/3] introduce two new tracepoints for udp

[PATCH net-next 0/3] introduce two new tracepoints for udp

[Tony Lu]

This series introduces two new tracepoints trace_udp_send and
trace_udp_queue_rcv, and removes redundant new line from
tcp_event_sk_skb.

Tony Lu (3):
  udp: introduce a new tracepoint for udp_send_skb
  udp: introduce a new tracepoint for udp_queue_rcv_skb
  tcp: remove redundant new line from tcp_event_sk_skb

 include/trace/events/tcp.h |  2 +-
 include/trace/events/udp.h | 88 ++++++++++++++++++++++++++++++++++++++
 net/ipv4/udp.c             |  2 +
 net/ipv6/udp.c             |  3 ++
 4 files changed, 94 insertions(+), 1 deletion(-)

-- 
2.21.0

[PATCH net-next 1/3] udp: introduce a new tracepoint for udp_send_skb

[Tony Lu]

This introduces a new tracepoint trace_udp_send, it will trace UDP
packets that are going to be send to the IP layer.

This exposes src and dst IP addresses and ports of the connection. We
could use kprobe or tcpdump to do similar things, however using tracepoint
makes it easier to use and to integrate into perf or ebpf.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
---
 include/trace/events/udp.h | 81 ++++++++++++++++++++++++++++++++++++++
 net/ipv4/udp.c             |  1 +
 net/ipv6/udp.c             |  2 +
 3 files changed, 84 insertions(+)

diff --git a/include/trace/events/udp.h b/include/trace/events/udp.h
index 336fe272889f..f2c26780e2a9 100644
--- a/include/trace/events/udp.h
+++ b/include/trace/events/udp.h
@@ -7,6 +7,38 @@
 
 #include <linux/udp.h>
 #include <linux/tracepoint.h>
+#include <linux/ipv6.h>
+#include <net/ipv6.h>
+#include <net/udp.h>
+
+#define TP_STORE_V4MAPPED(__entry, saddr, daddr)		\
+	do {							\
+		struct in6_addr *pin6;				\
+								\
+		pin6 = (struct in6_addr *)__entry->saddr_v6;	\
+		ipv6_addr_set_v4mapped(saddr, pin6);		\
+		pin6 = (struct in6_addr *)__entry->daddr_v6;	\
+		ipv6_addr_set_v4mapped(daddr, pin6);		\
+	} while (0)
+
+#if IS_ENABLED(CONFIG_IPV6)
+#define TP_STORE_ADDRS(__entry, saddr, daddr, saddr6, daddr6)		\
+	do {								\
+		if (sk->sk_family == AF_INET6) {			\
+			struct in6_addr *pin6;				\
+									\
+			pin6 = (struct in6_addr *)__entry->saddr_v6;	\
+			*pin6 = saddr6;					\
+			pin6 = (struct in6_addr *)__entry->daddr_v6;	\
+			*pin6 = daddr6;					\
+		} else {						\
+			TP_STORE_V4MAPPED(__entry, saddr, daddr);	\
+		}							\
+	} while (0)
+#else
+#define TP_STORE_ADDRS(__entry, saddr, daddr, saddr6, daddr6)	\
+	TP_STORE_V4MAPPED(__entry, saddr, daddr)
+#endif
 
 TRACE_EVENT(udp_fail_queue_rcv_skb,
 
@@ -27,6 +59,55 @@ TRACE_EVENT(udp_fail_queue_rcv_skb,
 	TP_printk("rc=%d port=%hu", __entry->rc, __entry->lport)
 );
 
+DECLARE_EVENT_CLASS(udp_event_sk_skb,
+
+	TP_PROTO(const struct sock *sk, const struct sk_buff *skb),
+
+	TP_ARGS(sk, skb),
+
+	TP_STRUCT__entry(
+		__field(const void *, skbaddr)
+		__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)
+	),
+
+	TP_fast_assign(
+		struct inet_sock *inet = inet_sk(sk);
+		__be32 *p32;
+
+		__entry->skbaddr = skb;
+		__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;
+
+		TP_STORE_ADDRS(__entry, inet->inet_saddr, inet->inet_daddr,
+			      sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
+	),
+
+	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c",
+		  __entry->sport, __entry->dport, __entry->saddr,
+		  __entry->daddr, __entry->saddr_v6, __entry->daddr_v6)
+);
+
+DEFINE_EVENT(udp_event_sk_skb, udp_send,
+
+	TP_PROTO(const struct sock *sk, const struct sk_buff *skb),
+
+	TP_ARGS(sk, skb)
+);
+
 #endif /* _TRACE_UDP_H */
 
 /* This part must be outside protection */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 8fb250ed53d4..3ff6fea9debe 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -898,6 +898,7 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
 		uh->check = CSUM_MANGLED_0;
 
 send:
+	trace_udp_send(sk, skb);
 	err = ip_send_skb(sock_net(sk), skb);
 	if (err) {
 		if (err == -ENOBUFS && !inet->recverr) {
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 07fa579dfb96..3a26990d5dc8 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -56,6 +56,7 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 #include <trace/events/skb.h>
+#include <trace/events/udp.h>
 #include "udp_impl.h"
 
 static bool udp6_lib_exact_dif_match(struct net *net, struct sk_buff *skb)
@@ -1177,6 +1178,7 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
 		uh->check = CSUM_MANGLED_0;
 
 send:
+	trace_udp_send(sk, skb);
 	err = ip6_send_skb(skb);
 	if (err) {
 		if (err == -ENOBUFS && !inet6_sk(sk)->recverr) {
-- 
2.21.0

[PATCH net-next 2/3] udp: introduce a new tracepoint for udp_queue_rcv_skb

[Tony Lu]

This introduces a new tracepoint trace_udp_queue_rcv, it will trace UDP
packets that are going to be queued on the socket receive queue.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
---
 include/trace/events/udp.h | 7 +++++++
 net/ipv4/udp.c             | 1 +
 net/ipv6/udp.c             | 1 +
 3 files changed, 9 insertions(+)

diff --git a/include/trace/events/udp.h b/include/trace/events/udp.h
index f2c26780e2a9..37daea5f7cb1 100644
--- a/include/trace/events/udp.h
+++ b/include/trace/events/udp.h
@@ -108,6 +108,13 @@ DEFINE_EVENT(udp_event_sk_skb, udp_send,
 	TP_ARGS(sk, skb)
 );
 
+DEFINE_EVENT(udp_event_sk_skb, udp_queue_rcv,
+
+	TP_PROTO(const struct sock *sk, const struct sk_buff *skb),
+
+	TP_ARGS(sk, skb)
+);
+
 #endif /* _TRACE_UDP_H */
 
 /* This part must be outside protection */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 3ff6fea9debe..262d76559bd5 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2238,6 +2238,7 @@ static int udp_unicast_rcv_skb(struct sock *sk, struct sk_buff *skb,
 		skb_checksum_try_convert(skb, IPPROTO_UDP, uh->check,
 					 inet_compute_pseudo);
 
+	trace_udp_queue_rcv(sk, skb);
 	ret = udp_queue_rcv_skb(sk, skb);
 
 	/* a return value > 0 means to resubmit the input, but
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 3a26990d5dc8..49473c5d3c4b 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -842,6 +842,7 @@ static int udp6_unicast_rcv_skb(struct sock *sk, struct sk_buff *skb,
 		skb_checksum_try_convert(skb, IPPROTO_UDP, uh->check,
 					 ip6_compute_pseudo);
 
+	trace_udp_queue_rcv(sk, skb);
 	ret = udpv6_queue_rcv_skb(sk, skb);
 
 	/* a return value > 0 means to resubmit the input */
-- 
2.21.0

[PATCH net-next 3/3] tcp: remove redundant new line from tcp_event_sk_skb

[Tony Lu]

This removes '\n' from trace event class tcp_event_sk_skb to avoid
redundant new blank line and make output compact.

Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>
---
 include/trace/events/tcp.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 2bc9960a31aa..cf97f6339acb 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -86,7 +86,7 @@ DECLARE_EVENT_CLASS(tcp_event_sk_skb,
 			      sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
 	),
 
-	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c state=%s\n",
+	TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c state=%s",
 		  __entry->sport, __entry->dport, __entry->saddr, __entry->daddr,
 		  __entry->saddr_v6, __entry->daddr_v6,
 		  show_tcp_state_name(__entry->state))
-- 
2.21.0

Re: [PATCH net-next 3/3] tcp: remove redundant new line from tcp_event_sk_skb

[Yafang Shao]

On Wed, May 29, 2019 at 9:08 PM Tony Lu <tonylu@linux.alibaba.com> wrote:
>
> This removes '\n' from trace event class tcp_event_sk_skb to avoid
> redundant new blank line and make output compact.
>
> Signed-off-by: Tony Lu <tonylu@linux.alibaba.com>

Acked-by: Yafang Shao <laoar.shao@gmail.com>

> ---
>  include/trace/events/tcp.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> index 2bc9960a31aa..cf97f6339acb 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -86,7 +86,7 @@ DECLARE_EVENT_CLASS(tcp_event_sk_skb,
>                               sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
>         ),
>
> -       TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c state=%s\n",
> +       TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c state=%s",
>                   __entry->sport, __entry->dport, __entry->saddr, __entry->daddr,
>                   __entry->saddr_v6, __entry->daddr_v6,
>                   show_tcp_state_name(__entry->state))
> --
> 2.21.0
>

Re: [PATCH net-next 0/3] introduce two new tracepoints for udp

[Alexei Starovoitov]

On Wed, May 29, 2019 at 6:10 AM Tony Lu <tonylu@linux.alibaba.com> wrote:
>
> This series introduces two new tracepoints trace_udp_send and
> trace_udp_queue_rcv, and removes redundant new line from
> tcp_event_sk_skb.

Though I like new tracepoints this patch set gets a nack, since commit log
says nothing about use case. Why kprobes cannot achieve the same?

Re: [PATCH net-next 0/3] introduce two new tracepoints for udp

[David Miller]

From: Tony Lu <tonylu@linux.alibaba.com>
Date: Wed, 29 May 2019 21:06:54 +0800

> This series introduces two new tracepoints trace_udp_send and
> trace_udp_queue_rcv, and removes redundant new line from
> tcp_event_sk_skb.

Why?

Is it faster than using kprobes?

Is it more reliable?

Are the events _so_ useful that they warrant a tracepoint and thus
creating a semi-stable interface for tracing and introspection via
ebpf and similar technologies?

Again, you have to say why in your log message(s).