Re: [IPFIX] WGLC for draft-ietf-ipfix-mib-variable-export-05

[Colin McDowall <[e-mail]>]

Hi Juergen, IPFIX List,

This is a great and detailed review thanks. Paul and I
have been working to address and resolve your comments.

Paul should have a new version of the draft checked in later today.

My comments are prefixed with "CM:"
I've merged Paul's replies in as well at "PA:"

I'll follow up with the replies to Benoit's reviews as well.

Colin.
     
On 18/04/2014 17:21, Juergen Schoenwaelder wrote:
> On Tue, Apr 15, 2014 at 12:01:11PM +1200, Nevil Brownlee wrote:
...
> I have reviewed the document and I believe it is not ready. The main
> points:
>
> - Terminology needs to be aligned with SMIv2
> - I spotted several errors in the examples
> - Major issues with the IE definitions in section 10
> - I also believe the introductionary text needs to improve.
>
CM:
These 4 top level issues have been addressed as noted below.

> /js
>
> I have reviewed draft-ietf-ipfix-mib-export-05 and I think it is not
> ready and requires improvements.
>
> 1) The introduction contains details of the solution. I think it
>     should instead contain the motivation and the architectural model
>     currently in section 2. The new introduction should then be
>     followed by a terminology section before an overview of the
>     solution is provided (so that the terminology is defined). This is
>     primarily text reorganization.
CM:
Ok - the introduction section needs reworked. - Benoit had a good suggestion for this
so I've implemented that.

>
> 2) I have trouble to understand the Figure "Architectural Overview".
>     I think this should be removed. Figure 4 is more useful and it is
>     at the right place in the document.
CM:
Yes, this is a simplified version of the same figure 4, added to help provide
a high level overview of the approach.
If it is more confusing than helpful it can be removed.
Benoit had an issue with this figure not helping so I've removed it.

>
> 3) I have terminology issues in several places. For example, RFC 2578
>     uses the term 'columnar object' for what this document seems to
>     call 'indexed object'.
CM:
I've updated all the references to indexed objects to columnar object.
Fields that contain a columnar object are still referred to as indexed fields.
This is because the export is including the index for that particular field.

>     Why is it useful to call Flow Records Data
>     Records? Using two terms for the same thing may just adds potential
>     for confusion.
CM:
As with Benoit's comment on this all the references to Flow Record have been updated
to use 7011 except:
      This document prefers the more generic term "Data Record"
      (as opposed to "Flow Record") in relation to the export of MIB objects.
We should only have Flow Record being used if we need a section discussing
a difference between Flow and Non Flow Data Records. I don't think that needs
discussion in this draft.

>      I also think more SMIv2 terms must be imported, such
>     as MIB module, MIB object, columnar object, ...
CM:
Agreed. I've imported more SMIv2 terms, the SYNTAX used in SMIv2 and the terms used to refer to SNMP contexts.


> 4) There is a statement that providing index information for columnar
>     objects is optional. I have some difficulty to understand why this
>     is useful.
CM:
The exporting process may have a way to provide information to make unambiguous
what the mib object value refers to via other key fields.
In particular it has been useful to allow customers/implementors the freedom to use ipfix
fields as best describes the problem being worked on.


>      What does "a MIB object is used purely as type
>     information" mean?
CM:
A MIB Object is providing a definition of a datatype that can be exported. This
was one of the original goals of this draft, to allow the use of types already defined
in the range of standard and non-standard MIBs without having to duplicate each object
into an IANA ipfix information elements.

When we are using the MIB modules purely to provide extra type information there should
be no requirement on the implementer of the exporting process to also implement access
to these via SNMP.

See the following in the current introduction:
> Rather than mapping existing MIB objects to IPFIX Information Elements on a case by case basis,
>it would be advantageous to enable the export of any existing or future MIB objects as part
>  of an IPFIX Data Record. This way, the duplication of data models [RFC3444], both as SMIv2 MIB
> objects and IPFIX Information Elements, out of the same information model [RFC3444] would be avoided.

