Re: [Linuxptp-devel] [PATCH 0/5] Add minimal gPTP (802.1 AS) support

[Keller, J. E <jac...@in...>]

> -----Original Message-----
> From: Richard Cochran [mailto:ric...@gm...]
> Sent: Saturday, July 14, 2012 9:57 AM
> To: Delio Brignoli
> Cc: lin...@li...
> Subject: Re: [Linuxptp-devel] [PATCH 0/5] Add minimal gPTP (802.1 AS)
> support
> 
> On Sat, Jul 14, 2012 at 12:53:04PM +0200, Delio Brignoli wrote:
> > Hello Richard,
> >
> > On Jul 14, 2012, at 11:11 AM, Richard Cochran wrote:
> > > On Fri, Jul 13, 2012 at 01:20:44PM +0200, Delio Brignoli wrote:
> > >> Hello Richard,
> > >
> > > Delio, thanks for getting the ball rolling with this. I know that
> > > there is some interest among other ptp4l users in getting gPTP
> > > supported. However, I would like to take a different approach than
> > > what you have in this patch series, and so I hope you will bear with
> > > me.
> > >
> > > First off, I would like to get some grey areas surrounding gPTP
> > > clarified before really diving into this. Despite what you might
> > > have read, the AVB standard is really different from 1588 on a
> > > number of important points.
> >
> > My knowledge of 1588 is very limited, I have noticed they are
> > different enough in some places, for instance, mac addresses used for
> > signaling and how some parameters are calculated. However, I would
> > like to know what you think these important points are so they can be
> discussed.
> 
> I summarized what I think are the important points in the list in my other
> mail. In general, I think you can immediately recognize that the models
> presented in AS Chapter 10 bear little resemblance to the code in ptp4l.
> 
> > > The gPTP standard seems to forbid adjusting the local clock! That
> > > would make the hardware features like PPS output useless. So one
> > > question to be answered is whether a slave clock, for example, is
> > > allowed to adjust its clock and still conform to the standard.
> >
> > I didn't spot this. Can you point me to the relevant section of the AS
> standard?
> 
> See Chapter 10.
> 
> > >> This series of patches add minimal support for gPTP (802.1 AS) and
> > >> a command line switch to select gPTP mode. 802.1 AS support is
> > >> incomplete: only slave mode is partially implemented.
> > >
> > > So, does this actually work as a gPTP slave?
> >
> > Yes, al least with the test hardware I have at the moment.
> 
> Well, the fact that it does work for you is probably the bottom line.
> 
> So, you don't need TLV parsing, cumulativeScaledRateOffset, or the
> alternate BCM?
> 
> > > Will you ever need gPTP grandmaster support?
> >
> > Probably yes, I just needed to get gPTP slave working so we can
> > continue with testing/development.
> >
> > > The standard seems to strictly enforce the idea of having at least
> > > three separate clocks, namely ClockMaster, ClockSlave, and
> > > LocalClock.*
> > >
> > > The whole idea seems to be founded on the idea that you have a
> > > non-adjustable LocalClock and update the clocks using a system tick.
> > > If you really implement this, then you must place part of the code
> > > into the operating system (or not use an OS at all). Clearly that
> > > model does fit here at all, neither to the Linux PHC code nor to the
> > > time synchronization hardware itself.
> > >
> > > * A fourth, ClockSource, corresponds to and external source like a
> > > PPS from a GPS.
> > >
> > > I studied 802.1AS-2011 a bit, and I think that the ptp4l program
> > > could also function as a "time-aware end station" (but not as a
> > > bridge). But I wonder whether there will be conformance issues, for
> > > example when we have ClockMaster = ClockSlave = LocalClock.
> > >
> > > What do you think about all of that?
> >
> > I was under the impression that the distinction between the three clocks
> is one of role.
> > I need to go back and give another look at the standard.
> 
> Again, chapter 10.
> 
> I only worry about the PICS. They are quite specific, but maybe you don't
> care about that.
> 
> > >> Patch 4/5 allows to  cross-compile without having to alter the
> > >> makefile. Patch 5/5 is optional but applies cleanly only if the
> > >> previous patches are applied.
> > >
> > > And, now to the patches...
> > >
> > >> Delio Brignoli (3):
> > >>  Add TransportSpecific field value for IEEE802.1AS (gPTP) to msg.h
> > >
> > > All of these patches...
> > >
> > >>  Define CC conditionally in makefile     Append to (rather than
> > >>    override) the compilation environment  Clarify usage string for
> > >> command line options that must appear before
> > >>    an interface option
> > >>
> > >> Eliot Blennerhassett (2):
> > >>  Additional debug output
> > >
> > > ... are really not about adding gPTP, and so do not belong in the
> > > same series.
> >
> > ack
> >
> > > I can say right now that I am not too keen to add lots more
> > > debugging messages to the code, but if you really think they are
> > > important, please post them separately.
> >
> > I will split the debug out of the series and asses its importance vs
> noisiness.
> 
> Sound good.
> 
> Thanks,
> Richard
> 

I spent a fair amount of time reading the standards. I didn't see anything about clocks not being adjustable. I read it the opposite (that you have to adjust clocks) but it was fairly confusing. I think we need TVL support if we want to be able to be a master (not just slave-only). I do think we should implement the rate adjustment features, but I wasn't sure how to go about that.

I agree with Richard that we should do best to treat gPTP as a profile, rather than as a config flag to enable gPTP mode.

I am very interested to know what hardware specifically you tested this with, Delio? I was previously working on this, and I couldn't get it to work with the AVG ethernet bridge I have. When I get some spare time I will take a look at running this patch set against that bridge.

- Jake