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

From: Colin McDowall <[email-masked]>
Date: Fri, 04 Jul 2014 17:00:08 +0100

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.

---

From: Colin McDowall <[email-masked]>
Date: Fri, 04 Jul 2014 17:08:31 +0100

Hi Benoit, all,

See CM: for
more replies for the last review of draft-ietf-ipfix-mib-variable-export-05.

On 25/04/2014 14:34, Benoit Claise wrote:
> Dear all,
>
> Some feedback on the sections I already reviewed.
> Note that I agree with Juergen's feedback.
...
> The following text could be cut/pasted from the Introduction to a new section "High Level Solution Overview", somewhere after the terminology section:
>
>     This document specifies a method for creating IPFIX Option Templates
>     that are used to export the extra data required to describe MIB
>     variables (seeSection 4.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.1>).
>
>     This allows IPFIX Templates to contain any combination of fields
>     defined by traditional IPFIX Information Element(s) and/or MIB Object
>     Identifier(s).  The MIB Object Identifiers can reference either non-
>     indexed or indexed MIB object(s).  Enterprise-specific MIB Object
>     Identifiers are also supported.
>
>     This document also defines three standard Option Templates (see
>     Section 4.2  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2>) that are used as part of the mechanism to export MIB
>     Object meta data:
>
>     o  mibFieldOption (Section 4.2.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.1>)
>
>     o  mibSubFieldOption (Section 4.2.2  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.2>)
>
>     o  mibTypeOption (Section 4.2.3  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.3>)
>
>     This document defines three classes of new IPFIX Information
>     Elements.  These are used to export values from the MIB, export
>     required Object Identifier information, and optionally export type
>     data from a MIB Module:
>
>     o  mibObjectValue Information Elements (Section 10.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-10.1>)
>
>     o  mibFieldOption Information Elements (Section 10.2  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-10.2>)
>
>     o  mibTypeInformation Information Elements (Section 10.3  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-10.3>)
>    
CM:

Agreed and implemented - the introduction is now a lot leaner and better.

> Agreed.
CM:

Ok that figure has been removed.

>>     call 'indexed object'. Why is it useful to call Flow Records Data
>  From RFC 7011:
>      +------------------+---------------------------------------------+
>      |                  |                 Contents                    |
>      |                  +--------------------+------------------------+
>      |       Set        |      Template      |         Record         |
>      +------------------+--------------------+------------------------+
>      |     Data Set     |          /         |     Data Record(s)     |
>      +------------------+--------------------+------------------------+
>      |   Template Set   | Template Record(s) |           /            |
>      +------------------+--------------------+------------------------+
>      | Options Template |  Options Template  |           /            |
>      |       Set        |      Record(s)     |                        |
>      +------------------+--------------------+------------------------+
>
>                      Figure A: Terminology Summary Table
>
> Section 3 provides:
>
>     This document prefers the more generic term "Data Record" (as opposed
>     to "Flow Record") in relation to the export of MIB objects.
>
> So flow record should be changed to data records throughout the doc.
CM: Done except for one mention in the above text you mentioned

     <t>
     </t>

Thanks,
colin

---

From: Colin McDowall <[email-masked]>
Date: Fri, 04 Jul 2014 17:23:24 +0100

Hi Benoit, all,

This is great feedback, thanks.

Please see replies at CM:

Paul should have draft -06 submitted today.

