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

["David McWalter" <DMcW at dataconnection.com>]

Dave,

Thanks for your thorough review of -07.  We did two iterations as a result.

Firstly, -08 addressed most of your feedback, as detailed inline below.

Then you kindly re-reviewed the draft, and provided two further points of feedback, which I addressed in -09.
-  The introduction should mention PIM-DM (as well as the abstract).
-  The interaction between pimStaticRPOverrideDynamic and pimStaticRPPrecedence was not legal.

Regards,
David McWalter.

-----Original Message-----
From: Dave Thaler [mailto:dthaler at windows.microsoft.com]
Sent: 09 December 2006 05:51
To: Romascanu, Dan (Dan); Raghava Sivaramu; James Lingard; David
McWalter; Bharat Joshi
Cc: Bill Fenner; PIM WG; mib-doctors at ietf.org
Subject: MIB Doctor review of draft-ietf-pim-mib-v2-07.txt


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.
[DMcW] Added.  Copied style from RFC4382, now all imported TCs point to their 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"
[DMcW] Done.

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.
[DMcW] Done.

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.
[DMcW] Removed references to 2434 and 3692.  3956 is referenced by PimGroupMappingOriginType.  I've added REFERENCE text for 3569 and ip-mcast-mib ipMcastSsmRangeTable in objects that deal particularly with SSM.

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.
[DMcW] Done.

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.
[DMcW] Yes, MGMD->MLD. Expanded the first use of acronyms, added references.  Removed version numbers from IGMP and MLD.

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.)
[DMcW] Timers 'not running' and 'running with infinite duration' are distinct protocol states, which the MIB therefore represents.  You're right that this is confusing.  If the PIM protocol RFC isn't clear on this point, I don't think we should fix it with commentary in the MIB.

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.
[DMcW] Yes, the agent.  Changed to 'is ignored' in -08, logic changed in -09.

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.
[DMcW] Text was stale.  Changed by pasting correct text from pimInterfaceStatus description.

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

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.
[DMcW] Done.

12) CONTACT-INFO clause SHOULD provide a pointer to the WG's web page,
per 
    RFC4181 section 4.5.
[DMcW] Added URL for PIM WG charter.

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)
[DMcW] Done.

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.
[DMcW] Added PIM-DM to abstract in -08, and introduction in -09.

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.
[DMcW] Will reference RFC 4601 sections 4.1.6, 4.3.1-4.3.4 and 4.5.1:
'The primary IP address of a neighbor is the address that it uses as the source of its PIM Hello messages.'
'On unnumbered interfaces on point-to-point links, the router's address should be the same as the source address it chose for the Hello message it sent over that interface.'

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"?
[DMcW] Changed to SHOULD.

17) Grammar error in pimStatusRPRowStatus DESCRIPTION: "valid values
have"
    should be "a valid value has"
[DMcW] Fixed.

18) Typo in DESCRIPTION of pimDiagnosticsGroup: "additonal"
[DMcW] Fixed.

19) Typo in security considerations section: "encapsulted"
[DMcW] Fixed.

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.
[DMcW] Updated pimInAsserts and pimOutAsserts to Counter64.

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.
[DMcW] All correct, but I would prefer not to fix.  There are 22 objects with syntax InetAddressType, some of which appear in all four compliances.  Adding ~50 syntax restrictions to the compliance statement would not eloquently express the restriction 'no DNS'.

22) Possible typo in DESCRIPTION of pimNeighborLossCount.  One paragraph

    says "This count" and the other says "This counter".   Should the 
    first be "This counter"?
[DMcW] It is inconsistent.  Fixed 'this count'->'this counter' (two instances).

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.
[DMcW] Agree.  I'll leave this unchanged.

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?
[DMcW] Indeed that's what the current compliance says.  I'm not against adding read-only compliances in principle, but no-one has put a case that would justify adding all that compliance text!

-Dave Thaler

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