Re: [tcpm] WGLC for draft-ietf-tcpm-accurate-ecn-23

Markku,

I have now written up the edits I proposed in order to address your 
review comments.
Offlist I've just sent you an rfcdiff of just the changes for you. I've 
addressed everyone's reviews now, but I'll post it in a few days to give 
you time to get back to us as soon as you can.
The list can also follow the github linked from the datatracker.

I've also added a couple of extra explanations below about what we've 
changed regarding "ACKs of ACKs" and "Updates 3168" (see [BB2] inline).

On 26/03/2023 17:28, Bob Briscoe wrote:
> Markku, Thank you for the review. Pls see [BB] inline...
>
> On 26/03/2023 08:52, Markku Kojo wrote:
>> Dear all,
>>
>> I have reviewed the doc. I didn't pass through all the details that I 
>> believe are in good shape. The doc reads well and is very thorough in 
>> most part. It is useful and good to have it published.
>>
>> However, there are a few of issues I am concerned about:
>>
>> 1. IANA considerations on allocating/using ECN flags.
>>
>> The design of the ECN negotiation and particularly how it takes into 
>> account potential any future ECN alternatives is the correct approach 
>> and very much appreciated.
>>
>> However, it is not made explicit in the draft. Instead, the draft 
>> hides this important information in the appendix B and discusses 
>> elsewhere in the draft, including the Sec 6 on IANA consideration, on 
>> the allocation of the AE flag in such a way that it is easily 
>> intrepeted as if the AC flag is solely for AccECN use (now and in the 
>> future).
>
> [BB] 1.1) The AccECN draft is not intended to give guidance on how to 
> design a future protocol that uses header flags for capability 
> negotiation.
> Indeed, the WG does not necessarily want to encourage consumption of 
> header flags for new capability negotiations. The case for AccECN's 
> use of header flags is very specific to AccECN and ECN's history.
>
> The AccECN draft describes how an AccECN implementation works, which 
> combinations of flags it uses, and justifies the design by explaining 
> how it supports backward compatibility.
> For information only, it provides an appendix that outlines /some/ 
> other combinations that are still available. But there is absolutely 
> no requirement for future uses of the TCP header to think of the set 
> of 3 flags used by AccECN as the only way of combining TCP flags in 
> future. Future protocol designers can use any cracks left between the 
> uses of AccECN and other protocols, if necessary in combination with 
> any other flag, or field, or whatever (e.g. the URG flag is a 
> possibility, as mentioned in the appendix).
>
>>
>> I'd suggest that the draft and the IANA assignment makes it explicit 
>> what the design actually does: it allocates (reassigns) the bit 7 of 
>> the TCP header flags to extend the use of the existing ECE and CWR 
>> flags in a backwards compatible way. That is, it specifies a 
>> three-bit TCP ECN Field out of the AE, ECE, and CWR flags and assigns 
>> the codepoint 111 to AccECN for negotiating ECN Capability in its 
>> initial SYN of 3whs. For backwards compability codepoint 011 is kept 
>> for negotiating Classic ECN in the initial SYN (and 000 for Not-ECN 
>> Capability) . After the initial SYN, the use of the TCP ECN Field 
>> depends solely on the variant of ECN Capability negotiated, i.e., the 
>> TCP ECN Field is overloaded to complete the 3whs as well as to 
>> provide ECN feedback during the TCP connection.
>
> [BB] 1.2) We are in the end-game wrt available bit combinations in 
> TCP. Even your multiple lines of text above are nowhere near enough to 
> describe AccECN's bit allocations. Even my longer description at Note 
> 1 below is only a summary. The complexity is a consequence of very 
> scarce header space combined with trying to side-step middlebox 
> tampering with most available combinations.
>
> Essentially, AccECN uses some combinations (not really codepoints) out 
> of the five flags: AE, CWR, ECE, ACK & SYN. And it also depends on 
> which combination of these five flags were used earlier in the 
> handshake sequence. See {Note 1}  below for details.
>
> The draft is intended primarily to be useful to implementers of 
> AccECN, not future protocol designers. To find the bit-space to 
> implement AccECN, we had to scratch around, reading the small print of 
> multiple RFCs as well as papers about actual behaviours and bugs (and 
> do our own measurements). We can't do this job for future protocol 
> designers, because only the future designer can tailor their specific 
> needs around the bit combinations that will be available in the future.
>
>> Consequently, the IANA assignment is best modified to reflect this by 
>> introducing a three-bit TCP ECN Field and allocating codepoints 000 
>> (non-ECN-setup SYN), 011 (RFC 3168 ECN-setup SYN), 111 (AccECN 
>> ECN-setup SYN) for the ECN-Capability negotiation in the initial SYN 
>> segment.
>
> [BB] 1.3) Again, I don't think the WG wants to recommend that new 
> behaviours use header flags like ECN did. And, as explained below, 
> there is no reason to confine future protocol designers to a 3 bit 
> field (or 5 header flags), for just the first handshake packet. They 
> will have to think through the whole handshake before they have a design.
>
> IOW, future inventive tricks to find spare bits do not need to copy 
> the most recent inventive trick.
>
>> I'd also suggest renaming the AE flag to EE (Extended ECN) to better 
>> cover also any ECN alternative beyond Classic ECN and not to be tied 
>> by name to AccECN only.
>
> [BB] 1.4) There is no reason to pre-judge that this flag has to be 
> used only for ECN in future.
> It is named for what it is used for now. Any future use of it can 
> rename it to whatever is appropriate at that future time.
> (Also the chairs closed the debate about naming this flag years ago).
>
>>
>> IMHO, this would make it all much clearer for everyone as well as 
>> easier to allocate new codepoints (by IANA) to any future ECN variant 
>> negotiation.
>
> [BB] 1.5) The TCP header bits are not plentiful enough to delegate 
> their assignment to IANA in any way such that the constraints would be 
> describable to IANA in advance.
>
> {Note 1}: I posted a full analysis of used and available bits to the 
> list many years ago during a relevant list discussion. The chart is 
> still available here:
> https://bobbriscoe.net/projects/latency/ecn-modes.pdf
> Here's a summary description of the allocation of combinations of 5 
> TCP header bits to AccECN:
>
>   * When (ACK,SYN) are (0,1) (i.e. AccECN SYN)
>       o AccECN uses 1 x 3-bit combination (AE, CWR, ECE) = (1,1,1), as
>         you say.
>       o In addition, RFC3168 uses 1 x 2-bit combination (CWR, ECE) =
>         (1,1) and doesn't use AE=1 (even for the nonce), so AccECN
>         effectively allocates (AE, CWR, ECE) = (0,1,1) as RFC3168.
>       o Non-ECN TCP didn't define any of (AE, CWR, ECE), but AccECN
>         effectively allocates 1 x 3-bit combination (0,0,0) to non-ECN.
>       o That leaves 5 x 3-bit combinations of (AE, CWR, ECE)
>         available, as listed in Appx B.3, but *only when **(ACK,SYN) =
>         (0,1)*
>   * When (ACK,SYN) are (1,1), but *only after an AccECN SYN* (i.e. an
>     AccECN SYN-ACK)
>       o AccECN uses 4 x 3-bit combinations of (AE, CWR, ECE)
>       o RFC3168 and RFC3540 (historic) use 2 x 3-bit combination (AE,
>         CWR, ECE) = (X,0,1)
>       o Non-ECN TCP didn't define any of (AE, CWR, ECE), but AccECN
>         effectively allocates 1 x 3-bit combination (0,0,0) to non-ECN.
>       o A widespread broken implementation uses the remaining 1 x
>         3-bit combination.
>       o So *no further combinations are available for a **SYN-ACK
>         after**an AccECN SYN*, but some will be available after
>         different SYNs.
>   * When (ACK,SYN) are (1,0), but *only after* an AccECN SYN and
>     AccECN SYN-ACK, *except* under the conditions that the AccECN
>     draft defines for the handshake encoding on the 3rd handshake ACK
>       o AccECN uses all 8 x 3-bit combinations of (AE, CWR, ECE) as
>         the ACE field.
>
>
>>
>> ====
>>
>> 2. The requirement for AccECN servers to interpret all ECN codepoints 
>> other than 000, 011, and 111 in the initial SYN same as 111, i.e., 
>> requesting AccECN.
>>
>> The draft reads in Sec. 3.1.3:
>>
>>  If a TCP server that implements AccECN receives a SYN with the three
>>  TCP header flags (AE, CWR and ECE) set to any combination other than
>>  000, 011 or 111, it MUST negotiate the use of AccECN as if they had
>>  been set to 111.
>>
>> Is this really what it meant to say, or was the intention to say:
>>
>>  If a TCP server that implements AccECN receives a SYN with the three
>>  TCP header flags (AE, CWR and ECE) set to any combination other than
>>  000, 011 or 111 *and does not understand such a combination*, it MUST
>>  negotiate the use of AccECN as if they had been set to 111.
>>
>> If not, then this sounds like too restrictive as in this case any 
>> future alternative spec that uses any of the other flag combinations 
>> and also supports AccECN would unnecessarily need to update this spec 
>> to allow negotiating other than AccECN behaviour.
>
> [BB] 2.1) We had assumed that any future use of these combinations 
> would need to update the AccECN spec.
> But I think you're right that it would be useful to add your phrase, 
> to make the intent clearer.
>     [Nonetheless, IMO, it's actually useful (and hardly onerous) to 
> explicitly say when a new protocol updates another RFC].
>
>>
>> In addition, the crucial pieces from Appendix B explaining the 
>> availability of the unused five codepoints for (any) future use is 
>> useful to include in the Sec 3.1.3.
>
> [BB] 2.2) Good. In §3.1.3 we'll add a ref to the appendix.
>
>>
>> ====
>>
>> 3. Acking pure acks. Current text is on the correct track but badly 
>> incomplete.
>>
>> The feature of acking Pure Acks in AccECN is introduced kinda as a 
>> sidenote in Sec 3.2.2.5.1 when discussing potential side-effects of 
>> ECN-marked pure ACKs that may trigger further ACKs. This is not quite 
>> appropriate because this would be a fundamental change to TCP that 
>> does not ack pure Acks. And there is no normative spec that says taht 
>> Pure Acks (with CE) MAY/SHOULD/MUST be acked. This also involves 
>> numerous caveats that has not been fully recognized (see below).
>>
>> The draft says:
>>
>> "Therefore, a host in AccECN mode that is sending ECN-capable pure 
>> ACKs SHOULD add one of the following additional checks when it tests 
>> whether an incoming pure ACK is a duplicate:
>>
>> -If SACK has been negotatiated for the connection, but there is no
>>  SACK option on the incoming pure ACK, it is not a duplicate;"
>>
>> [MK]: Cite RFC 6675 which already states this. But the actual problem 
>> of using this rule is that there is no normative spec available how a 
>> SACK-enabled receiver is supposed to ack a Pure Ack. The draft makes 
>> an well-educated assumption but in fact an implementator cannot 
>> implement this without a normative specthat is cited here to tell how 
>> to ack.
>
> [BB] 3.1) I'll cite 6675.
> I've included Richard in the 'To:', 'cos he's the best person to 
> respond, being the one who developed these rules.
>
>>
>> "-If timestamps are in use, and the incoming pure ACK echoes a
>>   timestamp older than the oldest unacknowledged data, it is not a
>>   duplicate.
>>
>> In the unlikely event that neither SACK nor timestamps are in use, or 
>> if the implementation has opted not to include either of the above 
>> two checks, it SHOULD NOT send ECN-capable pure ACKs. If it does, it 
>> could lead to false detection of duplicate ACKs, causing spurious 
>> retransmission(s) with a resulting unnecessary reduction in 
>> congestion window; but only in certain circumstances."
>>
>> [MK]: False Fast Retransmits are not the only problem although it 
>> alone necessitates MUST NOT, not just SHOULD NOT. Each extra dupack 
>> is subject to trigger an extra data segment in a numerous heuristics 
>> that are based on the packet conservation principle and will confuse 
>> these algorithms, including Limited Transmit, Fast Recovery, PRR, etc.
>> For this very reason RFC 5681 prevents (MUST NOT) a data receiver 
>> from acking more than once per incoming data packet. Ackinf Pure ACKs 
>> was not known by that time but this rule extends to cover acking of 
>> Pure AKCs because the consequence is the same.
>
> [BB] 3.2) I believe all these potential problems primarily affect the 
> host's own connection. The reason for the 'SHOULD' was to allow 
> implementers (in what is already a corner case) to judge the trade off 
> themselves between losing the benefit of ECN-capable pure ACKs and 
> losing the benefit of other performance improving algorithms like 
> those you describe.
>
>>
>> In addition, acking Pure Acks will break F-RTO and potentially also 
>> other algos. Even with Timestamps we are not on the safe side as this 
>> would also break Eifel detection.
>
> [BB] 3.3) Pls explain to me (this isn't my area of expertise). Surely, 
> as the spec says, timestamps can be used to prevent emitting a pure 
> ACK that could be mistaken for a DupACK in the first place.
>
>>
>> So, I hesitate a lot to introduce any algo for acking Pure acks in a 
>> standards track spec as unmature.
>>
>> In general, I don't see any benefit for Acking Pure acks for ack 
>> congestion control purposes because such feedback has no effect until 
>> the sender send data again. That is, postponing the feedback until 
>> there is data again has the same effect when used for controlling ack 
>> rate. Moreover, I find draft-ietf-tcpm-ack-rate-request much beter 
>> suited for this purpose.
>>
>> For the case where CE on Pure ACKs are mixed with data from the same 
>> sender, it is not at all clear how the sender should respond, or 
>> whether the ack CC should be handled separate from data CC. IMO, 
>> these are clearly research questions, definitely not ready for 
>> standards track.
>
> [BB] 3.4) How to respond to feedback is intended to be outside the 
> scope of the AccECN draft. The text about ACKs of ACKs only became 
> necessary when we noticed the potential interaction between ECN++ and 
> the Increment-Triggered ACKs rule.
>
> ECN++ is experimental track, so we could shift all this ACKing ACKs 
> stuff into that draft. None of it applies when ECN++ is not being used.
> Although specific congestion responses are also outside the scope of 
> the ECN++ draft, for certain cases, it does often include requirements 
> for a certain minimum congestion response. So it would be better to 
> include anything about congestion responses in the experimental ECN++ 
> draft.
>
> Wherever this congestion response text ends up, let me try to write 
> down the scope of your question. Your question is confined to the case 
> where ECN-capable pure ACKs are used and neither SACK nor timestamps 
> are being used (if I'm right about timestamps above).

[BB2] You'll see I've also rewritten the text about ACKs of ACKs that is 
left in AccECN - to make it much clearer which end is doing what.

>
>
>>
>> ====
>>
>> 4. Updates 3168:
>>
>> Make it clear that this does not obsolete/replace nor does require 
>> anyone who may want to implement Classic ECN to implement the updates 
>> in this doc. That is, this draft updates RFC 3168 solely to allow 
>> implementing AccECN specified in this doc.
>
> [BB] 4.1) 'Updates' does not make existing implementations of RFC3168 
> ECN feedback obsolete. But 'updates' does mean that no-one would be 
> /expected/ to write a new implementation of RFC3168 ECN feedback any 
> more. Once AccECN is negotiated, it still supports the call-backs used 
> by classical congestion controls that need just 1 bit of feedback per RTT.
>
> Also, the draft requires an AccECN client to implement RFC3168 
> feedback as well as AccECN (see the 'MUST' in note 2 under Table 2). 
> But it does not /require/ an AccECN server to implement RFC3168. The 
> agreed wording about a server not implementing RFC3168 support is:
>     "This ... is unlikely to be desirable, but it is allowed as a 
> possibility, e.g. for minimal TCP implementations."

