Subject: Re: [MIB-DOCTORS] MIB Doctor Review for draft-ietf-tictoc-ptp-mib-08.txt

From: vinays <[email-masked]>
Date: Wed, 20 Apr 2016 12:25:33 -0400

Thanks Dan for the valuable review and comments.

Please see below for what I have addressed in the MIB module. We will 
make the few changes
needed in the draft also as mentioned.

1. The current compile line lists only 2 errors, which are ok since they 
need the IANA number:

smilint -s -e -l 5 ./PTPBASE-MIB

./PTPBASE-MIB:417: [2] {bad-identifier-case} `XXX' should start with a 
lower case letter
./PTPBASE-MIB:417: [2] {object-identifier-not-prefix} Object identifier 
element `XXX' name only allowed as first element

2.  Have replaced the TEXTUAL CONVENTIONS as you suggested to add 'Ptp' 
prefix.

3. Have changed PtpClockIdentity to a SYNTAX OCTET STRING (SIZE(8))

4. Will replace with SMIv2 and SMIv1.

5. Yes, Intention is to refer to PTPv2 when using PTP. We will clarify this.

6. Have replaced with 'MIB module'.

7. We will address in the draft.

8.  If it is ok, we would prefer to leave this as is. Thanks.

9.  Same for this as well, Thanks

10. Its not an enumeration since there are ranges in the types.

11. Its not an enumeration since there are ranges in the types.

12. Am not sure what we need to change here, Please let us know. Will 
add as you need.

13. Would prefer to leave this as is, Thanks.

14. It can change following any configuration changes, hence would leave 
it as is. Thanks

15. Please help what should be mentioned here? Thanks

16. Same applies here as well since it can change per configuration.

17.  Will clarify in the draft.

18. This is the normal method used to extend for new types and hence 
have used it. Please suggest what needs to be done. Thanks.

19. we will make changes in the draft,

Thanks,
-Vinay.

