Re: [tcpm] New Version Notification for draft-ietf-tcpm-newcwv-11.txt

[[e-mail]]

The document editors have just uploaded a new revision of the CWV draft,
intended to address this issues raised in the IETF LC.  Specific comments
are listed below,

Best wishes,

Gorry & Raffaello

---


Mirja (TCPM)

pipeACK (section 4.1 and 4.2 and 4.5.1):
1) Can you maybe add a sentence that explains why FlightSize is not
sufficient.
I understand the difference between the two variables but are there actually
cases where it makes a big difference if one or the other is used?
- Added to section 1.

2) This is just an idea and does not make a big difference: Instead of saying
pipeACK is undefined, you could just set it to infinity or 'max' (what
every the
max in the implementation is) and this would always keep you in validated
phase.
- Not changed, this is an implementation choice, but setting to infinity
requires care when considering other uses of the pipeACK, e.g., when
calculating  cwnd = (Max(pipeACK,LossFlightSize))/2.

3) The implementation for pipeACK (based on HighACK) that you propose does
not
take into account that dupACK also acknowledge new data. Is this on
purpose? If
so please state explicitly.
- A sender MAY count TCP DupACKs that acknowledge new data when collecting
the pipeACK sample. Other equivalent methods may be used.

4) Text says "When no measurements are available, ..."
    Maybe say more explicitly "When no measurements are available as the
connection is idle" or "... as no data were sent during the last measurement
period" or "... no ACK were received...".
-  (e.g., a sender that has been idle will have sent no data and received
no ACKs)

5) I would appreciate a forward reference to section 4.5.1 here (as also done
for other stuff as for cwnd-limited in the next section).
- Done

6) Could you also include pseudo code in 4.5.1? I far as I know you have an
implementation, so it should not be extremely hard to come up with pseudo
code
and I personally think it would be really helpful...
We don’t plan to add this to 4.1, burt there is code available for Linux
and BSD.

7) nit in last paragraph: s/method defined on [RFC6675]is/ method defined in
[RFC6675] is/
- Done

8) Figure 1 (section 4.5.1): Maybe add a y-axis...?
- pipeACK sample (Bytes)

non-validated phase (section 4.3 and 4.4)

1) Maybe add one sentence saying "A connection is also in non-validated
phase if
pipeACK is zero which means the connection is idle". Saying this explicit
would
help me to understand things better.
- Idle isn’t quite correct, what it means is no new data was ACK’ed -
there could be data sent and pending ACK/ We changed 4.5.2 to clarify
this.

I just notice that this might actually not be true because in the section
above
you say that pipeACK is set to undefined if no measurements are available and
that would mean the connection goes into validated phase... okay I think
there
is some clarification needed here.
- see above


2) In section 4.4 you first say
    "cwnd neither grows nor reduces while the sender remains in this phase"
     and then
     "A sender that is cwnd-limited MAY use the standard TCP method to
increase
cwnd".
     Please clarify!
- A TCP sender that enters the non-validated phase preserves the cwnd
(i.e., the cwnd only increases after a sender fully uses the cwnd in this
phase, otherwise the cwnd neither grows nor reduces).

3) In section 4.4.1:
    "A sender that detects a packet-drop MUST record the current
    FlightSize in the variable LossFlightSize ..."
    Is this "packet-drop" or "congestion notification"? I guess the
difference
is that if ECN is used R=0. So that's okay. Please check again that this also
works correctly for ECN.
- Agree, we removed explicit discussion of ECN - We agree R=0 when ECN is
used and there is no loss, but we did not wish to further interpret the
correct response for ECN.

4) Is this
    cwnd = (Max(pipeACK,LossFlightSize) - R)/2
- This is correct and more conservative.
    or
    cwnd = (Max(pipeACK,(LossFlightSize - R))/2  [only LossFlightSize - R] ?
    Because the text reads as if it should be the second...
- Text has been fixed: The loss of segments indicates that the path
capacity was exceeded by at least R, and hence the calculated cwnd is
reduced by at least R before the window is halved.

5) "ssthresh is adjusted using the standard TCP method."
    This is not clear to me. Is ssthresh set to cwnd/2 before the
