[PATCH] ieee802154: Add trace events for rdev->ops

[PATCH] ieee802154: Add trace events for rdev->ops

[Guido Günther]

This is based on the rdev->ops tracing code from net/wireless/.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 net/ieee802154/Makefile   |   4 +-
 net/ieee802154/rdev-ops.h |  77 ++++++++++++---
 net/ieee802154/trace.c    |   7 ++
 net/ieee802154/trace.h    | 243 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 319 insertions(+), 12 deletions(-)
 create mode 100644 net/ieee802154/trace.c
 create mode 100644 net/ieee802154/trace.h

diff --git a/net/ieee802154/Makefile b/net/ieee802154/Makefile
index 05dab29..4adfd4d 100644
--- a/net/ieee802154/Makefile
+++ b/net/ieee802154/Makefile
@@ -3,7 +3,9 @@ obj-$(CONFIG_IEEE802154_SOCKET) += ieee802154_socket.o
 obj-y += 6lowpan/
 
 ieee802154-y := netlink.o nl-mac.o nl-phy.o nl_policy.o core.o \
-                header_ops.o sysfs.o nl802154.o
+                header_ops.o sysfs.o nl802154.o trace.o
 ieee802154_socket-y := socket.o
 
+CFLAGS_trace.o := -I$(src)
+
 ccflags-y += -D__CHECK_ENDIAN__
diff --git a/net/ieee802154/rdev-ops.h b/net/ieee802154/rdev-ops.h
index 7c46732..624413e 100644
--- a/net/ieee802154/rdev-ops.h
+++ b/net/ieee802154/rdev-ops.h
@@ -4,6 +4,7 @@
 #include <net/cfg802154.h>
 
 #include "core.h"
