Subject: [ipfix] AD review for: draft-ietf-ipfix-protocol-19.txt

From: "Wijnen, Bert (Bert)" <[email-masked]>
Date: Wed, 15 Mar 2006 01:21:35 +0100

Sorry for the long delay.

WG chairs/editors, pls let me (us ADs) know if you want to respin a new
rev first before we do IETF Last Call.

- Sect 3.
  States that Template Sets MUST be sent periodically when UDP is used.
  What if TCP or or SCTP are used? Might want to state what expected
  behaviour is in those cases as well.

- sect 3.1 under SOurce ID
  I do not understand the last sentence. WHat does it mean?
  pls clarify in text.

- Page 16 under padding
  s/zero (0) octets/zero (0) valued octets/
  or "octets with a value of zero (o)" ??
  Last sentence under "padding" may need a period or semicolon in
  the middle of the sentence?

- Sect 5/6 speak about ...DeltaUSeconds, but I cannot find that term in
  INFO spec. It talks about ...DeltaMicroSeconds. Is that what you mean?

- sect 6.1.3
  I'd like to see a reference to the IEEE spec for floats.

- sect 6.1.5 
  I am not sure that by saying "it is expected that strings will be coded
  in UTF-8 format" will guanrantee interoiperability?
  Maybe better use MUST (RFC2119) language.
  Last sentence of 6.1.5 talks about "NUL" characters. That again seems
  to be ASCII language. Maybe better state "zero valued octets" .

- sect 6.1.6
  YOu might want to add a sentence how long this can serve us.
  I believ it is something like 146 years,

- sect 6.2 speaks about signed64/unsigned64 and also for 32 and 16.
  Where are those types defined??

- one but last para on page 33
  "sufficient time" should be defined/explained somewhat don't
  you think so?

- Page 35.
  Should the "All Data Templates" be changed into "All Templates" ??

- I wonder if the one but last para of section 9 is a nice opening
  for DoS attacks?

- Sect 10.1 and 10.2 seem inconsistent in the use of SCTP-PR and
  PR-SCTP

- sect 10.2.6
  Where/how is the treshold specified?
  Same for sect 10.4.1.1

- sect 10.3.7
  It is unclear to me how/where the template lifetimes are specified,
  defined or configured.

- sect 11.4 last para
  I guess that just depends of who/where the attacker is, no?
  WHat if it is an internal ISP employee?

- sect 12.1
  I do not think the IPfix version number is an IANA controlled thing.
  I also think you want that to be changebale only by an IETF standards
  action (as opposed to just IETF consensus). Afetr all you want this
  protocol to be stds track. 

- sect 12.2 seems to be IPFIX-INFO material and not protocol material??

- sect 12 
  You have already gotten ports for UDP, SCTP and TCP.
  Sect 10.2.4.1 however does not list the port number.
  You may want to list the port there, and maybe in the IANA considerations
  you want to say/state that Nevil has already got port numbers for TCP, 
  UDP and SCTP assigned via IANA.

- sect 12. 
  Sect 11.1.1 talsk about IANA assigning Selectors. I guess this needs
  to be documented under IANA Considerations too.

- Sect 12.1
  The "IPFIX Set ID" starts (I think) a new registry. So you MUST
  describe under IANA considerations how to set it up and what the
  rules are for new assignements. You have that text (more or less) in
  sect 3.3.2, but you must spell it out here for IANA as well.

  Then you have a set of Set IDs that get assigned by this document
  and you should list them clearly for IANA so they can do the initial
  population of the registry.

- It seems to me that sect 13 is more an APPENDIX and so non-normative?
  Might be good to then list it as appendix and state that it is 
  non-normative.

- Refrences.
  - is 1889 really normative?

admin/nits/spelling

- references/citation issues:

  !! Contains embedded space:
  P051 L048:    by IANA, on a First Come First Served basis [RFC 2434], subject to

  !! Contains embedded space:
  P052 L005:    Expert Review [RFC 2434], i.e. review by one of a group of experts

  !! Missing Reference for citation: [RFC768]
  P041 L036:    [RFC768]

  !! Missing citation for Informative reference:
  P060 L046:    [RFC3955] Leinen, S., "Evaluation of Candidate Protocols for IP Flow

- add citation to RFC2119 first time you mention 2119.

- sect 3.1 remove one of the two words "format" 
  You migth also want to add a sentence to explain why the version
  field is 0x000a for this version.