See 2nd last paragraph of Section 2 (-05 draft):
>       Another advantage of exporting MIB objects via IPFIX is that IPFIX
>       would benefit from an extended series of types to be exported.  The
>       simple and application-wide data types specified in SMIv2
>       <xref target="RFC2578"/>, along with a new Textual Conventions, can be
>       exported within IPFIX and then decoded in the Collector.
>       ...
CM:
Also of note is that as documented in rfc 3410 the management information and the data definiton
languages are independent of the protocol used to export the data. This is just the first step
in providing an alternative/replacement to SNMP to export MIB Object data.
If exporting data about a columnar object without fully indexing it would always be
incorrect we can add a requirement that indexing MUST be used and remove the bits making it optional.

>      How can an exporting process not have access to
>     index information of a columnar object?
CM:
If there is not a SNMP or MIB process running on the exporting device.
In particular the exporting device may not implement the MIB Objects that are being
used as INDEX fields.

Alternatively an exporting device may be aggregating data from several simpler exporters
that don't understand fully how the data they are sending to the aggregating process needs
to be indexed.

>
> 5) The terminology sections says a "MIB Object Identifier" is an ASCII
>     character string of a certain format but it seems later a BER
>     encoded representation is used for mibObjectIdentifier. This seems
>     inconsistent.
>
CM:
I think this definition was originally taken from the SMIv2 document. I've replaced this with
the relevant text from rfc2578. Maybe this should be replaced with just a reference to SMIv2 as per
your comment 3 though.

> 6) "exporting a value from a MIB does not imply that the SNMP process
>     on the device supports that MIB"
>
>     Proper wording would be something like this:
>
>     "exporting a value of a MIB object defined in a certain MIB module
>     does not imply that the SNMP prcess on the device supports that
>     MIB module"
CM:
Fixed.

>
>     Similarly, "exporting MIBs in IPFIX" should be "exporting values of
>     MIB objects".
>
CM:
Fixed.

>     The paragraph on page 9 is also the first occurance of the term
>     "MIB Field" - without a clear definition what this means.
>
CM:
I could either change this to use the same terms as the rest of the document
or provide a definition for this term. Since this is a hypothetical, negative case
that we aren't standardizing I think I'll just make this paragraph more general.
Changed to:
>    The values of a MIB Object could be exported using a MIB specific Information Element without providing any Object Identifiers.
>    However, without exporting the actual MIB OID the full type of the data would be unknown and every Field containing MIB Object data would appear identical.
>    Without knowing which OID the contents of a Field map to the data would be incomprehensible to a Collector.


> 7) This text needs rewrite:
>
>     Since these values are statically defined in the MIB they are not
>     expected to change frequently.  However the additional information
>     about the MIB may help a Collecting Process that does not have access
>     to the MIB.
>
>     NEW
>
>     Type information is statically defined in a MIB module, it is not
>     expected to change.  However, the additional information about the
>     MIB Object may help a Collecting Process that does not have access
>     to the MIB module.
>
CM:
Replaced with your NEW version.

> 8) Is it mibTypeOption of mibSubTypeOption? Both words are used, or
>     are these different things?
>
CM:
Yep that should be mibTypeOption only, I've fixed the typo.

> 9) I fail to parse/understand these statements:
>
>       Forf each of these the options this draft specifies exactly which
>       mibObjectValue to use.
>
CM:
Removed the extra these and clarified.
Now reads:
>  For each of the SYNTAX clause options this draft specifies exactly which mibObjectValue to use.


>       [...]
>
>       If the SYNTAX clause contains a Textual Convention or Subtyping the
>       mibObjectSyntax Information Element SHOULD be used to export this
>       detail to the Collecting Process.
CM:
I'm not sure what is wrong with this. To paraphrase this a little, if the type
information is complicated the export process SHOULD export the full details of the
type to the collector. For example include the mibObjectSyntax field in a mibTypeOption
export.

The idea is that in the more complicated cases or with textual conventions the base type
of the MIB Object does not fully convey the type of the data. The collector either needs to
have access to the MIB module definition or be sent the SYNTAX text.

>
> 10) It escapes me how mibObjectValueSequence and mibObjectValueTable
>      are used / encoded. And why does the first one use an ASN.1 type
>      name while the second uses an SMIv2 conceptual term? This seems
>      inconsistent, either call them mibObjectValueSequence and
>      mibObjectValueSequenceOf or mibObjectValueRow and
>      mibObjectValueTable.
>
CM:
These are exported using IPFIX Structured data. The key point is that while
SNMP has all access as scalar single values with IPFIX we can define compound
data structures.

So rather than having a conceptual row - we can export an actual row with all
the values for that row together.

