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

[Yoshifumi Nishida <[e-mail]>]

Hi Neal,

On Fri, Mar 22, 2024 at 2:36 PM Neal Cardwell <[e-mail]> wrote:

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

Got it. Thanks for the explanation. I am personally fine with it although I
would like to see opinions from other folks.
BTW,  I might prefer to say "(bytes newly cumulatively acknowledged if
there's any)" as this looks rare to me. Also, I think we'll need to update
the definition of RecoverFS in Section 5 if we update this.
--
Yoshi