+#include "trace.h"
 
 static inline struct net_device *
 rdev_add_virtual_intf_deprecated(struct cfg802154_registered_device *rdev,
@@ -24,73 +25,127 @@ static inline int
 rdev_add_virtual_intf(struct cfg802154_registered_device *rdev, char *name,
 		      enum nl802154_iftype type, __le64 extended_addr)
 {
-	return rdev->ops->add_virtual_intf(&rdev->wpan_phy, name, type,
+	int ret;
+
+	trace_802154_rdev_add_virtual_intf(&rdev->wpan_phy, name, type,
 					   extended_addr);
+	ret = rdev->ops->add_virtual_intf(&rdev->wpan_phy, name, type,
+					  extended_addr);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_del_virtual_intf(struct cfg802154_registered_device *rdev,
 		      struct wpan_dev *wpan_dev)
 {
-	return rdev->ops->del_virtual_intf(&rdev->wpan_phy, wpan_dev);
+	int ret;
+
+	trace_802154_rdev_del_virtual_intf(&rdev->wpan_phy, wpan_dev);
+	ret = rdev->ops->del_virtual_intf(&rdev->wpan_phy, wpan_dev);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_set_channel(struct cfg802154_registered_device *rdev, u8 page, u8 channel)
 {
-	return rdev->ops->set_channel(&rdev->wpan_phy, page, channel);
+	int ret;
+
+	trace_802154_rdev_set_channel(&rdev->wpan_phy, page, channel);
+	ret = rdev->ops->set_channel(&rdev->wpan_phy, page, channel);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_set_cca_mode(struct cfg802154_registered_device *rdev,
 		  const struct wpan_phy_cca *cca)
 {
-	return rdev->ops->set_cca_mode(&rdev->wpan_phy, cca);
+	int ret;
+
+	trace_802154_rdev_set_cca_mode(&rdev->wpan_phy, cca);
+	ret = rdev->ops->set_cca_mode(&rdev->wpan_phy, cca);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_set_pan_id(struct cfg802154_registered_device *rdev,
 		struct wpan_dev *wpan_dev, __le16 pan_id)
 {
-	return rdev->ops->set_pan_id(&rdev->wpan_phy, wpan_dev, pan_id);
+	int ret;
+
+	trace_802154_rdev_set_pan_id(&rdev->wpan_phy, wpan_dev, pan_id);
+	ret = rdev->ops->set_pan_id(&rdev->wpan_phy, wpan_dev, pan_id);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_set_short_addr(struct cfg802154_registered_device *rdev,
 		    struct wpan_dev *wpan_dev, __le16 short_addr)
 {
-	return rdev->ops->set_short_addr(&rdev->wpan_phy, wpan_dev, short_addr);
+	int ret;
+
+	trace_802154_rdev_set_short_addr(&rdev->wpan_phy, wpan_dev, short_addr);
+	ret = rdev->ops->set_short_addr(&rdev->wpan_phy, wpan_dev, short_addr);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_set_backoff_exponent(struct cfg802154_registered_device *rdev,
 			  struct wpan_dev *wpan_dev, u8 min_be, u8 max_be)
 {
-	return rdev->ops->set_backoff_exponent(&rdev->wpan_phy, wpan_dev,
+	int ret;
+
+	trace_802154_rdev_set_backoff_exponent(&rdev->wpan_phy, wpan_dev,
 					       min_be, max_be);
+	ret = rdev->ops->set_backoff_exponent(&rdev->wpan_phy, wpan_dev,
+					      min_be, max_be);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_set_max_csma_backoffs(struct cfg802154_registered_device *rdev,
 			   struct wpan_dev *wpan_dev, u8 max_csma_backoffs)
 {
-	return rdev->ops->set_max_csma_backoffs(&rdev->wpan_phy, wpan_dev,
-						max_csma_backoffs);
+	int ret;
+
+	trace_802154_rdev_set_csma_backoffs(&rdev->wpan_phy, wpan_dev,
+					    max_csma_backoffs);
+	ret = rdev->ops->set_max_csma_backoffs(&rdev->wpan_phy, wpan_dev,
+					       max_csma_backoffs);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_set_max_frame_retries(struct cfg802154_registered_device *rdev,
 			   struct wpan_dev *wpan_dev, s8 max_frame_retries)
 {
-	return rdev->ops->set_max_frame_retries(&rdev->wpan_phy, wpan_dev,
+	int ret;
+
+	trace_802154_rdev_set_max_frame_retries(&rdev->wpan_phy, wpan_dev,
 						max_frame_retries);
+	ret = rdev->ops->set_max_frame_retries(&rdev->wpan_phy, wpan_dev,
+					       max_frame_retries);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 static inline int
 rdev_set_lbt_mode(struct cfg802154_registered_device *rdev,
 		  struct wpan_dev *wpan_dev, bool mode)
 {
-	return rdev->ops->set_lbt_mode(&rdev->wpan_phy, wpan_dev, mode);
+	int ret;
+
+	trace_802154_rdev_set_lbt_mode(&rdev->wpan_phy, wpan_dev, mode);
+	ret = rdev->ops->set_lbt_mode(&rdev->wpan_phy, wpan_dev, mode);
+	trace_802154_rdev_return_int(&rdev->wpan_phy, ret);
+	return ret;
 }
 
 #endif /* __CFG802154_RDEV_OPS */
diff --git a/net/ieee802154/trace.c b/net/ieee802154/trace.c
new file mode 100644
index 0000000..95f997f
--- /dev/null
+++ b/net/ieee802154/trace.c
@@ -0,0 +1,7 @@
+#include <linux/module.h>
+
+#ifndef __CHECKER__
+#define CREATE_TRACE_POINTS
+#include "trace.h"
+
+#endif
diff --git a/net/ieee802154/trace.h b/net/ieee802154/trace.h
new file mode 100644
index 0000000..81a6754
--- /dev/null
+++ b/net/ieee802154/trace.h
@@ -0,0 +1,243 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM cfg802154
+
+#if !defined(__RDEV_CFG802154_OPS_TRACE) || defined(TRACE_HEADER_MULTI_READ)
+#define __RDEV_CFG802154_OPS_TRACE
+
+#include <linux/tracepoint.h>
+
+#include <net/cfg802154.h>
+
+#define MAXNAME		32
+#define WPAN_PHY_ENTRY	__array(char, wpan_phy_name, MAXNAME)
+#define WPAN_PHY_ASSIGN	strlcpy(__entry->wpan_phy_name,	 \
+				wpan_phy_name(wpan_phy), \
+				MAXNAME)
+#define WPAN_PHY_PR_FMT	"%s"
+#define WPAN_PHY_PR_ARG	__entry->wpan_phy_name
+
+#define WPAN_DEV_ENTRY	__field(u32, identifier)
+#define WPAN_DEV_ASSIGN	(__entry->identifier) = (!IS_ERR_OR_NULL(wpan_dev) \
+					 ? wpan_dev->identifier : 0)
+#define WPAN_DEV_PR_FMT	"wpan_dev(%u)"
+#define WPAN_DEV_PR_ARG	(__entry->identifier)
+
+#define WPAN_CCA_ENTRY	__field(enum nl802154_cca_modes, cca_mode) \
+			__field(enum nl802154_cca_opts, cca_opt)
+#define WPAN_CCA_ASSIGN \
+	do {					 \
+		(__entry->cca_mode) = cca->mode; \
+		(__entry->cca_opt) = cca->opt;	 \
+	} while (0)
+#define WPAN_CCA_PR_FMT	"cca_mode: %d, cca_opt: %d"
+#define WPAN_CCA_PR_ARG __entry->cca_mode, __entry->cca_opt
+
+#define BOOL_TO_STR(bo) (bo) ? "true" : "false"
+
+/*************************************************************
+ *			rdev->ops traces		     *
+ *************************************************************/
+
+TRACE_EVENT(802154_rdev_add_virtual_intf,
+	TP_PROTO(struct wpan_phy *wpan_phy, char *name,
+		 enum nl802154_iftype type, __le64 extended_addr),
+	TP_ARGS(wpan_phy, name, type, extended_addr),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		__string(vir_intf_name, name ? name : "<noname>")
+		__field(enum nl802154_iftype, type)
+		__field(__le64, extended_addr)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		__assign_str(vir_intf_name, name ? name : "<noname>");
+		__entry->type = type;
+		__entry->extended_addr = extended_addr;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", virtual intf name: %s, type: %d, ea %llx",
+		  WPAN_PHY_PR_ARG, __get_str(vir_intf_name), __entry->type,
+		  __entry->extended_addr)
+);
+
+TRACE_EVENT(802154_rdev_del_virtual_intf,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev),
+	TP_ARGS(wpan_phy, wpan_dev),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		WPAN_DEV_ENTRY
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		WPAN_DEV_ASSIGN;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT, WPAN_PHY_PR_ARG,
+		  WPAN_DEV_PR_ARG)
+);
+
+TRACE_EVENT(802154_rdev_set_channel,
+	TP_PROTO(struct wpan_phy *wpan_phy, u8 page, u8 channel),
+	TP_ARGS(wpan_phy, page, channel),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		__field(u8, page)
+		__field(u8, channel)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		__entry->page = page;
+		__entry->channel = channel;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", page: %d, channel: %d", WPAN_PHY_PR_ARG,
+		  __entry->page, __entry->channel)
+);
+
+TRACE_EVENT(802154_rdev_set_cca_mode,
+	TP_PROTO(struct wpan_phy *wpan_phy, const struct wpan_phy_cca *cca),
+	TP_ARGS(wpan_phy, cca),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		WPAN_CCA_ENTRY
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		WPAN_CCA_ASSIGN;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_CCA_PR_FMT, WPAN_PHY_PR_ARG,
+		  WPAN_CCA_PR_ARG)
+);
+
+DECLARE_EVENT_CLASS(802154_le16_template,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		 __le16 le16arg),
+	TP_ARGS(wpan_phy, wpan_dev, le16arg),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		WPAN_DEV_ENTRY
+		__field(__le16, le16arg)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		WPAN_DEV_ASSIGN;
+		__entry->le16arg = __le16_to_cpu(le16arg);
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT ", pan id: 0x%04x",
+		  WPAN_PHY_PR_ARG, WPAN_DEV_PR_ARG, __entry->le16arg)
+);
+
+DEFINE_EVENT(802154_le16_template, 802154_rdev_set_pan_id,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		 __le16 le16arg),
+	TP_ARGS(wpan_phy, wpan_dev, le16arg)
+);
+
+DEFINE_EVENT_PRINT(802154_le16_template, 802154_rdev_set_short_addr,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		 __le16 le16arg),
+	TP_ARGS(wpan_phy, wpan_dev, le16arg),
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT ", sa: 0x%04x",
+		  WPAN_PHY_PR_ARG, WPAN_DEV_PR_ARG, __entry->le16arg)
+);
+
+TRACE_EVENT(802154_rdev_set_backoff_exponent,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		 u8 min_be, u8 max_be),
+	TP_ARGS(wpan_phy, wpan_dev, min_be, max_be),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		WPAN_DEV_ENTRY
+		__field(u8, min_be)
+		__field(u8, max_be)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		WPAN_DEV_ASSIGN;
+		__entry->min_be = min_be;
+		__entry->max_be = max_be;
+	),
+
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT
+		  ", min be: %d, max_be: %d", WPAN_PHY_PR_ARG,
+		  WPAN_DEV_PR_ARG, __entry->min_be, __entry->max_be)
+);
+
+TRACE_EVENT(802154_rdev_set_csma_backoffs,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		 u8 max_csma_backoffs),
+	TP_ARGS(wpan_phy, wpan_dev, max_csma_backoffs),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		WPAN_DEV_ENTRY
+		__field(u8, max_csma_backoffs)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		WPAN_DEV_ASSIGN;
+		__entry->max_csma_backoffs = max_csma_backoffs;
+	),
+
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT
+		  ", max csma backoffs: %d", WPAN_PHY_PR_ARG,
+		  WPAN_DEV_PR_ARG, __entry->max_csma_backoffs)
+);
+
+TRACE_EVENT(802154_rdev_set_max_frame_retries,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		 s8 max_frame_retries),
+	TP_ARGS(wpan_phy, wpan_dev, max_frame_retries),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		WPAN_DEV_ENTRY
+		__field(s8, max_frame_retries)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		WPAN_DEV_ASSIGN;
+		__entry->max_frame_retries = max_frame_retries;
+	),
+
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT
+		  ", max frame retries: %d", WPAN_PHY_PR_ARG,
+		  WPAN_DEV_PR_ARG, __entry->max_frame_retries)
+);
+
+TRACE_EVENT(802154_rdev_set_lbt_mode,
+	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
+		 bool mode),
+	TP_ARGS(wpan_phy, wpan_dev, mode),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		WPAN_DEV_ENTRY
+		__field(bool, mode)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		WPAN_DEV_ASSIGN;
+		__entry->mode = mode;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT
+		", lbt mode: %s", WPAN_PHY_PR_ARG,
+		WPAN_DEV_PR_ARG, BOOL_TO_STR(__entry->mode))
+);
+
+TRACE_EVENT(802154_rdev_return_int,
+	TP_PROTO(struct wpan_phy *wpan_phy, int ret),
+	TP_ARGS(wpan_phy, ret),
+	TP_STRUCT__entry(
+		WPAN_PHY_ENTRY
+		__field(int, ret)
+	),
+	TP_fast_assign(
+		WPAN_PHY_ASSIGN;
+		__entry->ret = ret;
+	),
+	TP_printk(WPAN_PHY_PR_FMT ", returned: %d", WPAN_PHY_PR_ARG,
+		  __entry->ret)
+);
+
+#endif /* !__RDEV_CFG802154_OPS_TRACE || TRACE_HEADER_MULTI_READ */
+
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH .
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE trace
+#include <trace/define_trace.h>
-- 
2.1.4

