[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

FW: AD review of draft-ietf-snmpconf-pm-11.txt - part 2



OK, here is part 2. I know... back on Jan 28 I promised
it would come "tomorrow"... I guess that means (for most
of you) just one night sleep :-() .. Oh well.

The part 1 is also attached at the bottom in case anyone 
wants it all together.

Continuing from Page 69

- sect 9.4.14 and sect 9.4.15

  I get confused. A character in a UTF-8 string can be composed of
  multiple octets. I am sure that that is intended. But are we sure
  everybody understands. Specifically with the focus that a 7-bit
  ASCII character is the same in ASCII as in UTF-8 may confuse people.
  The chr() function may return a string of multiple octets length
  and the octets used from the str argument in ord() function
  may be several octets long to return an integer that will exceed
  255. I'd add something to make that clear.

- sect 11 MIB definitions general comments:
  - StorageType object MUST specify which columns are
    to be writable for 'permanent' entries
  - RowStatus objects MUST specify which columns can be
    modified while a row is 'active'
  - in front of (before) various tables you have extensive
    MIB comment lines. I think a lot of that would be better
    placed in the DESCRIPTION clause of the table.

- once you want this to be considered for PS, you better change
  the import of experimental to mib-2 and do a
    ::= { mib-2 xxx } -- xxx to be assigned by IANA
- pls add a copyright statement to the DESCRIPTION clause of
  MODULE-IDENTITY as per
     http://www.ietf.org/IESG/STATEMENTS/MIB-COPYRIGHT.txt
- Pls change
        "The original version of this MIB, published as RFCXXXX."
  Into
        "The original version of this MIB, published as RFCXXXX."
         -- RFC-Editor assigns XXXX
- Why is there a UTF8String TC ??
  How does it differ (why must it differ) from SnmpAdminString?
  Same question for Utf8String from RFC2287 ??
  I would like to avoid a proliferation of various UTF*String TCs
- I see:
    pmPolicyAdminGroup OBJECT-TYPE
        SYNTAX      UTF8String (SIZE(0..8))
  That size restriction makes it 1 or 2 characters in some 
  languages, no? Is that wise?
- I see:
  pmPolicyPrecedenceGroup OBJECT-TYPE
    SYNTAX      UTF8String (SIZE (0..32))
    MAX-ACCESS  read-create
    STATUS      current
    DESCRIPTION
        "An administratively assigned string that is used to group
        policies. For each element, only one policy in the same
        precedence group may be active on that element. If multiple
        policies would be active on an element (because their
        conditions return non-zero), the execution environment will
        only allow the policy with the highest value of
        pmPolicyPrecedence to be active."
    ::= { pmPolicyEntry 3 }
  I think this will require the use of stringprep.
  I discussed this use of stringprep with authors already and had
  an APPS AD involved as well. I'll not repeat this comment, but
  it potentially applies to other objects, where UTF8 strings need
  to be sequenced or compared.
- I see various objects iwth RANGES like:
    pmPolicySchedule OBJECT-TYPE
       SYNTAX      Unsigned32 (0..65535)
  WOuld it be better to not have a range and specify such a range
  in a/the MODULE-COMPLIANCE ??
  But how this match to pmSchedIndex OBJECT-TYPE
                         SYNTAX      Unsigned32 (1..4294967295)
  Is that not a table to which this value is a link?

- pmPolicyElementTypeFilter OBJECT-TYPE
    SYNTAX      UTF8String (SIZE (0..128))
  Is this how you want to do that? WOuld it not be better to have
  a ptr to a table of OID entries? Now it is so limited in the
  number of OIDs you can put in here.
  In the last sentence in the DESCRIPTION clause it says "if a value
  is registered". WOuld you rather s/registered/included/ ??
- pmPolicyConditionScriptIndex OBJECT-TYPE
    SYNTAX      Unsigned32
  I think a range to exclude zero would be wise. Otherwise explain
  what special value zero means?
  RRef [23] is normative I think. 
- Similar comment for ActionScriptIndex
- pmPolicyDebugging OBJECT-TYPE
    SYNTAX      INTEGER {
                    off(0),
                    on(1)
                }
  we (MIB reviewers) tend to tell people to start enums at 1.
  I also wonder if
     pmPolicyDebuggingOn OBJECT-TYPE
        SYNTAX      TruthValue
  would not be better here
- pmPolicyAdminStatus OBJECT-TYPE
    SYNTAX      INTEGER {
                    disabled(0),
                    enabled(1),
                    enabledAutoRemove(2)
  start enum at 1.
  I wonder... do you want a OperStatus as well?
- pmPolicyStorageType OBJECT-TYPE
  You must explain/describe in DESCRIPTION clause which
  objects must be writable for 'permanent' entries

- pmPolicyCodeTable
  I see nothing about persistency of this table, nor do I see
  a STorageType object
  Should some (or all) of the comments before this table 
  not just go into the DESCRIPTION clause?
  Same for other tables.
- pmElementTypeRegTable
  Mmm... I wonder if we do not need to specify the contextName
  as well ??
- in DESCRIPTION clause of pmElementTypeRegOIDPrefix

        This object identifier value is specified down to the 'entry'
        component (i.e. ifEntry) of the identifier.
  s /i.e./e.g./ ??

        A special OBJECT IDENTIFIER '0.0' can be written to this
        object. '0.0' represents the single instance of the system

  Mmmm... ? This is an INDEX object and so is not accessible.
  So "can be written to this object" sounds pretty weird to me

- pmElementTypeRegDescription OBJECT-TYPE
    SYNTAX      UTF8String (SIZE (0..32))
  Mmm... seems only some 5 characters in some languages.
  Sounds short for a "descriptive label" to me

- pmRoleContextName OBJECT-TYPE
    SYNTAX      SnmpAdminString
  I believe that in all other mib modules that use ContextName
  we have a SIZE of 0..32. Might make sense here too

- pmRoleContextEngineID OBJECT-TYPE
    SYNTAX      OCTET STRING (SIZE (0..32))
  Would it not be better to use (5..32) as we have defined
  SnmpEngineID with that size in RFC3414 ??
  I also wonder about 
        .................... If the element is on the local system
        this object will be the empty string."
  Why not use the local SnmpEngineID ?? If you do use empty string
  then Size could be SIZE(0 | 5..32).

- pmCapabilitiesType
  talks about "traps", probably better to use "notifications"
  May need to talk about "MIB module" in stead of MIB or MIBs,
  Instead of "standard MIBs" I would say "MIB modules on the 
  standards track"
  Talks about OIDs registered with IANA. But I don't see any
  IANA considerations or a registry where they can be registered.

- pmCapabilitiesOverrideState OBJECT-TYPE
    SYNTAX      INTEGER {
                    invalid(0),
                    valid(1)
                }
  start enums at 1, or better use TruthValue ??

- page 101
            20000101T080000/20000131T120000

              January 1, 2000, 0800 through January 31, 2000, noon
  Might want to use T130000 for 1pm to show 24hour clock?

- in pmSchedEntry and pmSchedStorageType you talk about
   pmSchedHour and pmSchedMinute objects, but I cannot find them

- pmTrackingPEContextName OBJECT-TYPE
    SYNTAX      SnmpAdminString
  SIZE (0..32) ??

- pmTrackingPEContextEngineID
  Same questions as above for xxxContextEngineID

- pmTrackingEPContextName OBJECT-TYPE
    SYNTAX      SnmpAdminString
  pmTrackingEPContextEngineID OBJECT-TYPE
    SYNTAX      OCTET STRING (SIZE (0..32))
  Same questions as above

- pmTrackingEPStatus OBJECT-TYPE
    SYNTAX      INTEGER {
                    on(0),
                    forceOff(1)
                }
  start enum at 1. Possibly a TruthValue?

- pmDebuggingTable
  What happens with any entries if a system reboots/restarts?

-   pmDebuggingContextName      SnmpAdminString,
    pmDebuggingContextEngineID  OCTET STRING,

  Same questions as a
  By the way, why is ...ContextEngineID not of TC SnmpEngineID.
  Probably because of the empty string ?? Another reason to not
  do that.

-     pmConformance   OBJECT IDENTIFIER ::= { pmMib 20 }
  why the sudden jump to 20 ??

- Please check your Security COnsiderations section against the
  latest http://www.ops.ietf.org/mib-security.html
  guidelines.

- Pls split references in normative and informative references
  as per http://www.rfc-editor.org/policy.html

bove...
Thanks,
Bert 

-----Original Message-----
From: Wijnen, Bert (Bert) [mailto:bwijnen@lucent.com]
Sent: dinsdag 28 januari 2003 0:54
To: Snmpconf (E-mail)
Subject: AD review of draft-ietf-snmpconf-pm-11.txt - part 1


Here is a list of NITS, admin/bureucratic comments and
more or less serious concerns/questions.

- on the fornt page, I would not repeat the authors/editors'
  names under the title.
- At next revision, update copyright date
- the RFC-Editor does not want the abstract to be a numbered 
  section
- Section 2, we have a new MIB boilerplate since the SNMPv3
  RFCs have been published as STD 62.
  See: http:/www.ops.ietf.org
- We also have a new MIB security guidelines at the same
  place. You may want to check if that means some changes
  to your Security Considereations section.
- References need to be split in Normative and Informative
  See http://www.rfc-editor.org/policy.html
- last para on page 4 talsk about 
   "..this document defines standard management objects" and
   "... in a standard form..."
  The attribute of STDs track is assigned external to an RFC,
  so you should not state/claim that this is standard in the
  document itself.
- First list of bullets in section 3 seems to be a marketing
  statement. We do not need those in RFCs.
- In general, I would like the authors/editors to go through 
  the document and to properly use the terms MIB, MIB Module.
  I think too often then term "MIBs" is used when you really 
  mean "MIB Modules". I noticed it at least twice in sect 5.3
- Sect 5.5.1.
  s/are are/are/
- Section 6 states in 3rd para:
  "Some examples of the features that have been removed...."
  Not sure it is really "have been removed" or if it is better
  stated as "are not used" or "are not available" or 
  "are not inherited" or such.
- Has anyone checked the EBNF specification via a syntax checker?
  See http://www.ietf.org/ID-nits.html section 2 6th bullet,
  which also points to
  http://www.ietf.org/IESG/STATEMENTS/pseudo-code-in-specs.txt
  For example, I am not sure this is correct (page 18)
         unary_op:          '+' | '-' | ' | '!' | '++' | '--'
  What does that single quote between the 2nd and 3rd } mean?
- sect 6.2.1
  I get a bit confused when you talk about 8-bit elements for
  a string (in UTF-8 context). Steve tried to explain, but 
  the current text still confuses me. I think he was going to
  try and add some text to unconfuse me (and possibly others).
- When you use EBNF, LHS, RHS and such acronyms the first time 
  in the document, pls expand them Thisis another RFC-Editor
  policy.
- I had some conversation with Steve about the Library functions,
  on hos they are split in 4 categories and which of the
  "reserved words" can be used when and where. Also a question
  on the "which categories must be supported". I believe the 
  answer was all. Steve is going to clarify some of this.
- Sect 9.1.1 starts with a ">"... ??
  I saw that at a few more places
- Sect 9.1.1 I see
    snmp-function(...[, integer mPModel,
                        string tDomain, string tAddress,
                        integer secModel, string secName,
                        integer secLevel, string contextEngineID
    ]) 

  It seems to me we need to be able to express the contextName
  for local and for remote SNMP engines? How do we do that?
  I think when I look further in the document, that one of the
  "..." arguments is probably the contextName. Might be good to 
  explain some of that.

- when I see this
    snmp-function(...[, integer mPModel,
                        string tDomain, string tAddress,
                        integer secModel, string secName,
                        integer secLevel, string contextEngineID
    ]) 

  and if I understand it correctlym then all those extra args
  are positional and optional, is then not the better notation:

    snmp-function(...[, integer mPModel [,
                        string tDomain [, string tAddress [,
                        integer secModel [, string secName [,
                        integer secLevel [, string contextEngineID
    ]]]]]]]) 

- The sample that follows
    getVar("sysDescr.0", "", SNMPv3, "snmpUDPDomain",
           "192.168.1.1/161", SNMPv3, "joe", "NoAuthNoPriv");
  confuses me somewhat. 
  I think it is a getVar for sysDescr.0, in the default contextName
  but it seems to list a SN MPv3 secModel. I am not aware that
  we have such a secmodel, do we? We have a USM secModel.
  We also have an SNMPv1 and SNMPv2c, or even an any secModel.
  But not an SNMPv3 secModel.

- Sect 9.1.1 also talks about TDomain. I would suggest to consider
  to use terms from RFc3419, which includes values for IPv6, which
  is something that IESG really wants these days.

- bottom of page 34. Why do we allow a trailing dot if it can case 
  strncmp() problems?

- sect 9.1.3.1
  The empty (zero length)  string is the default context.
  so I guess if one omits it that one also gets the default
  context. Not sure if that is clear here.
  At least the text
              If 'contextName' is not present,
        this element's contextName will be used.
  Confuses me. What does that mean?
  This comes back in a lot of the function descriptions

- Sect 9.1.3.1 and following sections
  So do we realize that a "programmer" of these scripts needs
  to speak OID lingo as opposed to descriptors?
  Do we really believe that that is encouraging?

- Sect 9.1.3.2
    The exists() function is used to verify the existence of an
    SNMP MIB instance.
  Do you mean s/MIB/MIB object/ 
  You cannot be meaning a complete MIB instance, do you?
  Similar text is used in following sections too.

- I guess that exists() returns an integer cause we do not
  have a boolean in our language, right?

- Sect 9.1.3.4
  Mmm... why not rename "pattern" to "searchString" ??

  And is the programmer expected to use digits to express
  which mode to use? Or are there reserved words for that?

- Sect 9.1.3.6
  I wonder how rows that have multiple integers, or octet
  strings or such as index, how they get created?

  Does the last para on page 42 agree with the code fragment
  on the next page? Spefically, does the if createRow() call
  and and checking of return code and index agree with that
  para? Maybe I am just confused.

- page 44 talks about "noSuchName or noSuchObject"
  - do we still want to talk about noSuchName (SNMPv1 ??)
  - do we not need to talk about noSuchInstance?

- sect 9.1.4.1
  How about error-status and error-index?

- sect 9.1.4.2
  I wonder if pdu should not be prefixed with &pdu ?

  Can this function never return an error?

- sect 9.1.4.3 and 9.1.4.4
  I see
          int readVar(integer pdu, integer varBindIndex,
  and I see
    integer snmpSend(integer reqPDU, integer reqNumVarbinds,
  So I wonder, do we have both int and integer as a datatype?

- sect 9.1.4.5
  What if you pass a non-existing pdu handle?
  Will it return an error? so should it be:

  integer  readError(integer pdu, integer numVarbinds,
                     integer &errorStatus, integer &errorIndex,
                     integer &hasException)

  to show that it can return an error?
  The same question for some of the other functions in sect 9.1

- sect 9.2
  I wonder if this is not better sect 9.1.5 ??
  It is part of the SNMP library functions, no?

- sect 9.2
    const integer Integer       = 2;
  So does that potentially cause confusion for people to
  recognize the difference between integer with lowercase i
  and upprecase I ??
  In fact I wonder if these could not all better be prefixed
  with some string like SNMP or so, e.g.

  const integer SNMP_Integer       = 2;
  const integer SNMP_Integer32     = 2;
  etc

  You refer to RFC1905, better to refer to RFC3416

  You also refer to RFC2571, better to use RFC3411

- sect 9.3.1
  I fail to see how the function returns an indication on
  how the object is indexed.
  And does the prototype function not need arguments?

- sect 9.3.4 and 9.3.5
  I had asked steve if the function ev() and ec()
  could be better named elementValue() and elementCount().
  He answered that thes function will be used a lot, similar
  to argv and argc in C. So.. are we happy that these
  are such cryptic functiuon names? Is elemv() and elemc() 
  a better compromise. 
  I can live with ev() and ec() if the WG says it is OK.

- sect 9.3.4 and 9.3.5
  I wonder if it is easy to know what "this element" means.
  I think it is explained in sect 7... but that is quite
  a few pages back. 

- When I see things like the roleMatch function and I know
  that we are in a UTF-8 environment, then I think we need
  to include stringprep functionality as per RFC3454.
  In fact Patrik Faltstrom, APPS AD has confirmed that this
  is needed, and he has already pointed the authors to a
  sample.

- I guess we have another set of reserved words at the end of
  sect 9.3.7
  Maybe we need one section that lists them all, no?

- sect 9.4.6 last sentence
  "... twice if is an expression" ???

- sect 9.4.8
  I first thought that oid needed to be &oid.
  But the resulting OID is returned as a string (is what Steve
  told me). I guess the "...replace len subids in oid1.."
  is what confused me.

OK, part 2 comes tomorrow.

Bert


Thanks,
Bert