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

From: Paul Aitken <[email-masked]>
Date: Wed, 2 Mar 2016 13:58:53 +0000

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.

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

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.

5.6.8.4.Global Address mapping high threshold reached

-> Extra whitespace at the period: "paired address pooling behavior ."

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

8.2. Modified Information Elements / natEvent

-> Again, you can't modify the definitions of the existing values.

P.

---

From: Paul Aitken <[email-masked]>
Date: Mon, 7 Mar 2016 16:20:26 +0000

Senthil, please see inline:

> Please see inline for [Senthil]
>
> From: Paul Aitken <[email-masked] <mailto:[email-masked]>>
> Date: Wednesday, March 2, 2016 at 8:58 AM
> To: Senthil Sivakumar <[email-masked] 
> <mailto:[email-masked]>>, 
> "[email-masked] 
> <mailto:[email-masked]>" 
> <[email-masked] 
> <mailto:[email-masked]>>
> Cc: "[email-masked] <mailto:[email-masked]>" <[email-masked] 
> <mailto:[email-masked]>>
> Subject: Re: Last Call: <draft-ietf-behave-ipfix-nat-logging-06.txt> 
> (IPFIX Information Elements for logging NAT Events)
>
> " /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.

> [Senthil] Done.
> [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.

> [Senthil] Done.
> [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.

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

---

From: "Senthil Sivakumar (ssenthil)" <[email-masked]>
Date: Mon, 7 Mar 2016 16:00:16 +0000

From: Paul Aitken <[email-masked]<mailto:[email-masked]>>
Date: Wednesday, March 2, 2016 at 8:58 AM
To: Senthil Sivakumar <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
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.

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

          The IPFIX Information Elements that are NAT specific are created with

          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,

2. Introduction

-> No need to repeat "(IEs)" since this was already explained in the Terminology section.

   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.

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

5.6.8.4. Global Address mapping high threshold reached

[Senthil] Done.

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

[Senthil] Is there a process on how to modify/deprecate the previously defined values and replace it with new ones?

Thanks
Senthil

---

From: "Senthil Sivakumar (ssenthil)" <[email-masked]>
Date: Mon, 7 Mar 2016 18:15:03 +0000

<snip>

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.

[Senthil2] Ok, understood and fixed.
8.2. Modified Information Elements / natEvent

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.

[Senthil2] I have not heard any suggestions yet from IANA, I will wait for the review. Attaching the draft as it stands, with all the comments incorporated, let me know if you have any comments before I publish.

Thanks
Senthil

P.