Re: [PATCH] ieee802154: Add trace events for rdev->ops

[Alexander Aring]

Hi,

On Sun, Apr 12, 2015 at 02:07:33PM +0200, Guido Günther wrote:
> This is based on the rdev->ops tracing code from net/wireless/.
> 

this patch looks good. I tested it with trace-cmd:

1. trace-cmd record -e cfg802154:802154_rdev_set_pan_id

2. iwpan dev wpan0 set pan_id 0xbeaa

3. trace-cmd report

result:

version = 6
cpus=1
           iwpan-1266  [000]   892.921639: 802154_rdev_set_pan_id: phy0, wpan_dev(1), pan id: 0xbeaa

also test it with channel and it works pretty well. (I am not a
trace-cmd export at the moment, my previous use-case was enough to use
the raw ftrace interface).


I looked again for the byteorder handling and detected that trace-cmd has
a cfg802154 plugin [0]. It looks like the plugin is there, to doing
byteorder translation in userspace instead kernelspace.

Looking into "__le16_to_cpup" stuff in wireless trace.h show something
like [1].

Do you think we can do this in the same way? Then we need some
plugin_cfg802154 in trace-cmd tool.

- Alex

[0] http://git.kernel.org/cgit/linux/kernel/git/rostedt/trace-cmd.git/tree/plugin_cfg80211.c
[1] http://lxr.free-electrons.com/source/net/wireless/trace.h?v=3.19#L2160

