Fw: MIB Doctor review: publish draft-ietf-disman-remops-mib-v2-06.tx t

Hi -

Forwarded for your information.

Randy

----- Original Message ----- 
> From: "Juergen Schoenwaelder" <[email protected]>
> To: "Wijnen, Bert (Bert)" <[email protected]>
> Cc: "Juergen Quittek" <[email protected]>; "Randy Presuhn" <[email protected]>
> Sent: Tuesday, May 03, 2005 8:15 AM
> Subject: Re: MIB Doctor review: publish draft-ietf-disman-remops-mib-v2-06.tx t
>

> On Fri, Dec 31, 2004 at 03:25:26PM +0100, Wijnen, Bert (Bert) wrote:
>
> > Juergen, can you do MIB doctor review on this one?
>
> I am sorry that it took that long. Below are my comments. I suggest
> that Juergen Quittek takes a look at the comments I have first and
> then we perhaps do a quick phone call to work out any misunderstandings.
>
> /js
>
>
> MIB review <draft-ietf-disman-remops-mib-v2-06.txt>:
>
> - All three MIB modules compile without problems using smilint 0.4.3
>
> - The abstract has a duplicate "remote" - I suggest to remove the first
>   occurance of "remote".
>
> - The Intrduction could have been reworded. It is kind of strange to
>   start with the IETF keywords phrase followed by the statement that
>   the document originates from the DISMAN WG before actually saying
>   that the document is all about. (I am not even sure it is necessary
>   to point to the DISMAN WG in the introduction.)
>
> - Should the reference to gethostname() and gethostbyaddr() not be
>   replaced or at least be augmented with a reference to getnameinfo()
>   and getaddrinfo() [RFC 3493]? This also affects sections 1.3 and 3.
>
> - Taken the two last comments into account, I suggest:
>
> 1.  Introduction
>
>    This document defines standards-based MIB modules for performing
>    specific remote operations.  The remote operations defined by this
>    document consist of the ping, traceroute, and lookup functions.
>
>    Ping and traceroute are two very useful functions for managing
>    networks.  Ping is typically used to determine if a path exists
>    between two hosts, while traceroute shows an actual path.
>
>    Both ping and traceroute yield round-trip times measured in
>    milliseconds.  These times can be used as a rough approximation for
>    network transit time.
>
>    The lookup functions considered in this document are the
>    equivalents of name to address conversion functions such as
>    gethostbyname() / gethostbyaddr() and getaddrinfo() /
>    getnameinfo().
>
>    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>    "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in
>    this document are to be interpreted as described in RFC 2119
>    [RFC2119].
>
> - I am not sure the hostent structure is really needed to say that a
>   host may have multiple interfaces (IP addresses) and that multiple
>   names may be given to the same interface (IP address). (The
>   motivation is again to remove IPv4 only API specifics which may
>   mislead implementors.)
>
> - What is an official host name? Oh, I better do not ask this question.
>   So ignore this.
>
> - At the end of section 3.4, you may want to add a sentence explaining
>   why the RFC 2925 compliance statement was deprecated.
>
> - RFC 3291 has been obsoleted by RFC 4001. This affects the references
>   but also the comments in the IMPORT clauses of the three MIB modules.
>
>
> 2. DISMAN-PING-MIB:
>
> - reverse the revision statements (youngest first)
>
> - copyright year needs to be updated to 2005
>
> - pingSnmpQuery description clause "is an" -> "is using an"
>
> - I am wondering why the DEFVAL for pingCtlDescr is '00'H and not an
>   empty string.  (The DEFVAL has been so before so changing this may
>   not be worth it but I am still wondering why we did choose '00'H in
>   the first place.)
>
> - Indentation of TruthValue inconsistent (I am nitpicking here)
>
> - I am wondering whether pingProbeHistoryLastRC values should not be
>   defined by referring to some IANA registry instead of an ip_icmp
>   include file. But probably not worth to fix?
>
> - Is there a rationale why pingCtlRowStatus is not required in the
>   minimum compliance statement? As it stands, I can have an
>   implementation which supports the volatile(2) storage type but I am
>   kind of left alone how to create entries. Please explain what the
>   goal was you were trying to achieve with this construction.
>
> - The description of the DSCP parameter needs to be reworked to
>   actually match RFC 3260. Note that there are actually only 6 bits
>   and not 8 bits.
>
>
> 3. DISMAN-TRACEROUTE-MIB
>
> - reverse the revision statements (youngest first)
>
> - copyright year needs to be updated to 2005
>
> - The description of the DSCP parameter needs to be reworked to
>   actually match RFC 3260. Note that there are actually only 6 bits
>   and not 8 bits.
>
> - I am wondering what traceRouteCtlMiscOptions is good for other than
>   proprietary extensions which IMHO should go into table augmentations.
>   Otherwise, we could put opaque options in almost all tables. I suggest
>   to document how this object is actually used by existing implementations
>   or to deprecate or even obsolete it.
>
> - I am wondering why the DEFVAL for traceRouteCtlDescr is '00'H and
>   not an empty string.  (The DEFVAL has been so before so changing
>   this may not be worth it but I am still wondering why we did choose
>   '00'H in the first place.)
>
> - The description of traceRouteCtlTrapGeneration has a double "to".
>
> - The object traceRouteCtlTrapGeneration should have a DEFVAL clause
>   and the text should say it defaults to "an empty set" and not
>   "zero".
>
> - The referenced object traceRouteHopsIpTargetAddress has a different
>   name.
>
> - I think traceRouteHopsRttSumOfSquares is meant to store the sum of
>   the squares and not just the sum.
>
> - Saying "The compliance statement for the DISMAN-TRACEROUTE-MIB."  in
>   the description clause of a compliance statement when there are
>   multiple compliance statements for the same module is mildly
>   confusing. I suggest to add more meat when this compliance statement
>   applies. (I notice that this comment actually also applies to the
>   other MIB modules - I am not repeating it there to save space and
>   time.)
>
> - Substitute traceRoutengCtlRowStatusGroup with traceRouteCtlRowStatusGroup.
>
> - The minimum compliance statement correctly says that some ojects
>   must be readonly when some optional tables are not supported. I
>   think some similar statements should be made for the full compliance
>   statement since some tables are optionally there as well.
>
> - I have the same question as before about the RowStatus being
>   optional and the implications of being optional. I guess we should
>   discuss this over the phone (or I have to dive into the WG archive).
>
> - I am not sure I understand or like the traceRouteMinimumGroup - I
>   guess this goes back to the RowStatus object which I think is the
>   only one missing in the traceRouteMinimumGroup.
>
> - Summarizing the last two issues, why did you not follow the approach
>   taken by the DIFFSERV-MIB where they have clauses such as the
>   following:
>
>     OBJECT       diffServMaxRateStatus
>     SYNTAX       RowStatus { active(1) }
>     MIN-ACCESS   read-only
>     DESCRIPTION
>        "Write access is not required, and active is the only status that
>        needs to be supported."
>
>
> 4. DISMAN-NSLOOKUP-MIB
>
> - reverse the revision statements (youngest first)
>
> - copyright year needs to be updated to 2005
>
> - lookupCtlTable description referes to gethostbyname/gethostbyaddr as
>   does lookupCtlTargetAddressType and lookupResultsTable
>
> - The second paragraph in the description of lookupCtlEntry is
>   confusing and it is unclear why it is there since
>   lookupCtlTargetAddressType is _not_ a part of the index. I
>   suggest to drop this paragraph altogether.
>
> - The lookupCtlRc object suggests to report errno on systems
>   that have it. Note that errno is only significant if the
>   getnameinfo/getaddrinfo function returns EAI_SYSTEM. I think
>   this object should be described in terms of the standard return
>   codes of the getnameinfo/getaddrinfo functions.
>
> - Not sure what a primary host address is or how I determine that.
>
> - Compliance wording consistency: "SET operations" -> "set operations"
>   and "a SET operation" -> "set operations" (appears multiple times)
>
> - The row status minimum compliance statement here is both more
>   concrete and less concrete. I am wondering how you start a lookup if
>   the row is not dynamically created. Is the lookupCtlRowStatus
>   considered to exist and transition between notInServices and active?
>   (Note that notInService usually has a timer associated, which of
>   course does not make sense here.)
>
>
> 5.  Security Considerations
>
> - The following is stated:
>
>    However, the only information that might be
>    disclosed is the configuration and results of measurements that are
>    performed by implementations of the MIB modules.  This information
>    can only be mis-used in conjunction with the mis-use of further
>    information.
>
>   I am not sure what the last sentence hints at. It sounds like it is
>   trying to make this less a security problem. Note sure this is true.
>   Tracepaths reveals information about paths which some people tend to
>   block (actually becoming more and more popular in enterprise
>   networks it seems).
>
>
> -- 
> Juergen Schoenwaelder     International University Bremen
> <http://www.eecs.iu-bremen.de/>     P.O. Box 750 561, 28725 Bremen, Germany