[IPFIX] js review of draft-ietf-ipfix-mib-variable-export-03

Hi,

there was no time before the I-D cutoff to review the -03 version and
hence I am sending my comments straight to the list (even though I am
likely the one to act on some of them). But this may be useful input
for the WG meeting later this week.

/js

- You can't write 'indexed MIBs' or 'indexed MIB object. You probably
  mean 'columnar objects'?  This is the term used in RFC 2578.  We
  should in general check that this document uses RFC 2578 terms. I am
  also not sure about the usage of phrases such as "MIB Indexing". I
  guess I have to go through the doc and carefully mark all phrases
  that may need rewriting. (A phrase like 'the MIB' does not work
  either, it is the MIB module but often you really mean the object
  definition in RFC 2578 sense.)

- The I-D talks about a number of new information elements that are
  defined later and initially when you read from the beginning, the
  solution remains relatively unclear for quite some time. What is
  missing is perhaps a simple right at the beginning that makes things
  clear for the reader.

- In the enumeration in section 5.2, I suggest to inline the
  explanations rather than having the explanation below.

- Figure 4 helps getting an overview (but perhaps a bit late). But you
  seem to make the assumption that there is a fixed # of
  mibObjectValues mapping to the same fixed number of mibFieldOptions.
  This may be fine - I am just noting this (and if correct it might be
  good to spell it out).

- What about ordering requirements? I assume no lexicographic ordering
  needed when MIB data is moved via IPFIX?

- What about instead using mibObjectValue you use mibObjectValue<Type>
  (mibObjectValueCounter32, mibObjectValueCounter64, ...)? Since there
  is a limited number of base types, you can also benefit from knowing
  the length in many cases without having to have the corresponding 
  mibFieldOption. But then, if you need a mibFieldOption anyway, the
  current design makes sense.

- I have trouble to understand Figure 6 and Figure 7.

- I do not fully understand mibIndexList in section 5.3.5. I assume
  Figure 10 is a Data Record? Should the caption be "Format of a
  mibFieldOption Data Record for a Columnar Object"? But even then, I
  think there should have been a corresponding template record example
  before. Oh no, this is an option template for a columnar object.  So
  the caption should be "MIB Field Option Template Format for a
  Columnar Object".

- I think mibName should be renamed to mibModuleName.

- Should the mibContextIdentifier not go into mibFieldOption Template?
  If so, should the text in 5.4 be more specific?

- Second paragraph in section 6.1 I think is not needed.

- It seems the example in 6.1 does not match the new templates etc??

- Is section 6.3 really needed? The only important sentence is this:

    This example stresses that, even though the OID cpmCPUTotal1minRev is
    enterprise-specific, the E bit for the mibObjectValue and
    mibObjectIdentifier is set to "0" since the "mibObjectValue" and
    "mibObjectIdentifier" Information Element is not enterprise-specific.

  This should be generalized and written down where the normative text
  is of the mibObjectValue and mibObjectIdentifier definitions, e.g.

    The E bit of the mibObjectValue or mibObjectIdentifier Information
    Elements is set to "0" since they are not enterprise-specific. This
    holds true even if the data carried inside the mibObjectValue or
    mibObjectIdentifier may be enterprise specific.

- Section 6.4 talks about multiple indexes. This is confusing. RFC
  2578 calls this indexed by multiple objects. I do not know how to
  read "index 2 is not an index of index 1".

- Is Figure 22 a "Options Template Set"? I expected this to be a
  Template Set...

- Should ipIfStatsIPVersion not be InetVersion in Figure 24?

- I have not fully understood the mibIndexList - the example in 6.4
  did not fully help either.

- Am I correct that the example in 6.4 is supposed to show the value
  of ipIfStatsInForwDatagrams (10000 and 20000) for ip4 and ip6 on
  interface 10? If so, note that InetVersion encodes IPv4 using '1'
  and IPv6 using '2'. In any case, showing how a data record is
  encoded is very useful but requires some textual explanation.

- Section 6.6. has not been updated? Otherwise, I do not understand
  what mibIndexString is here. I assume this is outdated text. I guess
  the same applies to section 6.8.

- Why do we talk about IPFIX transports in section 8? All this should
  be pretty agnostic to the choice of the transport, no?

- I think there needs to be some more explicit text saying that the
  security administrator must make sure that IPFIX export of MIB
  objects does not cause a hole into the SNMP access control
  configuration. (Some SNMP hardliners might even say that IPFIX
  should access data through the SNMP access control subsystem, in
  which case there would need to be proper parameters such as a
  (securityName, securityModel, securityLevel) tuple...

- Section 11.2 says "the MIB OID" which is ambiguous at best. It
  should probably be "the OID of a MIB object" or even better "the OID
  assigned to the MIB object definition".

- Is it really useful to encode OIDs as strings? SNMP uses a binary
  BER encoding which is more compact (but less human readable).

- Do we still need mibIndexString? Is this just an additional add-on?

- Section 11.2.5:

  Description of mibObjectName: RFC 2578 calls this a
  descriptor. Perhaps use this text:

  The name (called a descriptor in RFC 2578) of an object type
  definition.

- Section 11.2.6 Description of mibObjectDescription. Perhaps use
  this text:

  The value of the DESCRIPTION clause of an object type definition.

- Section 11.2.7: This should be SMIv2 (a superset of SMIv1). Since
  you include Integer32 and Gauge32, do you expect that type to be
  reported that SNMP uses on the wire or the type that is acting as a
  base type in a MIB object definition? In a MIB module, I can define
  something to use Integer32, on the wire it is always INTEGER. Same
  for Gauge32.

- Section 11.2.8: Are you talking about using the content of the
  SYNTAX clause of an object type definition or a subset of it?  Note
  that the SYNTAX clause may contain refinements and enumerations.

- Section 11.2.10: An SNMP context is a type of a contextEngineID and
  a contextName. See section 3.3 of RFC 3411. What exactly does the
  mibContextIdentifier contain? Note that the contextName defaults to
  an empty string for the common case of the default context. To
  follow the SNMP architecture correctly, you would still have to
  explain which contextEngineID your data belongs to.

- Section 11.2.11: As said before, this should be called mibModuleName
  and the description should be "The name of an SMIv2 MIB module".

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1, 28759 Bremen, Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>
_______________________________________________
IPFIX mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/ipfix