Likewise with a table. We can export a list of rows to send a complete conceptual
table in one field. This means that an exporting process / export designer can correlate
a row of stats from a MIB with other IPFIX fields easily.
See section 4.8.2 for how a conceptual row is exported
See section 4.8.4 for how a complete table can be exported.

I like the names mibObjectValueRow and mibObjectValueTable though and since that moves
to using just SMIv2 terms I've renamed those.

> 11) This text is unclear and needs a rewrite. I think I understand
>      what you are trying to communicate but the wording needs
>      improvements. (And it is a MIB module here, not a MIB.)
>
>       This holds true even if the data carried inside the mibObjectValue or
>       mibObjectIdentifier may be related to an enterprise specific MIB.
>       The OID itself encodes if the Object is in an Enterprise specific MIB
>       Module.
>
CM:
Yes, this wording originated from the example you suggested cutting from a previous draft.
I've reworded it as follows:
        The MIB Object being exported may be defined in an enterprise
        specific MIB module but the Information Elements defined in this
        standard are still exported with the E bit set to "0".  The OID
        exported encodes that the MIB object was defined in a Enterprise
        specific MIB Module.

> 12) What does "Field Length (mib)" stand for? Is there anything
>      special or is this just a plain normal "Field Length"? If so, I
>      would rather remove the "(mib)" annotation.
CM:
This is so that the note below the figure can refer to this particular
part of the diagram. I could number the length fields if that is clearer or less
confusing.

>
> 13) I suggest to replace "more information about MIB indexing, extra
>      data from the MIB" with "additional information about the MIB
>      Object definition"
CM:
Updated.

>
> 14) Rewrite "... a reference to a MIB that ..." - I think you mean a
>      MIB Object.
>
CM:
Fixed.

>      Rewrite "may not have access to the MIB" -> "may not have access to
>      the MIB module".
CM:
Fixed.

>
>      And then I am getting lost here:
>
>        [..] It also allows the IPFIX Field Types to be extended with
>        any MIB Variable already defined purely through IPFIX.
>
>      It is not clear what is communicated here.
CM:
Ok, I've reworded that.

>
> 15) Section 4.4.6 has more inaccurate usages of the term "MIB" but
>      more important,
CM:
Fixed those.

>      I wonder what actually the OID is that is being
>      shipped in mibObjectIdentifier. Is it the OID assigned to the
>      object type definition or is it the OID of the instance of an
>      object type? Have I overlooked where this is clearly defined?
CM:
This is already defined in the definition for the mibObjectIdentifier IE:
Section 10.2.1:

> The mibObjectIdentifier Information Element contains the OID assigned to
> the MIB object definition encoded as ASN.1/BER
I can add the word type in there to make it clearer though:
Section 10.2.1 changed to:
> The mibObjectIdentifier Information Element contains the OID assigned to
> the MIB Object Type Definition encoded as ASN.1/BER
>


> 16) Terminology: "MIB Sequence Object's INDEX clause" -> "INDEX
>      clause of a conceptual row object".
CM:
Changed to:
> However, fields used as an INDEX MUST be in the same order as specified in the INDEX clause
> of the conceptual row MIB Object.
>


> 17) s/then the MIB context MUST/then the context MUST/
>
>      MIB context is not a defined term. There are only SNMP contexts.
>      (Perhaps they should have been MIB contexts but this is a
>      different story.)
>
CM: Fixed.

> 18) s/the exported to export/the exporter to export/
CM: Fixed.

>
> 19) Section 4.8 is rather confusing, primarily because the terminology
>      is not aligned with SMIv2 terminology. This affects almost all
>      sentences. In SMIv2, terms like "MIB SEQUENCE Objects" do not
>      exist. SMIv2 calls this a 'conceptual row object'. This section
>      needs to be rewritten to align with SMIv2 terminology (this means
>      getting rid of almost all occurrences of 'sequence' (in any
>      capitalization)). Terms such as "full OID" are ambiguous unless
>      you say what the OID is supposed to refer to.
Ok, I've updated this section to refer to conceptual rows and tables.
I think I was understanding an OID refereing to an instance of a MIB Object as
a "full OID". I  think this was in the initial version of this document - it is probably
where I learned this term.

