[MIB-DOCTORS] MIB-DOCTOR review of draft-ietf-magma-mgmd-mib-10.txt

I have just completed a MIB doctor review of draft -10.  (There have
been significant changes since my review of draft -08 a year
and a half ago.)  So here goes...

Section 1:
>  [RFC2236] or version 3 [3376] and the Multicast Listener Discovery

s/[3376]/[RFC3376]/

>  This version of the MIB obsoletes both rfc2933 [RFC2933] and rfc3019

s/MIB/MIB module/
s/rfc2933/RFC 2933/
s/rfc3019/RFC 3019/

Section 2:
> document are to be interpreted as described in RFC-2119 [KEYWORDS].

[KEYWORDS] should be [RFC2119], which is how it appears in the references
section.

Also s/RFC-2119/RFC 2119/

Section 3:

>    5. the reverse MGMD Host Table which contains one row for each
>       interface for which there are active multicast groups on a host,

This is not correct.  The mgmdInverseHostCacheTable has INDEX:
>   INDEX      { mgmdInverseHostCacheIfIndex,
>                mgmdInverseHostCacheAddressType,
>                mgmdInverseHostCacheAddress}

Hence it contains one row for each IP multicast group for which there
are members on a particular interface on a host (same as the
MGMD Host Cache Table), but they're in a different order.

>    6. the reverse MGMD Router Table which contains one row for each
>       interface for which there are active multicast groups on a
>       router,

Same error in the text above.

> as defined in RFC 2863, the MIB which decribes generic objects for

Grammar.  s/which/that/

Also s/MIB/MIB module/

Section 4:

MODULE-IDENTITY:
>From RFC 4181 section 4.5:
>  - If the module was developed by an IETF working group, then the
>    ORGANIZATION clause MUST provide the full name of the working
>    group, and the CONTACT-INFO clause MUST include working group
>    mailing list information.  The CONTACT-INFO clause SHOULD also
>    provide a pointer to the working group's web page.

Currently the MGMD doc contains the info in WG mailing list and web page
in the ORGANIZATION clause, and these need to be moved to the CONTACT-INFO
clause as noted above.

Also missing an initial REVISION clause as noted in RFC 4181 section 4.5.

>         "The MIB module for MGMD management.
                              ^^^^
Suggest expanding the MGMD acronym on first usage inside the MIB module.

mgmdHostInterfaceQuerier:

> The address of the IGMP or MLD Querier on the IP subnet to
> which this interface is attached. The InetAddressType, e.g.

Per section 5 of RFC 3810, all MLD messages MUST be sent from a link-local
source address.  Hence the IPv6 subnet prefix is irrelevant and the text
above incorrectly implies there can only be one subnet prefix on the link.
Instead, it should say it's the address seen as the source address of IGMP
or MLD Queries on the interface.

Same comment on the DESCRIPTION of the mgmdRouterInterfaceQuerier object.

mgmdHostInterfaceStatus:

RFC 4181 states regarding RowStatus objects...
>    - There either MUST be one columnar object with a SYNTAX value of
>      StorageType [RFC2579] and a MAX-ACCESS value of read-create, or
>      else the row object (table entry) DESCRIPTION clause MUST specify
>      what happens to dynamically-created rows after an agent restart.
>
>    - The DESCRIPTION clause of the status column MUST specify which
>      columnar objects (if any) have to be set to valid values before
>      the row can be activated.  If any objects in cascading tables
>      have to be populated with related data before the row can be
>      activated, then this MUST also be specified.
>
>    - The DESCRIPTION clause of the status column MUST specify whether
>      or not it is possible to modify other columns in the same
>      conceptual row when the status value is active(1).  Note that in
>      many cases it will be possible to modify some writable columns
>      when the row is active but not others.  In such cases, the
>      DESCRIPTION clause for each writable column SHOULD state whether
>      or not that column can be modified when the row is active, and
>      the DESCRIPTION clause for the status column SHOULD state that
>      modifiability of other columns when the status value is active(1)
>      is specified in the DESCRIPTION clauses for those columns (rather
>      than listing the modifiable columns individually).

None of the above MUSTs are currently met in the draft.
Same for the RowStatus objects of the other tables in the draft.

mgmdRouterInterfaceTable:
Currently the DESCRIPTION is identical to the description of the
mgmdHostInterfaceTable.  It SHOULD be different.