Re: [PATCH] ieee802154: Add trace events for rdev->ops

[Alexander Aring]

On Sun, Apr 12, 2015 at 02:07:33PM +0200, Guido Günther wrote:
...
> +DECLARE_EVENT_CLASS(802154_le16_template,
> +	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
> +		 __le16 le16arg),
> +	TP_ARGS(wpan_phy, wpan_dev, le16arg),
> +	TP_STRUCT__entry(
> +		WPAN_PHY_ENTRY
> +		WPAN_DEV_ENTRY
> +		__field(__le16, le16arg)
> +	),
> +	TP_fast_assign(
> +		WPAN_PHY_ASSIGN;
> +		WPAN_DEV_ASSIGN;
> +		__entry->le16arg = __le16_to_cpu(le16arg);

When "__entry->le16arg" is type of "__le16" which is given by
"__field(__le16, le16arg)". Then translation of __le16_to_cpu is here
wrong because __le16_to_cpu return u16.

> +	),
> +	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT ", pan id: 0x%04x",
> +		  WPAN_PHY_PR_ARG, WPAN_DEV_PR_ARG, __entry->le16arg)

When then we need to do the translation here. The format string requires
a host byte-order type. So it's __le16_to_cpu(__entry->le16arg), I don't
know why sparse (the C=2) doesn't report about that.

