Many thanks for your detailed review, Elwyn! Much appreciated. (And congrats to the team for a quality document!)
Jari On 14 Nov 2015, at 17:07, Elwyn Davies <[email protected]> wrote: > 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. > > > > > > _______________________________________________ > Gen-art mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/gen-art
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
