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

[Paul Aitken <[e-mail]>]

Senthil, please see inline:

> Please see inline for [Senthil]
>
> From: Paul Aitken <[e-mail] <mailto:[e-mail]>>
> Date: Wednesday, March 2, 2016 at 8:58 AM
> To: Senthil Sivakumar <[e-mail] 
> <mailto:[e-mail]>>, 
> "[e-mail] 
> <mailto:[e-mail]>" 
> <[e-mail] 
> <mailto:[e-mail]>>
> Cc: "[e-mail] <mailto:[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)
>
> Senthil, I hadn't realised you'd published a new version of the draft. 
> Please CC me if/when you update it again.
>
> I've quickly reviewed the diffs between -06 and -07 :
>
>
> 1. Terminology
>
> " /Any non-IPFIX terminology used to convey NAT events are described 
> in //this section./"
>
> -> which section is "/this/" referring to? Since this paragraph seems 
> only to serve as an introduction to the third paragraph which only 
> contains a single exception, would it be better to remove these two 
> lines and go directly to the third paragraph? :
>     However, that causes
>     confusion in terminology used in NAT specific terms and IPFIX IEs.
>     Any non-IPFIX terminology used to convey NAT events are described in
>     this section.
> [Senthil]    How does this read?
>
>           The IPFIX Information Elements that are NAT specific are 
> created with
>
>         NAT terminology. In order to avoid creating duplicate IEs, IEs
>
>         are reused if they convey the same meaning.
>
>         This document uses the term timestamp for the Information 
> element which
>
>         defines the time when an event is logged, this is the same as 
> IPFIX
>
>         term observationTimeMilliseconds as described in [IPFIX-IANA]. 
> Since
>
>         observationTimeMilliseconds is not self explanatory for NAT 
> implementors,
>
>         this document uses the term timeStamp.
>
>

That's good.


> 2. Introduction
> "This document details the IPFIX Information Elements(IEs)"
> -> No need to repeat "(IEs)" since this was already explained in the 
> Terminology section.
>
> -> Remove the duplicated text:
>     The IPFIX Protocol [RFC7011] defines a generic push mechanism for
>     exporting information and events.  The IPFIX Information Model
>     [IPFIX-IANA] defines a set of standard IEs which can be carried by
>     the IPFIX protocol.This document details the IPFIX Information Elements(IEs) that MUST be 
> logged by a NAT device that supports NAT logging using IPFIX.   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].
>
> [Senthil] Done.
>
> 5.4. Quota exceeded Event types
>
> -> In the "/The events that can be reported are .../", I'd like to see 
> the text be identical to the items listed in table 3 to remove any 
> possible ambiguity.
>
> [Senthil] All of the events do match the table. The text is a little 
> more verbose and descriptive, so that the table doesn’t have to have 
> long text message.
> Let me know if the below is any better than before.
>
> 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, Maximum active hosts limit
>      reached or maximum subscribers limit reached and
>      Maximum Fragments pending reassembly limit reached.
>              +---------------------------------------+--------+
>              |       Quota Exceeded Event Name       | Values |
>              +---------------------------------------+--------+
>              |        Maximum Session entries        |      1 |
>              |          Maximum BIB entries          |      2 |
>              |        Maximum entries per user       |      3 |
>              |  Maximum active hosts or subscribers  |      4 |
>              |  Maximum fragments pending reassembly |      5 |
>              +---------------------------------------+--------+

Good.


> 5.6.8.4.Global Address mapping high threshold reached
>
> -> Extra whitespace at the period: "paired address pooling behavior ."
>
> [Senthil] Done.
>
>
> 8.1. New Information Elements / natLimitEvent, natThresholdEvent,
>
> -> typo: " describer in Table below."
>
> -> you should remove the "Table 22" and "Table 23" descriptions under 
> those tables, because these won't make any sense when the text is 
> transcribed into IANA's registry. E
>
> [Senthil] I am not sure I understand why, because in section 8.1, for 
> natInstanceI, internalAddressRealm, externalAddressRealm we have this 
> format of name/description/data type and references.
> Why is natLimitEvent and natThresholdEvent different just because they 
> have their values defined?

You misunderstand: I mean the description text immediately underneath 
the tables, specifically the text which says: "/Table 22: Quota Exceeded 
event table/", "/Table 23: Threshold event table/", and "/Table 24: NAT 
Event ID table/". I'm not talking about the table contents.

IANA will take the relevant sections from your draft into their IPFIX 
registry. While "Table NN: whatever" makes perfect sense in the draft, 
it makes no sense in IANA's IPFIX registry - so IANA would have to 
selectively edit the definition you're providing. While I'm sure they're 
capable of doing that, the issue would be avoided if those tables didn't 
have descriptions.


> 8.2. Modified Information Elements / natEvent
>
> -> Again, you can't modify the definitions of the existing values.
>
> [Senthil] Is there a process on how to modify/deprecate the previously 
> defined values and replace it with new ones?

You can't modify the existing values, because there could be an unknown 
number of existing devices already using those values. I hope that IANA 
has already sent you feedback from the IPFIX expert reviewers indicating 
the best way forward.

P.