>
>      The first item in 4.8 talks about ingoring the indexing of
>      columnar objects and later says "a columnar object may be used
>      purely as a data type. I have a difficulty to understand how that
>      would be useful.
Again the a columnar MIB object can only ever have 1 set of index fields for the row
it is in. We want to be able to use the new MIB fields in combinations with
any other IPFIX field when useful / understandable.

Also for the simplest cases and simplest exporters we want to make it easy
to send data matching the MIB object type without having to introduce arbitary
indexes when not required.

We need to provide a mechanism to export indexing information, but we shouldn't
require it. I also can't see the use case of looking up a mib to check the value you have
just received, Ideally we want to replace snmp to provide access to the model, not just
send a duplicate traffic stream.

>
>      On page 33, a paragraph refers to section 7.7. of RFC 2578 and I
>      am confused what it tries to say here. So far, I assumed that the
>      OIDs all refer to columnar objects or conceptual row objects and
>      include no instance identifiers.
All the OIDs exported in the mibObjectIdentifier are OIDs for the object defitions.
I've clarified the text:
     
      The mibIndexIndicator marks the Fields whose value(s) should
      be added to the OID for the MIB Object type definition
      exported in mibObjectIdentifier to get the OID for the instance
      of the MIB Object.
I've also deleted the references to fully indexing an Object.

>
>      I do not understand the last paragraph before section 5.
OK, the idea was to prevent the daisy chain effect of exporting a lot of
mib objects just to satisify indexing requirements.
It is probably just better to use the provision to not provide the index details
rather than use an opaque number.

Taking account of your comment on the example that uses this it has been
removed here as well. This paragraph is now gone.

>
> 20) The first enumerated item in Section 5.1 says "flowStartSeconds
>      from [RFC7012]" but then RFC 7012 does not define this anymore
>      (but its predecessor RFC 5102 did). Since Section 5.2 has the same
>      text, the same issue needs to be resolved there.
CM:
Update to refer to IANA-IPFIX as the definition of these fields.

>
> 21) Sections 5.1. and 5.2 both contain references to themselfs "(see
>      Section 5.x)" which does not seem to make much sense. Last sentence
>      on page 38: s/this identical/this is identical/
CM: Fixed - the references weren't adding anything.
Sentence fixed

>
> 22) I looked up the definition of cpmCPUTotal1minRev and it turns out
>      that this is a columnar object (which makes sense since you can
>      have multiple CPUs). Since you do not export information about
>      which CPU the load value is coming from, how useful is the
>      information and does this make a good example?
>
CM:This is a good example of using the columnar object without providing
the full indexing of which CPU the information relates to. Without providing
any more fields the implicit scope would be interpreted as being the CPU
total for the exporting device. If there is only 1 CPU how has adding extra indexing
or a whole extra column helped with the export or clarity of the data.

However the text refers to the CPU usage as being a non columnar object, that
needs to be fixed.
Updated text:
     Although the cpmCPUTotal1minRev MIB Object is a columnar object in a conceptual row, while there
     is only 1 CPU no extra information is conveyed by providing the INDEX field. So in this case it is
     acceptable to not export the cpmCPUTotalIndex MIB Object. If there were multiple CPUs it would be appropriate
     to include this the cpmCPUTotalIndex field and specify the relationship.

> 23) Section 5.3. needs a title change. It is about exporting a subset
>      of a conceptual row. Perhaps something like this:
>
>      5.3.  Exporting a Conceptual Row: The OSPF Neighbor Sequence
>
CM:
Fixed using your text

>      The last sentence on page 40 refers to Table 7 but I assume you
>      mean Table 5.
>
CM:Fixed.

> 24) The BER encoding of 1.3.6.1.2.1.14.10.1 is 06082B060102010E0A01
>      (10 octets) and not the 22 octets shown in Figure 28.
>
CM:
This appears to have been an artifact of the code I was using to generate the
BER encoding - my binary/hex encoding appears to have been padding extra 0's that weren't required.

I did check each OID in the previous version that through a web based ASN decoder since I didn't
trust myself to get it right - http://lapo.it/asn1js/. The decoded output for
your version and the old one was identical. I've fixed that. Curious.
060f2b8006800180028001800e800a8001
06082B060102010E0A01

Anyway I'll update the OIDs throughout the document to use your ones and fix the sizes
of the examples as required and double check them.

> 25) Section 5.4. also needs a better title, e.g.
>
>      5.4.  Exporting an Augmented Conceptual Row: ifTable and ifXTable
CM:
Fixed.

