Re: Gen-ART Telechat review of draft-ietf-tictoc-ptp-mib-08

[vinays <[e-mail]>]

Sent too soon. Please see inline at VC2.

Thanks,
-Vinay.


On 4/20/16 10:41 PM, vinays wrote:
>
>
> Have addressed the MIB related comments below. The remaining will be 
> addressed
> by Tim in the latest revision to be submitted tomorrow morning UK time.
>
> Please see inline at VC.
>
>
> On 4/17/16 4:18 PM, Peter Yee wrote:
>> 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 wait for direction from your document shepherd or
>> AD before posting a new version of the draft.
>>
>> For more information, 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: April 21, 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.
>>
>> The following review was originally submitted for the LC (see
>> http://www.ietf.org/mail-archive/web/gen-art/current/msg13014.html). As
>> no update has been issued for the draft, the content of that review is
>> copied below.
>>
>> 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".
>
> VC: done.
>
>>
>> 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".
> VC: done.
>
>> Page 8, Acronyms, EUI: delete the period after the acronym expansion.
>
> VC: done.
>
>
>>
>> Page 12, PTP node definition: insert "A" before the sentence.
>
> VC: done.
>
>>
>> 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.
>
> VC: done. Have added MAC-48 in the acronym section.
>
>>
>> Page 17, ClockMechanismType description: replace "End to End” with
>> "end-to-end".  Replace "peer to peer" with "peer-to-peer".
>
> VC: done.
>
>
>>
>> Page 19, ClockPortTransportTypeAddress description, 2nd sentence: change
>> "Transport" to "transport".
>
> VC: done.
>
>
>>
>> Page 19, ClockPortTransportTypeAddress description, 4th sentence: insert
>> "an" before "address".  Change "and" to "or".
>
> VC: done.
>
>
>>
>> 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".
>
> VC: done.
>
>>
>> Page 23, ClockStateType description: insert "a" before "PTP".
>
> VC: done.
>>
>> Page 24, Holdover state text: change "ptp" to "PTP.  Change "FREERUN” to
>> "Freerun".
>>

VC2: done.


>> 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".

VC2: done.

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

VC2: done.


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

VC2: done.


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

VC2: done.


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

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

VC2: done.


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

VC2: done.


>>
>> 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.

VC2: done.

>>
>> 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?

VC2: All the above are done as well. Thanks,
-Vinay.



>>
>> Page 76, 4th paragraph, 1st sentence: delete "the" before 
>> "implementers”.
>> Delete "as".
>>
>> Page 76, 5th paragraph, 1st sentence: change "Further" to "Furthermore".
>>
>>
>