This is like wireless it does at [0], but with a pointer. When wireless
do that in kernelspace, then I have no idea why there is a cfg80211
plugin which doing byte order translation at [1]. I think this not for
parsing the argument, it's for correct handling of handling the display
of __le16_to_cpup (We use __le16_to_cpu here).

Then we need a cfg802154 plugin in trace_cmd for handling "__le16_to_cpu"
correct.

- Alex

[0] http://lxr.free-electrons.com/source/net/wireless/trace.h?v=3.19#L2160
[1] http://git.kernel.org/cgit/linux/kernel/git/rostedt/trace-cmd.git/tree/plugin_cfg80211.c

Re: [PATCH] ieee802154: Add trace events for rdev->ops

[Alexander Aring]

On Fri, Apr 17, 2015 at 12:03:55PM +0200, Alexander Aring wrote:
> Hi,
> 
> On Sun, Apr 12, 2015 at 02:07:33PM +0200, Guido Günther wrote:
> > This is based on the rdev->ops tracing code from net/wireless/.
> > 
> 
> this patch looks good. I tested it with trace-cmd:
> 
> 1. trace-cmd record -e cfg802154:802154_rdev_set_pan_id
> 
> 2. iwpan dev wpan0 set pan_id 0xbeaa
> 
> 3. trace-cmd report
> 
> result:
> 
> version = 6
> cpus=1
>            iwpan-1266  [000]   892.921639: 802154_rdev_set_pan_id: phy0, wpan_dev(1), pan id: 0xbeaa
> 
> also test it with channel and it works pretty well. (I am not a
> trace-cmd export at the moment, my previous use-case was enough to use
> the raw ftrace interface).
> 
> 
> I looked again for the byteorder handling and detected that trace-cmd has
> a cfg802154 plugin [0]. It looks like the plugin is there, to doing

