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

[Markku Kojo <[e-mail]>]

Hi,

just wanted to add one question: I'm not sure if the following piece of 
text correctly characterizes RFC 7661 behaviour (Gorry?):

  "it would allow using a value between _cwnd_ and
    _flight_size_ to calculate the new _ssthresh_"

Thanks,

/Markku

On Wed, 20 Jul 2022, Markku Kojo 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
>>       >>>
>>       >
>> 
>> 
>> 
>