* [PATCH net-next 0/2] Add IP/port information to UDP drop tracepoint
@ 2024-02-29 7:37 Balazs Scheidler
2024-02-29 7:37 ` [PATCH net-next 1/2] net: port TP_STORE_ADDR_PORTS_SKB macro to be tcp/udp independent Balazs Scheidler
2024-02-29 7:38 ` [PATCH net-next 2/2] net: udp: add IP/port data to the tracepoint udp/udp_fail_queue_rcv_skb Balazs Scheidler
0 siblings, 2 replies; 6+ messages in thread
From: Balazs Scheidler @ 2024-02-29 7:37 UTC (permalink / raw)
To: netdev; +Cc: Balazs Scheidler
Hi,
In our use-case we would like to recover the properties of dropped UDP
packets. Unfortunately the current udp_fail_queue_rcv_skb tracepoint
only exposes the port number of the receiving socket.
This patch-set will add the source/dest ip/port to the tracepoint, while
keeping the socket's local port as well for compatibility.
Balazs Scheidler (2):
net: port TP_STORE_ADDR_PORTS_SKB macro to be tcp/udp independent
net: udp: add IP/port data to the tracepoint
udp/udp_fail_queue_rcv_skb
include/trace/events/net_probe_common.h | 41 ++++++++++++++++++++++
include/trace/events/tcp.h | 45 ++-----------------------
include/trace/events/udp.h | 33 +++++++++++++++---
net/ipv4/udp.c | 2 +-
net/ipv6/udp.c | 3 +-
5 files changed, 75 insertions(+), 49 deletions(-)
--
2.40.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next 1/2] net: port TP_STORE_ADDR_PORTS_SKB macro to be tcp/udp independent 2024-02-29 7:37 [PATCH net-next 0/2] Add IP/port information to UDP drop tracepoint Balazs Scheidler @ 2024-02-29 7:37 ` Balazs Scheidler 2024-02-29 8:21 ` Kuniyuki Iwashima 2024-02-29 7:38 ` [PATCH net-next 2/2] net: udp: add IP/port data to the tracepoint udp/udp_fail_queue_rcv_skb Balazs Scheidler 1 sibling, 1 reply; 6+ messages in thread From: Balazs Scheidler @ 2024-02-29 7:37 UTC (permalink / raw) To: netdev; +Cc: Balazs Scheidler This patch moves TP_STORE_ADDR_PORTS_SKB() to a common header and removes the TCP specific implementation details. Previously the macro assumed the skb passed as an argument is a TCP packet, the implementation now uses an argument to the L3 header and uses that to extract the source/destination ports, which happen to be named the same in "struct tcphdr" and "struct udphdr" Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com> --- include/trace/events/net_probe_common.h | 41 ++++++++++++++++++++++ include/trace/events/tcp.h | 45 ++----------------------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/include/trace/events/net_probe_common.h b/include/trace/events/net_probe_common.h index 3930119cab08..50c083b5687d 100644 --- a/include/trace/events/net_probe_common.h +++ b/include/trace/events/net_probe_common.h @@ -41,4 +41,45 @@ #endif +#define TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb, protoh) \ + do { \ + struct sockaddr_in *v4 = (void *)__entry->saddr; \ + \ + v4->sin_family = AF_INET; \ + v4->sin_port = protoh->source; \ + v4->sin_addr.s_addr = ip_hdr(skb)->saddr; \ + v4 = (void *)__entry->daddr; \ + v4->sin_family = AF_INET; \ + v4->sin_port = protoh->dest; \ + v4->sin_addr.s_addr = ip_hdr(skb)->daddr; \ + } while (0) + +#if IS_ENABLED(CONFIG_IPV6) + +#define TP_STORE_ADDR_PORTS_SKB(__entry, skb, protoh) \ + do { \ + const struct iphdr *iph = ip_hdr(skb); \ + \ + if (iph->version == 6) { \ + struct sockaddr_in6 *v6 = (void *)__entry->saddr; \ + \ + v6->sin6_family = AF_INET6; \ + v6->sin6_port = protoh->source; \ + v6->sin6_addr = ipv6_hdr(skb)->saddr; \ + v6 = (void *)__entry->daddr; \ + v6->sin6_family = AF_INET6; \ + v6->sin6_port = protoh->dest; \ + v6->sin6_addr = ipv6_hdr(skb)->daddr; \ + } else \ + TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb, protoh); \ + } while (0) + +#else + +#define TP_STORE_ADDR_PORTS_SKB(__entry, skb, protoh) \ + TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb, protoh) + +#endif + + #endif diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h index 7b1ddffa3dfc..717f74454c17 100644 --- a/include/trace/events/tcp.h +++ b/include/trace/events/tcp.h @@ -295,48 +295,6 @@ TRACE_EVENT(tcp_probe, __entry->srtt, __entry->rcv_wnd, __entry->sock_cookie) ); -#define TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb) \ - do { \ - const struct tcphdr *th = (const struct tcphdr *)skb->data; \ - struct sockaddr_in *v4 = (void *)__entry->saddr; \ - \ - v4->sin_family = AF_INET; \ - v4->sin_port = th->source; \ - v4->sin_addr.s_addr = ip_hdr(skb)->saddr; \ - v4 = (void *)__entry->daddr; \ - v4->sin_family = AF_INET; \ - v4->sin_port = th->dest; \ - v4->sin_addr.s_addr = ip_hdr(skb)->daddr; \ - } while (0) - -#if IS_ENABLED(CONFIG_IPV6) - -#define TP_STORE_ADDR_PORTS_SKB(__entry, skb) \ - do { \ - const struct iphdr *iph = ip_hdr(skb); \ - \ - if (iph->version == 6) { \ - const struct tcphdr *th = (const struct tcphdr *)skb->data; \ - struct sockaddr_in6 *v6 = (void *)__entry->saddr; \ - \ - v6->sin6_family = AF_INET6; \ - v6->sin6_port = th->source; \ - v6->sin6_addr = ipv6_hdr(skb)->saddr; \ - v6 = (void *)__entry->daddr; \ - v6->sin6_family = AF_INET6; \ - v6->sin6_port = th->dest; \ - v6->sin6_addr = ipv6_hdr(skb)->daddr; \ - } else \ - TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb); \ - } while (0) - -#else - -#define TP_STORE_ADDR_PORTS_SKB(__entry, skb) \ - TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb) - -#endif - /* * tcp event with only skb */ @@ -353,12 +311,13 @@ DECLARE_EVENT_CLASS(tcp_event_skb, ), TP_fast_assign( + const struct tcphdr *th = (const struct tcphdr *)skb->data; __entry->skbaddr = skb; memset(__entry->saddr, 0, sizeof(struct sockaddr_in6)); memset(__entry->daddr, 0, sizeof(struct sockaddr_in6)); - TP_STORE_ADDR_PORTS_SKB(__entry, skb); + TP_STORE_ADDR_PORTS_SKB(__entry, skb, th); ), TP_printk("src=%pISpc dest=%pISpc", __entry->saddr, __entry->daddr) -- 2.40.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] net: port TP_STORE_ADDR_PORTS_SKB macro to be tcp/udp independent 2024-02-29 7:37 ` [PATCH net-next 1/2] net: port TP_STORE_ADDR_PORTS_SKB macro to be tcp/udp independent Balazs Scheidler @ 2024-02-29 8:21 ` Kuniyuki Iwashima 0 siblings, 0 replies; 6+ messages in thread From: Kuniyuki Iwashima @ 2024-02-29 8:21 UTC (permalink / raw) To: bazsi77; +Cc: balazs.scheidler, netdev, kuniyu From: Balazs Scheidler <bazsi77@gmail.com> Date: Thu, 29 Feb 2024 08:37:59 +0100 > This patch moves TP_STORE_ADDR_PORTS_SKB() to a common header and removes > the TCP specific implementation details. > > Previously the macro assumed the skb passed as an argument is a > TCP packet, the implementation now uses an argument to the L3 header and nit: s/L3/L4/ > uses that to extract the source/destination ports, which happen > to be named the same in "struct tcphdr" and "struct udphdr" > > Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com> > --- > include/trace/events/net_probe_common.h | 41 ++++++++++++++++++++++ > include/trace/events/tcp.h | 45 ++----------------------- > 2 files changed, 43 insertions(+), 43 deletions(-) > > diff --git a/include/trace/events/net_probe_common.h b/include/trace/events/net_probe_common.h > index 3930119cab08..50c083b5687d 100644 > --- a/include/trace/events/net_probe_common.h > +++ b/include/trace/events/net_probe_common.h > @@ -41,4 +41,45 @@ > > #endif > > +#define TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb, protoh) \ > + do { \ > + struct sockaddr_in *v4 = (void *)__entry->saddr; \ > + \ > + v4->sin_family = AF_INET; \ > + v4->sin_port = protoh->source; \ > + v4->sin_addr.s_addr = ip_hdr(skb)->saddr; \ > + v4 = (void *)__entry->daddr; \ > + v4->sin_family = AF_INET; \ > + v4->sin_port = protoh->dest; \ > + v4->sin_addr.s_addr = ip_hdr(skb)->daddr; \ > + } while (0) > + > +#if IS_ENABLED(CONFIG_IPV6) > + > +#define TP_STORE_ADDR_PORTS_SKB(__entry, skb, protoh) \ > + do { \ > + const struct iphdr *iph = ip_hdr(skb); \ > + \ > + if (iph->version == 6) { \ > + struct sockaddr_in6 *v6 = (void *)__entry->saddr; \ > + \ > + v6->sin6_family = AF_INET6; \ > + v6->sin6_port = protoh->source; \ > + v6->sin6_addr = ipv6_hdr(skb)->saddr; \ > + v6 = (void *)__entry->daddr; \ > + v6->sin6_family = AF_INET6; \ > + v6->sin6_port = protoh->dest; \ > + v6->sin6_addr = ipv6_hdr(skb)->daddr; \ > + } else \ > + TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb, protoh); \ > + } while (0) > + > +#else > + > +#define TP_STORE_ADDR_PORTS_SKB(__entry, skb, protoh) \ > + TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb, protoh) > + > +#endif > + > + > #endif > diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h > index 7b1ddffa3dfc..717f74454c17 100644 > --- a/include/trace/events/tcp.h > +++ b/include/trace/events/tcp.h > @@ -295,48 +295,6 @@ TRACE_EVENT(tcp_probe, > __entry->srtt, __entry->rcv_wnd, __entry->sock_cookie) > ); > > -#define TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb) \ > - do { \ > - const struct tcphdr *th = (const struct tcphdr *)skb->data; \ > - struct sockaddr_in *v4 = (void *)__entry->saddr; \ > - \ > - v4->sin_family = AF_INET; \ > - v4->sin_port = th->source; \ > - v4->sin_addr.s_addr = ip_hdr(skb)->saddr; \ > - v4 = (void *)__entry->daddr; \ > - v4->sin_family = AF_INET; \ > - v4->sin_port = th->dest; \ > - v4->sin_addr.s_addr = ip_hdr(skb)->daddr; \ > - } while (0) > - > -#if IS_ENABLED(CONFIG_IPV6) > - > -#define TP_STORE_ADDR_PORTS_SKB(__entry, skb) \ > - do { \ > - const struct iphdr *iph = ip_hdr(skb); \ > - \ > - if (iph->version == 6) { \ > - const struct tcphdr *th = (const struct tcphdr *)skb->data; \ > - struct sockaddr_in6 *v6 = (void *)__entry->saddr; \ > - \ > - v6->sin6_family = AF_INET6; \ > - v6->sin6_port = th->source; \ > - v6->sin6_addr = ipv6_hdr(skb)->saddr; \ > - v6 = (void *)__entry->daddr; \ > - v6->sin6_family = AF_INET6; \ > - v6->sin6_port = th->dest; \ > - v6->sin6_addr = ipv6_hdr(skb)->daddr; \ > - } else \ > - TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb); \ > - } while (0) > - > -#else > - > -#define TP_STORE_ADDR_PORTS_SKB(__entry, skb) \ > - TP_STORE_ADDR_PORTS_SKB_V4(__entry, skb) > - > -#endif > - > /* > * tcp event with only skb > */ > @@ -353,12 +311,13 @@ DECLARE_EVENT_CLASS(tcp_event_skb, > ), > > TP_fast_assign( > + const struct tcphdr *th = (const struct tcphdr *)skb->data; > __entry->skbaddr = skb; > > memset(__entry->saddr, 0, sizeof(struct sockaddr_in6)); > memset(__entry->daddr, 0, sizeof(struct sockaddr_in6)); > > - TP_STORE_ADDR_PORTS_SKB(__entry, skb); > + TP_STORE_ADDR_PORTS_SKB(__entry, skb, th); > ), > > TP_printk("src=%pISpc dest=%pISpc", __entry->saddr, __entry->daddr) > -- > 2.40.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] net: udp: add IP/port data to the tracepoint udp/udp_fail_queue_rcv_skb 2024-02-29 7:37 [PATCH net-next 0/2] Add IP/port information to UDP drop tracepoint Balazs Scheidler 2024-02-29 7:37 ` [PATCH net-next 1/2] net: port TP_STORE_ADDR_PORTS_SKB macro to be tcp/udp independent Balazs Scheidler @ 2024-02-29 7:38 ` Balazs Scheidler 2024-02-29 8:27 ` Kuniyuki Iwashima 1 sibling, 1 reply; 6+ messages in thread From: Balazs Scheidler @ 2024-02-29 7:38 UTC (permalink / raw) To: netdev; +Cc: Balazs Scheidler The udp_fail_queue_rcv_skb() tracepoint lacks any details on the source and destination IP/port whereas this information can be critical in case of UDP/syslog. Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com> --- include/trace/events/udp.h | 33 +++++++++++++++++++++++++++++---- net/ipv4/udp.c | 2 +- net/ipv6/udp.c | 3 ++- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/include/trace/events/udp.h b/include/trace/events/udp.h index 336fe272889f..cd4ae5c2fad7 100644 --- a/include/trace/events/udp.h +++ b/include/trace/events/udp.h @@ -7,24 +7,49 @@ #include <linux/udp.h> #include <linux/tracepoint.h> +#include <trace/events/net_probe_common.h> TRACE_EVENT(udp_fail_queue_rcv_skb, - TP_PROTO(int rc, struct sock *sk), + TP_PROTO(int rc, struct sock *sk, struct sk_buff *skb), - TP_ARGS(rc, sk), + TP_ARGS(rc, sk, skb), TP_STRUCT__entry( __field(int, rc) __field(__u16, lport) + + __field(__u16, sport) + __field(__u16, dport) + __field(__u16, family) + __array(__u8, saddr, sizeof(struct sockaddr_in6)) + __array(__u8, daddr, sizeof(struct sockaddr_in6)) ), TP_fast_assign( + const struct inet_sock *inet = inet_sk(sk); + const struct udphdr *uh = (const struct udphdr *)udp_hdr(skb); + __be32 *p32; + __entry->rc = rc; - __entry->lport = inet_sk(sk)->inet_num; + __entry->lport = inet->inet_num; + + __entry->sport = ntohs(uh->source); + __entry->dport = ntohs(uh->dest); + __entry->family = sk->sk_family; + + p32 = (__be32 *) __entry->saddr; + *p32 = inet->inet_saddr; + + p32 = (__be32 *) __entry->daddr; + *p32 = inet->inet_daddr; + + TP_STORE_ADDR_PORTS_SKB(__entry, skb, uh); ), - TP_printk("rc=%d port=%hu", __entry->rc, __entry->lport) + TP_printk("rc=%d port=%hu family=%s src=%pISpc dest=%pISpc", __entry->rc, __entry->lport, + show_family_name(__entry->family), + __entry->saddr, __entry->daddr) ); #endif /* _TRACE_UDP_H */ diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index a8acea17b4e5..d21a85257367 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -2051,8 +2051,8 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) drop_reason = SKB_DROP_REASON_PROTO_MEM; } UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite); + trace_udp_fail_queue_rcv_skb(rc, sk, skb); kfree_skb_reason(skb, drop_reason); - trace_udp_fail_queue_rcv_skb(rc, sk); return -1; } diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 3f2249b4cd5f..e5a52c4c934c 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -34,6 +34,7 @@ #include <linux/slab.h> #include <linux/uaccess.h> #include <linux/indirect_call_wrapper.h> +#include <trace/events/udp.h> #include <net/addrconf.h> #include <net/ndisc.h> @@ -661,8 +662,8 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) drop_reason = SKB_DROP_REASON_PROTO_MEM; } UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite); + trace_udp_fail_queue_rcv_skb(rc, sk, skb); kfree_skb_reason(skb, drop_reason); - trace_udp_fail_queue_rcv_skb(rc, sk); return -1; } -- 2.40.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] net: udp: add IP/port data to the tracepoint udp/udp_fail_queue_rcv_skb 2024-02-29 7:38 ` [PATCH net-next 2/2] net: udp: add IP/port data to the tracepoint udp/udp_fail_queue_rcv_skb Balazs Scheidler @ 2024-02-29 8:27 ` Kuniyuki Iwashima 2024-02-29 8:56 ` Balazs Scheidler 0 siblings, 1 reply; 6+ messages in thread From: Kuniyuki Iwashima @ 2024-02-29 8:27 UTC (permalink / raw) To: bazsi77; +Cc: balazs.scheidler, netdev, kuniyu From: Balazs Scheidler <bazsi77@gmail.com> Date: Thu, 29 Feb 2024 08:38:00 +0100 > The udp_fail_queue_rcv_skb() tracepoint lacks any details on the source > and destination IP/port whereas this information can be critical in case > of UDP/syslog. > > Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com> > --- > include/trace/events/udp.h | 33 +++++++++++++++++++++++++++++---- > net/ipv4/udp.c | 2 +- > net/ipv6/udp.c | 3 ++- > 3 files changed, 32 insertions(+), 6 deletions(-) > > diff --git a/include/trace/events/udp.h b/include/trace/events/udp.h > index 336fe272889f..cd4ae5c2fad7 100644 > --- a/include/trace/events/udp.h > +++ b/include/trace/events/udp.h > @@ -7,24 +7,49 @@ > > #include <linux/udp.h> > #include <linux/tracepoint.h> > +#include <trace/events/net_probe_common.h> > > TRACE_EVENT(udp_fail_queue_rcv_skb, > > - TP_PROTO(int rc, struct sock *sk), > + TP_PROTO(int rc, struct sock *sk, struct sk_buff *skb), > > - TP_ARGS(rc, sk), > + TP_ARGS(rc, sk, skb), > > TP_STRUCT__entry( > __field(int, rc) > __field(__u16, lport) > + > + __field(__u16, sport) > + __field(__u16, dport) duplicating lport just for reusing TP_STORE_ADDR_PORTS_SKB() ? Then, I think we should define udp-specific macro. > + __field(__u16, family) > + __array(__u8, saddr, sizeof(struct sockaddr_in6)) > + __array(__u8, daddr, sizeof(struct sockaddr_in6)) > ), > > TP_fast_assign( > + const struct inet_sock *inet = inet_sk(sk); > + const struct udphdr *uh = (const struct udphdr *)udp_hdr(skb); > + __be32 *p32; > + > __entry->rc = rc; > - __entry->lport = inet_sk(sk)->inet_num; > + __entry->lport = inet->inet_num; > + > + __entry->sport = ntohs(uh->source); > + __entry->dport = ntohs(uh->dest); > + __entry->family = sk->sk_family; > + > + p32 = (__be32 *) __entry->saddr; > + *p32 = inet->inet_saddr; > + > + p32 = (__be32 *) __entry->daddr; > + *p32 = inet->inet_daddr; nit: double space here. > + > + TP_STORE_ADDR_PORTS_SKB(__entry, skb, uh); > ), > > - TP_printk("rc=%d port=%hu", __entry->rc, __entry->lport) > + TP_printk("rc=%d port=%hu family=%s src=%pISpc dest=%pISpc", __entry->rc, __entry->lport, > + show_family_name(__entry->family), > + __entry->saddr, __entry->daddr) > ); > > #endif /* _TRACE_UDP_H */ > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index a8acea17b4e5..d21a85257367 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -2051,8 +2051,8 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) > drop_reason = SKB_DROP_REASON_PROTO_MEM; > } > UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite); > + trace_udp_fail_queue_rcv_skb(rc, sk, skb); > kfree_skb_reason(skb, drop_reason); > - trace_udp_fail_queue_rcv_skb(rc, sk); > return -1; > } > > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c > index 3f2249b4cd5f..e5a52c4c934c 100644 > --- a/net/ipv6/udp.c > +++ b/net/ipv6/udp.c > @@ -34,6 +34,7 @@ > #include <linux/slab.h> > #include <linux/uaccess.h> > #include <linux/indirect_call_wrapper.h> > +#include <trace/events/udp.h> > > #include <net/addrconf.h> > #include <net/ndisc.h> > @@ -661,8 +662,8 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) > drop_reason = SKB_DROP_REASON_PROTO_MEM; > } > UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite); > + trace_udp_fail_queue_rcv_skb(rc, sk, skb); > kfree_skb_reason(skb, drop_reason); > - trace_udp_fail_queue_rcv_skb(rc, sk); > return -1; > } > > -- > 2.40.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/2] net: udp: add IP/port data to the tracepoint udp/udp_fail_queue_rcv_skb 2024-02-29 8:27 ` Kuniyuki Iwashima @ 2024-02-29 8:56 ` Balazs Scheidler 0 siblings, 0 replies; 6+ messages in thread From: Balazs Scheidler @ 2024-02-29 8:56 UTC (permalink / raw) To: Kuniyuki Iwashima; +Cc: balazs.scheidler, netdev On Thu, Feb 29, 2024 at 12:27:11AM -0800, Kuniyuki Iwashima wrote: > From: Balazs Scheidler <bazsi77@gmail.com> > Date: Thu, 29 Feb 2024 08:38:00 +0100 > > The udp_fail_queue_rcv_skb() tracepoint lacks any details on the source > > and destination IP/port whereas this information can be critical in case > > of UDP/syslog. > > > > Signed-off-by: Balazs Scheidler <balazs.scheidler@axoflow.com> > > --- > > include/trace/events/udp.h | 33 +++++++++++++++++++++++++++++---- > > net/ipv4/udp.c | 2 +- > > net/ipv6/udp.c | 3 ++- > > 3 files changed, 32 insertions(+), 6 deletions(-) > > > > diff --git a/include/trace/events/udp.h b/include/trace/events/udp.h > > index 336fe272889f..cd4ae5c2fad7 100644 > > --- a/include/trace/events/udp.h > > +++ b/include/trace/events/udp.h > > @@ -7,24 +7,49 @@ > > > > #include <linux/udp.h> > > #include <linux/tracepoint.h> > > +#include <trace/events/net_probe_common.h> > > > > TRACE_EVENT(udp_fail_queue_rcv_skb, > > > > - TP_PROTO(int rc, struct sock *sk), > > + TP_PROTO(int rc, struct sock *sk, struct sk_buff *skb), > > > > - TP_ARGS(rc, sk), > > + TP_ARGS(rc, sk, skb), > > > > TP_STRUCT__entry( > > __field(int, rc) > > __field(__u16, lport) > > + > > + __field(__u16, sport) > > + __field(__u16, dport) > > duplicating lport just for reusing TP_STORE_ADDR_PORTS_SKB() ? > Then, I think we should define udp-specific macro. I left "lport" in for compatibility with the previous users of the same tracepoint. The sk->inet_num can be different from the dport in the packet, for instance when TPROXY style redirects are done. In that case the TPROXY rule would look up a socket and queue the incoming packet there, even if their port numbers differ. If I was adding this tracepoint now, I would probably skip the "lport" value, I agree this is redundant. But there are other issues as well: * I think that the name "lport" is confusing, all other tracepoints have saddr/daddr sport/dport. * Sometimes address information is stored stored as separate fields (e.g. family, saddr/daddr, sport/dport), in other cases they are stored as "struct sockaddr" that bundles family, address/port information. With that said, I could either go ahead and: 1) break tracepoint compatibility by removing lport completely 2) change the interpretation of lport and store the dport in that field (still incompatible, but would not break stuff), this would leave the confusing lport name. 3) retain lport and add the redundant fields (compatible at a cost of an extra u16 field) I've chosen number 3) above, as it fully retains compatibility and sometimes the extra lport field can come handy in case someone is using TPROXY with UDP. > > > > + __field(__u16, family) > > + __array(__u8, saddr, sizeof(struct sockaddr_in6)) > > + __array(__u8, daddr, sizeof(struct sockaddr_in6)) > > ), > > > > TP_fast_assign( > > + const struct inet_sock *inet = inet_sk(sk); > > + const struct udphdr *uh = (const struct udphdr *)udp_hdr(skb); > > + __be32 *p32; > > + > > __entry->rc = rc; > > - __entry->lport = inet_sk(sk)->inet_num; > > + __entry->lport = inet->inet_num; > > + > > + __entry->sport = ntohs(uh->source); > > + __entry->dport = ntohs(uh->dest); > > + __entry->family = sk->sk_family; > > + > > + p32 = (__be32 *) __entry->saddr; > > + *p32 = inet->inet_saddr; > > + > > + p32 = (__be32 *) __entry->daddr; > > + *p32 = inet->inet_daddr; > > nit: double space here. Thanks, I fixed this. > > > > + > > + TP_STORE_ADDR_PORTS_SKB(__entry, skb, uh); > > ), > > > > - TP_printk("rc=%d port=%hu", __entry->rc, __entry->lport) > > + TP_printk("rc=%d port=%hu family=%s src=%pISpc dest=%pISpc", __entry->rc, __entry->lport, > > + show_family_name(__entry->family), > > + __entry->saddr, __entry->daddr) > > ); > > > > #endif /* _TRACE_UDP_H */ > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > > index a8acea17b4e5..d21a85257367 100644 > > --- a/net/ipv4/udp.c > > +++ b/net/ipv4/udp.c > > @@ -2051,8 +2051,8 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) > > drop_reason = SKB_DROP_REASON_PROTO_MEM; > > } > > UDP_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite); > > + trace_udp_fail_queue_rcv_skb(rc, sk, skb); > > kfree_skb_reason(skb, drop_reason); > > - trace_udp_fail_queue_rcv_skb(rc, sk); > > return -1; > > } > > > > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c > > index 3f2249b4cd5f..e5a52c4c934c 100644 > > --- a/net/ipv6/udp.c > > +++ b/net/ipv6/udp.c > > @@ -34,6 +34,7 @@ > > #include <linux/slab.h> > > #include <linux/uaccess.h> > > #include <linux/indirect_call_wrapper.h> > > +#include <trace/events/udp.h> > > > > #include <net/addrconf.h> > > #include <net/ndisc.h> > > @@ -661,8 +662,8 @@ static int __udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) > > drop_reason = SKB_DROP_REASON_PROTO_MEM; > > } > > UDP6_INC_STATS(sock_net(sk), UDP_MIB_INERRORS, is_udplite); > > + trace_udp_fail_queue_rcv_skb(rc, sk, skb); > > kfree_skb_reason(skb, drop_reason); > > - trace_udp_fail_queue_rcv_skb(rc, sk); > > return -1; > > } > > > > -- > > 2.40.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-29 8:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-02-29 7:37 [PATCH net-next 0/2] Add IP/port information to UDP drop tracepoint Balazs Scheidler 2024-02-29 7:37 ` [PATCH net-next 1/2] net: port TP_STORE_ADDR_PORTS_SKB macro to be tcp/udp independent Balazs Scheidler 2024-02-29 8:21 ` Kuniyuki Iwashima 2024-02-29 7:38 ` [PATCH net-next 2/2] net: udp: add IP/port data to the tracepoint udp/udp_fail_queue_rcv_skb Balazs Scheidler 2024-02-29 8:27 ` Kuniyuki Iwashima 2024-02-29 8:56 ` Balazs Scheidler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).