Re: Last Call: <draft-ietf-behave-ipfix-nat-logging-06.txt> (IPFIX Information Elements for logging NAT Events)

[Paul Aitken <[e-mail]>]

Senthil,

> Hi Paul,
> Thanks for taking the time to do a detailed review. Please see inline 
> for [Senthil]. I have incorporated most of your comments,
> I have a few questions embedded inline.

Replies inline @@PJ2.


> From: Paul Aitken <[e-mail]>
> Date: Thursday, February 11, 2016 at 9:36 AM
> To: "[e-mail]" 
> <[e-mail] 
> <mailto:[e-mail]>>
> Cc: "[e-mail]" <[e-mail] <mailto:[e-mail]>>
> Subject: Re: Last Call: <draft-ietf-behave-ipfix-nat-logging-06.txt> 
> (IPFIX Information Elements for logging NAT Events)
>
>
>
> Section 1, second paragraph:
>     The IPFIX Information elements that are NAT specific are created with
>     NAT terminology.  In order to avoid creating duplicate IE's, IE's
>     that are reused if they convey the same meaning.
>
> Capitalise "Elements" and remove the redundant "that" in "that are 
> reused".
>
> The plural of "IE" is "IEs". See section 5 of RFC 7012. Please 
> s/IE's/IE/ throughout the draft.
>
> The draft defines "Information Element (IE)" in section 1 and 
> "Information Elements (IEs)" in section 2. There's no need to repeat 
> (IEs) in sections 2 and 5.2.
>
>
> [Senthil] Ok, done.
>
>
>
> Section 2, first paragraph:
>     This document details
>     the IPFIX Information Elements(IEs) that MUST be logged by a NAT
>     device that supports NAT logging using IPFIX.  The document will
>     specify the format of the IE's that SHOULD be logged by the NAT
>     device and all the optional fields.  The fields specified in this
>     document are gleaned from [RFC4787 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_rfc4787&d=CwMF-g&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoCgBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=aFOj9PLeooXMHRXima8gIP54ipb4rlFwPk4XSgkDnFs&s=fPl32NCjRXlcp_YL261dUPyU67jiIq7tyzzfC7tKpvw&e=>] and [RFC5382 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_rfc5382&d=CwMF-g&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoCgBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=aFOj9PLeooXMHRXima8gIP54ipb4rlFwPk4XSgkDnFs&s=16Ab6VcDIcMdMKTX968UOAJXu7V-HfXqiwrnDfs0EAk&e=>].
>
> I can't reconcile the "MUST" with the "SHOULD" and the "optional".
>
> [Senthil] How about
>     The document will specify the format of the IE's that SHOULD be logged by the NAT
>     device and all the optional fields.  The fields specified in this
>     document are gleaned from [RFC4787 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_rfc4787&d=CwMF-g&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoCgBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=aFOj9PLeooXMHRXima8gIP54ipb4rlFwPk4XSgkDnFs&s=fPl32NCjRXlcp_YL261dUPyU67jiIq7tyzzfC7tKpvw&e=>] and [RFC5382 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_rfc5382&d=CwMF-g&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoCgBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=aFOj9PLeooXMHRXima8gIP54ipb4rlFwPk4XSgkDnFs&s=16Ab6VcDIcMdMKTX968UOAJXu7V-HfXqiwrnDfs0EAk&e=>].
> The optional fields are described in the specific events. For example 
> Table 5, describes a nat session create event,
> There are a few mandatory fields and a few optional fields.

@@PJ2 The MUST was good since this is a standards track document. 
Whereas SHOULD gives wiggle room for RFC-compliant but non-interoperable 
implementations.

How about this:

    This document details
    the IPFIX Information Elements(IEs) that MUST be logged by a NAT
    device that supports NAT logging using IPFIX,    and all the optional fields.  The fields specified in this
    document are gleaned from [RFC4787 
<https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_rfc4787&d=CwMF-g&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoCgBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=aFOj9PLeooXMHRXima8gIP54ipb4rlFwPk4XSgkDnFs&s=fPl32NCjRXlcp_YL261dUPyU67jiIq7tyzzfC7tKpvw&e=>] and [RFC5382 
<https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_rfc5382&d=CwMF-g&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoCgBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=aFOj9PLeooXMHRXima8gIP54ipb4rlFwPk4XSgkDnFs&s=16Ab6VcDIcMdMKTX968UOAJXu7V-HfXqiwrnDfs0EAk&e=>].




