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

[Neal Cardwell <[e-mail]>]

Thanks, Bob. All of your responses/updates sound good to me as well, for
what it's worth.

Best,
neal


On Tue, Jun 22, 2021 at 11:05 PM Vidhi Goel <vidhi_goel=
[e-mail]> wrote:

>
> 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/
>
>
>