- sect 3.1 under SOurce ID
  remove one of the two "the combination" 

- sect 6.2
  s/smaller type/smaller size/ ??

- sect 7
  s/MUST/MUST be/

- page 33
  s/Disused/Unused/ ??

- the last par of the "Interllectual Property Statement" on page 63
  is normally not included in the RFC. So I would remove it.

Bert

--
Help        mailto:[email-masked] and say "help" in message body
Unsubscribe mailto:[email-masked] and say
"unsubscribe ipfix" in message body
Archive     http://ipfix.doit.wisc.edu/archive/

---

From: Benoit Claise <[email-masked]>
Date: Fri, 31 Mar 2006 00:30:10 +0200

Bert, all,

Thanks for your comments. See inline.
For clarity purposes, I removed the obvious editorial improvements, 
which have been inserted already.

Note that I made the following changes:
- I modified all occurrences of Source ID to Observation Domain ID.
- As proposed by Brian Trammell, I clarified:

    Add to the end of 10.2.4.1 Association Establishment:
    "Each SCTP stream over which TLS [TLS] will be used MUST be
    established as a fully reliable bidirectional stream as required by
    RFC 3436."

    Insert a new third paragraph to 10.2.4.2 Association Teardown:
    "If TLS [TLS] is used, the Process initiating association teardown
    SHOULD send a close_notify alert over each TLS stream before closing
    the SCTP association."

    Append to the second paragraph to 10.2.4.3 Stream:
    "Only fully reliable bidirectional streams are available if TLS
    [TLS] as used, as required by RFC 3436."

          [RFC3436] added as a normative reference
>   behaviour is in those cases as well.
I agree this is confusing, as we don't know yet at this stage that UDP 
is a MAY.
Since these are examples, since the first two examples don't discuss any 
transport protocol issues and since the transport protocol is explained 
at length in the section 10, I have removed the sentences.

>   I do not understand the last sentence. WHat does it mean?
As proposed in http://ipfix.doit.wisc.edu/archive/3145.html, I moved it 
into the section "3.4.2.1 Scope... where it naturally belongs

>   or "octets with a value of zero (o)" ??
Done. "the padding octet(s) MUST be composed of zero (0) valued octets. "
>   the middle of the sentence?
Done.
>   INFO spec. It talks about ...DeltaMicroSeconds. Is that what you mean?
Corrected already. Someone mentioned it on the mailing
>   I'd like to see a reference to the IEEE spec for floats.
Corrected already. This was discussed on the mailing list.

 6.1.3   float32
   
   The float32 data type is encoded as an IEEE single-precision 32-bit
   floating point-type, as specified in [IEEE.754.1985].  

 6.1.4   float64
   
   The float64 data type is encoded as an IEEE double-precision 64-bit
   floating point-type, as specified in [IEEE.754.1985].
>   Maybe better use MUST (RFC2119) language.
I have right now:

    The data type string represents a finite length string of valid
    characters of the Unicode character encoding set.  The string data
    type MUST be encoded in UTF-8 format.  The string is sent as an
    array of octets using an information element of fixed or variable
    length. The length of the information element specifies the length
    of the octetarray.

Since this is required for interoperability, I think the "MUST" is more 
appropriate. So I changed all definitions in 6.1.X: from "is encoded" to 
"MUST be encoded"

Note: we have the same sentence in [IPFIX-INFO]:

   It is expected that strings
   will be encoded in UTF-8 format, which is identical in encoding for
   ASCII characters, but also accommodates other Unicode multi-byte
   characters.

I think this should be removed, as the information model should not tell how to encode the data.

>   to be ASCII language. Maybe better state "zero valued octets" .
Following some discussions, the sentence "In case of fixed length 
Information Element, if padding is required, padding MUST be composed of 
NUL character(s)." has been removed. All padding related info is now in 
the padding section.
>   I believ it is something like 146 years,
136 years. Added.

The data type dateTimeSeconds represents a time value in units of 
seconds normalised to the GMT timezone.  It MUST be encoded in a 32-bit 
integer containing the number of seconds since 0000 UTC Jan 1st 1970. 
The 32-bit integer allows the time encoding up to 136 years.

>   Where are those types defined??
Previous text:

 6.1.1   Integral Data Types 
    
   Integral data types - octet, unsigned16, unsigned32 and unsigned64 - 
   are encoded using the default canonical format in network byte 
   order. 