> Section 5, first paragraph:
>     The creation and
>     deletion of NAT sessions and bindings are examples of events as it
>     results in the resources (addresses and ports) being allocated or
>     freed.
>
> s/as it results in the resources/as they result in resources/
>
> [Senthil] Done.
>
> Section 5, first paragraph:
>     The events can happen either through the processing of data
>     packets flowing through the NAT device or through an external entity
>     installing policies on the NAT router or as a result of an
>     asynchronous event like a timer.
>
> Since this is either/or/or, simply remove the "either".
> [Senthil] Yes. Done.
>
>
>
> Section 5, first paragraph:
>     The list of events are provided inSection 4.1 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__tools.ietf.org_html_draft-2Dietf-2Dbehave-2Dipfix-2Dnat-2Dlogging-2D06-23section-2D4.1&d=CwMF-g&c=IL_XqQWOjubgfqINi2jTzg&r=Xx9729xYDYoCgBDdcp1FKt5PyYd1TCoXNKhyPY8CFp8&m=aFOj9PLeooXMHRXima8gIP54ipb4rlFwPk4XSgkDnFs&s=wWv4DJ-C66KkaISni6E6o7kuLUdi-NXOQiPBAl6ITcI&e=>.
>
> There is no section 4.1.
> [Senthil] Fixed by pointing to the table that lists the events.
>
>
> Section 5, second paragraph:
>     A collector may receive NAT events from multiple CGN devices and MUST
>     be able to distinguish between the devices.  Each CGN device should
>     have a unique source ID to identify themselves.  The source ID is
>     part of the IPFIX template and data exchange.
>
> No; this requires ID synchronisation across devices which is not 
> required for IPFIX. IPFIX uniqueness is guaranteed by the combination 
> of source address, source port, and source ID. The source address 
> provides uniqueness across devices. The source port provides 
> uniqueness when there are multiple exporters within a device (ie, the 
> source addresses are identical). The source ID provides uniqueness 
> when an exporter exports information from multiple unique sources (ie, 
> the source address and source port are identical).
>
> In IPFIX, the source ID is called the Observation Domain ID. See 
> section 3.1 of RFC 7011.
>
> [Senthil] Ok, the paragraph can be safely removed, as I understand?

@@PJ2: It could be removed, or clarity the issue for readers who may not 
be familiar with IPFIX, eg:

    A collector may receive NAT events from multiple CGN devices. The collector
    distinguishes between the devices using the source IP address, source port,
    and Observation Domain ID in the IPFIX header.



> Section 5, third paragraph:
>     The templates can be
>     exchanged as frequently as required given the reliability of the
>     connection.  There SHOULD be a configurable timer for controlling the
>     template refresh.
>
> It's not just about the reliability of the connection. eg the 
> collecting process could restart with no knowledge of the previously 
> exported templates.
>
> [Senthil] True, in the case of the restart all the exported data will 
> be thrown away by the collector until the template refresh timer kicks 
> in.
> Are you suggesting any changes to what should/shouldn’t be said?

@@PJ Consider adding an xref to section 8.4 of RFC 7011 which 
specifically mentions template refresh for UDP.



> Section 5, third paragraph:
>     NAT device SHOULD combine as many events as
>     possible in a single packet to effectively utilize the network
>     bandwidth.
>
> Say "The NAT device ...".
>
> [Senthil] Ok done.
>
>
>
>
> Section 5.2, table 1:
>
>     |        sourceIPv6Address         |     27 |   128 |  Source IPv6  |
>     |                                  |        |       |    address    |
>
> Most IPv6 addresses have more than 27 bits. The size and ID values 
> appear to be swapped.
>
> [Senthil] :-), Good catch, fixed now.
>
> Section 5.3:
> 	The list can be expanded in the future as necessary.
>
> Define the process for expanding the table, eg Expert Review. Consider 
> putting the table under IANA control to avoid implementers having to 
> refer to a chain of RFCs for the complete definition.
>
> [Senthil] Is the process explained somewhere that I can just point to? 
> Also, I am not clear on what you mean by “putting the table under IANA 
> control”. Can you please elaborate?

@@PJ2: If this is to be a standard table that everyone can use and 
anyone can potentially define new values, then we need to know who 
controls the allocation of new entries and how those are requested. 
Where/how are the new values defined, and who reviews / approves the 
allocation?

See section 4.1 of RFC5226.

eg, new IPFIX fields are requested through IANA, who asks a designated 
group of experts to review and approve them. It could make sense to 
define the table in an IANA registry with a designated expert, possibly 
even as a sub-table in IANA's IPFIX registry.



> Section 5.3, Table 2: NAT Event ID table
>
> When / where is this table used? It seems to be an extension of the 
> existing natEvent IE, though no mention of this is made in the IANA 
> section and the proposed values constrain the existing "create" and 
> "delete" events to be NAT44 specific.
> [Senthil] The value of the natEvent IE uniquely identifies the event 
> that is being reported. Do we need IANA to assign all the possible 
> valid values that natEvent can have?

