On Wed, 2017-12-20 at 18:22 +0200, Kalle Valo wrote:
> Luca Coelho <luca@xxxxxxxxx> writes:
>
> > On Wed, 2017-12-20 at 17:49 +0200, Kalle Valo wrote:
> > > Luca Coelho <luca@xxxxxxxxx> writes:
> > >
> > > > From: Mordechay Goodstein <mordechay.goodstein@xxxxxxxxx>
> > > >
> > > > This helps matching tx cmd with other trace events, like
> > > > net_dev_xmit
> > > > and net_dev_queue etc.
> > > >
> > > > Signed-off-by: Mordechay Goodstein <mordechay.goodstein@intel.c
> > > > om>
> > > > Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx>
> > >
> > > [...]
> > >
> > > > @@ -120,9 +121,9 @@ TRACE_EVENT(iwlwifi_dev_tx,
> > > > __get_dynamic_array(buf1
> > > > ),
> > > > skb->len - hdr_len);
> > > > ),
> > > > - TP_printk("[%s] TX %.2x (%zu bytes)",
> > > > + TP_printk("[%s] TX %.2x (%zu bytes) skbaddr=%p",
> > > > __get_str(dev), ((u8
> > > > *)__get_dynamic_array(buf0))[0],
> > > > - __entry->framelen)
> > > > + __entry->framelen, __entry->skbaddr)
> > > > );
> > >
> > > Just so that you know there has been a lot of changes recently
> > > about
> > > %p,
> > > I think the addresses are nowadays hashed due to security
> > > reasons. No
> > > idea if it afffects tracing events, but just wanted to point out
> > > this.
> >
> > Yeah, I've seen some discussions and I even commented that in our
> > internal review. But I reckoned that if they are all hash, it
> > should
> > still be possible to match them.
>
> Good point, matching is of course possible with hashes.
>
> > It would be good to check if that's done also for traces...
>
> Yeah, please let me know if you find out that.
Moti investigated this a bit further. This is what he concluded:
---8<---
Hi from my checking in the trace code it looks like
Tracepoins infrastructure copies as is the skbaddr to the cyclic buffer
User space tools get the content as is, from the cyclic buffer but you
need to be root to read it. The use of tp_printk is for dumping the
output without userspace tools. (trace-cmd)
It's when you do cat trace in debugfs.
Like
>> cd /sys/kernel/debug/tracing
>> [enable the tracing points]
>> cat trace
It calls
kernel/trace/trace_seq.c :void trace_seq_printf
lib/seq_buf.c: int seq_buf_vprintf
lib/vsprintf.c : int vsnprintf
so if printk is changing to print hash (%p) also tp_printk would print
the hash https://patchwork.kernel.org/patch/10031785/ the changed are
done on vsnprintf but the user that has access to the cyclic buffer
would still have the ability to get the address itself.
--->8---
Thanks for checking Moti!
--
Cheers,
Luca.
