Re: [PATCH v27 1/5] trace-cmd: Add timestamp synchronization per vCPU

[Tzvetomir Stoyanov <tz.stoyanov@gmail.com>]

On Wed, Feb 3, 2021 at 10:20 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon,  1 Feb 2021 18:03:35 +0200
> "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:
>
> > --- a/lib/trace-cmd/trace-timesync.c
> > +++ b/lib/trace-cmd/trace-timesync.c
> > @@ -22,6 +22,8 @@
> >  #include "event-utils.h"
> >  #include "trace-tsync-local.h"
> >
> > +typedef __be16 be16;
>
> I'm curious to why you added this, because I have a machine that doesn't
> define __be16. I'm guessing it comes from "#include <linux/limits.h>", was
> there a reason to use that?
>
I added it to be consistent with the other messages, defined in trace-msg.c -
for signed / unsigned 32bit and 64bit fields. The other reason is that I used
to define packet's structure in that way in my previous job - to be
clear exactly
how many bits are exchanged over the network.
I agree that that is useless here and will replace it with an unsigned short.

> I really don't see the difference between __be16 and unsigned short. I
> think it's mostly used for static parsers. Perhaps its best to use __u16,
> or "unsigned short" to make sure it compiles on older systems.
>
> -- Steve
>
>
> > +
> >  struct tsync_proto {
> >       struct tsync_proto *next;
> >       char proto_name[TRACECMD_TSYNC_PNAME_LENGTH];
> > @@ -34,9 +36,13 @@ struct tsync_proto {
> >       int (*clock_sync_free)(struct tracecmd_time_sync *clock_context);
> >       int (*clock_sync_calc)(struct tracecmd_time_sync *clock_context,
> >                              long long *offset, long long *scaling,
> > -                            long long *timestamp);
> > +                            long long *timestamp, unsigned int cpu);
> >  };
> >
> > +struct tsync_probe_request_msg {
> > +     be16    cpu;
> > +} __packed;
>


-- 
Tzvetomir (Ceco) Stoyanov
VMware Open Source Technology Center