Re: [tsvwg] A review of draft-ietf-tsvwg-l4s-arch-09

[Vidhi Goel <[e-mail]>]

Hi Bob,

Thanks for taking care of my comments so promptly. I am satisfied with all your responses.
I agree that referring to the Linux implementation for BBRv2 would be more informative wherever the draft talks about implementation.

Looking forward to read the updated draft.

Thanks,
Vidhi

> On Jun 22, 2021, at 2:15 PM, Bob Briscoe <[e-mail]> wrote:
> 
> Vidhi, 
> 
> Thank you for this constructive review.
> Inline tagged [BB]...
> 
> 
> On 22/06/2021 05:02, Vidhi Goel wrote:
>> Dear L4S authors,
>> 
>> I have a few comments on the architecture draft.
>> 
>> 1. Section 4, Protocols
>>  >> much more frequent signals--too often to require an equivalent
>>     excessive degree of drop from non-ECN flows;
>> I am a little confused by this statement. Is it saying that the frequent signaling in L4S is too often that it is equivalent to the “drop” signal in case of non-ECN flows? Or does the too often ECN signal requires an a certain degree of drop from non-ECN? 
>> I think it means the former but still bit a confusing to read.
>> 
>> Similar question about the second bullet, too soon for L4S here means it will result in the same effect as drop for non-ECN, right?
>>  >> immediately tracking every fluctuation of the queue--too soon
>>     to warrant dropping packets from non-ECN flows.
>> 
> 
> [BB] Yes, I admit I was trying to pack too much into the bullets. Does the following make better sense?:
> 
>    a.                                        ... responded to by hosts.
>        L4S needs networks and hosts to support a more fine-grained
>        meaning for each ECN signal that is less severe than a drop, so
>        that the L4S signals:
> 
>        *  can be much more frequent;
> 
>        *  can immediately track every fluctuation of the queue.
> 
>        A drop is both an impairment and a congestion signal, so while
>        ECN was equivalent to drop, it could not be used too frequently
>        or too immediately without non-ECN flows having to be subjected
>        to an equivalent degree of loss, and therefore excessive
>        impairment.  To enable L4S,...
> 
> 
>> 2. Section 4, Network
>>   >> Then the Classic
>>      AQM such as CoDel or PIE is applied to non-ECN or ECT(0) packets,
>>      while the shallow threshold is applied to ECT(1) packets, to give
>>      sub-millisecond average queue delay.
>> If the bottleneck with CoDel has both classic and L4S packets in the queue at the same time, 
>> then a “shallow threshold” for L4S can’t guarantee sub-ms queue delay, right?
> 
> [BB] Yes, we didn't make the modification clear. Is this clearer:
> 
>    b.  A scheduler with per-flow queues such as FQ-CoDel or FQ-PIE can
>        be used for L4S by simply modifying the classifiers in the
>        existing design.  For instance within each queue of an FQ-CoDel
>        system, as well as a CoDel AQM, there is typically also ECN
>        marking at an immediate (unsmoothed) shallow threshold to support
>        use in data centres (see Sec.5.2.7 of [RFC8290]).  This can be
>        modified so that the shallow threshold is solely applied to
>        ECT(1) packets.  Then if there is a flow of non-ECN or ECT(0)
>        packets in the per-flow-queue, the Classic AQM such as CoDel or
>        PIE is applied; while if there is a flow of ECT(1) packets in the
>        queue, the shallower threshold is applied to give sub-millisecond
>        average queue delay.  In addition, a mix of ECT(1) and ECT(0) or
>        not-ECT packets (e.g. due to a hash collision or a VPN) can be
>        classified into separate per-flow queues.
> 
> 
> 
>> 
>> 3. Section 4, Hosts
>>     >> Also the L4S ECN part of
>>     BBRv2 [I-D.cardwell-iccrg-bbr-congestion-control <https://datatracker.ietf.org/doc/html/draft-ietf-tsvwg-l4s-arch-09#ref-I-D.cardwell-iccrg-bbr-congestion-control>] is a scalable
>>     congestion control intended for the TCP and QUIC transports,
>>     amongst others.
>> 
>> The BBR draft has no mention of L4S ECN so far. Are we expecting an updated BBR draft and reference? 
>> There is another reference in Section 5.2. 
> 
> [BB] It is in the published implementation, but you're correct that the the draft is still at revision -00 from Jul 2017 and describes BBRv1 without L4S. For now I have referred to the Linux implementation when talking specifically about v2, and I have referred to the draft when just discussing generally how the delay-based part of BBR works.
> 
>> 
>> 4. Section 5.1
>>     >> The consequent smaller amplitude sawteeth
>>     fit between a very shallow marking threshold and an empty
>>     queue, so queue delay variation can be very low, without risk
>>     of under-utilization.
>> Is there a description of what "very shallow marking threshold” means in terms of time or anything else? 
>> I think it would be good to quantify this term, even if it is approximate.
> 
> [BB] I've changed to "very shallow (~1 ms) marking threshold"
> This quantification is discussed in much more detail in the AQM spec: draft-ietf-tsvwg-aqm-dual-queue-coupled
> 
>> 
>> 5. Section 5 has a lot of redundant info, perhaps it can be shortened. Some of it can be combined with Section 3 - terminology.
> 
> [BB] I think some of the redundancy is between the Architecture Components Section (4) and the Rationale section (5) as well.
> The rationale section is useful to avoid having to justify everything earlier on, so everything can be introduced more quickly before going into detail in a second pass. But sometimes you feel you have to justify something when you first introduce it. I'll try to be more ruthless.
> 
> I'm going to have to use old technology (print out the draft and write on it by hand), to try to see where there is duplication, and which duplicate to keep. 
> 
> 
>> 
>> 6. Section 5.2
>>    >> The L4S architecture does not require the IETF to commit to
>>     one approach over the other, because it supports both, so that
>>     the market can decide.
>> Minor suggestion: s/market/industry
> 
> [BB] I know that is technically more accurate, because not everything is for sale, but "Let the market decide" is a common idiom used across the IETF whether or not something is for sale, e.g. RFC5218, RFC6670. If you feel strongly, I can change it, but for now I have just pt 'market' in quotes.
> 
>> 
>> I would really appreciate if you could re-organize Section 5 as some of the content in that section was previously covered. 
>> If needed, I can work with you on this.
> 
> 
> [BB] Thanks. I'm starting work on that now. I'll be in touch if I need a second opinion on anything, and whatever, I'll run the diffs past you before posting.
> 
> And thank you for the review. Always useful to get new eyes on a draft to see things in plain sight that those of us familiar with a draft no longer notice.
> 
> Cheers
> 
> 
> Bob
> 
> 
> 
>> 
>> Thanks,
>> Vidhi
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
>> 
> 
> -- 
> ________________________________________________________________
> Bob Briscoe                               http://bobbriscoe.net/ <http://bobbriscoe.net/>