Re: [tsvwg] New rev of docsis-q-protection draft related to L4S

[Bob Briscoe <[e-mail]>]

Gorry,

This has gone to the RFC Editor, but Eliot has asked me to let him know 
pronto if we want changes. There are some.
I'm off to bed now, so I'll put these into effect in the UK morning. 
Hopefully this will give you time to come back on anything if you need to.

See bold text for whether there's a change or not, tagged [BB] inline.

On 12/05/2022 14:34, Gorry Fairhurst wrote:
> On 11/05/2022 22:37, Bob Briscoe wrote:
>> tsvwg,
>>
>> This document is adjacent to the L4S work in TSVWG.
>> Apologies, another rev with 3 more nits fixed:
>> Pls see diff here if interested:
>> https://datatracker.ietf.org/doc/draft-briscoe-docsis-q-protection/history/
>> (A missing abbreviation expansion, excess white space removed and a 
>> full-stop where a comma was intended.)
>>
>>
>> Bob
>
> Thanks again for writing this!!
>
> I did previously offer comments in one of the earlier review stages, 
> so sorry to give a few more minor comments at this stage, but I hope 
> these comments relate only to document words - not the mechanism:
>
> 1. Would ECN benefit from one or more REF in the RFC series when first 
> used?
> /explicit congestion notifications (ECN)/... e.g. RFC 8087 and RFC 3168?
>

[BB] I didn't want to refer to 3168 because that would have been the 
wrong ref for the first use of ECN, which happens to be about ECN for 
low latency:

    Low queuing delay depends on hosts sending their data smoothly,
    either at low rate or responding to explicit congestion notifications
    (ECN).


The normatives refs for ECN later in the intro of this doc are:
* RFC8311, which updates RFC3168,
* and the ecn-l4s-id draft, which defines the L4S ECN codepoint.
I believe it is normal to refer to the latest RFC, and referring to the 
one that was updated isn't common practice.

So I guess I should add these two after the first use as well. *I'll do 
that.*

I did refer to RFC3168 informatively when talking about the ECN field, 
rather than a specific ECN codepoint. I wasn't sure whether that should 
have been a normative ref. But I guess RFC8311 didn't update the 
definition of the IP-ECN field as a whole, so the normative ref for the 
whole field is still RFC3168. *I'll move 3168 from informative to 
normative refs.*

>
> 2. /TCP that needs about a round trip of buffering to fully utilize 
> the link/
> … I am not sure which “link” in this particular sentence - is this the 
> “bottleneck link along the Internet path”?
> - or do you mean something more specific about the DOCSIS link?
>

[BB] The subject of this full sentence is the Classic queue:
   "The Classic queue is only necessary for
    traffic such as traditional (Reno/Cubic) TCP that needs about a round
    trip of buffering to fully utilize the link"

So, I think it's pretty unlikely anyone would think that this means a 
link other than the link that the queue is feeding.
If, at certain times, this link is not the bottleneck, a queue won't 
build here. But the link still needs that amount of buffer DRAM for when 
it is the bottleneck.

This doc explains the aspects of the wider AQM and DualQ technology that 
the reader needs to know to understand QProt. But I think context like 
an operator's strategy for which links to deploy AQMs in the first place 
wouldn't be expected to be found in the present document. The full para 
starts with a ref to the LLD overview, which is where you'd expect to 
find issues like which links are likely to be the bottlenecks. 
Specifically, it says:

    The “Queuing” delay is mainly caused by the current TCP protocol and
    its variants. Applications today that need to seek out as much
    bandwidth as possible use a transport protocol like TCP (or the
    TCP-replacement known as QUIC), which uses a “congestion control”
    algorithm (such as Reno, Cubic, or BBR) to adjust to the available
    bandwidth at the bottleneck link through the network. Typically, this
    will be the last mile link—the DOCSIS link for cable customers—where
    the bandwidth available for each application often varies rapidly
    as the activity of all the devices in the household varies.

So, in this instance, *I don't think there's a case for adding 
explanation* that it would only use the buffer for queuing if it was the 
path bottleneck.

>
> 3. Should the formal definition of ECN in section 1.2 also have a 
> reference to an RFC in thius case at least RFC 3168?
>

[BB] One or two of the terminology items define what a term means. 
Others (CM, CMTS, DOCSIS, ECN) just expand the abbreviation without a 
definition or reference. References could have been added, but instead 
references have been given in the earlier part of the doc where there is 
an explanation of these items. *I don't plan to change this* - it's just 
the way the style of this document went.

