On 3/4/20 1:36 PM, Johannes Berg wrote: > On Wed, 2020-03-04 at 13:32 +0100, Markus Theil wrote: >> Yes, its trace-cmd output. Without this patch, the print fmt in the >> trace data file looks like this: >> print fmt: "%s, netdev:%s(%d), %pM, proto: 0x%x, unencrypted: %s", >> REC->wiphy_name, REC->name, REC->ifindex, (REC->dest), >> (__u16)__builtin_bswap16((__u16)(( __u16)(__be16)(REC->proto))), >> (REC->unencrypted) ? "true" : "false" >> >> With the patch, the builtin_bswap16 does not get placed there: > Sure. > > But trace-cmd has infrastructure to handle such "function calls" in the > output format, so it should be able to handle this pretty easily. > > So really it's mostly a presentation issue, and having the data in big > endian when it's that way over the air etc. IMHO does make sense. Sure. Should cfg80211_rx_control_port then also adopt this behavior? It currently does the conversion in the way I changed cfg80211_tx_control port to. IMHO, as long as the trace events are just printed, it is cleaner to do the endiannes conversion already in the kernel. TRACE_EVENT(cfg80211_rx_control_port, ... __entry->proto = be16_to_cpu(skb->protocol); __entry->unencrypted = unencrypted; ), TP_printk(NETDEV_PR_FMT ", len=%d, " MAC_PR_FMT ", proto: 0x%x, unencrypted: %s", NETDEV_PR_ARG, __entry->len, MAC_PR_ARG(from), __entry->proto, BOOL_TO_STR(__entry->unencrypted))
