Gen-ART LC review of draft-ietf-tictoc-ptp-mib-08

["Peter Yee" <[e-mail]>]

I am the assigned Gen-ART reviewer for this draft.  The General Area Review
Team (Gen-ART) reviews all IETF documents being processed by the IESG for
the IETF Chair.  Please treat these comments just like any other last call
comment.  For background on Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>

Document: draft-ietf-tictoc-ptp-mib-08
Reviewer: Peter Yee
Review Date: March 8, 2016
IETF LC End Date: March 8, 2016
IESG Telechat date: March 17, 2016

Summary: This draft is basically ready for publication as a Standards Track
RFC, but has nits and a couple of minor issues that should be
fixed/considered before publication. [Ready with issues]

This draft defines an SMIv2 MIB for use with the Precision Time Protocol v2
(IEEE Std. 1588-2008).  I do not have access to this standard, so my review
primarily addresses nits within the module and not whether the module itself
is a fair representation of a MIB suitable for use with PTP v2.

Major issues: None

Minor issues:

Given that the draft seems to quote a fair amount of text (at least 5 pages)
from IEEE Std. 1588-2008 in the definitions section if not elsewhere, it
might be good to make sure that there are no copyright issues.

Many of the object descriptions use identical text.  While that's helpful
from the direct lookup point of view, it doesn't help to distinguish these
items from others of their ilk.  I'm not sure if this matters, but perhaps
enhancing the descriptions in their contexts might make them more useful.

Nits:

General:

Replace all occurrences of "create logical group" with "create a logical
group".

Watch the case and spacing of object names in their descriptions.  Sometime
there's a space between two words, sometimes not.  Some words are
capitalized in one place and not another.  Use of camel case conventions
comes and goes.  It's probably best to treat the object names as English
words in the descriptions, separating words with spaces and generally
writing them in lower case.  Sure, the inconsistency doesn't hurt a lot, but
it ends up looking a wee bit sloppy.  The RFC Editor may not be sure how to
handle the inconsistency as some word combinations might appear to be
technical MIB terms that should be left as is.

Drop the draft name from the center, top header.  Generally a shortened form
of the draft title is placed here.  Perhaps you could use "PTPv2 MIB"?

I'll leave the serial comma be.  It appears in some places and not others.
I like them, but there are enough missing ones that I'm just going to let it
slide. ;-)

When citing the official name of IEEE 1588-2008, use a lower case "a" before
"Precision".  That aligns with IEEE's title.

The authors' addresses in Section 9 have way too many punctuation marks in
them.  A few less commas would go a long way.  Maybe change some into serial
commas for sprinkling elsewhere in the document? ;-)

Specific:

Page 2, Section 1, 1st paragraph, 2nd sentence: delete "the" before
"ordinary".  Make "ordinary clock" and "transparent clock" plural.  Insert a
comma and "and" after "transparent clocks".

Page 3, Section 1.1, 1st paragraph, 2nd sentence: append a comma after
"e.g.".

Page 5, 1st bullet item: delete the comma after "STD 62".

Page 5, 2nd bullet item, 2nd sentence: insert "is" before "described".

Page 5, 3rd bullet item, 2nd sentence: insert "is" before "described".

Page 5, 3rd bullet item, last sentence: insert "is" before "described".

Page 7, MIB Description, 1st paragraph, 1st sentence: change "MIB is to
support" to "MIB supports".

Page 8, Acronyms, EUI: delete the period after the acronym expansion.

Page 12, PTP node definition: insert "A" before the sentence.

Page 16, ClockIdentity description: the term "MAC-48" has not been defined
or previously referenced in the draft.  It's also not quite an exact synonym
for EUI-48, so perhaps it should be defined.

Page 17, ClockMechanismType description: replace "End to End" with
"end-to-end".  Replace "peer to peer" with "peer-to-peer".

Page 19, ClockPortTransportTypeAddress description, 2nd sentence: change
"Transport" to "transport".

Page 19, ClockPortTransportTypeAddress description, 4th sentence: insert
"an" before "address".  Change "and" to "or".

Page 20, ClockQualityAccuracyType description: change "section" to
"sections".  Insert "of" before "RFC 2578", which should be placed in square
brackets.  Change "to start with" to "starting at".

Page 23, ClockStateType description: insert "a" before "PTP".

Page 24, Holdover state text: change "ptp" to "PTP.  Change "FREERUN" to
"Freerun".

Page 25, ClockTimeSourceType description: change "section" to "sections".
Insert "of" before "RFC 2578", which should be placed in square brackets.
Change "to start with" to "starting at".

Page 26, ClockTimeSourceType reference: change "Section" to "Sections".

Page 32, ptpbaseClockCurrentDSOffsetFromMaster and
ptpbaseClockCurrentDSMeanPathDelay references: the indentation looks a bit
off here.

Page 32, ptpbaseClockCurrentDSMeanPathDelay description: change "measure" to
"measured".

Page 35, ptpbaseClockParentDSOffset description (near top of page): change
"clocks" to "clock's".

Page 35, ptpbaseClockParentDSClockPhChRate description: change "clocks" to
"clock's".

Page 41, ptpbaseClockRunningState description: insert "a" before "PTP".
Append a comma after "engine".

Page 42, top: Rather than duplicate the state descriptions information here,
why not merely point back to ClockStateType?  Btw, PTP engine hasn't been
defined.

Page 42, Holdover state, 1st sentence: if you do not remove this paragraph
as suggested in the previous comment, change "ptp" to "PTP".

Page 42, ptpbaseClockRunningPacketsSent description: replace "packet Unicast
and multicast" with "unicast and multicast packets".

Page 42, ptpbaseClockRunningPacketsReceived description: replace "packet
Unicast and multicast" with "unicast and multicast packets".

Page 44, ptpbaseClockTimePropertiesDSCurrentUTCOffsetValid description:
insert "the" before "UTC".

Page 45, ptpbaseClockTimePropertiesDSCurrentUTCOffset description (at top of
page): insert "the" before "UTC".

Page 49, ptpbaseClockTransDefaultDSDelay description (at top of page):
change "shall be" to "of".  Change "If" to "if".

Page 56, ptpbaseClockPortRunningTable description: should that be "a
dataset" or "datasets".  Plain "dataset" doesn't work.

Page 57, ptpbaseClockPortRunningEntry description: change "runing" to
"running" unless Viking writing is involved.

Page 58, ptpbaseClockPortRunningState description: you probably don't need
to include the states here since they are already found in the definition of
ClockPortState.

Page 60, ptpbaseClockPortRunningEncapsulationType description: change "eg."
to "e.g.,".

Page 60, ptpbaseClockPortRunningRxMode description: change "specifie" to
"specifies".

Page 63, ptpbaseClockPortTransDSPeerMeanPathDelay description: delete the
parentheses and append a comma after PTP.  On the following page, change
"value is" to "value of".

Page 64, ptpbaseClockPortAssociateTable description: change "which" to
"that".

Page 75, ptpbaseMIBClockPortTransDSGroup description: does " TransparentDS
Dataset" contain a redundancy?

Page 76, 4th paragraph, 1st sentence: delete "the" before "implementers".
Delete "as".

Page 76, 5th paragraph, 1st sentence: change "Further" to "Furthermore".