[conex] Feedback on draft-ietf-conex-tcp-modifications-05

[Nandita Dukkipati <[e-mail]>]

(feedback as individual contributor)

No need to sugar coat: this draft needs lot more work. But happy to
help to shape it to its final form.

---------

Table of Contents

* Section 2 is Sender side changes. I was looking for equivalent
Receiver side changes, but there's no section for that. Should there
be one?

* Section 3 My suggestion is to swap the ECN and loss detection
sections, because loss detection methods are implicit and can be used
today, while ECN requires... well ECN to be deployed.

Introduction

* At the end of Introduction lay out the organization of the remainder
of the draft so a reader is informed of how the flow looks like.

* Need to set up the problem being addressed
Jumping to Section 2 on Sender side modifications felt abrupt. What's
missing is a setup of the problem that the draft is proposing TCP
changes for. Can we place this problem setup either in the Intro. or a
separate section in itself. An example of what might go into the
problem set up is: Algorithms for a ConEx sender to keep track of byte
wise congestion.

Section 2. Sender-side modifications

* "A ConEx sender MUST negotiate for both SACK and ECN.... Thus a
ConEx SHOULD also implement SACK and ECN".
The para appears incorrect to me. If you mandate a MUST for SACK/ECN
negotiation, then shouldn't there be a MUST also for implementing SACK
and ECN?

* Pseudo code.
Section 2 begs for pseudo code to succinctly describe the proposal.
Else the text is unnecessarily wordy.

* Section 2 ends abruptly. It sets up the problem and I was looking
for more details, but I guess they are punted to the other sections in
the draft.

Section 3. Accounting congestion

* "... ConEx marked either with the E bit or the L bit..."
AFAICT this is the first occurrence of E and L bits, and hence should
define them.

* Section 3 uses "outstanding bytes" often to describe the algorithms.
On immediate reading it was confusing, because in TCP implementations
outstanding bytes refers to the data that's been sent out but not
ACKed yet.

What you really mean by outstanding bytes here is a count of the
remaining bytes that are yet to be ConEx marked with either ECN or
loss bits. It's worthwhile having an explicitly and precisely efined
term here.

Section 3.1

* The algorithmic description of DeliveredData is laborious. Can you
see if you can simplify the description. Also take a look at [PRR RFC]
and see how DeliveredData is described there.

* Is Section 3.1 the best place to describe DeliveredData?
DeliveredData is trivial when there's no loss - it's just the number
of bytes acked. So clearly the value of DeliveredData is when there
are packet losses. So then shouldn't you be describing in the loss
sesction and using that definition in the ECN section.

This also goes back to an earlier comment I made that you should
consider swapping the two placement of these two sections, 3.1 and
3.2.

* For ease of reading and simplicity, please split up the equation for
DeliveredData into the SACK and non-SACK case.

Section 3.1.2 Classic ECN support

* ".... it will receive one full RTT of delayed ACKs with the ..."
I don't remember exactly, but some implementations may disable delayed
ACKs on receiving a CE mark. What's the significance of mentioning
delayed ACKs here?

* Why not recommend (SHOULD?) to turn off delayed ACKs when rceiving
CE marks. May be this belongs to another draft and not here?

* The description of the more sophisticated heuristic to extract
congestion notification at the end of Section 3.1.2 sounds half
hearted to me. My suggestion: either describe it nicely and clearly
with pseudo code etc., or give a high level idea here and ditch the
details to an Appendix. At least that way the flow of the draft is
maintained.

Section 3.2 Loss Detection

* Do retransmitted packets carry ConEx L bit or just the original data
segments? Whatever the answer is, why?

Section 3.2.1 Without SACK Support

* Isn't this the right place to discuss DeliveredData w/o SACK support?

Section 3.2.1 Without SACK support

* General comment that's not sepcific to this section: LEC and LEG are
very close in naming, but mean different things. Can we coose better
names?

* Is there any implicit relation between LEC and LEG counters? Seems
like there is, otherwise why do you say that LEG shoul be increased by
LEC?

* It seems to me that you have a nice algorithm here, but sorry you
totally lost me. My only suggestion would be to try and put the algo
in a nice pseudo code.

Section 4

* A lot of what appears in this section seems like should be in the
sendder side changes.

Section 4.1

* What's the reasoning behind: "If the CEG or LEG counter is negative,
the respective counter SHOULD be reset to sero within one RTT after it
was decreased..."

Section 4.2

* Please clarify the relation between credit counter and the previous
counters LEG/LEC? Are they independent?

* Could you expand more on how you arrived at: every fourth packet
will allow teh respective number of credits in Slow Start..." I can
see how it works. Is there a formulae connecting the Slow Start
exponent and the the nth packet that needs to be marked?

* Mention how credits should be replenished in congestion avoidance phase.

Section 6 Timeliness of the ConEx Signals

* You discuss the event when receiving both ECN and loss signals. But
how should the sender respond? Send ConEx signals for summation of
both ECN/loss (I would think not) or choose the max?

Editorial changes

I have tons of editorial changes marked all over the draft - too many
to spend time to cut+paste in this email. I'll just scan the sheets
and send them to you.