David,
thanks for your review.
Find answers and comments inline...
Black_David(_at_)emc(_dot_)com wrote:
> I've reviewed this document as part of the transport area
> directorate's ongoing effort to review key IETF documents.
> These comments were written primarily for the transport
> area directors, but are copied to the document's authors
> for their information and to allow them to address any
> issues raised.
>
> --- Summary ---
>
> This draft is in generally good shape, but has some issues that
> need to be addressed. The three most important issues are:
> [1] Side effects of out-of-order message processing
> [2] Dealing with UDP's lack of congestion control
> [3] X.509 certificate interoperability
> They're tagged with these numbers below. Issue [3] is
> fairly easy to address (cite RFC 3280), but can cause
> complex interoperability disasters when it is not addressed.
>
> --- Comments ---
>
> Section 3.5:
>
> Note also that while SCTP
> guarantees to preserve message boundaries even if the message sent is
> larger than the path MTU, there is no such facility in TCP or TLS.
>
> Please explain why this matters. The TCP or TLS receiver
> receives a byte stream and parses the IPFIX messages based
> on that byte stream. As long as the byte stream contents are
> sufficient to identify IPFIX message boundaries, how/why
> does SCTP's preservation of them matter to IPFIX
> implementations?
>
We initially wrote this note because CPs over a datagram-oriented
transport (UDP, SCTP) can use the packet
boundaries instead of having to use the Message Length field to read the
message off the wire. This simplifies the CP implementation (one read()
to get a Message instead of two (one for the message header and one for
the rest). Anyway I'm not completely sure this is relevant and fits here
(nor it does mentioning TLS at this stage...) so my proposal is to
completely cut this sentence
> Section 6.1:
>
> There is an additional service provided by SCTP and useful in
> conjunction with PR-SCTP: unordered delivery. This also works on a
> per-message basis by declaring that a given message should be
> delivered at the receiver as soon as it is received rather than kept
> in sequence; however, it should be noted that unless explicitly
> requested by the sender even messages sent partially reliably will
> still be delivered in order.
>
> [1] Isn't this likely to cause problems when messages are
> processed out of order? If three messages are sent containing
> absolute counts of 45, 87, and 138, out of order processing
> combined with a delay to the first message could result in the
> count being 45 at the recipient after the three messages are
> processed, which seems rather wrong. Whose responsibility
> is it to prevent this? In contrast, partial reliability skips
> updates that would be generated by processing the dropped
> messages, and hence seems ok.
I propose adding the following text at the end of the paragraph (to
recommend not using unordered delivery when order may matter):
Unordered delivery SHOULD NOT be used when the order of IPFIX Messages
may matter: e.g., a Template or Options Template.
>
> o Increase the bandwidth of the links through which the Data Records
> are exported.
>
> Both instances of this should be rephrased to:
>
> o Increase the bandwidth available for communicating the exported
> Data Records.
>
> There are a number of scenarios in which link bandwidth
> increases do not benefit all flows on a link.
>
ok, changed.
> Section 6.2: UDP
>
> [2] I don't see any discussion of congestion control in here.
> Something needs to be done to avoid a high rate IPFIX protocol
> session over UDP worsening a congestion situation because not
> only is the IPFIX flow non-responsive to congestion, but
> congested conditions may increase the volume of IPFIX data
> to be reported. At the very least, this draft should repeat
> and reinforce the discussion in Section 10.3.1 of
> draft-ietf-ipfix-protocol-24.txt, which says that UDP should
> not be used over congestion sensitive network paths - I'm not
> thrilled about that approach, but the ipfix-protocol draft
> is already in the RFC Editor Queue. A general recommendation
> against UDP when TCP or SCTP is possible may be appropriate,
> and the first item in Section 10.1 is related to this concern,
> as it requires availability of a congestion-controlled protocol.
UDP is not the recommended protocol for IPFIX and is intended for use in
cases in which IPFIX is replacing an existing NetFlow infrastructure,
with the following properties:
1. a dedicated network
2. within a single administrative domain
3. where SCTP is not available due to implementation constraints
4. and the collector is as close as possible to the exporter.
Would you like to see this text in the document or is the current text
at the beginning of 6.2 enough:
"UDP is useful in simple systems where an SCTP stack is not available,
and where there is insufficient memory for TCP buffering.
However, UDP is not a reliable transport protocol, and IPFIX messages
sent over UDP might be lost as with unreliable or partially-reliable
SCTP streams. Therefore we recommend that UDP only be used for
systems where data loss is acceptable."
>
> The possibility of Exporter vs. Collector misconfiguration
> on template resend interval vs. template expiry time is
> unfortunate and (as described) can cause interoperability
> issues. Can IPFIX define a template for an Exporter to
> report its resend interval? That would enable a Collector
> to determine an appropriate expiry time for each Exporter,
> and might obviate the need for the packet-count-based
> resend mechanism discussion.
>
> Before using a Template for the first time, the Exporter may send it
> in several different IPFIX messages in quick succession to increase
> the likelihood that the Collector has received the Template.
>
> That's exactly the wrong thing to do if there's a congested
> tail-drop queue in the path, as it increases the likelihood of
> all of the messages being dropped. It's better to space these
> out over some period of time to avoid a transient congestion
> spike causing all the copies of the template to be lost.
>
proposed NEW text:
Before using a Template for the first time, the Exporter may
send it in several different IPFIX Messages spaced out over a period of
time less than the Template resend time in order to increase the
likelihood that the Collector has received the Template.
> Section 7.1:
>
> The text describing each of Figures 1-4 should provide an
> example of a middlebox or two that has the function shown
> in the Figure.
>
agreed
> Section 7.3:
>
> Only in the case of composed middleboxes with well defined and well
> separated internal middlebox functions, for example a combined NAT
> and firewall, MAY an Observation Point be inside a middlebox, but in
> any case it SHOULD be located in between the middlebox functions.
>
> That's a poor use of "MAY". Here's a suggestion for
> replacement text:
>
> If an Observation Point is located inside a middlebox, the middlebox
> MUST have well defined and well separated internal functions,
> for example a combined NAT and firewall, and the Observation Point
> SHOULD be located on a boundary between middlebox functions rather
> than within one of the functions.
>
ok and done
> Section 7.4:
>
> Still, if possible, IPFIX implementations co-located with uncovered
> middleboxes (i.e. of type 7 or 11 - 20) MAY follow the
> recommendations given in this section if they can be applied in a way
> that reflects the intention of these recommendations.
>
> I would change "MAY" to "should" (lower-case), as I think
> the intent is to make a recommendation, but one that
> does not rise to the strength of an RFC 2119 "SHOULD".
>
agree, done as suggested
> Section 8.2:
>
> [3] This appears to only reference X.509 for certificates. That's
> not interoperable, and there's no X.509 reference in the list
> of references. An appropriate reference to RFC 3280 will
> correct these issues.
ok, added a reference to RFC 3280.
>
> --- Nits ---
>
> Section 3.4:
> PSAMP acronym needs expansion or definition before it is used.
done
>
> Section 4.4:
>
> At first, reporting the running total may seem to be the obvious
> choice, but requires that the system accurately maintains the flow
> over a long time without any loss or error,
>
> "maintains the flow" -> "maintains information about the flow"
>
done
> Delta counters offer some advantages: flows don't have to be
> permanently maintained,
>
> "flows" --> "information about flows"
>
done
> Note that delta counters have an origin of zero, and that a
> Collecting Process receiving delta counters for a new flow must
> assume the deltas are from zero.
>
> "new flow" --> "flow that is new to the Collecting Process"
>
done
> Section 4.5.2 and 4.5.3
>
> Both sections talk about a "limited" need for alignment without
> providing justification. They would be clearer if they only asserted
> that the 32-bit alignment that always occurs is sufficient.
>
4.5.2 OLD:
There isn't any means for aligning Information Element specifiers within
Template Records, but there is a limited need for it and Information
Element specifiers are aligned to 32-bit address boundaries anyway.
4.5.2 NEW:
There isn't any means for aligning Information Element specifiers within
Template Records, but there is a limited need for it as Information
Element specifiers are always aligned to 32-bit address boundaries and
this alignment is sufficient.
4.5.3 OLD:
There is no means for aligning Template Records or Option Template
Records within a Set. However, for these records the need for alignment
is limited and they are aligned to 32-bit boundaries anyway.
4.5.3 NEW
There is no means for aligning Template Records or Option Template
Records within a Set. However, for these records the need for alignment
is limited the 32-bits alignment that always occur is sufficient.
> Section 7.1/7.2:
> Figure 4 should be after the Section 7.2 heading, if possible.
>
> idnits 2.04.12 found a couple of minor reference nits:
>
> == Unused Reference: 'RFC2434' is defined on line 1516, but no
> explicit
> reference was found in the text
>
> == Outdated reference: A later version (-12) exists of
> draft-ietf-ipfix-as-11
>
updated
Thanks,
Elisa
_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www1.ietf.org/mailman/listinfo/ietf