netdev - Re: [PATCH v5 3/4] net: dropreason: use new __print

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <294c30a62d415c9b641be32df2b527cfff19ef62.camel@redhat.com>
Date: Tue, 18 Jun 2024 10:26:11 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Johannes Berg <johannes@...solutions.net>, linux-kernel@...r.kernel.org
Cc: linux-trace-kernel@...r.kernel.org, netdev@...r.kernel.org, 
	linux-wireless@...r.kernel.org, Johannes Berg <johannes.berg@...el.com>
Subject: Re: [PATCH v5 3/4] net: dropreason: use new __print_sym() in tracing

On Fri, 2024-06-14 at 10:19 +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@...el.com>
> 
> The __print_symbolic() could only ever print the core
> drop reasons, since that's the way the infrastructure
> works. Now that we have __print_sym() with all the
> advantages mentioned in that commit, convert to that
> and get all the drop reasons from all subsystems. As
> we already have a list of them, that's really easy.
> 
> This is a little bit of .text (~100 bytes in my build)
> and saves a lot of .data (~17k).
> 
> Signed-off-by: Johannes Berg <johannes.berg@...el.com>
> ---
>  include/net/dropreason.h   |  5 +++++
>  include/trace/events/skb.h | 16 +++-----------
>  net/core/skbuff.c          | 43 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 51 insertions(+), 13 deletions(-)
> 
> diff --git a/include/net/dropreason.h b/include/net/dropreason.h
> index 56cb7be92244..c157070b5303 100644
> --- a/include/net/dropreason.h
> +++ b/include/net/dropreason.h
> @@ -42,6 +42,11 @@ struct drop_reason_list {
>  extern const struct drop_reason_list __rcu *
>  drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
>  
> +#ifdef CONFIG_TRACEPOINTS
> +const char *drop_reason_lookup(unsigned long long value);
> +void drop_reason_show(struct seq_file *m);
> +#endif
> +
>  void drop_reasons_register_subsys(enum skb_drop_reason_subsys subsys,
>  				  const struct drop_reason_list *list);
>  void drop_reasons_unregister_subsys(enum skb_drop_reason_subsys subsys);
> diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
> index 07e0715628ec..8a1a63f9e796 100644
> --- a/include/trace/events/skb.h
> +++ b/include/trace/events/skb.h
> @@ -8,15 +8,9 @@
>  #include <linux/skbuff.h>
>  #include <linux/netdevice.h>
>  #include <linux/tracepoint.h>
> +#include <net/dropreason.h>
>  
> -#undef FN
> -#define FN(reason)	TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason);
> -DEFINE_DROP_REASON(FN, FN)
> -
> -#undef FN
> -#undef FNe
> -#define FN(reason)	{ SKB_DROP_REASON_##reason, #reason },
> -#define FNe(reason)	{ SKB_DROP_REASON_##reason, #reason }
> +TRACE_DEFINE_SYM_FNS(drop_reason, drop_reason_lookup, drop_reason_show);
>  
>  /*
>   * Tracepoint for free an sk_buff:
> @@ -44,13 +38,9 @@ TRACE_EVENT(kfree_skb,
>  
>  	TP_printk("skbaddr=%p protocol=%u location=%pS reason: %s",
>  		  __entry->skbaddr, __entry->protocol, __entry->location,
> -		  __print_symbolic(__entry->reason,
> -				   DEFINE_DROP_REASON(FN, FNe)))
> +		  __print_sym(__entry->reason, drop_reason ))

Minor nit: if you have to repost for other reasons,       ^^ here
checkpatch complains for the extra space.

Otherwise LGTM,

Thanks!

Paolo