Re: [tcpm] CUBIC rfc8312bis / WGLC Issue 6 and proposed text

[Yuchung Cheng <[e-mail]>]

The latest text looks good to me.

On Thu, Jul 28, 2022, 3:29 PM Markku Kojo <kojo=
[e-mail]> wrote:

> Hi Vidhi,
>
> On Thu, 28 Jul 2022, Vidhi Goel wrote:
>
> >
> >
> > Hi Markku,
> > Can you review the PR? As I have adopted the change that you suggested,
> it might be an easy one. We would like to converge on
> > this soon.
>
> I'm fine with the text except the sentence that I spotted only after
> re-reading the text later and already commented on the wg mailing list:
>
>   "the mechanisms described in [RFC7661] can be used to mitigate this
>    issue as it would allow using a value between _cwnd_ and
>    _flight_size_ to calculate the new _ssthresh_ in Figure 5."
>
> AFAIK "using a value between _cwnd_ and _flight_size_ to calculate the new
> _ssthresh_" does not correctly capture the algo in RFC 7661. In the
> non-validated RFC 7661 uses
>
>   (Max(pipeACK,LossFlightSize))/2
>
> and adjusts it with R (=retransmitted data) in the end of recovery phase.
> And, in the validated phase RFC 7661 uses FlightSize/2.
>
> It would be good to have Gorry to review this piece of text as the text
> seems to have been modified from what Gorry originally wrote.
>
> Good to check that Yuchung also is fine with all the latest text.
>
> Thanks,
>
> /Markku
>
>
>
> >
> [png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABAQMAAAAl21bKAAAAA1BMVEUAAACnej3aAAAAAXRSTlMAQObYZgAAAApJREFUCNdjYAAAAAIAAeIhvDMAA
> > AAASUVORK5CYII=]
> > restrict use of cwnd directly on a congestion event by goelvidhi · Pull
> Request #148 · NTAP/rfc8312bis
> > github.com
> >
> >
> > Thanks,
> > Vidhi
> >
> >       On Jul 24, 2022, at 2:09 PM, Vidhi Goel <[e-mail]>
> wrote:
> >
> >       Hi Markku,
> > I have updated the PR with your suggested text to converge faster. Can
> you take a look
> > - https://github.com/NTAP/rfc8312bis/pull/148/
> >
> > Vidhi
> >
> >       On Jul 19, 2022, at 8:03 PM, Markku Kojo <kojo=
> [e-mail]> wrote:
> >
> > Hi Vidhi (Cc'ing also Gorry because he has better insight into RFC 7661),
> >
> > Thanks for the text. I'll reply here to keep the discussion on the wg
> list so that anyone interested can more
> > easily follow the discussion (jumping back and worth between ML and
> github may become intractable).
> >
> > First, I want to stress that I am not expecting nor insisting to stick
> with my proposed text (with necessary
> > changes to address the comments on the list) but it could possibly
> result in faster conclusion(?).
> >
> > Second, I'd like to highlight that my proposed additional text was
> intended just to bring up the caveats of simply
> > using cwnd and give advise that is in line with other standards track CC
> RFCs. It was not intended to specify any
> > algorithm (or a part of an algorithm).
> >
> > The new text in github IMO has the following issues:
> >
> > 1. It does not mention the caveat of letting cwnd to grow
> >   beyond rwnd which is a known implementation problem in
> >   the past (so it can easily be repeated by some new
> >   implementors) nor does it explain why simply using
> >   cwnd is incorrect.
> >
> > 2. Even though this draft is intended to be applicable
> >   with QUIC, it misses the pointer to RFC 9002 that
> >   articulates the corresponding requirement for QUIC.
> >
> > 3. It tries to create an ad-hoc algorithm in a single
> >   sentence that captures the RFC 7661 algorithm for
> >   those using cwnd. RFC 7661 is some 20 pages of which
> >   several pages are used to specify the algorithm.
> >   That is, the proposed text ignores several crucial
> >   actions of RFC 7661. And, my proposal was not to
> >   provide an algorithm for those using cwnd.
> >
> > 4. The sentence / rule "when flight_size is less than 1/2
> >   of the cwnd" is not capturing what was discussed earlier
> >   in github and now on the wg list, and it is also in
> >   conflict with RFC 9002 and effectively also with RFC
> >   5681. None of these effectively allows the sender not
> >   reacting to a congestion event but the "new" rule would
> >   allow it once the cwnd has grown more than 43% above the
> >   flightsize.
> >
> > Apologies for finding so many issues with the proposed text.
> >
> > Thanks,
> >
> > /Markku
> >
> > On Sun, 17 Jul 2022, Vidhi Goel wrote:
> >
> >       Thank you everyone for the discussion. Since some of the text was
> becoming duplicate, I moved the
> >       below text a
> >       few lines up and added a restriction for using cwnd directly. And
> the recommendation to use RFC 7661
> >       now
> >       applies to both approaches.
> >       Some implementations of CUBIC currently
> >       use *cwnd* ….
> >       Here is the PR - https://github.com/NTAP/rfc8312bis/pull/148
> >       I would really appreciate if folks can review it so that we can
> close this issue before meeting at
> >       IETF 114
> >       next week.
> >       Thanks,
> >       Vidhi
> >
> >            On Jul 17, 2022, at 8:33 AM, Yuchung Cheng <[e-mail]>
> wrote:
> >       Glad we agree on improving the text. I think Vidhi will come up
> with a good diff that captures our
> >       discussion.
> >       On Thu, Jul 14, 2022 at 4:47 PM Markku Kojo <kojo=
> [e-mail]> wrote:
> >            On Thu, 14 Jul 2022, Yuchung Cheng wrote:
> >
> >            > On Thu, Jul 14, 2022 at 4:12 PM Markku Kojo
> >            > <kojo=[e-mail]> wrote:
> >            >>
> >            >> Hi Yuchung,
> >            >>
> >            >> On Tue, 12 Jul 2022, Yuchung Cheng wrote:
> >            >>
> >            >>> AFAIK, the latest (and previous) Linux Cubic
> implementation does not follow
> >            >>> "The implementations that use _cwnd_ MUST use other
> measures to avoid
> >            >>> _cwnd_ from growing beyond the receive window"
> >            >>>
> >            >>> I don't see the need to add that check in Linux as the
> effective
> >            >>> window is always the min of cwnd and rwnd.
> >            >>
> >            >> That does not help. If cwnd is allowed to grow way beyond
> rwnd (more than
> >            >> 43% for CUBIC or more than 100% for Reno CC), a sender
> would not respond
> >            >> to an arriving congestion signal effectively at all (and
> may even
> >            >> increase the cwnd). That is against the very basic rule in
> congestion
> >            >> control principles we have.
> >            >>
> >            >>> On the other hand, Linux does restrict cwnd growth if
> flight size is
> >            >>> below cwnd but the
> >            >>> actual logic is more sophisticated than a "cwnd <
> inflight" check to
> >            >>> work w/ TSO chunking issue well.
> >            >>>
> https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp_output.c#L1881
> >            >>> (Neal cc'd here is the inventor of the advanced check)
> >            >>
> >            >> I'm not sure I follow your logic. Doesn't that effectively
> restrict cwnd
> >            >> from growing beyond rwnd? To my understanding with that
> check Linux
> >            >> applies "other measures to avoid _cwnd_ from growing
> beyond the receive
> >            >> window". It does not matter whether the conncetion is
> >            >> receiver-application limited via rwnd or
> sender-application limited. If
> >            >> rwnd limits flight size, cwnd will not grow more. Maybe I
> am missing
> >            >> something?
> >            > That's right. Limiting cwnd growth on flight size
> automatically
> >            > prevents cwnd goes far beyond rwnd for the congestion case
> you are
> >            > concerned about.
> >            >
> >            > Your suggested text "The implementations that use _cwnd_
> MUST use
> >            > other measures to avoid _cwnd_ from growing beyond the
> receive window"
> >            > is easily interpreted as a line of code "cwnd = min(cwnd,
> rwnd)"
> >
> >            That was not the intent. So, I think we agree.
> >
> >            > What I am saying is that this sentence is not necessary and
> overly
> >            > strict. Limiting cwnd growth based on inflight which is
> based on the
> >            > effective sending window (= min(cwnd, rwnd)) already
> prevents cwnd
> >            > bloats over rwnd.
> >
> >            Sure, I misinterepted your previous text ;)
> >
> >            > I don't see why Cubic needs to have more strict
> >            > rules than other C.C.s.
> >
> >            It does not have to be more strict rule. So, to my
> understanding we just
> >            need to tweak the text such that it is not misinterpreted the
> way you
> >            thought but still clearly involves the message that the
> sender needs to
> >            prevent cwnd to grow beyond rwnd.
> >
> >            Maybe(?):
> >
> >              "... The implementations that use _cwnd_ MUST use
> [other|alternative]
> >               measures to not allow _cwnd_ to grow when bytes in flight
> is
> >               smaller than cwnd_. That also effectively avoids _cwnd_
> from
> >               growing beyond the receive window. Such measures are
> important
> >               to prevent a CUBIC sender from using an arbitrarily ..."
> >
> >            Thanks,
> >
> >            /Markku
> >
> >            >>
> >            >> Thanks,
> >            >>
> >            >> /Markku
> >            >>
> >            >>> Just want to reflect a major implementation status.
> >            >>>
> >            >>>
> >            >>> On Tue, Jul 12, 2022 at 2:01 PM Vidhi Goel
> >            >>> <vidhi_goel=[e-mail]> wrote:
> >            >>>>
> >            >>>> Thank you Markku for proposing the text. Some of this is
> already covered in the latest
> >            draft but I can do some edits to your proposed text and
> create a PR.
> >            >>>> I spoke to other co-authors about this suggestion as
> well and we are mostly ok with it.
> >            >>>>
> >            >>>>
> >            >>>> Vidhi
> >            >>>>
> >            >>>>> On Jul 11, 2022, at 5:37 PM, Markku Kojo <kojo=
> [e-mail]> wrote:
> >            >>>>>
> >            >>>>> Hi all,
> >            >>>>>
> >            >>>>> I promised to propose some text to some of the
> remaining issues.
> >            >>>>> This thread starts the discussion on the issue 6 and
> proposes text to solve the issue:
> >            >>>>>
> >            >>>>> Issue 6) Flightsize:
> >            >>>>>
> >            >>>>>   The current text in Sec 4.6 w.r.t using FlightSize
> vs. cwnd for
> >            >>>>>   calculating multiplicative decrease is fine except
> that it does
> >            >>>>>   not quite correcly reflect what stacks that use cwnd
> instead of
> >            >>>>>   flightsize should do and actually do. AFAIK and what
> was
> >            >>>>>   discussed in github all stacks apply some sort of
> restrictions
> >            >>>>>   to not allow cwnd to grow beyond rwnd and to not use
> an
> >            >>>>>   arbitrarily high (old) cwnd value to calculate new
> cwnd
> >            >>>>>   when a congestion event occurs.
> >            >>>>>
> >            >>>>>
> >            >>>>> Current text in Sec 4.6::
> >            >>>>>
> >            >>>>>  Some implementations of CUBIC currently use _cwnd_
> instead
> >            >>>>>  of _flight_size_ when calculating a new _ssthresh_
> using Figure 5.
> >            >>>>>
> >            >>>>> Proposed new text:
> >            >>>>>
> >            >>>>>  Some implementations of CUBIC currently use _cwnd_
> instead
> >            >>>>>  of _flight_size_ when calculating a new _ssthresh_
> using Figure 5.
> >            >>>>>  The implementations that use _cwnd_ MUST use other
> measures to
> >            >>>>>  avoid _cwnd_ from growing beyond the receive window
> and to not
> >            >>>>>  allow _cwnd_ to grow when bytes in flight is smaller
> than
> >            >>>>>  _cwnd_. This prevents a CUBIC sender from using an
> arbitrarily
> >            >>>>>  high _cwnd_ value in calculating the new value for
> _ssthresh_
> >            >>>>>  and _cwnd_ when a congestion event is signalled, but
> it is not
> >            >>>>>  as robust as the mechanisms described in [RFC7661].
> >            >>>>>  [Many|Most|All] TCP implementations of CUBIC that use
> _cwnd_ apply
> >            >>>>>  such measures. Likewise, a QUIC sender that also uses
> congestion
> >            >>>>>  window to calculate a new value for the congestion
> window and
> >            >>>>>  slow-start threshold is required to apply similar
> mechanisms
> >            >>>>>  [RFC 9002].
> >            >>>>>
> >            >>>>>
> >            >>>>> Any comments and help in formulating the text are
> welcome.
> >            >>>>>
> >            >>>>> Need also some guidance from TCP implementations of
> CUBIC to finish up the second but
> >            last sentence.
> >            >>>>>
> >            >>>>> Thanks,
> >            >>>>>
> >            >>>>> /Markku
> >            >>>>>
> >            >>>>> _______________________________________________
> >            >>>>> tcpm mailing list
> >            >>>>> [e-mail]
> >            >>>>> https://www.ietf.org/mailman/listinfo/tcpm
> >            >>>>
> >            >>>> _______________________________________________
> >            >>>> tcpm mailing list
> >            >>>> [e-mail]
> >            >>>> https://www.ietf.org/mailman/listinfo/tcpm
> >            >>>
> >            >
> >
> > _______________________________________________
> > tcpm mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/tcpm
> >
> >
> >
> >_______________________________________________
> tcpm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/tcpm
>