On 25/04/2014 14:32, Benoit Claise wrote:
> Dear all,
>
> I stopped my review at section 4.4.2 (spent a couple of hours already), and I'm afraid the draft is not ready.
> Here is my feedback.
>
> - Section 1
>
>     For example,
>     in the IPFIX information model [RFC7012  <http://tools.ietf.org/html/rfc7012>], Information Elements coming
>     from the SNMP world have already been specified, e.g.,
>     ingressInterface and egressInterface both refer to the ifIndex
>     defined in [RFC2863  <http://tools.ietf.org/html/rfc2863>].
>
> As Jürgen noted, RFC 7012 doesn't specify the IPFIX information elements any longer.
> This is done in the IANA registry.
CM:
updated to refer to IANA. The only remaining reference t rfc7012  is in Paul's
new semantics section that is refering to a section of that draft rather than IEs.

>
> - Section 2
>
>        In the best case, assuming very tight integration of
>        an IPFIX Collector with and SNMP polling engine, SNMP data is
>        retrieved shortly after Data Records have been received, which
>        implies a delay of the sum of the active or inactive timeouts (if
>        not null) plus the time to export the Flow Record to the
>        Collector.
>
> For active and inactive timeouts, provide a reference to RFC 5470 , section 5.1.1. "Flow Expiration"
> Even if RFC 5470 doesn't call them active and inactive timeouts (btw, don't change that, these are well known terms), this is a good reference.
CM:
Added the reference.

>
> -  Section 2
>
>     This draft does not specify SNMP notifications, even if the
>     specifications in this document could potentially allow this.
>
> draft -> document
> Same remark for:
>
>     For each of these the options this draft specifies exactly which
>     mibObjectValue to use.
>
>     This draft defines two forms of indexing that can be used for
>     SEQUENCE MIB Objects.
CM:
All uses of draft in the main body changed, now only mentioned in the Acknowledgements.

>
> - Section 2
>
>     The
>     simple and application-wide data types specified in SMIv2 [RFC2578  <http://tools.ietf.org/html/rfc2578>],
>     along with a new Textual Conventions
>
> Remove the s
CM: Went the other way and removed the 'a' to make the sentence read:
     
     The simple and application-wide data types specified in SMIv2
     <xref target="RFC2578"/>, along with new Textual Conventions, can be
     exported within IPFIX and then decoded in the Collector.

>
> - Section 3
>
>   mibObjectValue
>
>        Refers to any and all of the mibObjectValue Information Elements
>        generically.  Any restriction or requirement in this document that
>        refers to mibObjectValue applies to the following Information
>        Elements defined inSection 10.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-10.1>: mibObjectValueInteger,
>        mibObjectValueOctetString, mibObjectValueOID, mibObjectValueBITS,
>        mibObjectValueCounter, mibObjectValueGauge, mibObjectValueTime,
>        mibObjectValueUnsigned, mibObjectValueTable and
>        mibObjectValueSequence.
>
> Change mibObjectValue to mibObjectValue Information Elements.
> This is how it's used in some sentences in the rest of the document.
> However, I also see:  mibObjectValue Field or mibObjectValue field
> I don't know what the term field refers to. Please change this to mibObjectValue Information Elements
CM:
wow didn't realise field wasn't an ipfix term.

I've updated all the references to mibObjectValue fields to refer to mibObjectValue Information Elements

>
> - Section 4
>
>     The Exporting process MAY extract the data values for mibObjectValue
>     fields from a Process that resides on the same device or MAY capture/
>     create the data required to match the definition of the MIB Object.
>     In particular exporting a value from a MIB does not imply that the
>     SNMP process on the Device supports that MIB.
>
> Why is Process capitalized?
CM:
I think it was originally Exporting Process - but the process capturing values from
an on board mib may not be the actual Exporting Process

> Also MAY to may. This is not a RFC 2119 keyword required for the implementation.
CM:
ok moved to lower case.
>
> - Section 4
>     The main issue that arises from exporting MIBs in IPFIX is that MIB
>     Object Identifiers do not fit into the standard IPFIX Template format
>     [RFC7011  <http://tools.ietf.org/html/rfc7011>], as this only provides a 16-bit Information Element
>     identifier and the length of the Information Element.
>
> I would remove "and the length of the Information Element", which doesn't add anything to this sentence
CM: Agreed,  fixed.

>
> - Section 4
>
>     One approach to this problem would be to extend the IPFIX standard to
>     allow extended field specifiers so metadata about Fields can be
>     included in Data Templates.
>
> Fields -> field
CM:
lowercased but kept it in the plural:
     so metadata about fields can be included in Data Templates.

>
> - Section 4
>
>     However, future versions of IPFIX MAY export the required MIB
>     metadata as part of newer set versions.
>
> MAY -> may.
> Please review all RFC 2119 keywords
CM:
This was an attempt to not limit future implementations of IPFIX from
having to implement the export of the OID data in the same format. I've moved
that to lower case though, since we can only define behaviour for the current
version in this draft.

I've considered all the 2119 keywords and I think those left are now required.

>
> - Section 4
>     This is a
>     standard IPFIX Option Template Set that MUST include a minimum set of
>     required fields (seeSection 4.2.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.1>) and MAY include extra fields to
>     provide more meta information about one of the mibObjectValue fields.
>
> While you're not yet in the specifications in Section 4, I would not introduce MUST and MAY here.
> This also contradicts the REQUIRED and RECOMMENDED in section 4.2.1
> Please group the RFC 2119 keywords in section 4.2.1
CM:
Ok, the RFC 2119 keywords relating to the option export have been grouped into that section.

>
> - Throughout the document
> Option Template -> Options Template
> Note: IIRC, there were inconsistencies in RFC 5101, which were corrected in RFC 7011
CM: Fixed throughout

>
> - Major issue: I confused by mibFieldOption
> Is this a IE or an Options Template?
>
> The TOC shows:
>     4.2.  MIB Field Options - Specifications and Required Fields  .  11
>         4.2.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.1>.  mibFieldOption  . . . . . . . . . . . . . . . . . . .12  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-12>
>         4.2.2  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.2>.  mibSubFieldOption . . . . . . . . . . . . . . . . . .13  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-13>
>         4.2.3  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.3>.  mibTypeOption . . . . . . . . . . . . . . . . . . . .13  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-13>
>       4.3  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.3>.  IPFIX and MIB Data Model  . . . . . . . . . . . . . . . .14  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-14>
>       4.4  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.4>.  MIB Field Option Template Formats . . . . . . . . . . . .15  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-15>
>         4.4.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.4.1>.  Data Template containing a mibObject Field  . . . . .15  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-15>
>         4.4.2  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.4.2>.  mibFieldOption Template . . . . . . . . . . . . . . .17  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-17>
>         4.4.3  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.4.3>.  mibFieldOption Data Records . . . . . . . . . . . . .18  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-18>
>         4.4.4  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.4.4>.  Option Template containing a mibObject Field  . . . .19  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-19>
>         4.4.5  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.4.5>.  mibFieldOption Template with Semantics Fields . . . .20  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#page-20>
>         4.4.6.  mibFieldOption Template with extra MIB Object Details  21
>
> It's not clear!
>
> Ok, so these are Options Templates.
> If this is the case, change the TOC section titles
>
> Then I see in figure 1
>                                         +------------------------+
>                                         | mibFieldOptionTemplate |
>                                         +------------------------+
> What is a mibFieldOptionTemplate ?
>
>     This document defines a mibFieldOption Template to export the extra
>     meta information required for a mibObjectValue field.
>
> Well, this one above is wrong. This should be Options Template, right?
>
> Then I see
>    2.  A mibFieldOption Template Set
>
>            The mibFieldOption Template describes which metadata will be
>            sent for each mibObjectValue being exported.
>
> These should be Options Template Set, right?
>
> Then, I look at
>
>
>         4.2.1. mibFieldOption
>
>
>     Three fields are REQUIRED to unambiguously export a standalone
>     mibObjectValue Field with a mibFieldOption:
>
>
> Then I look at
>
>
>         4.4.2. mibFieldOption Template
>
>     The mibFieldOption Template is a Standard Option Template which
>     defines the Fields that will be exported to provide enough metadata
>     about a mibObjectValue so that the Collector can tie the data values
>     in the mibObjectValue back to the definition of the MIB Object.
>
> And I finally understand that mibFieldOption is an Options Template.
>
> What is very confusing to me is that mibFieldOption is apparently simply an Options Template, like we have defined in IPFIX (http://tools.ietf.org/html/rfc7011#section-4.1) or  PSAMP (http://tools.ietf.org/html/rfc5476#section-6.5.1)
> However, this Options Template is called like an IPFIX IE: mibFieldOption
> This should be called "MIB Field Options Template" throughout the document.
>
> OLD:
> NEW:
>     o  MIB Field Options Template (Section 4.2.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.1>)
>
>     o  MIB SubField Options Template (Section 4.2.2  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.2>)
>
>     o  MIB Type Options Template (Section 4.2.3  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.3>)
CM:
I think I just wanted to give a handle/name to the particular option export, to make it clearer
to refer to throughout the document. If it was more confusing that is bad. I've updated the
names of the Options exports to match your suggestions.

>
>
> - Major: Why do we have ...
> http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.1
> http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.4.2
> ... with slightly different specifications?
> As I mentioned previously, there are also some conflicting spec rules at the beginning of the section 4
CM:
I've removed the duplicate list of required fields and just added a reference to the required minimum section.

There are now no spec rules or keywords in the initial part of this section.

>
> -
>
>      0                   1                   2                   3
>        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |          Set ID = 3           |          Length = 26          |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |        Template ID = 258      |        Field Count = 4        |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |  Scope field count = 2        |0| IE = templateId             |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |        Field Length = 2       |0| IE = informationElementIndex|
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |        Field Length = 2       |0| IE = mibObjectIdentifier    |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |        Field Length = 65535   |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>        Figure 19: Example of tcpCurrEstab mibFieldOption Template Set
>
> This is not an Template Set, but an Options Template Set.
> Same remark for all the examples.
CM:
I've checked and fixed the titles for all the examples

>
> - Section 4.2
>
> Why a single in this sentence?
>     For each mibObjectValue field that is defined in an IPFIX Template, a
>     single mibFieldOption Data Record MUST be exported that provides the
>     required minimum information to define the MIB object that is being
>     exported (seeSection 4.2.1  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.2.1>).
>
> What if we export with UDP? We might want to re-export this metadata information, without which the collector can't interpret the data.
CM:
Agreed - the point is we want to treat the Data Records with the OIDS like templates.
Removed the single

Updated to
     If Multiple MIB Field Options Data Records that refer to a mibObjectValue are received the latest MUST be used.

> This contradicts:
>     Note that the ID of an identical mibFieldOptionTemplate which has
>     already been exported MAY be reused without exporting the Template
>     again.
CM: Resolved

>
> - Section 4.2
>
>     This mibFieldOption Data is defined in a template referred to in this
>     document as a mibFieldOption Template with the format specified in
>     Section 4.4  <http://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-05#section-4.4>.
>
> What's mibFieldOption Data?
> Template -> Options Template
CM: Updated to
The MIB Field Options Data Records are defined in a template referred to in this document as a MIB Field Options Template with the format specified in <xref target="sectionFormats"/>.

>
> - Section 4.2
>
>    _  A Template that uses a mibObjectValue Field MUST be exported prior to
>     the corresponding mibFieldOption Data Records._   It is RECOMMENDED
>     that these are all exported in the same IPFIX Message.  Note that
>     this places an implicit size constraint on the export.
>
> _    These MUST all be exported prior to the corresponding Data Records
>     which depend upon them._   i.e. referring to Figure 1, the export order
>     MUST be:
>
> The two underlined sentences are redundant.
CM:
Not quite.
There are 2 different types of Data records in play here.
The MIB Field Option Data Records contain the OIDs. The Object value data
records contain values.

This is saying that you need the oid data record before
you can understand the values.

I've reworded it as follows which is clearer:
   The MIB Field Options Template and MIB Field Options Data Records MUST be exported in the same IPFIX
   Message as any Template that is using a mibObjectValue Information Element.
   Note that this places an implicit size constraint on the export.
   This whole set of Templates and MIB Field Options Data Records MUST all be exported prior to the corresponding Data  Records  which depend upon them.
i.e. referring to <xref target="figure_overview_field"/>, the export order MUST be:
       <list style="numbers">
     <t>Data Template for mibObjectValue Information Elements</t>
     <t>MIB Field Options Template</t>
     <t>MIB Field Options Data Records</t>
     <t>MIB Object Value Data Records</t>
       </list>
>
> - Major
>     While the following are optional, they are nevertheless RECOMMENDED
>     in certain circumstances as they are per field:
>
> You don't explain those circumstances
CM:
Added a reference for each of these optional IEs to the section that explains when
to use them.
>
> - I arrived at section 4.3, I'm not clear when I need to use mibSubFieldOption ormibTypeOption
>
> For mibSubFieldOption, this only thing I read so far is:
>     To export a mibObjectValue that is contained in a
>     mibObjectValueSequence Field with a mibSubFieldOption three fields
>     are REQUIRED:
>
> Not too clear.
CM:

Agreed.

The mibSubIdentifier usage is now part of the standard MIB Field Options Template. This
is clearer.

I've added a note on when to use the mibSubIdentifier and a reference to the correct section.

The MIB Type Options Export is entierly optional. Since it is just RECOMMENDED I've taken all the fields
together into 1 list.

>
> - Major, Section 4.4.1
>
>     Multiple
>     mibFieldOption that refer to a single mibObjectValue MUST NOT be
>     exported.
>
> What is Multiple mibFieldOption?
> Multiple Data Records. No
> Multiple different Data Records. I agree
> Multiple Options Template Records. No
> Multiple different Options Template Records. I agree
CM:

Reworded to match standard template behaviour on UDP - use the latest version:
      
      If Multiple MIB Field Options Data Records that refer to a mibObjectValue are received the latest MUST be used.
     This matches the expected behaviour of IPFIX Templates.
  
>
> - Major, Section 4.4.1
>
> OLD:
>      0                   1                   2                   3
>        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |          Set ID = 2           |          Length               |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |       Template ID             |         Field Count = 2       |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |0| IE = Existing IPFIX Field   |        Field Length           |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |0| IE = mibObjectValueInteger  |        Field Length (mib)     |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>            Figure 5: IPFIX Template Set using mibObjectValue Field
>
>
> NEW:
>      0                   1                   2                   3
>        0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |          Set ID = 2           |          Length               |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |       Template ID             |         Field Count = 2       |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |0| IE = Existing IPFIX Field   |        Field Length           |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>       |0| IE = <mibObjectValue>      |        Field Length (mib)     |
>       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>            Figure 5: IPFIX Template Set using mibObjectValue Field
>
> Then you have to explain that <mibObjectValue> could be
>       mibObjectValueInteger,
>        mibObjectValueOctetString, mibObjectValueOID, mibObjectValueBITS,
>        mibObjectValueCounter, mibObjectValueGauge, mibObjectValueTime,
>        mibObjectValueUnsigned, mibObjectValueTable and
>        mibObjectValueSequence.
CM:
Done.

>
>
> I stopped my review at section 4.4.2, but plenty of my feedback would apparently be equivalent for the rest of the draft.
I've tried to apply each of your points throughout the document.

Thanks,
colin