Re: [tcpm] I-D Action: draft-ietf-tcpm-prr-rfc6937bis-06.txt

[Neal Cardwell <[e-mail]>]

On Thu, Mar 21, 2024 at 6:30 PM Yoshifumi Nishida <[e-mail]>
wrote:

> Hi Neal,
> On Wed, Mar 20, 2024 at 1:32 PM Neal Cardwell <[e-mail]>
> wrote:
>
>>
>>
>> On Wed, Mar 20, 2024 at 3:29 PM Yoshifumi Nishida <[e-mail]>
>> wrote:
>>
>>> Hi Neal,
>>>
>>> On Wed, Mar 20, 2024 at 6:55 AM Neal Cardwell <[e-mail]>
>>> wrote:
>>>
>>>>
>>>> On Wed, Mar 20, 2024 at 3:07 AM Yoshifumi Nishida <[e-mail]>
>>>> wrote:
>>>>
>>>>>
>>>>> On Mon, Mar 18, 2024 at 8:13 PM Neal Cardwell <ncardwell=
>>>>> [e-mail]> wrote:
>>>>>
>>>>>>
>>>>>> But your point still stands, and you raise a great point: simply
>>>>>> initializing RecoverFS to "pipe" is not safe, because packets that were
>>>>>> marked lost and removed from pipe may actually have been merely reordered.
>>>>>> So if those packets are delivered later, they will increase the numerator
>>>>>> of prr_delivered / RecoverFS without increasing the denominator, thus
>>>>>> leading to a result above 1.0, and thus potentially leading to a target for
>>>>>> Sent_so_far that is above ssthresh, causing the algorithm to erroneously
>>>>>> exceed ssthresh.
>>>>>>
>>>>>
>>>>> Hmm. I have a native question here. If packets were merely reordered,
>>>>> isn't it ok for cwnd to be bigger than ssthresh?
>>>>>
>>>>
>>>> Yes, if packets were merely reordered and none were lost, then I agree
>>>> it would be OK for cwnd to be bigger than ssthresh. And in fact I would
>>>> argue that cwnd should be reverted back to its value before fast recovery.
>>>> And this is in fact what Linux TCP would do, using loss recovery "undo"
>>>> mechanisms based on TCP timestamps or DSACKs.
>>>>
>>>> However, in the kind of scenario Markku described, there was not merely
>>>> reordering, but also real packet loss: "1 packet is lost (P1), and 24
>>>> packets are delayed (packets P2..P25)". In the traditional loss-based
>>>> Reno/CUBIC paradigm, any non-zero amount of real packet loss in fast
>>>> recovery should result in the same multiplicative decrease in cwnd,
>>>> regardless of the combination of reordering and loss. We could argue about
>>>> whether that approach is the best approach (BBR, for example, takes a
>>>> different approach), but that is a very different discussion. :-) For now
>>>> AFAICT we are focused on PRR's faithful enactment of the congestion control
>>>> algorithms decision to reduce cwnd toward ssthresh when there is any
>>>> non-zero amount of real packet loss in fast recovery.
>>>>
>>>
>>> Got it. But, I just would like to clarify whether we are discussing the
>>> inflation of sndcnt during the recovery process or cwnd after the exit of
>>> recovery.
>>>
>>
>> Good point. We are talking about inflation of sndcnt during the recovery
>> process.
>>
>>
>>> Because I'm personally not very keen to address miscalculation of lost
>>> packets due to reordering during the recovery process as it seems to be
>>> tricky.
>>>
>>
>> It is tricky, but I think it is feasible to address. What do folks think
>> about my suggestion from above in this thread:
>>
>>   existing text:
>>      pipe = (RFC 6675 pipe algorithm)
>>      RecoverFS = pipe              // RFC 6675 pipe before recovery
>>
>>   proposed new text:
>>      RecoverFS = snd.nxt - snd.una + (bytes newly cumulatively
>> acknowledged)
>>
>
> Hmm. Sorry. I'm not very sure about the difference between snd.nxt -
> snd.una and snd.nxt - snd.una + (bytes newly cumulatively acknowledged)
> Could you elaborate a bit? I thought we don't have data which are
> cumulatively acked in case of reordering.
>

The  addition of (bytes newly cumulatively acknowledged) is unrelated to
reordering. It just seems necessary AFAICT to avoid another way for the
target number of packets to send to be greater than ssthresh.

As I noted above, I'm proposing here to add in  (bytes newly cumulatively
acknowledged) because in a (probably rare) corner case where the ACK that
initiates fast recovery advances snd.una, then the   (bytes newly
cumulatively acknowledged)  on that first ACK will be non-zero, and will be
added into prr_delivered. So if RecoverFS does not include (bytes newly
cumulatively acknowledged) in such cases, then we could again trigger the
kind of scenario Markku raised where prr_delivered > RecoverFS. In such a
case that could trigger the connection to send more than ssthresh bytes in
flight.

However, I think it won't be good if it's propagated to ater the recovery.
>>> But, don't we reset cwnd to ssthresh at the end of recovery?
>>>
>>
>> Yes, Linux TCP does reset cwnd to ssthresh at the end of recovery. And we
>> discussed putting that in the draft last year in the
>> thread "draft-ietf-tcpm-prr-rfc6937bis-03: set cwnd to ssthresh exiting
>> fast recovery?". But it looks to me from reviewing the draft that we never
>> got around to inserting that in the draft. I will do that now, and post the
>> proposal in that thread for discussion.
>>
>
> Thanks! I believe that reflects the consensus of the WG.
>

Great!

thanks,
neal



> --
> Yoshi
>
>>