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

From: Elwyn Davies <[email-masked]>
Date: Sat, 14 Nov 2015 15:07:26 +0000

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.

---

From: Benoit Claise <[email-masked]>
Date: Thu, 19 Nov 2015 09:30:30 +0100

Thank you Elwyn for your review.

Regards, Benoit
> (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:
> 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.
>
> 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.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).
>
> that the value 0xffff/65535 indicates variable length encoding  per 
> RFC 7011 Section 3.2 and that the RECOMMENDED use of variable length 
> 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'.
>
> 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 
> improved by reducing the width of the Object column by 7 characters 
> and reallocating them to the ID (+4) and mibObjectValue (+3) columns.  
> 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.
>
> 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.
>
>
>
>
>
> .
>

---

From: Jari Arkko <[email-masked]>
Date: Thu, 19 Nov 2015 13:12:37 +0200

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-masked]> wrote:

> 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:
> 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.
> 
> 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.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)
> 
> 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-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: Colin McDowall <[email-masked]>
Date: Sat, 21 Nov 2015 00:47:33 +0000

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.
>
>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.
>
>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:

PJ: done.

>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.

>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.
>

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
CM:
Good idea : Added a reference.

>(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.

---

From: Elwyn Davies <[email-masked]>
Date: Thu, 26 Nov 2015 15:38:22 +0000

Hi Colin and Paul.

Thanks for your responses.  I think -10 clears up most of the points 
below.  There is one point and a query that don't seem to be addressed:

> in the relevant length fields.  ******Presumably Collector implementations  MUST
> being explicit about this (this might usefully be said in Section 8).******

PJ: done.

Thoughts on an addendum to s8?

Does this need to be explained or a reference added?

Cheers,
Elwyn

---

From: Paul Aitken <[email-masked]>
Date: Mon, 30 Nov 2015 20:59:58 +0000

Elwyn,

>> that the

Collectors must accept both fixed and variable length per the MUST in 
section 7 of RFC 7011.

I didn't see a need to re-state that here, though there's no harm in 
doing so.See the suggested text below.

> not an

It's explained in section 8.1 of RFC 7011 (IPFIX Protocol 
Specification). I think it would be reasonable to expect that 
implementers should already be familiar with that.

Again, there's no need and no harm in adding a note. See the suggested 
text below.

Please let us know whether you're happy with these additions, and 
whether you'd like to see a -11 or are happy for them to be inserted in 
the AUTH48.

Suggested texts:

    5.7.2 Template Withdrawal and Reuse

    IPFIX Template Withdrawal is discussed in Section 8.1 of RFC 7011.

    Data Records containing mibObjectValue Information Elements MUST NOT
    be exported if their corresponding Data Template or MIB Field Options
    Template has been withdrawn, since the MIB Field Options Template
    MUST be exported in the same IPFIX Message as the Data Template which
    it annotates, except as allowed by the caveat ofSection 5.7.1  <https://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-10#section-5.7.1>.

    MIB Field Options Template IDs MUST NOT bewithdrawn or  reused while they are
    required by any existing Data Templates.

(Alternatively, the new paragraph could be added to the end of the 
section instead.)

    8.  The Collecting Process's Side

    The specifications insection 9 of [RFC7011]  <https://tools.ietf.org/html/rfc7011#section-9>  also apply to Collectors
    that implement this specification.  In addition, the following
    specifications should be noted.

    A Collecting Process that implements this specification MUST store
    the data records containing the OID Object Type Definitions with the
    same retention policy as templates.

    A Collecting Process that implements this specification SHOULD have
    access to MIB modules in order to look up the received MIB Object
    Identifiers and find the full type definition and name of MIB OID
    fields used in received templates.

    It should be noted that since reduced size encoding MAY be used by
    the Exporting Process, the Collecting Process cannot assume a
    received size for a field is the maximum size it should expect for
    that field.

    SincemibObjectIdentifier fields may be exported using  variable length
    mibObjectIdentifier fields, the Collecting Process MUST support
    Variable-Length Information Element encoding per Section 7 of RFC 7011.

    If a Collecting Process receives a MIB Object Identifier that it
    cannot decode, it MAY log a warning.

    A Collecting Process MUST support the 3 options for handling columnar
    objects detailed inSection 5.8  <https://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-10#section-5.8>.

P.

---

From: Elwyn Davies <[email-masked]>
Date: Mon, 30 Nov 2015 23:37:47 +0000

Hi.

Thanks for the response.

I think that the suggested text for s5.7.2 is fine.

To be clear, on s5.4.2 and s8, the point I was trying to get across is 
that the variable length encoding is a MAY/RECOMMENDED. Accordingly 
agents can alternatively use a fixed length encoding.  I was suggesting 
that it would probably be worth mentioning in s8 that the collector MUST 
be prepared to accept the fixed length encoding as well as variable 
length encoding.  Possibly this is the default situation (I am not 
familiar with the innards of IPFIX) but I thought collector implementers 
needed to be reminded of the need to support fixed length encoding as 
well as variable length encoding for the relevant fields so that they 
are liberal in what they accept!

I have no problem with -11 or AUTH48 as long as the text gets in. 
Depends if you have sent it to the RFC Editor already I guess.

Cheers,
Elwyn

On 30/11/2015 20:59, Paul Aitken wrote:
> doing so. See the suggested text below.
>
>
> whether you'd like to see a -11 or are happy for them to be inserted 
> in the AUTH48.
>
> Suggested texts:
>     it annotates, except as allowed by the caveat ofSection 5.7.1 
> <https://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-10#section-5.7.1>.
>
>    8.The Collecting Process's Side
>  Since mibObjectIdentifier fields may be exported using variable 
> length mibObjectIdentifier fields, the Collecting Process MUST support 
>     objects detailed inSection 5.8 
> <https://tools.ietf.org/html/draft-ietf-ipfix-mib-variable-export-10#section-5.8>.
>
> P.
>