mgmdRouterInterfaceQueryInterval:
>   SYNTAX     Unsigned32 (0..31744)
>   DESCRIPTION
>           "The frequency at which IGMP or MLD Host-Query packets are
>           transmitted on this interface. This variable must be a
>           non-zero value."

If it must be non-zero, why is 0 in the legal syntax range?  Seems like
the range should be (1..31744) and the last sentence deleted.

mgmdRouterInterfaceVersion:
Fix incorrect capitalization of "To" in the DESCRIPTION.

mgmdRouterInterfaceQueryMaxResponseTime:
The reference clause points to RFC 3810 section 9.3, but not to the
equivalent for IPv4 (RFC 3376 section 8.3).

Many objects:
>   SYNTAX     InetAddressType (SIZE(4|16))

This is incorrect.  This is an integer, not an octet string, so the SIZE
restriction is wrong.  It should instead be:
>   SYNTAX     InetAddressType { ipv4(1), ipv6(2) }
if you want to restrict what is legal to IPv4 and IPv6.

mgmdRouterInterfaceRobustness has a syntax restriction of (1..255) which
is fine, but mgmdHostInterfaceVersion3Robustness has none and just has the
statement "The variable must be a non-zero value" in the DESCRIPTION.
It should instead have a range restriction.

mgmdHostCacheLastReporter's DESCRIPTION says:
> ... If no membership report has been received, this object has a value of 0.

Since this is an InetAddress object, shouldn't this say 0.0.0.0 for IPv4
or :: for IPv6?  (And same for mgmdRouterCacheLastReporter)

mgmdRouterCacheExpiryTime:
The sentence in the DESCRIPTION "The value must always be greater than or
equal to 1" can be deleted since it's redundant with the range restriction
in the SYNTAX clause.

There's an odd difference between the mgmdInverseHostCacheTable and the
mgmdRouterInverseCacheTable.  The former defines its own columnar objects
for the INDEX, whereas the latter uses the objects in the mgmdRouterCacheTable
in its INDEX.  Why the inconsistency between these tables?

mgmdRouterSrcListEntry:
Fix indentation in INDEX clause.

Section 5:

>   a) The mgmdRouterInterfaceTable provides read-create acces to 2

s/acces/access/

Section 6:
There are two section 6's.  "IANA Considerations" and "Contributors"
are both numbered 6.

Section 6: IANA Considerations
>  This MIB introduces a new term to refer to two existing multicast
>  protocols, Multicast Group Membership Discovery. It encompasses both
>  the IPv4 Multicast discovery protocol, IGMP, and the IPv6 Multicast
>  discovery protocol, MLD, as defined in RFCs 2933 and 3019
>  respectively.

I don't see what the above paragraph has to do with IANA considerations
or with anything else in that section.  It looks like something that belongs
in the introduction section.

Also s/MIB/MIB module/

Section 9:

References should be listed in order.

idnits output:

tmp/draft-ietf-magma-mgmd-mib-10.txt:

  Checking boilerplate required by RFC 3978 and 3979, updated by RFC 4748:
  ----------------------------------------------------------------------------

  ** The document seems to lack an RFC 3979 Section 5, para 1 IPR Disclosure
     Acknowledgement -- however, there's a paragraph with a matching
     beginning. Boilerplate error?

  Checking nits according to http://www.ietf.org/ietf/1id-guidelines.txt:
  ----------------------------------------------------------------------------

  == Mismatching filename: the document gives the document name as
     'draft-ietf-magma-mgmd-mib-09', but the file name used is
     'draft-ietf-magma-mgmd-mib-10'

  == No 'Intended status' indicated for this document; assuming Proposed
     Standard

  == It seems as if not all pages are separated by form feeds - found 0 form
     feeds but 34 pages

  Checking nits according to http://www.ietf.org/ID-Checklist.html:
  ----------------------------------------------------------------------------

  ** There are 6 instances of too long lines in the document, the longest one
     being 3 characters in excess of 72.

  Miscellaneous warnings:
  ----------------------------------------------------------------------------

     No issues found here.

  Checking references for intended status: Proposed Standard
  ----------------------------------------------------------------------------

     (See RFC 3967 for information about using normative references to
     lower-maturity documents in RFCs)

  -- Looks like a reference, but probably isn't: '3376' on line 70

  == Missing Reference: 'KEYWORDS' is mentioned on line 83, but not defined

  -- Possible downref: Undefined Non-RFC (?) reference : ref. 'KEYWORDS'

  == Missing Reference: 'RFC 1112' is mentioned on line 1176, but not defined

  == Missing Reference: 'RFC 2236' is mentioned on line 1232, but not defined

  ** Obsolete undefined reference: RFC 2236 (Obsoleted by RFC 3376)

  == Missing Reference: 'RFC 2710' is mentioned on line 1279, but not defined

  == Missing Reference: 'RFC 3376' is mentioned on line 1344, but not defined

  == Unused Reference: 'RFC3376' is defined on line 1730, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC2863' is defined on line 1740, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC4001' is defined on line 1743, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC2119' is defined on line 1749, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC3414' is defined on line 1765, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC3415' is defined on line 1769, but no explicit
     reference was found in the text

  == Unused Reference: 'RFC4605' is defined on line 1773, but no explicit
     reference was found in the text

  ** Obsolete normative reference: RFC 2236 (Obsoleted by RFC 3376)

