Re: Re: MIB Doctor review: publish draft-ietf-disman-remops-mib-v2-06.txt

Dear all,

One IETF meeting ago, I promised a follow-up of the
message below for finalizing the discussion of remops
issues that Juergen Schoenwaelder raised in the MIB
doctor review.

I am very sorry for delaying this follow-up until today!

After reviewing again all issues that Juergen S. raised,
I found only two non-editorial issues that are still open:

1. Referring to the hostent structure

>  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.)

I agree on this comment addressing the section 3.3.3 on the
lookupResultsTable.  This section needs to be re-written in order
to become independent of the IP version.  This will also effect
objects lookupCtlTable and lookupCtlRc in the DISMAN-NSLOOKUP-MIB
module.

2. pingCtlRowStatus in the minimum compliance statement?

>  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.

I read again the discussion we had with Eduardo Cardona on the mailing
list in June and July 2004.  There we agreed on not including this
object in the minimum requirements in order to reduce the minimal
number of objects to be supported by light-weight implementations.
A consequence was a deprecation of the pingGroup in the conformance
section.

We could reconsider this decision and include pingCtlRowStatus
in the minimum compliance statement and contimue using the pingGroup.

We also could address Juergen's comment by further restricting the
pingCtlStorageType when pingCtlRowStatus is not implemented.


I will discuss both issues with Juergen Schoenwaelder this week at
the IETF meeting and post suggestions for solutions on the list at
the end of the week.

    Juergen Q.
-- 
Juergen Quittek        [email protected]       Tel: +49 6221 90511-15
NEC Europe Ltd.,       Network Laboratories        Fax: +49 6221 90511-55
Kurfuersten-Anlage 36, 69115 Heidelberg, Germany   http://www.netlab.nec.de


--On 09.07.2005 18:20 Uhr +0200 Juergen Quittek wrote:

> Dear all,
>
> Please find replies on Juergen's comments below.
> I will cover a few of the comments in separate messages.
>
> --On 5/3/2005 5:15 PM +0200 Juergen Schoenwaelder wrote:
>
>> 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
>>   occurrence of "remote".
>
> done.
>
>> - 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].
>
>  done.
>
>> - 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.)
>
> Please see separate message on this issue.
>
>> - 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.
>
> Would you be fine with
> OLD
>    In addition, the DISMAN-PING-MIB module and the DISMAN-TRACEROUTE-MIB
>    modules each have a deprecated compliancy statement that was current
>    in RFC 2925.
> NEW
>    In addition, the DISMAN-PING-MIB module and the DISMAN-TRACEROUTE-MIB
>    modules each have a deprecated compliancy statement that was current
>    in RFC 2925. Semantically, the new full compliancy statements are
>    identical with the deprecated ones. But the object groups in the
>    compliancy sections needed to be restructured which is reflected by
>    the new full compliancy statements.
> ?
>
>> - 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.
>
> fixed.
>
>> 2. DISMAN-PING-MIB:
>>
>> - reverse the revision statements (youngest first)
>
> fixed.
>
>> - copyright year needs to be updated to 2005
>
> done.
>
>> - pingSnmpQuery description clause "is an" -> "is using an"
>
> fixed.
>
>> - 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.)
>
> replaced OLD
>     DEFVAL { '00'H }
> with NEW
>     DEFVAL { ''H }
>
>> - Indentation of TruthValue inconsistent (I am nitpicking here)
>
> looks better now.
>
>> - 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?
>
> OLD
>            "The last implementation method specific reply code received.
>            If the ICMP Echo capability is being used then a successful
>            probe ends when an ICMP response is received that contains
>            the code ICMP_ECHOREPLY(0).  The ICMP responses are defined
>            normally in the ip_icmp include file."
> NEW
>            "The last implementation method specific reply code received.
>            If the ICMP Echo capability is being used then a successful
>            probe ends when an ICMP response is received that contains
>            the code ICMP_ECHOREPLY(0).  The ICMP codes are maintained
>            by IANA.  Standardized ICMP codes are listed at
>            http://www.iana.org/assignments/icmp-parameters.
>            The ICMPv6 codes are listed at
>            http://www.iana.org/assignments/icmpv6-parameters."
>
>> - 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.
>
> Please see separate message on this issue.
>
>> - 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.
>
> Suggestion: let's use the TC Dscp from MIB module DIFFSERV-DSCP-TC
> (RFC 3289) for the SYNTAX clause and add RFC 3260 to the references:
>
> OLD
>     pingCtlDSField OBJECT-TYPE
>        SYNTAX      Unsigned32 (0..255)
>        MAX-ACCESS  read-create
>        STATUS      current
>        DESCRIPTION
>            "Specifies the value to store in the Differentiated
>            Services (DS) Field in the IP packet used to
>            encapsulate the ping probe.  The DS Field is defined
>            as the Type of Service (TOS) octet in a IPv4 header
>            or as the Traffic Class octet in a IPv6 header.
>
>            The value of this object must be a decimal integer
>            in the range from 0 to 255.  This option can be used
>            to determine what effect an explicit DS Field setting
>            has on a ping response.  Not all values are legal or
>            meaningful.  A value of 0 means that the function
>            represented by this option is not supported.  DS Field
>            usage is often not supported by IP implementations and
>            not all values are supported.  Refer to RFC 2474 for
>            guidance on usage of this field."
>        REFERENCE
>            "Refer to RFC 2474 for the definition of the
>            Differentiated Services Field and to RFC 1812
>            Section 5.3.2 for Type of Service (TOS)."
>        DEFVAL { 0 }
>        ::= { pingCtlEntry 22 }
> NEW
>     pingCtlDSField OBJECT-TYPE
>        SYNTAX      Dscp
>        MAX-ACCESS  read-create
>        STATUS      current
>        DESCRIPTION
>            "Specifies the value to store in the Differentiated
>            Services (DS) Field in the IP packet used to
>            encapsulate the ping probe.  The DS Field is defined
>            as part of the Type of Service (TOS) octet in the IPv4
>            header or as part of the Traffic Class octet in the
>            IPv6 header, respectively.
>
>            This option can be used to determine what effect an
>            explicit DS Field setting has on a ping response.
>            Not all values are legal or meaningful.  A value of 0
>            means that the function represented by this option is
>            not supported.  DS Field usage is often not supported
>            by IP implementations and not all values are supported.
>            Refer to RFC 2474 and RFC 3260 for guidance on usage of
>            this field."
>        REFERENCE
>            "Refer to RFC 2474 and RFC 3260 for the definition of
>            the Differentiated Services Field."
>        DEFVAL { 0 }
>        ::= { pingCtlEntry 22 }
>
> NEW
>        Dscp
>            FROM DIFFSERV-DSCP-TC;           -- RFC3289
>
>> 3. DISMAN-TRACEROUTE-MIB
>>
>> - reverse the revision statements (youngest first)
>
> fixed.
>
>> - copyright year needs to be updated to 2005
>
> done.
>
>> - 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.
>
> See pingCtlDSField. traceRouteCtlDSField changed analogously.
>
>> - 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.
>
> Please see separate message on this issue.
>
>> - 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.)
>
> replaced OLD
>     DEFVAL { '00'H }
> with NEW
>     DEFVAL { ''H }
>
>> - The description of traceRouteCtlTrapGeneration has a double "to".
>
> fixed.
>
>> - The object traceRouteCtlTrapGeneration should have a DEFVAL clause
>>   and the text should say it defaults to "an empty set" and not
>>   "zero".
>
> OLD
>            The value of this object defaults to zero, indicating
>            that none of the above options have been selected."
> NEW
>            The value of this object defaults to an empty set,
>            indicating that none of the above options have been
>            selected."
>        DEFVAL { { } }
>
>> - The referenced object traceRouteHopsIpTargetAddress has a different
>>   name.
>
> fixed.
>
>> - I think traceRouteHopsRttSumOfSquares is meant to store the sum of
>>   the squares and not just the sum.
>
> OLD
>        DESCRIPTION
>            "This object contains the sum of all traceroute responses
>            received for this hop.  Its purpose is to enable standard
>            deviation calculation."
> NEW
>        DESCRIPTION
>            "This object contains the sum of the squares of all
>            round-trip-times received for this hop.  Its purpose is
>            to enable standard deviation calculation."
>
>> - 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.)
>
> Please see separate message on this issue.
>
>> - Substitute traceRoutengCtlRowStatusGroup with traceRouteCtlRowStatusGroup.
>
> fixed.
>
>> - 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.
>
> Please see separate message on this issue.
>
>> - 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."
>
> Please see separate message on this issue.
>
>>
>> 4. DISMAN-NSLOOKUP-MIB
>>
>> - reverse the revision statements (youngest first)
>
> fixed.
>
>> - copyright year needs to be updated to 2005
>
> done.
>
>> - lookupCtlTable description referes to gethostbyname/gethostbyaddr as
>>   does lookupCtlTargetAddressType and lookupResultsTable
>
> Please see separate message on this issue.
>
>> - 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.
>
> OLD
>        DESCRIPTION
>            "Defines an entry in the lookupCtlTable.  A
>            lookupCtlEntry is initially indexed by
>            lookupCtlOwnerIndex, which is of type SnmpAdminString,
>            a textual convention that allows for use of the SNMPv3
>            View-Based Access Control Model (RFC 3415, VACM)
>            and also allows an management application to identify
>            its entries.  The second index element,
>            lookupCtlOperationName, enables the same
>            lookupCtlOwnerIndex entity to have multiple outstanding
>            requests.
>
>            The value of lookupCtlTargetAddressType determines which
>            lookup function to perform.  Specification of dns(16)
>            as the value of this index implies that the gethostbyname
>            function should be performed to determine the numeric
>            addresses associated with a symbolic name via
>            lookupResultsTable entries.  Use of a value of either
>            ipv4(1) or ipv6(2) implies that the gethostbyaddr function
>            should be performed to determine the symbolic name(s)
>            associated with a numeric address at a remote host."
> NEW
>        DESCRIPTION
>            "Defines an entry in the lookupCtlTable.  A
>            lookupCtlEntry is initially indexed by
>            lookupCtlOwnerIndex, which is of type SnmpAdminString,
>            a textual convention that allows for use of the SNMPv3
>            View-Based Access Control Model (RFC 3415, VACM)
>            and also allows an management application to identify
>            its entries.  The second index element,
>            lookupCtlOperationName, enables the same
>            lookupCtlOwnerIndex entity to have multiple outstanding
>            requests.  The value of lookupCtlTargetAddressType
>            determines which lookup function to perform."
>
>> - 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.
>
> Please see separate message on this issue.
>
>> - Not sure what a primary host address is or how I determine that.
>
> Please see separate message on this issue.
>
>> - Compliance wording consistency: "SET operations" -> "set operations"
>>   and "a SET operation" -> "set operations" (appears multiple times)
>
> changed all of them.
>
>> - 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.)
>
> Please see separate message on this issue.
>
>>
>> 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).
>
> OLD
>    see above
> NEW
>    However, the only information that can be
>    disclosed without encryption is the configuration and results of
>    measurements that are performed by implementations of the MIB
>    modules.
>
>> --
>> Juergen Schoenwaelder		    International University Bremen
>> <http://www.eecs.iu-bremen.de/>	    P.O. Box 750 561, 28725 Bremen, Germany
>
> Again many thanks for the very detailed and constructive review.
>
>     Juergen Q.
> --
> Juergen Quittek        [email protected]        Tel: +49 6221 90511-15
> NEC Europe Ltd.,       Network Laboratories        Fax: +49 6221 90511-55
> Kurfuersten-Anlage 36, 69115 Heidelberg, Germany   http://www.ccrle.nec.de
>
>