[BB2] GIven AccECN has the /ability/ to replace classic ECN (superset), 
we didn't want to /not/ say 'replace'. The trick that Gorry suggested 
was to make it clear that each host has the choice to replace classic 
ECN on a per-connection basis.

Please see my responses to Gorry on the ML about what changes have been 
made to address this point.
I have now fixed the remaining occurrence of replace for you in a 
similar way.

Here's a quick paste of one of the relevant responses to Gorry's points 
on this:
==================
>> This document specifies an update to the ECN feedback scheme of RFC 
>> 3168 that provides more accurate information and could be used by 
>> these and potentially other future TCP extensions.
>> - could we note this is an alternative rather than replacement:
>> This document specifies an update to the ECN feedback scheme of RFC 
>> 3168 that provides an alternative more accurate information and could 
>> be used by these and potentially other future TCP extensions.
>
> [BB3] Sorry, I overlooked this in the first pass.
> AccECN is definitely not an alternative (as explained in the sentence 
> "not a fork in the design of TCP"). It is a superset.
> I know you are trying to avoid any implication that AccECN obsoletes 
> RFC3168, but there was no such implication in this sentence as it 
> stood. That is captured by the well-understood word "update".
>
> Nonetheless, I would like to add the following important point to this 
> sentence, which doesn't seem to have been said anywhere in the Intro:
>     This document specifies an update to the ECN feedback scheme of 
> RFC 3168 that provides more accurate information and could be used by 
> these and potentially other future TCP extensions*, while still also 
> supporting the pre-existing TCP congestion controllers that use just 
> one feedback signal per round*.
>
> A later sentence makes it even clearer that RFC3168 feedback has not 
> gone away:
>     "AccECN feedback complements TCP's loss feedback and it can 
> coexist alongside 'classic' [RFC3168 
> <https://www.rfc-editor.org/info/rfc3168>] TCP/ECN feedback."
==================

Regards

Bob

>
>>
>>
>> Nits:
>>
>> "flag available for use by the AccECN experiment instead."
>>
>> - Experiment is leftover from the time when targetting Experimental?
>>   There are a few other such occurances. Might want to
>>   consider deleting.
>
> [BB] Alex Burr also pointed this out during WGLC. And I also removed 
> the description of MPTCP as experimental, now that it isn't any more.
>
>>
>> "RST packet"  -> "RST segment"
>> "SYN packett" -> "SYN segment"
>>
>> - there are a number of such intanses. If you want to use packet 
>> instead of segment, then it might be better to make this explicit 
>> early in the draft similar to RFC 3168 does:
>>
>>  "In a mild abuse of terminology, in this document we refer to `TCP
>>  packets' instead of `TCP segments'.
>
> [BB] I'll do that. Otherwise I'll have 162 occurrences of 'packet' to 
> check.
>
> Thank you again for your review.
>
>
>
> Bob
>
>>
>>
>> Hope this is helpful.
>>
>> Best regards,
>>
>> /Markku
>>
>> _______________________________________________
>> tcpm mailing list
>> tcpm@ietf.org
>> https://www.ietf.org/mailman/listinfo/tcpm
>
> -- 
> ________________________________________________________________
> Bob Briscoehttp://bobbriscoe.net/

-- 
________________________________________________________________
Bob Briscoehttp://bobbriscoe.net/