(Note that the reference to RFC 2236 is actually correct.  It was actually
wrong for RFC 3376 to obsolete RFC 2236, it actually updates it since it
does not respecify the IGMPv2 behavior (e.g. it normatively references 2236 for
the packet format of the v2 report) and actually mandates that it use
IGMPv2 under certain conditions.  Hence I believe it was in fact a process
error that 3376 was allowed to claim it obsoleted 2236.  Anyway, the point
is that referencing 2236 is correct and the "obsolete normative reference"
complaint should be ignored.)

Also the smi checker output (which Bert already ran, so just copying his output here) is below.
Some of these are the same as items I noted above.

> I get this from SMICng:
>
> C:\bwijnen\smicng\work>smicng mgmd.inc
> E: f(mgmd.mi2), (90,38) A SIZE or range clause may not be used with ENUM
> E: f(mgmd.mi2), (246,38) A SIZE or range clause may not be used with
> ENUM
> E: f(mgmd.mi2), (487,38) A SIZE or range clause may not be used with
> ENUM
> E: f(mgmd.mi2), (590,38) A SIZE or range clause may not be used with
> ENUM
> E: f(mgmd.mi2), (644,27) A SIZE or range clause may not be used with
> TimeTicks
> E: f(mgmd.mi2), (751,38) A SIZE or range clause may not be used with
> ENUM
> E: f(mgmd.mi2), (835,38) A SIZE or range clause may not be used with
> ENUM
> E: f(mgmd.mi2), (797,5) Item "mgmdRouterInterfaceIfIndex" in sequence
> "MgmdRouterInverseCacheEntry" is not a column for
> row "mgmdRouterInverseCacheEntry"
> E: f(mgmd.mi2), (798,5) Item "mgmdRouterCacheAddressType" in sequence
> "MgmdRouterInverseCacheEntry" is not a column for
> row "mgmdRouterInverseCacheEntry"
> E: f(mgmd.mi2), (799,5) Item "mgmdRouterCacheAddress" in sequence
> "MgmdRouterInverseCacheEntry" is not a column for row
> "mgmdRouterInverseCacheEntry"
> E: f(mgmd.mi2), (910,5) Item "mgmdRouterCacheAddressType" in sequence
> "MgmdRouterSrcListEntry" is not a column for row "
> mgmdRouterSrcListEntry"
> E: f(mgmd.mi2), (911,5) Item "mgmdRouterCacheAddress" in sequence
> "MgmdRouterSrcListEntry" is not a column for row "mgmd
> RouterSrcListEntry"
> E: f(mgmd.mi2), (912,5) Item "mgmdRouterCacheIfIndex" in sequence
> "MgmdRouterSrcListEntry" is not a column for row "mgmd
> RouterSrcListEntry"
> E: f(mgmd.mi2), (784,1) Row "mgmdRouterInverseCacheEntry" must have leaf
> objects defined under it
> W: f(mgmd.mi2), (791,18) Row "mgmdRouterInverseCacheEntry" does not have
> a consistent indexing scheme - cannot specify a
> n index item from additional "base row" mgmdRouterInterfaceEntry, since
> can have only one "base row" which is mgmdRouter
> CacheEntry
>
> *** 14 errors and 1 warning in parsing
>
> And I get this from smilint:
>
> C:\bwijnen\smicng\work>smilint -m -l 6 -s ./MGMD-STD-MIB
> ./MGMD-STD-MIB:39: [3] {revision-missing} revision for last update is
> missing
> ./MGMD-STD-MIB:90: [2] {size-illegal} illegal size restriction for
> non-octet-string parent type `InetAddressType'
> ./MGMD-STD-MIB:100: [2] {sequence-type-mismatch} type of
> `mgmdHostInterfaceQuerierType' in sequence and object type defi
> nition do not match
> ./MGMD-STD-MIB:246: [2] {size-illegal} illegal size restriction for
> non-octet-string parent type `InetAddressType'
> ./MGMD-STD-MIB:256: [2] {sequence-type-mismatch} type of
> `mgmdRouterInterfaceQuerierType' in sequence and object type de
> finition do not match
> ./MGMD-STD-MIB:487: [2] {size-illegal} illegal size restriction for
> non-octet-string parent type `InetAddressType'
> ./MGMD-STD-MIB:494: [2] {sequence-type-mismatch} type of
> `mgmdHostCacheAddressType' in sequence and object type definiti
> on do not match
> ./MGMD-STD-MIB:590: [2] {size-illegal} illegal size restriction for
> non-octet-string parent type `InetAddressType'
> ./MGMD-STD-MIB:597: [2] {sequence-type-mismatch} type of
> `mgmdRouterCacheAddressType' in sequence and object type defini
> tion do not match
> ./MGMD-STD-MIB:644: [2] {subtype-illegal} subtyping not allowed
> ./MGMD-STD-MIB:751: [2] {size-illegal} illegal size restriction for
> non-octet-string parent type `InetAddressType'
> ./MGMD-STD-MIB:756: [2] {sequence-type-mismatch} type of
> `mgmdInverseHostCacheAddressType' in sequence and object type d
> efinition do not match
> ./MGMD-STD-MIB:798: [2] {sequence-type-mismatch} type of
> `mgmdRouterCacheAddressType' in sequence and object type defini
> tion do not match
> ./MGMD-STD-MIB:835: [2] {size-illegal} illegal size restriction for
> non-octet-string parent type `InetAddressType'
> ./MGMD-STD-MIB:842: [2] {sequence-type-mismatch} type of
> `mgmdHostSrcListAddressType' in sequence and object type defini
> tion do not match
> ./MGMD-STD-MIB:910: [2] {sequence-type-mismatch} type of
> `mgmdRouterCacheAddressType' in sequence and object type defini
> tion do not match
> ./MGMD-STD-MIB:796: [3] {sequence-no-column} SEQUENCE element #1
> `mgmdRouterInterfaceIfIndex' is not a child node under
> `mgmdRouterInverseCacheEntry'
> ./MGMD-STD-MIB:784: [5] {index-element-not-accessible} warning: exactly
> one index element of row `mgmdRouterInverseCache
> Entry' must be accessible
> ./MGMD-STD-MIB:909: [3] {sequence-no-column} SEQUENCE element #1
> `mgmdRouterCacheAddressType' is not a child node under
> `mgmdRouterSrcListEntry'
> ./MGMD-STD-MIB:909: [3] {sequence-no-column} SEQUENCE element #2
> `mgmdRouterCacheAddress' is not a child node under `mgm
> dRouterSrcListEntry'
> ./MGMD-STD-MIB:909: [3] {sequence-no-column} SEQUENCE element #3
> `mgmdRouterCacheIfIndex' is not a child node under `mgm
> dRouterSrcListEntry'
> ./MGMD-STD-MIB:102: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:258: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:496: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:524: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:599: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:620: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:758: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:844: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:862: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object
> ./MGMD-STD-MIB:917: [5] {inetaddress-inetaddresstype} warning:
> `InetAddress' object should have an accompanied preceding
>  `InetAdressType' object

Finally, it might be helpful, though not required, to have a section that
summarizes the changes from RFC 2933 and RFC 3019.  So feel free to ignore this one.

-Dave

_______________________________________________
MIB-DOCTORS mailing list
MIB-DOCTORS@ietf.org
https://www1.ietf.org/mailman/listinfo/mib-doctors