Hi Elwyn,
Many thanks for the review, agree this one is a little dry :-)
Please see below for the replies to your comments. The changes have been made
for draft 10.
PJ: Paul Aitken
CM: Colin McDowall
Thanks,
Colin and Paul
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)
PJ: done.
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.
PJ: done.
s4, para 2: s/non columnar/non-columnar/ (?)
PJ: done.
s4, third para from end: s/One common type/Two common types/
PJ: done.
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 IPFIX Set versions [or just Sets]'?
PJ: Each IPFIX Sets has a version number, so "newly defined Set versions".
s5.1, para after Table 1: s/encoding references/encoding reference/
PJ: done.
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).
PJ: done.
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).
PJ: done.
s5.7.2, para 1: The MUST ought to be qualified by 'except as allowed by the
caveat of Section 5.7.1'.
PJ: done.
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)
PJ: done.
s5.8, last para: is missing its terminal period/full stop. :-(
PJ: done.
s5.8.1, last para: s/be exported/to be exported/
PJ: done.
s6.2, bullet 2 after Table 3: is missing its terminal period/full stop. :-(
CM:
Fixed.
s6.2, 3rd from last para (top of page 40): s/encoded/encode/;
CM: Fixed
It would also be useful to point the reader back to the template for
mibObjectValueGauge in Table 23 where the encoding size is specified.
CM:
Good idea : Added a reference.
s6.3: This section is somewhat politically incorrect in that it deals
(only) with IPv4 addresses ;-)
CM:
Well the OSPFv2 MIB RFC4750 is IPv4 only. Since SNMP has a native type
for IPv4 addresses but not IPv6 it is sort of justified to have an example
using them. Having an IPv6 example as well would be nice but the mechanism works
identically.
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.
CM: Yes - I've reworked these tables so they are clearer. I've had to hyphenate
the "ospfNbrAddressLessIndex" Object name - but the other columns are now much
clearer.
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) .
CM:
Yes added a note here and in the section detailing the use of structured data.
Reference added to IANA Structured Data Semantics registery and a brief
note in section 5.8.2. None of the existing semantics seem appropriate for
general exports of MIB tables - but if the row or table is being related to
other ipfix IE some of the existing semantics may be sensible. For example a
selection of routes could be associated with a flow of traffic with exactlyOneOf
to report that 1 of those routes was used to forward the traffic.
It would also
be clearer to s/=FF/=0xFF/g. Also applies to Figure 31 and Figure 42.
CM: Done
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.
CM:
I think this is now pretty well covered with the additions to this section
following the
SecDir review and in particular the details recomending Anonymization via
RFC6235.