On 3/23/16 1:58 PM, Romascanu, Dan (Dan) wrote:
> This is the MIB Doctor Review for draft-ietf-tictoc-ptp-mib-08.txt.
> Note that this document has a long history. A MIB Doctor review was 
> performed already by Bert Wijnen back in 2011.
> I do not believe that this document is ready for approval in its 
> current form.
> Here are my findings.
> 1.PARSING
> Using smilint:
> smistrip -d mibs docs/draft-ietf-tictoc-ptp-mib-08.txt
> timeout 10 smilint -s -e -l 5 mibs/PTPBASE-MIB 2>report.txt
>
> You can access any intermediately created files, the processing report 
> (which might be empty if no errors or warnings have been found), and 
> output files (in case of a conversion request) for reading and 
> download from a temporary serverdirectory 
> <http://www.ibr.cs.tu-bs.de/projects/libsmi/tools/tmp/691tQ1/>for 
> approx. 24 hours.
>
> While processing your request the following errors and/or warnings 
> have been found:
>
> mibs/PTPBASE-MIB:426: [2] {bad-identifier-case} `XXX' should start 
> with a lower case letter
> mibs/PTPBASE-MIB:426: [2] {object-identifier-not-prefix} Object 
> identifier element `XXX' name only allowed as first element
> mibs/PTPBASE-MIB:26: [2] {module-identity-registration} illegal module 
> identity registration
> Using smicng (thanks to Bert Wijnen):
>
> C:\bw\smicng\work>smicng ptpbase.inc
>
> Successful parsing
>
> C:\bw\smicng\work>
>
> **** now setup for STRICT checking:
>
> C:\bw\smicng\work>smicstrict
>
> C:\bw\smicng\work>smicng ptpbase.inc
>
> W: f(rfc2863.mi2), (274,17) Item "ifPhysAddress" should have SIZE 
> specified
>
> E: f(rfc2863.mi2), (1092,23) Index item "ifRcvAddressAddress" must be 
> defined with syntax that includes a SIZE
>
> W: f(rfc2863.mi2), (1084,1) Row "ifRcvAddressEntry" has indexing that 
> may create variables with more than 128 sub-ids
>
> W: f(rfc2863.mi2), (1103,17) Item "ifRcvAddressAddress" should have 
> SIZE specified
>
> W: f(rfc2863.mi2), (1146,15) Variable "ifIndex" in notification 
> "linkDown" is an index for a table
>
> W: f(rfc2863.mi2), (1158,15) Variable "ifIndex" in notification 
> "linkUp" is an index for a table
>
> W: f(rfc2863.mi2), (1691,1) OBJECT-GROUP "ifOldObjectsGroup" is not 
> used in a MODULE-COMPLIANCE in current module
>
> E: f(ptpbase.mi2), (413,19) Sub-Id for item "ptpbaseMIB" must be 
> "number" or "name(number)" format
>
> *** 2 errors and 6 warnings in parsing
>
> C:\bw\smicng\work>
>
> As the two errors refer to the RFC 2863 module rather than to the 
> PTBASE-MIB: the compilation seems clean.
> 2.TEXTUAL CONVENTIONS are prefixed differently than the MIB objects. 
> This is not forbidden, but it creates inconsistency. Also the prefix 
> ‘Clock’ for the TCs hints to a more generic functionality, although 
> the TCs seem to be PTP-related. I would suggest prefixing the TCs also 
> with PTP or Ptp
> 3.The following TC
>
> ClockIdentity ::= TEXTUAL-CONVENTION
>
> STATUS          current
>
> DESCRIPTION
>
>         "The clock Identity is an 8-octet array...
>
> REFERENCE       "Section 7.5.2.2.1 from [IEEE
>
> SYNTAX          OCTET STRING (SIZE (1..255))
>
> If this is 8-Octet why is the OCTET STRING size 255?
> 4.I see in the Abstract:
>
>    This memo specifies a MIB module in a manner that is both compliant
>    to the SNMPv2 SMI, and semantically identical to the peer SNMPv1
>    definitions.
>
> Do you actually mean SMIv2 rather than SNMPv2 SMI? I am not sure why 
> this is important, but if this is important, please provide a 
> reference to the document that includes the ‘peer SNMPv1’ definitions 
> (probably SMIv1)
>
> 5.Most of the document speaks about PTP, but in the DESCRIPTION clause 
> at page 7 it mentions:
>
>         [IEEE 1588-2008] defines a protocol enabling precise
>         synchronization of clocks in measurement and control systems
>         implemented with packet-based networks, the Precision Time
>         Protocol Version 2 (PTPv2).  This MIB does not address the
>         earlier version IEEE Std. 1588(TM)-2002 (PTPv1).
>
> So, when it says ‘PTP’ does it mean PTPv2 or any version of PTP? It 
> would be good to clarify.
>
> 6. The document uses the improper terminology ‘MIB’ when it means MIB 
> module. For example ‘Relationship to other Profiles and MIBs’ or ‘This 
> MIB is intended to be used …’ etc., etc.,
>
> 7.Idnits complains about an obsolete reference to RFC 1906:
> -- Obsolete informational reference (is this intentional?): RFC 1906
>      (Obsoleted by RFC 3417)
> 8.I do not believe that Section 3 includes any useful information.
> 9.The DESCRIPTION clause is unusually long and includes a lot of 
> abbreviations, terminology, architectural details and other pieces of 
> information which is not clear why they need to be hardcoded in the 
> MIB module. Maybe the place for all these is in the currently 
> empty-content section 3?
> 10.I do not understand how the ClockPortTransportTypeAddress TC works. 
> If this TC defines an address type why is it not an enumeration? What 
> is the string that for example indicates IPv6? Is it ‘IPv6’? who 
> guarantees that a manager and an agent spell the same? Or is the 
> intention to use the list of identifiers for ‘Well Known transport 
> types for PTP communication’at page 64? If yes, how is this extended? 
> Probably an IANA maintained enumerated TC would be a better solution.
> 11.Why is not the SYNTAX of ClockQualityClassType an enumeration, when 
> the DESCRIPTION indicates clearly that this is an enumeration.
>
> 12.I do not understand how ClockTimeIntervalworks. The example does 
> not help either. It says ‘For example, 2.5 ns is expressed as 0000 
> 0000 0002 8000 in Base16.’ and than the SYNTAX is
>
> SYNTAX          OCTET STRING (SIZE (1..255))
>
> What is the STRING for the object in this example?
> 13.For objects like ptpDomainIndex there is no need to copy again the 
> possible values in the DESCRIPTION clause, because these are already 
> listed in the TC.
> 14.Why is ptpDomainClockPortsTotal of SYNTAX Gauge32? Does this value 
> change all the time? Does it latch at a max value?
> 15.There is no indication of behavior for counter discontinuity, or 
> object for counter discontinuity – see section 4.6.1.2 in RFC 4181
> 16.Why is the SYNATX of ptpbaseClockTransDefaultDSNumOfPorts 
> Counter32? This does not seem to be a counter, but an unsigned integer.
>
> 17.The DESCRIPTION clause of the ptpbaseClockPortDSPTPVersion objects 
> reads: "This object specifies the PTP version being used.” However, 
> the DESCRIPTION clause of the MIB module says that this module refers 
> only to PTPv2. So what does this object stand for? And in any case, 
> how is this coded? (1) for v1 and (2) for v2? Then why is it not an 
> enumerated value?
>
> 18.How does ptpbaseClockPortAssociateAddressType work? What would be 
> included under the AutonomousType SYNTAX to make possible 
> interoperability?
>
> 19.The Security Considerations section does not follow the template 
> defined at http://trac.tools.ietf.org/area/ops/trac/wiki/mib-security.
>
> I hope this helps.
>
> Regards,
>
> Dan
>