s/cfg802154/cfg80211/

> byteorder translation in userspace instead kernelspace.
> 
> Looking into "__le16_to_cpup" stuff in wireless trace.h show something
> like [1].
> 
> Do you think we can do this in the same way? Then we need some
> plugin_cfg802154 in trace-cmd tool.
> 

Please read the other mail which I also mark an issue and we should move
the byteorder translation into the format string argument. I suppose, what
wireless do with the cfg80211 plugin is to printout the handling of function
__le16_to_cpup (in our case __le16_to_cpu) more correct. So we need
both, translation byteorder in kernelspace (for the __le16 parameter)
and (to show the __le16_to_cpu in userspace correct).

- Alex

Re: [PATCH] ieee802154: Add trace events for rdev->ops

[Varka Bhadram]

On 04/12/2015 05:37 PM, Guido Günther wrote:
> This is based on the rdev->ops tracing code from net/wireless/.

Instead of writing the above commit message its better to write
what exactly this code is for and how can we use this from
the user space (if possible with commands).

If you want to write your above commit message put it in trace.c/h file 

> Signed-off-by: Guido Günther <agx@sigxcpu.org>
> ---
>  net/ieee802154/Makefile   |   4 +-
>  net/ieee802154/rdev-ops.h |  77 ++++++++++++---
>  net/ieee802154/trace.c    |   7 ++
>  net/ieee802154/trace.h    | 243 ++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 319 insertions(+), 12 deletions(-)
>  create mode 100644 net/ieee802154/trace.c
>  create mode 100644 net/ieee802154/trace.h
>
> diff --git a/net/ieee802154/Makefile b/net/ieee802154/Makefile
> index 05dab29..4adfd4d 100644
> --- a/net/ieee802154/Makefile
> +++ b/net/ieee802154/Makefile
> @@ -3,7 +3,9 @@ obj-$(CONFIG_IEEE802154_SOCKET) += ieee802154_socket.o
>  obj-y += 6lowpan/
>  
>  ieee802154-y := netlink.o nl-mac.o nl-phy.o nl_policy.o core.o \
> -                header_ops.o sysfs.o nl802154.o
> +                header_ops.o sysfs.o nl802154.o trace.o
>  ieee802154_socket-y := socket.o
>  
> +CFLAGS_trace.o := -I$(src)

What is this for..? 


-- 
Varka Bhadram

Re: [PATCH] ieee802154: Add trace events for rdev->ops

[Guido Günther]

On Fri, Apr 17, 2015 at 05:19:58PM +0530, Varka Bhadram wrote:
> On 04/12/2015 05:37 PM, Guido Günther wrote:
> > This is based on the rdev->ops tracing code from net/wireless/.
> 
> Instead of writing the above commit message its better to write
> what exactly this code is for and how can we use this from
> the user space (if possible with commands).

Adjusted in v2.

> 
> If you want to write your above commit message put it in trace.c/h file 
> 
> > Signed-off-by: Guido Günther <agx@sigxcpu.org>
> > ---
> >  net/ieee802154/Makefile   |   4 +-
> >  net/ieee802154/rdev-ops.h |  77 ++++++++++++---
> >  net/ieee802154/trace.c    |   7 ++
> >  net/ieee802154/trace.h    | 243 ++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 319 insertions(+), 12 deletions(-)
> >  create mode 100644 net/ieee802154/trace.c
> >  create mode 100644 net/ieee802154/trace.h
> >
> > diff --git a/net/ieee802154/Makefile b/net/ieee802154/Makefile
> > index 05dab29..4adfd4d 100644
> > --- a/net/ieee802154/Makefile
> > +++ b/net/ieee802154/Makefile
> > @@ -3,7 +3,9 @@ obj-$(CONFIG_IEEE802154_SOCKET) += ieee802154_socket.o
> >  obj-y += 6lowpan/
> >  
> >  ieee802154-y := netlink.o nl-mac.o nl-phy.o nl_policy.o core.o \
> > -                header_ops.o sysfs.o nl802154.o
> > +                header_ops.o sysfs.o nl802154.o trace.o
> >  ieee802154_socket-y := socket.o
> >  
> > +CFLAGS_trace.o := -I$(src)
> 
> What is this for..? 