New text:

 6.1.1   Integral Data Types 
    
   Integral data types - octet, signed octet, unsigned16, signed16, unsigned32
   signed32, signed64 and unsigned64 - are encoded using the default canonical 
   format in network byte order.  Signed Integral data types are represented in
   two's complement notation.

>   "sufficient time" should be defined/explained somewhat don't
I added the last sentence.
"The Template Withdraw Message MUST not be sent until sufficient time 
has elapsed to allow the Collecting Process to receive and process the 
last Data Record using this Template information. This time MUST be 
configurable.  A suitable default value is 5 seconds after the last Data 
Record has been sent.

> - I wonder if the one but last para of section 9 is a nice opening
The sentence is: "If the Collecting Process receives a Template Withdraw 
message for a Template Record it has not received before, it MUST reset 
the SCTP association, discard the IPFIX Message, and SHOULD log the 
error as it does for malformed IPFIX Messages."

The point is that we need that statement because it means that the 
collector is confused about its own status. And the only solution is to 
shutdown the SCTP association, so that the exporter will resend all the 
templates with a new SCTP association.
>   Where/how is the treshold specified?
I'm not sure how to answer the questions:
    Where? on the exporter
    How? based on the user appreciation. This is exactly why we have a 
MAY statement.
We could say "exceeds a locally configured threshold. By default this 
time is foo secs." However, I have no clue what the value of foo should be.
If the statements are too vague without the default value, I simply 
propose to remove them.
>   It is unclear to me how/where the template lifetimes are specified,
We wrote the following sentence:
   The Template lifetime at 
   the Collecting Process MUST be at least 3 times higher that the 
   Template refresh timeout configured on the Exporting Process.  
Isn't it enough?

>   WHat if it is an internal ISP employee?
Old text:

   The use of a dedicated network prevents IPFIX Messages from being 
   inspected by an attacker. 

New text:
   The use of a dedicated network prevents IPFIX Messages from being 
   inspected by an external attacker. 

>   protocol to be stds track. 
It was actually the intend behind the sentence:

   "Changes in either IPFIX Version Number or IPFIX Set ID assignments 
   require an IETF Consensus, i.e. they are to be made via RFCs 
   approved by the IESG."

Should we simply erase the section 12.1?
Or make clearer?
   "Changes in either IPFIX Version Number or IPFIX Set ID assignments 
   are not controlled by IANA, but by an RFC approved by the IESG." 

> - sect 12.2 seems to be IPFIX-INFO material and not protocol material??
Section 12.2 erased.

>   sect 3.3.2, but you must spell it out here for IANA as well.
New section "12.x Set ID"

Set ID value identifies the IPFIX Set. Set ID values of 0 and 1 are not 
used for historical reasons [RFC3954]. A value of 2 is reserved for the 
Template Set. A value of 3 is reserved for the Option Template Set. All 
other values from 4 to 255 are reserved for future use. Values above 255 
are used for Data Sets.
The new Set ID values are defined as "Specification Required" [RFC 2434] 
prior to the assignment by IANA.

>   Might be good to then list it as appendix and state that it is 
I moved the appendix just before the reference section.
ftp://ftp.rfc-editor.org/in-notes/rfc-editor/instructions2authors.txt 
doesn't mention that we should mention something about non-normative ... 
unless I missed something?

>   - is 1889 really normative?
Moved to informative
>   P060 L046:    [RFC3955] Leinen, S., "Evaluation of Candidate Protocols for IP Flow
Removed this reference.
>   field is 0x000a for this version.
I inserted this proposal

Version

Version of Flow Record format exported in this message.  The value of 
this field is 0x000a for the current version, incrementing by one the 
version used in the NetFlow services export version 9 [RFC3954].

>   s/Disused/Unused/ ??

Let me copy the entire paragraph:
   Disused Templates SHOULD be deleted. Before reusing a Template ID 
   the disused Template MUST be deleted.  In order to delete an 
   allocated Template, the Template is withdrawn through the use of a 
   Template Withdraw Message.  

The term "unused" is already applied in this section with a different meaning.
For example "an unused Template ID MUST be used". This means: allocate a new Template ID.
However, in the sentence you pointed out: we want to say: the templates that are not used anymore.
I propose:
	"The Templates that are not used anymore SHOULD be deleted. Before reusing a Template ID, the Template MUST be deleted"

Thanks again for your comments. 

Regards, Benoit.