Subject: [Sigtran] FW: [Tsvwg] AD review of: draft-ietf-sigtran-sctp-mib-07.txt - Be rt Wijnen

From: "Ong, Lyndon" <[email-masked]>
Date: Tue, 27 Aug 2002 15:58:41 -0700

-----Original Message-----
From: Wijnen, Bert (Bert) [mailto:[email-masked]]
Sent: Monday, August 26, 2002 8:45 AM
To: [email-masked]
Cc: Shawn Routhier (E-mail); Bert Wijnen (E-mail)
Subject: [Tsvwg] AD review of: draft-ietf-sigtran-sctp-mib-07.txt

I have tried to not duplicate SHawn's comments. Appology if 
by accident I do have a duplicate.

More or less serious issues:
- You must decide what to do with the DeleteTCB.
  We cannot leave it as an open issue when doc goes to IESG
  And this of cours eimpacts last para of sect 3.1.3.1

- Your MODULE-IDENTITY macro MUSt include a REVISION clause.
  I would suggest to change:
      DESCRIPTION
           "The MIB module for managing an SCTP implementation."
      ::= {  mib-2 xxxx } -- IANA needs to choose this value
                          -- when sent to the RFC editor
  into:
      DESCRIPTION
           "The MIB module for managing an SCTP implementation."
      REVISION "200202060000Z"       -- 06th February 2002
      DESCRIPTION " Initial version, published as RFC XXXX"
      -- RFC Editor to assign XXXX
      ::= {  mib-2 xxxx } -- IANA to assign xxxx
  Pls keep in mind that REVISION date must be same as LAST-UPDATED.

- I am missing a reference to the/a discontinuity timer object
  for your Counters. Such is needed as per RFC2978, sect 7.1.6
  and sect 7.1.10

- A number of objects like sctpRtoMin, sctpRtoMax etc have 
  as data type Unsigned32. That means it can have a value of zero.
  I wonder if a zero value makes sense. If it does, then maybe
  it has special meaning? If so then pls describe. If it
  does not make sense, then better add a range to exclude it.
  I also wonder if a value of 1 or 2 millisecodns or such makes
  sense. If not, maybe say some words about it or add a lower range
  that sort of specifies the minimum that makes sense.
  Does Max have to be GE than Min ??

- sctpMaxAssoc (why not name it sctpMaxAssociations ??)
  I guess valid values are -1, and 0-0x7fffffff ??
  If so, then probably best to specify so as a range

- sctpAssocId is an Unsigned32. Value zero is reserved for
  "no association". But this is an INDEX object. So first
  of all, we do not want zero as an index in a table, but second,
  what would a zero entry represent?
  Shawn does discuss this too.
  By the way, what is "Initiate Verification Tag" ??
  Probably known to SCTPers, but not to me... Maybe add a 
  REFERENCE clause to where it is defined.

- sctpAssocRemHostName is OCTET STRING
  Mmm... currently I believe DNS names are US ASCII
  If that gets changed in the future, then I would assume it would
  possibly some form of UTF-8 ?? Not sure what the best approach 
  is here... but at least I would like to make the WG aware of 
  OCTET STRING meaning that it can be any string of octets. So
  please make sure to think about this before you decide.

- sctpAssocRemPrimaryAddressType DESCRIPTION clause talks about 
  "destination", I think it better uses "remote" instead. 
  And the 2nd para of this DESCRIPTION is not parsable by me. 
  Is it real english?
  But more serious...
  - The 2nd sentence of the DESCRIPTION clause can be removed.
    You have IMPORTed this data type from INET-ADDRESS-MIB
    so everyone understands that they need to look there to
    find out what it exactly means.
  - I would think that untill the value is know, that this
    object gets value unknown (0). And as long as this
    is zero (unknown) the value of sctpAssocRemPrimaryAddress
    should probably be the zero length OCTET STRING.
  - What are you trying to say? That you only support or
    recognize IPv4 and IPv6 addresses? If so, then pls do say
    so and refer to types ipv4 and ipv6. So the only values
    that have to be supported are unknown, ipv4 and ipv6

- sctpAssocRemPrimaryAddress DESCRIPTION clause talks about 
  "destination", I think it better uses "remote" instead. 
  But more serious:
  - The 2nd sentence I think better be worded as: The type 
    of this address is determined by the value of 
    sctpAssocRemPrimaryAddressType.
  - Explain that the value will be the zero length OCTET
    STRING untill a known value is received.

- sctpAssocHeartBeatTimer
  what would a value of zero mean?
  do values like 1 or 2 milliseconds make sense?