---

From: vinays <[email-masked]>
Date: Wed, 20 Apr 2016 22:41:43 -0400

(Adding Benoit; Dan is on vacation)

We have addressed the MIB doctor review comments as described. We have 
addressed the Gen-ART reviewer comments from Peter Yee also.

Tim will be addressing the editorial comments in the draft and submit a 
new version tomorrow 4/21 (early morning UK time), so as to make it 
before the telechat. Appreciate all the valuable feedback and comments 
to see this through,

Thanks,
-Vinay.

On 4/20/16 12:25 PM, vinays wrote:
> 1. The current compile line lists only 2 errors, which are ok since 
> they need the IANA number:
>
> ./PTPBASE-MIB:417: [2] {object-identifier-not-prefix} Object 
> 2.  Have replaced the TEXTUAL CONVENTIONS as you suggested to add 
> 'Ptp' prefix.
>
> 14. It can change following any configuration changes, hence would 
> leave it as is. Thanks
>
>> You can access any intermediately created files, the processing 
>> report (which might be empty if no errors or warnings have been 
>> found), and output files (in case of a conversion request) for 
>> reading and download from a temporary serverdirectory 
>> mibs/PTPBASE-MIB:26: [2] {module-identity-registration} illegal 
>> module identity registration
>> the TCs seem to be PTP-related. I would suggest prefixing the TCs 
>> also with PTP or Ptp
>> 3.The following TC
>>
>> 5.Most of the document speaks about PTP, but in the DESCRIPTION 
>> clause at page 7 it mentions:
>>
>> module. For example ‘Relationship to other Profiles and MIBs’ or 
>> ‘This MIB is intended to be used …’ etc., etc.,
>>
>> 10.I do not understand how the ClockPortTransportTypeAddress TC 
>> works. If this TC defines an address type why is it not an 
>> enumeration? What is the string that for example indicates IPv6? Is 
>> it ‘IPv6’? who guarantees that a manager and an agent spell the same? 
>> Or is the intention to use the list of identifiers for ‘Well Known 
>> transport types for PTP commu nication’at page 64? If yes, how is 
>> this extended? Probably an IANA maintained enumerated TC would be a 
>> better solution.
>> 11.Why is not the SYNTAX of ClockQualityClassType an enumeration, 
>> when the DESCRIPTION indicates clearly that this is an enumeration.
>>