@@PJ2: It isn't clear whether this table defines new values for the 
natEvent IE. If so then it needs to be discussed in the IANA section of 
the draft because the new values must be requested through IANA.



> Section 5.4:
>     The Quota exceeded events are generated when the hard limits set by
>     the administrator has reached or exceeded.
>
> Say "has been reached or exceeded."
>
> The text should mention that the values are used for the natLimitEvent 
> element in section 8.
>
> [Senthil] Ok. Is there an example of how the values are to be 
> specified for IANA? Thanks.

@@PJ2: See section 6 of RFC7012



> Section 5.4:
>     The events that can be reported are the Maximum session entries limit
>     reached, Maximum BIB entries limit reached, Maximum session/BIB
>     entries per user limit reached and maximum subscribers or hosts limit
>     reached.
>
> Capitalise "maximum subscribers". The "Maximum fragments pending 
> reassembly" event isn't mentioned.
>
> [Senthil] Done.
>
> However there's really no reason to duplicate the information from the 
> table in the preceding description.
>
>
>
> Section 5.5:
>
> The text doesn't describe the "Global Address mapping high threshold 
> event" in Table 4.
>
> The text should mention that the values are used for the 
> natThresholdEvent element in section 8.
>
> [Senthil] Ok.
>
> Section 5.6:
>
>     The following is the template of events that will be logged.  The
>     events below are identified at the time of this writing but the set
>     of events is extensible.
>
> Describe the process for extending the list of events, eg Expert Review.

@@PJ2: see the discussion above.


> Tables 5 - 21:
>
> There's no need to describe the size of each field since that 
> information has already been given in Table 1.
> It draws unnecessary attention to the field sizes which should be 
> invariant.
>
> [Senthil] I find it useful to quickly refer to it and say the size of 
> the record. I am inclined to leave it there.

@@PJ2: OK.


P.


> Section 5.6.5:
>     The following is a template of the event.  Note that either the NAT
>     pool name or the nat pool identifier SHOULD be logged, but not both.
>
> No mention is made of how the NAT pool name could/should be logged 
> (IPFIX IE #284). natPoolID is mandatory in Table 9 which suggests that 
> mention of the NAT pool name should be removed from the text.
> [Senthil] Right, probably a spill over from a revision that I didn’t 
> clean up the pool name.
>
>
>
> Sections 5.6.7.1 and 5.6.7.2:
>     The maximum ... is generated when
>
> Say "The maximum ... *event* is generated when" or "This event is 
> generated when".
>
>
>
> Section 5.6.7.1:
>     when the administratively configured limit is reached.
>
> Define what the limit is, eg "when the administratively configured 
> *NAT session *limit is reached."
>
>
>
> Section 5.6.7.2:
>     when the administratively configured limit is reached.
>
> Define what the limit is, eg "when the administratively configured 
> *BIB entries *limit is reached."
>
>
>
> Section 5.6.7.3:
>     when a single user reaches the administratively configured limit.
>
> Define what the limit is, eg "when a single user reaches the 
> administratively configured *IPv4 or IPv6 address* limit."
>
> [Senthil]  The limit is the number of NAT translations per user. Point 
> taken though.
>
> Section 5.6.8
>     This event will be generated
>
> Say, "These events will be generated"
>
>
>
> Section 5.6.8
>     The threshold reached events are described in the section above.
>
> Please add an xref to the relevant section.
>
>
>
> Section 5.6.8.4
>     This event is generated when the high is reached
>
> Say, "This event is generated when the high *threshold* is reached"
>
>
>
> Section 5.6.8.4
>     This is generated only by NAT devices that use a address pooling behavior of paired.
>
> Would it be clearer to say, "... that use a paired address pooling 
> behavior." ?
>
> [Senthil] Ok.
>
> Section 5.6.9
>     This binding event happens when the first packet of the first flow
>     from a host in the private realm.
>
> Say, "These binding events happen when". The remainder of the sentence 
> seems incomplete?
>
>
>
> Section 7:
>
> s/Trammel/Trammell/
>
>
>
> Section 9:
>     Some management considerations is covered
>
> s/is/are/
>
>
>
> Section 9.1:
>     An IPFIX collector MUST be able to collect events from multiple NAT
>     devices and be able to decipher events based on the sourceID in the
>     IPFIX header.
>
> s/sourceID/Observation Domain ID/ per RFC 7011, section 3.1.
>
>
>
> Section 11.2:
>
> The References to [RFC5101bis] and [RFC5102bis] should be updated to 
> RFC7011 and RFC7012 respectively.
>
> [Senthil] Done for the above items.
> P.
>