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

[Vidhi Goel <[e-mail]>]

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.

https://github.com/NTAP/rfc8312bis/pull/148/" dir="ltr" width="300">

https://opengraph.githubassets.com/45c2ba21a1688cb9568bafb2cd2eedc1602e83033597754e1d95b2f32a76d764/NTAP/rfc8312bis/pull/148" class="lp-rich-link-mediaImage" width="300" height="150">

https://github.com/NTAP/rfc8312bis/pull/148/">restrict use of cwnd directly on a congestion event by goelvidhi · Pull Request #148 · NTAP/rfc8312bis https://github.com/NTAP/rfc8312bis/pull/148/">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/" rel="nofollow">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