---

From: Tim Frost <[email-masked]>
Date: Thu, 21 Apr 2016 11:13:36 +0000

I have now submitted the -09 version of the draft. Hopefully this answers Dan Romascanu's comments, as reviewed by Vinay below.  I updated the security section to match the newer template, and made a few typographical corrections, mainly for line length.

Best regards,
Tim

Tim Frost   |   Calnex Solutions Ltd.   |   +44 (0) 7825 706 952   |   www.calnexsol.com<http://www.calnexsol.com/>

From: vinays [mailto:[email-masked]]
Sent: 21 April 2016 03:42
To: MIB Doctors (E-mail) <[email-masked]>; Benoit Claise <[email-masked]>
Cc: Romascanu, Dan (Dan) <[email-masked]>; [email-masked]; Tim Frost <[email-masked]>; Laurent Montini <[email-masked]>; Greg Dowd <[email-masked]>; Karen O'Donoghue <[email-masked]>; Vinay Shankarkumar <[email-masked]>
Subject: Re: MIB Doctor Review for draft-ietf-tictoc-ptp-mib-08.txt

We have addressed the MIB doctor review comments as described. We have addressed the Gen-ART reviewer comments from Peter Yee also.

Tim will be addressing the editorial comments in the draft and submit a new version tomorrow 4/21 (early morning UK time), so as to make it before the telechat.

Appreciate all the valuable feedback and comments to see this through,

Thanks,
-Vinay.

Please see below for what I have addressed in the MIB module. We will make the few changes
needed in the draft also as mentioned.

1. The current compile line lists only 2 errors, which are ok since they need the IANA number:

./PTPBASE-MIB:417: [2] {bad-identifier-case} `XXX' should start with a lower case letter
./PTPBASE-MIB:417: [2] {object-identifier-not-prefix} Object identifier element `XXX' name only allowed as first element

2.  Have replaced the TEXTUAL CONVENTIONS as you suggested to add 'Ptp' prefix.

12. Am not sure what we need to change here, Please let us know. Will add as you need.

14. It can change following any configuration changes, hence would leave it as is. Thanks

18. This is the normal method used to extend for new types and hence have used it. Please suggest what needs to be done. Thanks.

Thanks,
-Vinay.

Note that this document has a long history. A MIB Doctor review was performed already by Bert Wijnen back in 2011.

I do not believe that this document is ready for approval in its current form.

Here are my findings.

1.PARSING

Using smilint:

You can access any intermediately created files, the processing report (which might be empty if no errors or warnings have been found), and output files (in case of a conversion request) for reading and download from a temporary server directory<http://www.ibr.cs.tu-bs.de/projects/libsmi/tools/tmp/691tQ1/> for approx. 24 hours.

While processing your request the following errors and/or warnings have been found:

mibs/PTPBASE-MIB:426: [2] {bad-identifier-case} `XXX' should start with a lower case letter

mibs/PTPBASE-MIB:426: [2] {object-identifier-not-prefix} Object identifier element `XXX' name only allowed as first element

mibs/PTPBASE-MIB:26: [2] {module-identity-registration} illegal module identity registration

Successful parsing

C:\bw\smicng\work>

W: f(rfc2863.mi2), (274,17) Item "ifPhysAddress" should have SIZE specified

E: f(rfc2863.mi2), (1092,23) Index item "ifRcvAddressAddress" must be defined with syntax that includes a SIZE

W: f(rfc2863.mi2), (1084,1) Row "ifRcvAddressEntry" has indexing that may create variables with more than 128 sub-ids