adjustment or
to cwnd-1 after the adjustment? The first option would restart the
connection in
Slow Start... Please clarify!
- We use the standard method, with no change.

6) "then this can result in the final cwnd after loss recovery being 1/4
of the
cwnd"
    Shouldn't this be "being at max 1/4 of the cwnd"...?
-  at most one quarter of the cwnd

7) "Subsequent updates to cwnd do not therefore reflect pipeACK history
before any
    congestion event."
    Not sure I understand this sentence... which subsequent updates?
- This reduction is conservative, and pipeACK is then reset to undefined,
hence cwnd updates after a congestion event do not depend upon the pipeACK
history before congestion was detected.

8) nit: s/updated during loss recoverySection 4.2/ updated during loss
recovery
as stated in Section 4.2/
- Done

Non-validated phase (section 4.4.3 and 5)
1) I would recommend to change the title to
"4.4.3 Adjustment at the end of the Non-Validated Phase (NVP)"
- Done

2) "An application that remains in the non-validated phase for a period
    greater than the NVP is required to adjust its congestion control
    state.  If the sender exits the non-validated phase after this
    period, it MUST update the ssthresh:"

    This is not fully clear to me. Do I have to adapt cwnd and ssthresh right
after NVP or as soon as I leave the non-validated phase after being for at
least
NVP time in this phase? If I'm not idle, but only sending with a low rate, I
should probably do this adjustment with the next ACK, no...?
- Not sure of the difference, e hope the new intro helps with this.

3) Further, these adjustments mean that the connection will be in Slow
Start. If
I understand this correctly, this is not a problem as long as the connection
stays in non-validated as the cwnd will not be further increased. But as
soon as
it leaves non-validated phase it will increase its cwnd as in Slow Start. I
would like to have this spelled out explicitly.
- ???

4) This section does not tell/specify what value NVP period should have.
Please add a
sentence that this is on purpose and add a reference to section 5.
- We see this problem= as a definition in the info section and added text
to 4.4.

5) Section 5 says "A period of five minutes was chosen for this NVP."
    This seems to be a left over from an older version as nothing was
specified
so far. Please check!
- Fixed

-----------------------------------------------------------------------------------------

Martin (AD)

The comment:
- Section 5:
"   Routing algorithms may modify the network path, disrupting the RTT
    measurement and changing the capacity available to a TCP connection,
    however such changes do not usually occur within a time frame of a
    few minutes."

That is an odd assumption, as a path change can happen at any time,
though it is not very usually under normal conditions. Did actually mean
to say that it is uncommon to happen?
- Routing algorithms may change the the network path that is used by a
transport. Although a change of path can in turn disrupt the RTT
measurement and may result in a change of the capacity available to a TCP
connection, we assume these path changes do not usually occur frequently
(compared to a  time frame of a few minutes).


The two nits:

- Section 4.4.1
"   The pipeACK value is not updated during loss recoverySection 4.2. If
    there is a valid pipeACK value, the new cwnd is adjusted to reflect
    that a non-validated cwnd may be larger than the actual FlightSize,
    or recently used FlightSize (recorded in pipeACK).  The updated cwnd
    therefore prevents overshoot by a sender significantly increasing its
    transmission rate during the recovery period."

Typo in first sentence "recoverySection 4.2"
- Done

- Section 4.5.2.