>
>      The text in section 5.4 confuses ifXEntry and IfXEntry. A possible
>      rewrite:
>
>        The ifTable defined in the IF-MIB [RFC2863] is augmented by the
>        ifXTable (defined in the same MIB module). The OID of the
>        ifEntry is 1.3.6.1.2.1.2.2.1 while the OID of the augmenting
>        ifXEntry is 1.3.6.1.2.1.31.1.1.1. This example demonstrates how
>        columnar objects from the base conceptual row and the augmenting
>        row can be exported in a single mibObjectValueSequence data
>        record.
CM:
Replaced using your text but updating the mibObjectValueSequence to mibObjectValueRow
and data record -> field

>
> 26) Why is the Field Length of mibObjectValueOctetString (on page 46)
>      16 octets? The ifName's restriction is 255 octets. And should this
>      not be variable length?
CM:
The longest name we are trying to export in this example is 16 bytes long.
This field is being reduce length encoded, it is one reasonable choice for the
export process developer to get the longest string the they are going to be using and
have that as the length.
The better solution is, as you say,to use variable length, but I don't think it adds much to
this example.

>
> 27) page 47: s/VLAN=11/VLAN=10/
CM: Fixed

>
>      On page 48, I am surprised that ifType and ifMTU are both 16-bit
>      fields. Does the template not say they are 4 octets long?
CM:
Yep that is a mistake.
Fixed in the template to specify both fields are 2 bytes long.

>
> 28) Section 5.5. also needs a new title and the terminology used is
>      wrong as well. Perhaps it is:
>
>      5.5.  Exporting a Columnar Object: ipIfStatsInForwDatagrams
>
>      Please do not write 'ipIfStatsTable SEQUENCE'. The correct SMIv2
>      term is conceptual table. (And also note that the ASN.1 type of a
>      table is a SEQUENCE OF and not a SEQUENCE.)
>
I've removed all refererences to SEQUENCE except when refering to the SYNTAX clause
in "Section 4.3 - IPFIX and MIB Data Model"

>      The template in Figure 33 says the interface index is two octets.
>      This may be true for a certain exporter but may not be generally
>      true - it may be worth to be explicit about the assumption made
>      here that all possible interfaces are numbered such that they fit
>      into 16 bits.
CM: Added a note to that regard.

>
>      The BER encodings and the corresponding VLEN fields are all wrong
>      in Figure 35:
>
>      1.3.6.1.2.1.4.31.3.1.1  -> 060A2B06010201041F030101 (12 octets)
>
>      1.3.6.1.2.1.4.31.3.1.2  -> 060A2B06010201041F030102 (12 octets)
>
>      1.3.6.1.2.1.4.31.3.1.12 -> 060A2B06010201041F03010c (12 octets)
>
CM: Fixed OIDs and lengths of Variable fields and whole set.

> 29) Section 5.6. needs a better title. This is about a columnar object
>      where the index information is provided by IPFIX information
>      elements.
>
>      5.6.  Exporting a Columnar Object index by IEs: ifOutQLen
CM: Fixed

>
>      s/be done be/be done by/
CM: Fixed

>
>      The text below Table 8 needs to be rewritten, e.g.
>
>      The ifOutQLen MIB object defined in the IF-MIB [RFC2863] provides
>      the length of the output packet queue. This columnar object is
>      part of the ifEntry conceptual row and indexed by the interface
>      index (ifIndex).
CM: Agreed and updated to use the above text.

>
>      I am not sure I agree on the second paragraph on page 53. I tend
>      to believe that the index information must be provided for generic
>      applications to make sense out of the data.
CM: While I think there are many MIB Objects that will only make sense if the
full indexing is provided, there will be many others where the index information is
optional or an alternative can be provided.

Regardless this paragraph doesn't add anything useful, explain it well or belong in this section
so I've removed it.

>
>      The BER encoding and the corresponding VLEN in Figure 39 is wrong:
>
>      1.3.6.1.2.1.2.2.1.21 -> 06092B0601020102020115 (11 octets)
>
CM: Fixed

