netdev - Re: [Patch net-next 2/5] net_sched: add tracepoints for qdisc_reset() and qdisc

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] [day] [month] [year] [list]

Date: Thu, 29 Feb 2024 14:05:13 -0500
From: Steven Rostedt <rostedt@...dmis.org>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: netdev@...r.kernel.org, vaclav.zindulka@...pnet.cz, Jamal Hadi Salim
 <jhs@...atatu.com>, Jiri Pirko <jiri@...nulli.us>
Subject: Re: [Patch net-next 2/5] net_sched: add tracepoints for
 qdisc_reset() and qdisc_destroy()

On Tue, 26 May 2020 21:35:24 -0700
Cong Wang <xiyou.wangcong@...il.com> wrote:

> Add two tracepoints for qdisc_reset() and qdisc_destroy() to track
> qdisc resetting and destroying.
> 
> Sample output:
> 
>   tc-756   [000] ...3   138.355662: qdisc_reset: dev=ens3 kind=pfifo_fast parent=ffff:ffff handle=0:0
>   tc-756   [000] ...1   138.355720: qdisc_reset: dev=ens3 kind=pfifo_fast parent=ffff:ffff handle=0:0
>   tc-756   [000] ...1   138.355867: qdisc_reset: dev=ens3 kind=pfifo_fast parent=ffff:ffff handle=0:0
>   tc-756   [000] ...1   138.355930: qdisc_destroy: dev=ens3 kind=pfifo_fast parent=ffff:ffff handle=0:0
>   tc-757   [000] ...2   143.073780: qdisc_reset: dev=ens3 kind=fq_codel parent=ffff:ffff handle=8001:0
>   tc-757   [000] ...1   143.073878: qdisc_reset: dev=ens3 kind=fq_codel parent=ffff:ffff handle=8001:0
>   tc-757   [000] ...1   143.074114: qdisc_reset: dev=ens3 kind=fq_codel parent=ffff:ffff handle=8001:0
>   tc-757   [000] ...1   143.074228: qdisc_destroy: dev=ens3 kind=fq_codel parent=ffff:ffff handle=8001:0
> 
> Cc: Jamal Hadi Salim <jhs@...atatu.com>
> Cc: Jiri Pirko <jiri@...nulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
> ---
>  include/trace/events/qdisc.h | 52 ++++++++++++++++++++++++++++++++++++
>  net/sched/sch_generic.c      |  4 +++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/include/trace/events/qdisc.h b/include/trace/events/qdisc.h
> index 0d1a9ebf55ba..2b948801afa3 100644
> --- a/include/trace/events/qdisc.h
> +++ b/include/trace/events/qdisc.h
> @@ -8,6 +8,8 @@
>  #include <linux/netdevice.h>
>  #include <linux/tracepoint.h>
>  #include <linux/ftrace.h>
> +#include <linux/pkt_sched.h>
> +#include <net/sch_generic.h>
>  
>  TRACE_EVENT(qdisc_dequeue,
>  
> @@ -44,6 +46,56 @@ TRACE_EVENT(qdisc_dequeue,
>  		  __entry->txq_state, __entry->packets, __entry->skbaddr )
>  );
>  
> +TRACE_EVENT(qdisc_reset,
> +
> +	TP_PROTO(struct Qdisc *q),
> +
> +	TP_ARGS(q),
> +
> +	TP_STRUCT__entry(
> +		__string(	dev,		qdisc_dev(q)	)
> +		__string(	kind,		q->ops->id	)
> +		__field(	u32,		parent		)
> +		__field(	u32,		handle		)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(dev, qdisc_dev(q));

I'm doing updates to __assign_str() and __string() and the below errored
out because "qdisc_dev(q)" is not a string.

How does the above work?

static inline struct net_device *qdisc_dev(const struct Qdisc *qdisc)
{
	return qdisc->dev_queue->dev;
}

Where:


struct net_device {
	/* Cacheline organization can be found documented in
	 * Documentation/networking/net_cachelines/net_device.rst.
	 * Please update the document when adding new fields.
	 */

	/* TX read-mostly hotpath */
	__cacheline_group_begin(net_device_read_tx);
	unsigned long long	priv_flags;
	const struct net_device_ops *netdev_ops;

What looks to be returned from qdisc_dev() is not a string??

Is this a bug? You don't really expect this to work do you?

-- Steve



> +		__assign_str(kind, q->ops->id);
> +		__entry->parent = q->parent;
> +		__entry->handle = q->handle;
> +	),
> +
> +	TP_printk("dev=%s kind=%s parent=%x:%x handle=%x:%x", __get_str(dev),
> +		  __get_str(kind), TC_H_MAJ(__entry->parent) >> 16, TC_H_MIN(__entry->parent),
> +		  TC_H_MAJ(__entry->handle) >> 16, TC_H_MIN(__entry->handle))
> +);
> +
> +TRACE_EVENT(qdisc_destroy,
> +
> +	TP_PROTO(struct Qdisc *q),
> +
> +	TP_ARGS(q),
> +
> +	TP_STRUCT__entry(
> +		__string(	dev,		qdisc_dev(q)	)
> +		__string(	kind,		q->ops->id	)
> +		__field(	u32,		parent		)
> +		__field(	u32,		handle		)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_str(dev, qdisc_dev(q));
> +		__assign_str(kind, q->ops->id);
> +		__entry->parent = q->parent;
> +		__entry->handle = q->handle;
> +	),
> +
> +	TP_printk("dev=%s kind=%s parent=%x:%x handle=%x:%x", __get_str(dev),
> +		  __get_str(kind), TC_H_MAJ(__entry->parent) >> 16, TC_H_MIN(__entry->parent),
> +		  TC_H_MAJ(__entry->handle) >> 16, TC_H_MIN(__entry->handle))
> +);
> +
>  #endif /* _TRACE_QDISC_H */
>  
>  /* This part must be outside protection */
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 7a0b06001e48..abaa446ed01a 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -911,6 +911,8 @@ void qdisc_reset(struct Qdisc *qdisc)
>  	const struct Qdisc_ops *ops = qdisc->ops;
>  	struct sk_buff *skb, *tmp;
>  
> +	trace_qdisc_reset(qdisc);
> +
>  	if (ops->reset)
>  		ops->reset(qdisc);
>  
> @@ -965,6 +967,8 @@ static void qdisc_destroy(struct Qdisc *qdisc)
>  	module_put(ops->owner);
>  	dev_put(qdisc_dev(qdisc));
>  
> +	trace_qdisc_destroy(qdisc);
> +
>  	call_rcu(&qdisc->rcu, qdisc_free_cb);
>  }
>