Re: [PATCH v2] tracing/synthetic: use union instead of casts

[Steven Rostedt]

On Mon, 14 Aug 2023 11:34:20 +0000
David Laight <David.Laight@xxxxxxxxxx> wrote:

> > No need to create a structure around a single element union. Also, I would
> > like to name it for what it is for.
> > 
> > union trace_synth_field {
> > 	u8				as_u8;
> > 	u16				as_u16;
> > 	u32				as_u32;
> > 	u64				as_u64;
> > 	struct trace_dynamic_info	as_dynamic;
> > };
> > 
> > Other than that, the patch looks good. Although I still need to test it.  
> 
> I was wondering if you need the u8 and u16 members at all?
> Can't the values just be treated as 32bit?
> Both char and short aren't really 'proper' arithmetic types.

Not sure what you mean by "arithmatic types".

If you do something like:

 ~# echo 's:skb u16 protocol; u64 delta;' >> /sys/kernel/tracing/dynamic_events
 ~# echo 'hist:keys=skbaddr:ts=common_timestamp.usecs' >> /sys/kernel/tracing/events/net/netif_receive_skb/trigger
 ~# echo 'hist:keys=skbaddr:delta=common_timestamp.usecs-$ts:onmatch(net.netif_receive_skb).trace(skb,protocol,$delta)' >> /sys/kernel/tracing/events/skb/kfree_skb/trigger

Where it records the protocol as u16, I'm guessing that if we didn't use
this union, it might break on BIG_ENDIAN machines.

For reference, the kfree_skb event looks like:

system: skb
name: kfree_skb
ID: 1735
format:
        field:unsigned short common_type;       offset:0;       size:2; signed:0;
        field:unsigned char common_flags;       offset:2;       size:1; signed:0;
        field:unsigned char common_preempt_count;       offset:3;       size:1; signed:0;
        field:int common_pid;   offset:4;       size:4; signed:1;

        field:void * skbaddr;   offset:8;       size:8; signed:0;
        field:void * location;  offset:16;      size:8; signed:0;
        field:unsigned short protocol;  offset:24;      size:2; signed:0;
        field:enum skb_drop_reason reason;      offset:28;      size:4; signed:0;

-- Steve