> 30) I find the example in section 5.7 rather strange. Why would one
>      export the ifIndex value using mibSubIdentifier and not by
>      including the ifIndex proper? There does not seem to be any
>      savings and this mibSubIdentifier approach of course is only
>      applicable where the number of sub-identifier is constant for all
>      conceptual rows. Since I do not see that this approach is needed
>      nor that it adds any value in terms of more compact encodings,
>      I would actually prefer this to be removed and perhaps even be
>      disallowed.
>
>      The BER encoding of the ifOutQLen OID is wrong, see above for the
>      correct value.
>
>      (The caption of Figure 44 is kind of strange because it says
>       "using ifIndex" but then the example is about not using ifIndex
>       but instead an opaque number.)
>
>      This section 5.7. should really be removed I think.
>
CM: On reflection I agree, making indexing optional is enough flexibility.

This was originally added to prevent a cascade of having to support all the fields in the INDEX,
but it really just adds an extra option that is not needed and confusion.

The main problem I see is your point about not being sure of the number of subidentifiers,
we would have to make it a list and even more complexity for no gain.

mibSubIdentifier is now only defined to be used in a Mib Field Option so I've moved it's
definition with the other mibFieldOption fields.

> 31) In section 5.8,
>
>      s/ospfNbarEntry/ospfNbrEntry/
CM: fixed throughout

>
>      The OID encoding and the VLEN field in Figure 46 is wrong:
>
>      1.3.6.1.2.1.14.10.1 -> 06082B060102010E0A01 (10 octets)
>
CM: Fixed.

>      RFC 3411 uses '800002b804616263'H as an snmpEngineID in examples.
>
CM: Ok updated to use the same example. And fixed the lengths to match.

> 32) What is the meaning of this:
>
>        If a Collecting Process receives a MIB Object Identifier that it
>        cannot decode, it SHOULD log an error.
>
>      What does 'cannot decode' mean? I mean, a simple collector may
>      just store the records in some file / database. So what is
>      expected here? And why is it an error and not a warning?
CM: This requirement carried over from the first version, where the data being
sent was just some bytes and without access to the MIB module the collector
could not understand the format at all. Now a collector that supports all the
fields will know the basic type of the data it is much more reasonable to store
the data for later decoding.

I've added instead a paragraph here that mentions that the OIDs be stored with the same policy as templates.

I've downgraded the unknown thing to a warning. In some cases the collector
would be expecting to understand each MIB and if it gets an  unexpected or corrupt
value reporting a warning is reasonable behaviour.

>
>      I am also not sure what the last paragraph in section 7 tells me.
>      What does 'purely semantic information' mean? For me, if you miss
>      the semantics, the data has no value. But I understand that IPFIX
>      people have a very different terminology at times.
>
CM: It is sort of making the point that the export may not be from a full MIB
implementation on the exporting process - and that the Object Definition is
just being used in place of a pure IPFIX IE number.

> 33) It is unclear to me how the export of conceptual rows deals with
>      non-existing variables (aka table holes). Is there a mechanism in
>      IPFIX to indicate that a certain field of a flow record does not
>      have a value?
CM: This is the subject of draft-aitken-ipfix-unobserved-fields-02. There needs
to be a general solution to this problem, which would apply well here, but this
is not the draft to try to solve the problem.

>
> Issues with section 10:
>
> a) In the SMIv2, we distinguish between Counter32 and Counter64 and
>     I think this difference important to capture since the rollover
>     is different. The I-D seems to map both to mibObjectValueCounter.
>
PA:

mibObjectValueCounter simply contains the value of the counter, which is independent of the type.
The mibType Information Elements should be used to distinguish between Counter32 and Counter64.

> b) The descriptions all use the phrase "from a MIB" but I think it
>     should be "of a MIB object".
>
PA:
Done.

> c) The SMIv2 type is IpAddress and not IPAddress.
PA:
Done.

>
> d) The definition of mibObjectValueCounter says "Data Type Semantics:
>     totalCounter". I think this is wrong since SMIv2 counters do not
>     start with zero. Furthermore, a Counter32 rolls over after 32-bit
>     and not after 64-bit (but mibObjectValueCounter is an unsigned64).
PA:

The point of the "totalCounter" semantic is to indicate that the value is absolute rather than a delta.

I've added a new IPFIX "SNMPtotalCounter" semantic for this.

Despite the 64-bit mibObjectValueCounter definition, Counter32 can be exported in a 32 bit mibObjectValueCounter using "Reduced Size Encoding" per http://tools.ietf.org/html/rfc7011#section-6.2
The correct semantic (counter32, counter64) can easily be determined from the field length.