So trace.h is found due to the ftrace magic. See
samples/trace_events/Makefile.

Cheers,
 -- Guido

Re: [PATCH] ieee802154: Add trace events for rdev->ops

[Guido Günther]

Hi Alexander,
On Fri, Apr 17, 2015 at 12:27:53PM +0200, Alexander Aring wrote:
> On Sun, Apr 12, 2015 at 02:07:33PM +0200, Guido Günther wrote:
> ...
> > +DECLARE_EVENT_CLASS(802154_le16_template,
> > +	TP_PROTO(struct wpan_phy *wpan_phy, struct wpan_dev *wpan_dev,
> > +		 __le16 le16arg),
> > +	TP_ARGS(wpan_phy, wpan_dev, le16arg),
> > +	TP_STRUCT__entry(
> > +		WPAN_PHY_ENTRY
> > +		WPAN_DEV_ENTRY
> > +		__field(__le16, le16arg)
> > +	),
> > +	TP_fast_assign(
> > +		WPAN_PHY_ASSIGN;
> > +		WPAN_DEV_ASSIGN;
> > +		__entry->le16arg = __le16_to_cpu(le16arg);
> 
> When "__entry->le16arg" is type of "__le16" which is given by
> "__field(__le16, le16arg)". Then translation of __le16_to_cpu is here
> wrong because __le16_to_cpu return u16.
> 
> > +	),
> > +	TP_printk(WPAN_PHY_PR_FMT ", " WPAN_DEV_PR_FMT ", pan id: 0x%04x",
> > +		  WPAN_PHY_PR_ARG, WPAN_DEV_PR_ARG, __entry->le16arg)
> 
> When then we need to do the translation here. The format string requires
> a host byte-order type. So it's __le16_to_cpu(__entry->le16arg), I don't
> know why sparse (the C=2) doesn't report about that.
> 
> This is like wireless it does at [0], but with a pointer. When wireless
> do that in kernelspace, then I have no idea why there is a cfg80211
> plugin which doing byte order translation at [1]. I think this not for
> parsing the argument, it's for correct handling of handling the display
> of __le16_to_cpup (We use __le16_to_cpu here).

Agreed. If we want to keep this le until it's printed we should delay conversion
up to the format string.

> Then we need a cfg802154 plugin in trace_cmd for handling "__le16_to_cpu"
> correct.

I think this is only needed you don't use the kernel's built in printing
so this could be done as a follow up once this is in.
Cheers,
 -- Guido

Re: [PATCH] ieee802154: Add trace events for rdev->ops

[Alexander Aring]

On Mon, Apr 20, 2015 at 08:35:55PM +0200, Guido Günther wrote:
> On Fri, Apr 17, 2015 at 05:19:58PM +0530, Varka Bhadram wrote:
> > On 04/12/2015 05:37 PM, Guido Günther wrote:
> > > This is based on the rdev->ops tracing code from net/wireless/.
> > 
> > Instead of writing the above commit message its better to write
> > what exactly this code is for and how can we use this from
> > the user space (if possible with commands).
> 
> Adjusted in v2.
> 

or you can steal it from 14e8a3c47e808772a5ba8118ef1f9a8d604dbbe5
("cfg80211: add tracing to rdev-ops").

Or refer this commit in your commit message, but keeping style then
$ID ("$MSG").

- Alex