netdev - Re: [PATCH net-next v2 2/2] net: udp: add IP/port data to the tracepoint udp/udp_fail_queue_rcv

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAL+tcoCXCteEBhQXrFuM_9vXaLd+0JQy04yj9nzti75u3twuzw@mail.gmail.com>
Date: Thu, 21 Mar 2024 11:34:21 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Balazs Scheidler <bazsi77@...il.com>
Cc: netdev@...r.kernel.org, Balazs Scheidler <balazs.scheidler@...flow.com>
Subject: Re: [PATCH net-next v2 2/2] net: udp: add IP/port data to the
 tracepoint udp/udp_fail_queue_rcv_skb

On Wed, Mar 20, 2024 at 12:39 AM Balazs Scheidler <bazsi77@...il.com> wrote:
>
> 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@...flow.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..f04270946180 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);

Oh, I forgot to say: is this macro a duplication? I mean that it's
good to use the macro but we need to clear the related part (recording
port and addr manually) above this line.

Thanks,
Jason

>         ),
>
> -       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
>
>