>
> e) The definition of mibObjectValueGauge "Data Type Semantics:
>     totalCounter". I think this is wrong, a Gauge32 is not a counter.
>     And it might be useful to call this mibObjectValueGauge32.
PA:
Changed the semantics to "gauge" and added a new IPFIX "SNMPgauge" semantic for this.

Naming it "mibObjectValueGauge32" unnecessarily conflates the type and the size, so a new type would be needed for each different gauge size (eg gauge64).
The correct semantic (gauge32, gauge64) can easily be determined from the field length.

>
> f) Why did you call mibObjectValueTime not mibObjectValueTimeTicks?
>     The "Abstract Data Type: dateTimeMilliseconds" also seems to be
>     wrong since TimeTicks are _not_ measured in milliseconds since
>     1970-01-01T00:00. You may want to use unsigned32 and spell out the
>     semantics.
PA:
Renamed to "mibObjectValueTimeTicks" and changed to u32.

>
> g) mibObjectValueUnsigned used the "Abstract Data Type: unsigned64"
>     which is wrong since there is only a 32-bit unsigned type in the
>     SMIv2. I suggest to use unsigned32 and to rename this to
>     mibObjectValueUnsigned32.
PA:

Unsigned64 is used to allow future exports of 64 bit values. IPFIX's "Reduced Size Encoding" allows 32 bit values to be exported despite the u64 semantic.


>
> h) The phrase "a complete MIB 'SEQUENCE OF X' or conceptual table
>     value" reads strange. Similarly, "a MIB SEQUENCE or a row from a
>     conceptual table" reads strange. Perhaps simply use "a complete
>     conceptual table" and "a row of a conceptual table".
PA:

Done.

>
> i) The lest sentence of the Description in 10.1.11 seems to be missing
>     some words.
PA:

Good catch: "The template specified in the subTemplateList MUST be an Option Template..."

>
> j) The description of 10.2.2 is somewhat confusing: "... that serves
>     as INDEX MIB Objects of Information Elements for a mibField". In
>     SMIv2, the INDEX is associated with a conceptual row.
PA:

Changed to, "that serve as INDEX MIB objects for an indexed Columnar MIB object." for consistency with the remainder of the paragraph.

>
> k) What does "sampled by SNMP" mean? Do you hook into the
>     instrumentation or do you access things via the SNMP agent?
PA:

That's an implementation issue. Our concern is only to indicate how fresh the data value is.

>
> l) p74: s/the MIB the Flow/the MIB when the Flow/
PA:
Good catch.

>
>     (Note that you are talking about a flow here and not about a data
>     record. I think this is goodness and proves that a new term is not
>     needed.)
PA:

A Flow is a sequence of packets which we're observing. It's an input.
A Flow Record contains information about an observed Flow. It's an internal object.
A Data Record contains values indicating observations which were made. It's an output.

A Flow could have started a long time before the metering process began, so I clarified the text to:

     begin - The value for the MIB object is captured from the MIB when the Flow is first observed
     end - The value for the MIB object is captured from the MIB when the Flow ends
     export - The value for the MIB object is captured from the MIB at export time
     average - The value for the MIB object is an average of multiple captures from the MIB over the life of the Flow

CM:
I updated the average case to mention the observed life of the flow

>
> m) Replace the Description in 10.3.4 with this:
>
>        Description: The textual name of the MIB module that defines a MIB
>        Object.
PA:

Done.

>
> n) I do not understand what mibObjectSyntax would contain for an object
>     representing a conceptual table or a conceptual row. For say ifEntry,
>     would it be just "SEQUENCE {
>          ifIndex                 InterfaceIndex,
>     ifDescr                 DisplayString,
>     -- lots of stuff left out
>     }"? And what would it be for the object representing the conceptual
>     table? Or do you just mean what is literally in the SYNTAX clause,
>     excluding the referenced ASN.1 type definitions?
PA:

Literally just the SYNTAX clause.

CM:

Yes I meant that the Syntax  is literally the output in the SYNTAX clause.

> o) Remove "( also known as snmpEngineID) that" since it is potentially
>     misleading.
>
>     Explanation: Every SNMP engine has a unique snmpEngineID. The
>     combination of a snmpEngineID value and a context names provides
>     the SNMP context. Note that an SNMP engine may provide access to a
>     non-local context, in which case the values of snmpEngineID and
>     contextEngineID would be different.
>
>
PA:

Done.