>
> 4. The phrase: /uses to ECN-mark packets (Classic or coupled queuing 
> is not relevant)/
> - I was unsure how to read this, could it be that something more lie 
> this was intended to show that the choice is not relevant, rather than 
> the mechanisms being irrelevant:
> /uses to ECN-mark packets (both Classic and
>    coupled queuing are relevant)/
>

[BB] This should have said "Classic or coupled marking is not relevant." 
I'll change it. The full sentence is:

    The scoring
    algorithm uses the same internal variable, probNative, that the AQM
    for the low latency queue uses to ECN-mark packets (Classic or
    coupled queuing is not relevant).

The doc hasn't introduced what coupled marking is. So *I'll change the 
parenthesis to*:
"(The other two forms of marking, Classic and coupled, are driven by 
Classic traffic and therefore not relevant to protection of the LL queue)"


>
> 5. I stumbled over this, as being close to IETF guidance, and wonder if
> /This is because it is legitimate for
>    capacity-seeking flows to induce a continuous low level of congestion
>    in order to track available capacity./
> could be said as something more like:
> /This is because the DOCSIS QProt algorithm regards it as legitimate for
>    capacity-seeking flows to induce a continuous low level of congestion
>    in order to track available capacity./
>

[BB] Eh? It has been intrinsic to the Internet architecture for 
capacity-seeking flows to induce a continuous low level of congestion 
since Van Jacobson introduced TCP congestion avoidance in 1988. Or is 
this a question of the definition of 'congestion'? Perhaps you are 
thinking 'congestion' means excessive congestion? AFAI'mC, in a doc 
about latency, any amount of queuing no matter how small is congestion.

Whatever, to avoid any implication that there is an IETF rule about 
this, *I'll replace* legitimate with 'unavoidable'. The whole sentence 
will then be:
/This is because it is unavoidable for
    capacity-seeking flows to induce a continuous low level of congestion
    as they track available capacity./



Bob

>
> Best wishes,
>
> Gorry
>
>
>>
>> On 10/05/2022 20:48, Bob Briscoe wrote:
>>> tsvwg,
>>>
>>> This document is adjacent to the L4S work in TSVWG.
>>> A new rev with a couple of minor edits has just been posted.
>>> See links and explanation of changes below, if interested.
>>>
>>>
>>> Bob
>>>
>>> -------- Forwarded Message --------
>>> Subject: 	Re: Martin Duke's Yes on 
>>> conflict-review-briscoe-docsis-q-protection-00: (with COMMENT)
>>> Date: 	Tue, 10 May 2022 20:45:21 +0100
>>> From: 	Bob Briscoe <[e-mail]>
>>> To: 	Martin Duke <[e-mail]>, Eliot Lear 
>>> <[e-mail]>
>>> CC: 	[e-mail]
>>>
>>>
>>>
>>> Martin, Eliot,
>>>
>>> You may have seen the text I proposed to resolve your comment below 
>>> requesting a more understandable definition of congestion-rate.
>>> At the request of Eliot, as ISE, I've now posted a new rev 
>>> containing that text.
>>> https://datatracker.ietf.org/doc/html/draft-briscoe-docsis-q-protection-04
>>>
>>> The diff is here:
>>> https://www.ietf.org/rfcdiff?url2=draft-briscoe-docsis-q-protection-04.txt
>>> I also corrected a nit I noticed in the abstract (I'd added a 4th 
>>> item to the list in the last sentence without moving the 'and' 
>>> before the 3rd item).
>>>
>>>
>>> Bob
>>>
>>> On 08/04/2022 22:05, Martin Duke via Datatracker wrote:
>>>> The document, along with other ballot positions, can be found here:
>>>> https://datatracker.ietf.org/doc/conflict-review-briscoe-docsis-q-protection/
>>>>
>>>> ----------------------------------------------------------------------
>>>> COMMENT:
>>>> ----------------------------------------------------------------------
>>>>
>>>> This document is adjacent to the L4S work in TSVWG.
>>>>
>>>> One nit:
>>>> In (1.2), a definition refers to "marking all the packet's bytes." 
>>>> I'm not sure
>>>> what the authors mean; perhaps s/bytes/ECN bits?
>>>>
>>>> Reviewing the pseudocode in detail was not necessary for the 
>>>> conflict review,
>>>> and I did not do so.
>>>>
>>>>
>>>>
>>>
>>> -- 
>>> ________________________________________________________________
>>> Bob Briscoehttp://bobbriscoe.net/
>>>
>>
>> -- 
>> ________________________________________________________________
>> Bob Briscoehttp://bobbriscoe.net/
>
>

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