- sctpAssocState
  Instead of saying "it is appropriate to return badValue"
  I think it is better to say: "If any other value is
  received, then the agent must return a badValue error""".
  I believe that it is common practice that lots of agents do
  accept a SET for a value that is exactly the same as the
  current value (it basically is a NO-OP) and so they would
  return noErr. If you wanto allow that, then a note to that
  extend is a good idea.
  
- The counters in the sctpAssocTable also need to specify which
  timer object fiunctions to indicate discontinuity.

- sctpAssocLocalAddressType DESCRIPTION clause
  - remove 2nd sentence
  - "when the endpoint does still not determeined the address"
    I cannot parse
  - "This will be filled in after the INIT or INIT ACK ...."
    I do not understand this. This is an INDEX column. I would
    think that as long as you have not reeived the INIT or 
    INIT ACK, then there seems to be no value to me to have
    an entry at all, does it?
    So entries would (in my view) only show up when there
    is real (INDEX) information available.
- sctpAssocLocalAddress DESCRIPTION clause
  - 2nd sentence to be replaced by: The type of this address 
    is determined by the value of sctpAssocLocalAddressType.
  - Again, since this is an INDEX object, it seems to me
    that you would only instanciate an instance once you
    do have received the INIT or INIT ACK, and so no need
    to talk about unknown values. It sounds to me as if
    you only support ipv4 and ipv6 values (but I am not 100%
    sure of that... so you probably can tell me).
  - In any event, a "zero value" is kind of a strange concept
    for a data type (TC) that has an OCTET STRING as the 
    underlying base data type. Better explain what zero means
    if you do want to use that.
  - Last sentence, probably better: The sctpAssocLocalAddress
    may not be empty due to the SIZE restriction
  - Shawn discusses some of this as well

- sctpAssocRemAddresstable
  - same story about the indexing objects as above
    for the index objects in LocalAddressTable

- sctpAssocRemAddressStatus
  Enumerations are prefered to start at 1 and not at zero
  (see RFC2578 sect 7.1.1.)
  Instead of your own enumeration, you could use
     sctpAssocRemAddressActive
         SYNTAX TruthValue
  A reference to RFC2960 is better made using the 
  REFERENCE clause as per RFC2578, sect 7.6

- sctpAssocRemAddressHBFlag
  same story as for sctpAssocRemAddressStatus.
  Instead of your own enumeration, you could use
     sctpAssocRemAddressHBActive
         SYNTAX TruthValue
  A reference to RFC2960 is better made using the 
  REFERENCE clause as per RFC2578, sect 7.6

- sctpLookupLocalPortTable DESCRIPTION clause
  I think I understand what you mean to say
  but it sounds weird in my ears (maybe it is just me)
  May I suggest:
    This table allows a management application to find out
    all the associations that use a specific local port.

- sctpLookupLocalPortEntry DESCRIPTION clause
  I think I understand what you mean by the 2nd sentence,
  but it sounds a repeat of what is in the table description
  May I suggest for DESCRIPTION:
    Each entry represents the mapping of one association
    to a specific local port.
    
- sctpLookupLocalPortStartTime
  do you need this object? Is it not the same value as one
  would find in sctpAssocStartTime?  If so, then the question
  is: would a mgmt app want to find just all the associations
  per port and the startTime for such associations.
  If the answer is: yes,then fine.
  If the answer is no, it would need/want to know more about
  the associations, then it needs to go to the assoc table
  anyway and can pick up the time from there
  If the answer is: but we need at least one readable object,
  then maybe the question is: is this the best/most usefull
  object? Would assocState not be more usefull?

- Similar questions for the other reverse lookup tables.    

- In the MODULE-COMPLIANCE
  - change "SNMPv3" in "SNMP".
  - The discussion about only IPv4/IPv6 and such belongs in another
    place. Namely in the OBJECT clauses, as in fact you already do.
    I think I would write the MODULE-COMPLIANCE as follows:
      sctpCompliance MODULE-COMPLIANCE
        STATUS  current
        DESCRIPTION
           "The compliance statement for SNMP entities which implement
           SCTP MIB."
  - I find the optional groups in the MODULE compliance strange.
       They say: "This will be implemented when statistics are required"
       and "This should be implemented.... when it is required"
    But it is the task of the COMPLIANCE statement to specify when it 
    is or is not required. Possibly you want to indicate that these
    groups are optional. If so, then just say that they are
    optional at the discretion of the implemneter.
    I think however that I would rather make them mandatory, so all
    groups would (in my view) better be mandatory... 

  - The OBJECT clauses like:
           OBJECT  sctpAssocRemPrimaryAddressType
           SYNTAX  InetAddressType { unknown(0),
                                     ipv4(1),
                                     ipv6(2)
                                   }
           DESCRIPTION
                "It is only required to have IPv4 and IPv6 addresses
                without zone indices. Unknown values must also be
                supported in case the IP address has a zero string length
      s/IP address/Internet address/
      s/zero string length/zero length string/
                or an invalid/unknown format.

                The address with zone indices is required if an
                implementation can connect multiple zones."
   
    is great. But you also need (I think) to add the same restriction
    for the Address itself, aka

           OBJECT  sctpAssocRemPrimaryAddress
           SYNTAX  InetAddress (SIZE(0|4|16))
           DESCRIPTION
               "An implementation is only required to support globally
                unique IPv4 and globally unique IPv6 addresses."

  - Same for other address types/addresses

  - For the object  sctpAssocState, you can formally indicate that
    only one enum is writable, namely:
           OBJECT    sctpAssocState
           WRITE-SYNTAX  INTEGER { deleteTCB(9) }
           DESCRIPTION
               "Only the value deletedTCB(9) can be written"           

- Section 5 ...Mmmm... let us see

    5. Compiling Notes

    After compiling the MIB the following warnings can be got:

    @ index of row `sctpLookupRemHostNameEntry' can exceed OID size limit
       by 141 subidentifier(s)

Could limit it with a SIZE. Do you think that the remote hostname indeed
will exceed 80 octets? If you do not want to limit it... I guess
that in practice names will not be longer than that.
People have been saying that this OID limit (128 subids) is a CLR
(Crappy Little Rule). But I would not be sruprised if many SNMP
implementations
in fact will not handle more than 128 subids (since that is what the spec
RFC1905, sect 4.1) says

    @ index of row `sctpLookupRemPrimIPAddrEntry' can exceed OID size
       limit by 142 subidentifier(s)

Same story. Here you can actually limit it, cause you claim that you only
want to support IPv4 and IPv6. In fact you also use a SIZE limit for
sctpAssocLocalAddress for example.

    These two come from the fact that sctpAssocRemPrimaryAddress has the
    default InetAddress size of (0..255) which exceeds OID size
    limitations. Introducing a size restriction on
    sctpAssocRemPrimaryAddress would make the warning go away û although
    it would be one of those more arbitrary restrictions.

But see my note that there probably are several SNMP engines that will
not handle the more than 128 SUBIDs

    Some compilers can also show the following warnings:

    @ refined object `sctpAssocLocalAddressType' not listed in a
       mandatory or optional group

    @ refined object `sctpAssocRemAddressType' not listed in a mandatory
       or optional group

    The compliance statement refines these object which are not listed in
    one of the groups. The interesting thing is that you can't list them
    in one of the groups since the SMIv2 disallows to list not accessible
    objects. (It can actually be considered as a bug in SMIv2.) One
    solution would be to just put these two refinements into a
    DESCRIPTION clause or a comment.

Interesting ... thanks for the input. Need to check this.
Keep it for now.

- In the Security Considerations section, you must also list which
  objects are sensitive in read-only (if any) and explain why that
  is so and what the risks are.

Admin/bureaucratic/editorial notes:
- Pls remove "using SMIv2" from the title
  All MIB documents MUST use SMIv2 these days.
- RFC2851 has in the meanwhile been obsoleted by RFC3291
  So you can now update the references
- Why is ICANN listed in the Abbreviations list?
  It is not used anywhere in the text.
- SNMP boiler plate (Sect 2) is not the current one. You are
  not including the intro document RFC2570. See www.ops.ietf.org
  for current boiler plate (will soon change by the way when
  the SNMPv3 STD documents come out... but you may use the
  current boiler plate).
- By the way, I like section 3. I wonder if the "figure" in 
  sect 3.1.3.1 would not be better represented with a similar
  diagram as the one in sect 3.
- You have some "Redmond characters" in the text (for example
  last para on page 8 and also sect 3.2)
- I wonder if the first para in sect 3.2 is actually a good
  sentence. In any event, I cannot parse it.
- References must be split in normative and non-normative
  See http://www.rfc-editor.org/policy.html
  For the SNMP documents, here is the split:
  > > Normative        Informative
  > > ----------       ------------
  > >  1905                1155
  > >  1906                1157
  > >                      1212
  > >                      1215
  > >  2571                1901
  > >  2572                2570
  > >  2573
  > >  2574
  > >  2575
  > >  2578
  > >  2579 
  > >  2580
- IPR section missing as per RFC2026 Sect 10.
- In the MODULE-IDENTITY macro, pls add to the CONTACT-INFO
  a ptr to the WG mailing list and how to subscribe.
  Allison Mankin, should this be tsvwg mailing list?
  If so, then maybe the ORGANIZATION should also list TSVWG ??
- This is very subjective I guess... but I wonder (and these
  are just a few examples):
    why not use                      instead of
    sctpCurrEstablishedAssociations  sctpCurrEstab
    ...
    sctpAbortedAssociations          sctpAborteds
    ...
    sctpOutControlChunks             sctpOutCtrlChunks
    sctpOutOrderedChunks             sctpOutOrderChunks
    sctpOutUnorderedChunks           sctpOutUnorderChunks
    ..
    sctpFragmentedUserMessages       sctpFragUsrMsgs
    sctpReassembledUserMessages      sctpReasmUsrMsgs
    ...
    sctpOutPackets                   sctpOutSCTPPacks  
  In other words, make the "descriptors" or "names" recognizable
  for humans who may have to read them. Those names are supposed
  to be used by people instead of OIDs.
- sctpAssocInStreams, sctpAssocOutStreams
  It seems to me that the sentence "This parameter has to be
  read-only by the manager." is confusing. Better remove it.
  Shawn discusses this too
- In section 10, at the beginning you may want to add a note to
  RFC-Editor to remove that section before publication as RFC.

Oh well... hope this helps,
Bert 
_______________________________________________
tsvwg mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/tsvwg
_______________________________________________
Sigtran mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/sigtran