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

["Senthil Sivakumar (ssenthil)" <ssenthil@xxxxxxxxx>]

Hi Paul,

Please look for [Senthil2]. 

From: Paul Aitken <paitken@xxxxxxxxxxx>
Date: Saturday, February 13, 2016 at 6:46 AM
To: Senthil Sivakumar <ssenthil@xxxxxxxxx>, "draft-ietf-behave-ipfix-nat-logging@xxxxxxxx" <draft-ietf-behave-ipfix-nat-logging@xxxxxxxx>
Cc: "ietf@xxxxxxxx" <ietf@xxxxxxxx>
Subject: Re: Last Call: <draft-ietf-behave-ipfix-nat-logging-06.txt> (IPFIX Information Elements for logging NAT Events)

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 <paitken@xxxxxxxxxxx>
Date: Thursday, February 11, 2016 at 9:36 AM
To: "draft-ietf-behave-ipfix-nat-logging@xxxxxxxx" <draft-ietf-behave-ipfix-nat-logging@xxxxxxxx>
Cc: "ietf@xxxxxxxx" <ietf@xxxxxxxx>
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] and [RFC5382].

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] and [RFC5382].

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] and [RFC5382].

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 in Section 4.1. 

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.

[Senthil2] There is already an entry in IPFIX IANA registry for natEvent. There are 3 events (create/delete/pool exhausted) defined there. So I will add text to point to IPFIX IANA registry.

I assume I have to add the additional events to the IANA considerations section of this draft to have IANA assign these. However, I am not clear how do I request these new values for

natEvent.  Is the following the right format to ask for the new values?

Name : natEvent

   Description: This Information Element identifies an Instance of the
   NAT that runs on a NAT middlebox function after the packet passed the
   Observation Point.

   Abstract Data Type: unsigned8

   Data Type Semantics: identifier

   Element ID : 230

   New values requested : The values 4-16 are requested as described in Table 2. 

   Reference:

   See RFC 3022 [RFC3022] for the definition of NAT.  See RFC
   3234 [RFC3234] for the definition of middleboxes.

Thanks

Senthil

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.

[Senthil2]. Yes, it does. I will add this to the IANA section.

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.