W: f(rfc2863.mi2), (1103,17) Item "ifRcvAddressAddress" should have SIZE specified

W: f(rfc2863.mi2), (1146,15) Variable "ifIndex" in notification "linkDown" is an index for a table

W: f(rfc2863.mi2), (1158,15) Variable "ifIndex" in notification "linkUp" is an index for a table

W: f(rfc2863.mi2), (1691,1) OBJECT-GROUP "ifOldObjectsGroup" is not used in a MODULE-COMPLIANCE in current module

E: f(ptpbase.mi2), (413,19) Sub-Id for item "ptpbaseMIB" must be "number" or "name(number)" format

C:\bw\smicng\work>

As the two errors refer to the RFC 2863 module rather than to the PTBASE-MIB: the compilation seems clean.

2.TEXTUAL CONVENTIONS are prefixed differently than the MIB objects. This is not forbidden, but it creates inconsistency. Also the prefix 'Clock' for the TCs hints to a more generic functionality, although the TCs seem to be PTP-related. I would suggest prefixing the TCs also with PTP or Ptp

3.The following TC

    STATUS          current

    DESCRIPTION

4.I see in the Abstract:

Do you actually mean SMIv2 rather than SNMPv2 SMI? I am not sure why this is important, but if this is important, please provide a reference to the document that includes the 'peer SNMPv1' definitions (probably SMIv1)

5.       Most of the document speaks about PTP, but in the DESCRIPTION clause at page 7 it mentions:

So, when it says 'PTP' does it mean PTPv2 or any version of PTP? It would be good to clarify.

6.        The document uses the improper terminology 'MIB' when it means MIB module. For example 'Relationship to other Profiles and MIBs' or 'This MIB is intended to be used ...' etc., etc.,

     (Obsoleted by RFC 3417)

9.The DESCRIPTION clause is unusually long and includes a lot of abbreviations, terminology, architectural details and other pieces of information which is not clear why they need to be hardcoded in the MIB module. Maybe the place for all these is in the currently empty-content section 3?

10.   I do not understand how the ClockPortTransportTypeAddress TC works. If this TC defines an address type why is it not an enumeration? What is the string that for example indicates IPv6? Is it 'IPv6'? who guarantees that a manager and an agent spell the same? Or is the intention to use the list of identifiers for 'Well Known transport types for PTP com

11.   mu

12.nication'at page 64? If yes, how is this extended? Probably an IANA maintained enumerated TC would be a better solution.

13.Why is not the SYNTAX of ClockQualityClassType an enumeration, when the DESCRIPTION indicates clearly that this is an enumeration.

14.   I do not understand how ClockTimeInterval works. The example does not help either. It says 'For example, 2.5 ns is expressed as 0000 0000 0002 8000 in Base16.' and than the SYNTAX is

15.For objects like ptpDomainIndex there is no need to copy again the possible values in the DESCRIPTION clause, because these are already listed in the TC.

16.Why is ptpDomainClockPortsTotal of SYNTAX Gauge32? Does this value change all the time? Does it latch at a max value?

17.There is no indication of behavior for counter discontinuity, or object for counter discontinuity - see section 4.6.1.2 in RFC 4181

18.Why is the SYNATX of ptpbaseClockTransDefaultDSNumOfPorts Counter32? This does not seem to be a counter, but an unsigned integer.

19.   The DESCRIPTION clause of the ptpbaseClockPortDSPTPVersion objects reads: "This object specifies the PTP version being used." However, the DESCRIPTION clause of the MIB module says that this module refers only to PTPv2. So what does this object stand for? And in any case, how is this coded? (1) for v1 and (2) for v2? Then why is it not an enumerated value?

20.   How does ptpbaseClockPortAssociateAddressType work? What would be included under the AutonomousType SYNTAX to make possible interoperability?

21.   The Security Considerations section does not follow the template defined at http://trac.tools.ietf.org/area/ops/trac/wiki/mib-security.

I hope this helps.

Regards,

Dan