"   A sender needs to implement a method that detects the cwnd-limited
    condition (see Section 4.4.  This detects a condition where a sender"

closing brackets after 4.4. missing
- Done

-----------------------------------------------------------------------------------------

Sec

The security considerations defer to those of RFC 5681 with the claim that
this document just describes an algorithm for one aspect of the congestion
control procedures and thus that the generic security considerations
apply; this claim seems true.  The security considerations section of RFC
5681 feels a little bit short, but it does seem to cover the relevant
risks, namely that an attacker could make the sender send more slowly or
more quickly, with the latter possibly driving the network into congestion
collapse.  These are issues that protocol designers and implementors must
be aware of (and avoiding congestion collapse is more important).

The authors seem to have taken sufficient care to avoid this algorithm
contributing to congestion collapse, and the concern about an attacker
being able to slow down a sender is not really feasible to mitigate at
this level, so the security considerations of RFC 5681, short as they are,
seem adequate here.  One would hope that implementors know that failing to
follow the specification could lead to congestion collapse, so an explicit
warning is not needed.

The nits are basically all grammatical; I'll include them below and expect
people other than the document authors/shepherd to stop reading here.

-Ben

My apologies in advance if I am applying too much of an American English
perspective to this document written in British English.


In the abstract, "TCP is used to support traffic that [...]", is "support"
the best verb to use (as opposed to "transmit", "carry", "transport",
etc.)?
- Done

The first paragraph of the Introduction might have a little more text to
clarify how the cwnd differs from the FlightSize, for non-experts such as
myself.  (The cwnd is how much the sender is permitted to have
outstanding/unack'd and the FlightSize is how much the sender actually has
outstanding/unack'd?)  Just another sentence after the second one, "The
FlightSize is how many unacknowledged packets/bytes that are currently
outstanding, and the cwnd is the maximum value the sender will allow
FlightSize to take on" would be plenty.
- Added Explanation, and reworked this text.

Introduction, third paragraph, """[CWV] introduced the terminology of
"application limited periods".  This document describes [...]"""  Usually,
in an RFC, "this document" refers to the RFC itself, but in this case,
"this document" seems to be being used to refer to the other document, RFC
2861.  So, the references and text could be made more clear.
- Updated to refer to RFC2861.

A couple sentences later, "or by changing the rate the application sends",
maybe put an "at which" in there somewhere?  Absent context, I would
expect "the rate the application sends" to mean that an application
protocol was conveying a number which is interpreted as a rate in some
fashion.
- Changed

Still in the Introduction, in the summary of the document, paragraph for
section 4, maybe s/does this in a way/does so in a way/ ?  Also, in the
parenthetical at the end of that sentence, I think it reads better as
"including both application-limited and idle senders".
- Changed

Still in the introduction, in the "goals of this update" list, first item,
do bulk transfers consume the cwnd or fill it?
- changed text

I'm not sure that the fifth item is fully grammatical.  Maybe add in an
"to send extra traffic" somewhere?
- changed text

In the sixth item, there's a singular/plural mismatch between flows and
network (unless it's supposed to be "the network").
- changed to benefiting both the flows and other flows sharing the network
path

Section 2, first paragraph, "the behaviour where a sender transmitted at a
rate less than allowed by cwnd", is "where" or "when" better?
- when

Section 2, second paragraph, third sentence: I think it should start
"While this" instead of just "This".  The first comma in this sentence
could also be removed, if desired.
- While this

In section 3 (Terminology), it might be nice to clarify that the listed
terms are new to this document, or in addition to the RFC 5681 terms, or
something like that.  (I.e., "Additionally, the following terminology
...".)
- additional

Section 4.2, last paragraph, a space is missing after the [RFC6675]
citation.
- Done

In section 4.4: if I understand correctly, the sliding window for pipeAck
calculation is smaller than the NVP, so it is not the absolute amount of
data which the sender transmits which can push pipeAck over (1/2)*cwnd,
but rather the rate at which the data is transmitted.
- pipeACK is a measure per RTT (like cwnd)

Also in section 4.4, there is some superficial inconsistency between "a
sender that enters the non-validated phase SHOULD preserve the cwnd" and
the text telling the sender how to reduce cwnd if it encounters congestion
and the (outer) bullet point wherein "a sender determines whether to
increase the cwnd based on ...".  The last paragraph of the section helps
clarify the situation, but it might help readability to clarify the
exceptions to the SHOULD (and the rationale for them) right after the
claim is made.
- Reworked.


In section 4.4.1, there is a missing space after "recovery" and before the
internal reference to Section 4.2.
- Done

Section 4.5.1, comma after "e.g.".
- Done

-----------------------------------------------------------------------------------------

Gen-ART

Nits/editorial comments:

1.        I am not sure that there is a need for the first sentence in
section
1.2
- Moved to ACK section.

2.        In section 4.4.1 MSS is used but not expanded Maximum Segment  Size
(MSS)
- Done