[pim] MIB Doctor review of draft-ietf-pim-mib-v2-07.txt

[Dave Thaler <dthaler at windows.microsoft.com>]

I have completed by review of draft-ietf-pim-mib-v2-07.txt, as requested
by Dan Romascanu.  Except for the items identified below, everything
else
in the MIB Review Checklist was OK.

I have organized my comments into lists of what I feel are MUST, SHOULD,
and MAY fix items.

MUST fix:

1)  Missing normative reference for IANA-RTPROTO-MIB as required by
RFC4181 
    section 3.6.   Citation should be added to last sentence of section
3 
    where it is mentioned along with other modules which do have
citations.

2)  ORGANIZATION clause MUST have the "full name" of the working group,
per 
    RFC4181 section 4.5.  Per the IETF WG page, this should be the
"Protocol 
    Independent Multicast (PIM) Working Group"

3)  The copyright notice on page 1 is now obsolete, should say The IETF
Trust
    (see the ID-guidelines).  The copyright notice at the end is fine.

4)  Per RFC4181 section 3.6, all references must be cited in the text.  
    The following appear as uncited references: [RFC3569], [RFC3956], 
    [I-D.ietf-mboned-ip-mcast-mib], [RFC2434], [RFC3692]. 
    These need to be either cited or removed.
 
5)  Per RFC3978 section 5.6, the copyright statement text in the
DESCRIPTION
    clause of the MODULE-IDENTITY is incorrect.  That wording is only
for 
    MIBs that are IANA maintained.   The other wording from RFC3978
section 
    5.6 needs to be used instead.

6)  DESCRIPTION of pimInterfaceStubInterface uses acronyms IGMP and MGMD

    which are not expanded or defined, nor is a reference provided for
them.  
    pimStarGILocalMembership also uses MLD without expanding/defining/
    referencing it.  Was MGMD supposed to be MLD too?
pimSGILocalMembership 
    then mentions IGMPv3 and MLDv2.  pimAnycastRPSetTable then mentions
    MSDP.

7)  DESCRIPTION of pimSGRptIPruneExpiryTimer is unclear.  What's the 
    difference between a timer not running, and a timer having an
infinite
    expiry time?  How do I know which one to use?  (I did look at RFC
4601 
    section 4.5.4 as indicated in the REFEFENCE clause, but that didn't 
    answer the question.)

8)  DESCRIPTION of pimStaticRPPrecedence contains this statement:

            If this object is present, then pimStaticRPOverrideDynamic
            is ignored.

    The main problem with the above is it doesn't say who it's ignored
by,
    the agent?  The management station?  Presumably you mean the agent.
 
    Also, does "ignored" mean the agent need not report a correct value 
    when read either?  If so, it seems really odd to make 
    pimStaticRPOverrideDynamic be a mandatory object, even for agents
that 
    implement pimStaticRPPrecedence, in which case
pimStaticRPOverrideDynamic
    would be a useless object.

9)  DESCRIPTION of pimAnycastRPSetRowStatus says:

            ... There are no other other
            writeable columnar objects in this entry."

    Typo ("other other").  Also, the statement is either incorrect or at
    least confusing, since pimAnycastRPSetStorageType is in the entry
and
    has a MAX-ACCESS of read-create.  Is the intent that the storage
type
    is not writable, except during row creation?  Please clarify.

10) References for RFC4601, RFC4610, and RFC3692 still contain an I-D
    filename and say they're works in progress.

SHOULD fix:

11) The text in IANA considerations section SHOULD be crafted so that
after 
    publication it will serve to document the OID assignment.  See
RFC4181 
    section 3.5.2 for an example.

12) CONTACT-INFO clause SHOULD provide a pointer to the WG's web page,
per 
    RFC4181 section 4.5.

13) The top-level structure doesn't follow the RECOMMENDED layout in RFC
4181 
    appendix D:
        pimMIBObjects should be removed
        pimTraps should be { pimStdMIB 0 }
        pim should be { pimStdMIB 1 }
        (pimMIBConformance is fine)

14) Since the MIB module contains a whole group of objects (pimDmGroup)
that 
    are *only* used by PIM-DM, then the abstract should also mention
PIM-DM.

15) What is the value of pimInterfaceAddress if the interface is an 
    unnumbered IPv4 interface?  DESCRIPTION should answer, or just add a

    REFERENCE if the term "primary IP address" is defined in an RFC
section 
    that answers the question.

16) Regarding pimInterfacePropagationDelay: the SYNTAX says 0 is a legal

    value, but the DESCRIPTION says "Implementations should enforce a
lower 
    bound on the permitted values for this object to allow for
scheduling 
    and processing delays within the local router."  The use of "should"
is 
    vague.  If this is to be read as a MUST, then 0 wouldn't be legal.
    Do you mean "SHOULD"?

17) Grammar error in pimStatusRPRowStatus DESCRIPTION: "valid values
have"
    should be "a valid value has"

18) Typo in DESCRIPTION of pimDiagnosticsGroup: "additonal"

19) Typo in security considerations section: "encapsulted"

MAY fix:

20) Could pimInAsserts ever wrap in less than an hour in any realistic 
    deployment?  (say with lots of interfaces, lots of groups, lots of 
    sources, lots of routers on the LAN, etc)  If so, then this should 
    probably be a Counter64 per the rule of thumb in RFC4181.

21) There are a number of InetAddressType objects where the InetAddress 
    objects have a SIZE constraint of (0|4|8|16|20), but the compliance 
    statements (as allowed by a MAY in RFC4001) do not include any 
    constraints on the values of InetAddressType objects (i.e. right now

    a type of dns would be legal as long as the value happens to meet 
    the address size constraint).  I don't know if this is intentional,
    but I suspect not.

22) Possible typo in DESCRIPTION of pimNeighborLossCount.  One paragraph

    says "This count" and the other says "This counter".   Should the 
    first be "This counter"?

23) There's no reason pimStarGRPFNextHop needs to be able to contain a 
    zone id, since you always have an interface index in
pimStarGRPFIfIndex
    if there's a next hop.  It's fine to allow one, just redundant.  
    Same for pimStarGIAssertWinnerAddress, pimSGUpstreamNeighbor, 
    pimSGRPFNextHop, etc.

24) Since there are no read-only compliance statements for writable
    objects, currently any agent which implements an object which is
    declared read-write or read-create MUST support write access.  It's
    not allowed to support such objects for reading only.  Is this
    intentional?

-Dave Thaler


_______________________________________________
pim mailing list
pim at ietf.org
https://www1.ietf.org/mailman/listinfo/pim