Re: [Gen-art] Gen-art Last Call/Telechat Review of draft-ietf-ipfix-mib-variable-export-09

[Benoit Claise <[e-mail]>]

Thank you Elwyn for your review.

Regards, Benoit
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair. Please wait for direction from your
> document shepherd or AD before posting a new version of the draft.
>
> For more information, please see the FAQ at
>
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>
> Document: draft-ietf-ipfix-mib-variable-export-09.txt
> Reviewer: Elwyn Davies
> Review Date: 2015/11/14
> IETF LC End Date: 2015/10/26
> IESG Telechat date: 2015/11/19
>
> Summary: Ready except for some minor nits, chiefly associated with the 
> unexplained use of 'magic numbers' that are defined elsewhere in the 
> IPFIX specifications (see comments on ss5.3, 5.4.2 and 6.3).  I was 
> (however) impressed by the quality of the document, which is 
> consistent in its presentation and deals clearly with a complex (and, 
> frankly, pretty dry as dust) specification.  Most of the points below 
> are primarily to make the document more easily accessible to people 
> (like me) with limited exposure to IPFIX. Caveat:  I have read through 
> the examples (Section 6) but I cannot say that I have analysed them in 
> gory detail.
>
> Apologies for the late delivery of this review.  I missed the 
> assignment during the last call period.  In the light of the quality 
> of the document, I don't think this should have any effect on the 
> progress of the document!
>
> Major issues:
> None.
>
> Minor issues:
> None.
>
> Nits/editorial comments:
> General: s/i.e./i.e.,/g (4 instances)
> s/e.g./e.g.,/ (1 instance in s10)
>
> s2, para 1:  Probably good to provide a pointer to the doc/section 
> where Template Record and Data Record are defined as this section 
> precedes s3 where the terminology is specified.
>
> s4, para 2: s/non columnar/non-columnar/ (?)
>
> s4, third para from end: s/One common type/Two common types/
>
> s5, para 5:
>>     However, future versions of IPFIX may export the required MIB
>>     metadata as part of newer set versions.
> Is the phrase 'newer set versions' a term of art here?  Maybe change 
> to 'newly defined version(s)' or maybe 'newly defined Set versions [or 
> just Sets]'?
>
> s5.1, para after Table 1: s/encoding references/encoding reference/
>
> s5.3, bullet points: To clarify Set ID entries in the figures 
> describing the various templates/data sets it would be worth noting 
> the SetID(s) that can be used with the various template and data 
> records and a reference to RFC 7011 Section 3.3.2.
> I think that bullet 1 has Set ID 2, bullet 2 has Set ID 3 and bullets 
> 3 and 4 have Set IDs from 256 upwards (implementation choice).
>
> s5.4.2, last para and Figure 5:  It would be helpful to remind people 
> that the value 0xffff/65535 indicates variable length encoding  per 
> RFC 7011 Section 3.2 and that the RECOMMENDED use of variable length 
> encoding for mibObjectIdentifier fields is indicated in subsequent 
> figures by placing 65535 in the relevant length fields.  Presumably 
> Collector implementations  MUST accept a specific length encoding in 
> the usual IETF spirit!  It might be worth being explicit about this 
> (this might usefully be said in Section 8).
>
> s5.7.2, para 1:  The MUST ought to be qualified by 'except as allowed 
> by the caveat of Section 5.7.1'.
>
> s5.7.2: is there any need to explain how withdrawal is achieved? I am 
> not an IPFIX expert so I am not aware how the withdrawal might be 
> achieved.
>
> s5.8, para 5: s/may be used purely use as a data type./may be used 
> purely as a data type./ ( I think)
>
> s5.8, last para: is missing its terminal period/full stop. :-(
>
> s5.8.1, last para: s/be exported/to be exported/
>
> s6.2, bullet 2 after Table 3: is missing its terminal period/full 
> stop. :-(
>
> s6.2, 3rd from last para (top of page 40): s/encoded/encode/;  It 
> would also be useful to point the reader back to the template for 
> mibObjectValueGauge in Table 23 where the encoding size is specified.
>
> s6.3:  This section is somewhat politically incorrect in that it deals 
> (only) with IPv4 addresses ;-)
>
> s6.3, Table 4 (also Table 9):  The aesthetics of this table could be 
> improved by reducing the width of the Object column by 7 characters 
> and reallocating them to the ID (+4) and mibObjectValue (+3) columns.  
> Similarly in Table 5, moving a character from the Entity column to the 
> Full OID column.
>
> s6.3, Figure 28:  For the benefit of less clued up readers, it would 
> be worth pointing out that this is a structured data type 
> specification using the 'undefined' (= 0xFF) semantic (RFC 6313, 
> Section 11.4/11.4.1) .  It would also be clearer to s/=FF/=0xFF/g. 
> Also applies to Figure 31 and Figure 42.
>
> s10: The discussion I think effectively covers issues of privacy 
> inherited both from SNMP/MiBs and IPFIX but it might be worth putting 
> in the 'P word' and expanding a bit more on this subject to make it 
> clear that accessing MiB objects  via IPFIX opens up a whole new 
> opportunity for privacy violations.
>
>
>
>
>
> .
>