Re: [tcpm] [EXTERNAL] Re: A review of draft-ietf-tcpm-rack-10

[Yuchung Cheng <[e-mail]>]

On Mon, Sep 21, 2020 at 4:37 PM Praveen Balasubramanian
<pravb=[e-mail]> wrote:
>
> >> Segment.retransmitted is meant to reflect whether the most recent send is a retransmission or not
> The "most recent send" here is confusing. Why not just state that this reflects whether the segment was ever retransmitted?
We'll make the change based on your suggestions:

“Segment.retransmitted” is true if the segment has ever been
retransmitted. False otherwise.

Also RACK_detect_loss() can be further simplified by removing the
checks of Segment.retransmitted, as long as Segment.xmit_ts is
invalidated upon loss.

        “Segment.xmit_ts” is the time of the last transmission of a
data segment, including retransmissions, if any, with a clock
granularity specified in the Requirements section. A maximum value
INFINITE_TS indicates an invalid timestamp that represents that the
Segment is not currently in flight.

New RACK_detect_lost():
    timeout = 0
    RACK.reo_wnd = RACK_update_reo_wnd()

    For each segment, Segment, not acknowledged yet:
        // removed the old lines to check Segment.retransmitted
        //
        If RACK_sent_after(RACK.xmit_ts, RACK.end_seq,
                           Segment.xmit_ts, Segment.end_seq):
            remaining = Segment.xmit_ts + RACK.rtt +
                        RACK.reo_wnd - Now()
            If remaining <= 0:
                Segment.lost = TRUE
                Segment.xmit_ts = INFINITE_TS //<--- new line
            Else:
                timeout = max(remaining, timeout)
    Return timeout

So it's more clear and simpler :-)

>
> -----Original Message-----
> From: tcpm <[e-mail]> On Behalf Of Yuchung Cheng
> Sent: Monday, September 21, 2020 4:12 PM
> To: Yi Huang <huanyi=[e-mail]>
> Cc: Yuchung Cheng <ycheng=[e-mail]>; [e-mail]
> Subject: Re: [tcpm] [EXTERNAL] Re: A review of draft-ietf-tcpm-rack-10
>
> On Mon, Sep 21, 2020 at 3:18 PM Yi Huang <huanyi=[e-mail]> wrote:
> >
> > Thanks Yuchung, I responded inline.
> >
> > -----Original Message-----
> > From: tcpm <[e-mail]> On Behalf Of Yuchung Cheng
> > Sent: Monday, September 21, 2020 2:53 PM
> > To: Yi Huang <huanyi=[e-mail]>
> > Cc: [e-mail]
> > Subject: [EXTERNAL] Re: [tcpm] A review of draft-ietf-tcpm-rack-10
> >
> > On Sun, Sep 20, 2020 at 6:16 PM Yi Huang <huanyi=[e-mail]> wrote:
> > >>
> > >> Hi,
> > >>
> > >> I reviewed the latest version of RACK-TLP draft and have a few comments/questions.
> > >>
> > >> 1. In section 5.1, "Segment.retransmitted is true if it was retransmitted in the most recent transmission."
> > >> There are also pseudo-code explaining how retransmitted flag is set. AFAICT, retransmitted is set if the segment is ever retransmitted since the flag is never reset. Is this variable supposed to mean ever retransmitted?
> > >Thanks for catching that. Segment.retransmitted MUST be reset upon a
> > >(re)transmission so it does not ever-retransmitted (i.e. it's only
> > >meant for the very last transmission)
> >
> > > RACK_transmit_data(Segment):
> > >           Segment.xmit_ts = Now()
> > >           Segment.lost = FALSE
> > >           Segment.retransmitted = FALSE
> >
> > >   RACK_retransmit_data(Segment):
> > >           RACK_transmit_data(Segment)
> > >           Segment.retransmitted = TRUE
> >
> > Sorry, I still don't quite get it. RACK_transmit_data is for previously unsent data (aka new data) right? If so, it will be only called once for a given segment. Then, RACK_retransmit_data only sets it to TRUE. So where do we reset it? Looks like we still just initialize it to FALSE and set it to TRUE upon retransmission.
>
> Apologize for this confusion. I now see better how the text and code confuse you. Segment.retransmitted is meant to reflect whether the most recent send is a retransmission or not. But since a segment can be (only) retransmitted N times consecutively, the flag is sticky throughout once set. To avoid future confusion, how about having two separate routines.
>
> /* Send newly unsent data */
> RACK_transmit_new_data(Segment):
>            Segment.xmit_ts = Now()
>            Segment.lost = FALSE
>            Segment.retransmitted = FALSE
>
> /* Retransmit previously sent data */
> RACK_retransmit_data(Segment)
>           Segment.xmit_ts = Now()
>           Segment.lost = FALSE
>           Segment.retransmitted = TRUE
>
>
> >
> > >>
> > >> 2. In section 6.2, p14,
> > >> "   If the sender has not yet observed any reordering based on the
> > >>    previous step, then RACK prioritizes quick loss recovery by using
> > >>    setting RACK.reo_wnd to 0 when the number of SACKed segments exceeds
> > >>    DupThresh, or during loss recovery.
> > >>
> > >>    Aside from those special conditions, RACK starts with a conservative
> > >>    reordering window of RACK.min_RTT/4.  This value was chosen because
> > >>    Linux TCP used the same factor in its implementation to delay Early
> > >>    Retransmit [RFC5827] to reduce spurious loss detections in the
> > >>    presence of reordering, and experience showed this worked reasonably
> > >>    well [DMCG11]."
> > >>
> > >> If I understand this correctly, if no reordering observed, TCP should still enter loss recovery when DupACK >= DupThresh (RFC6675). If reordering has been seen, this RFC6675 rule is not honored and TCP only enters loss recovery when RACK detects loss? I think it would be great to clarify this in the spec.
> >
> > > Your understanding is correct. will this text revision improves clarity"
> > > ""
> > > If no reordering has been observed based on the previous step, the sender enters Fast Recovery when the number of SACKed segments exceeds DupThresh (similar to RFC6675). The RACK.reo_wnd is set to 0 both upon entering and during the loss recovery.
> >
> > > Otherwise if some reordering has been observed, RACK does not honor
> > RFC6675 rule and starts with a conservative RACK.reo_wnd of RACK.min_RTT/4. This .... reasonably well [DMCG11].
> > > ""
> >
> > Yeah, that's pretty clear now.
> >
> > > "   If the sender has not yet observed any reordering based on the
> > >    previous step, then RACK prioritizes quick loss recovery by using
> > >    setting RACK.reo_wnd to 0 when the number of SACKed segments exceeds
> > >    DupThresh, or during loss recovery.
> > >
> > >    Aside from those special conditions, RACK starts with a conservative
> > >    reordering window of RACK.min_RTT/4.  This value was chosen because
> > >    Linux TCP used the same factor in its implementation to delay Early
> > >    Retransmit [RFC5827] to reduce spurious loss detections in the
> > >    presence of reordering, and experience showed this worked reasonably
> > >    well [DMCG11]."
> > >
> > > 3. [NIT] In section 8, " Zero window probe (ZWP) timer" -> "Zero Window Probe (ZWP) timer".
> > will fix thanks
> > >
> > > Thanks,
> > >
> > > Yi
> > >
> > > _______________________________________________
> > > tcpm mailing list
> > > [e-mail]
> > > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> > > ietf.org%2Fmailman%2Flistinfo%2Ftcpm&amp;data=02%7C01%7Chuanyi%40mic
> > > ro
> > > soft.com%7C71748af3b9ae4232346008d85e78d063%7C72f988bf86f141af91ab2d
> > > 7c
> > > d011db47%7C1%7C0%7C637363220297828477&amp;sdata=eHYh0FXRGNS8T2LFq8sz
> > > gH
> > > x43Hsfwlrog%2BZxQwJyiq0%3D&amp;reserved=0
> >
> > _______________________________________________
> > tcpm mailing list
> > [e-mail]
> > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> > ietf.org%2Fmailman%2Flistinfo%2Ftcpm&amp;data=02%7C01%7Cpravb%40micros
> > oft.com%7C4cc0adb558a64fedcfac08d85e83dca9%7C72f988bf86f141af91ab2d7cd
> > 011db47%7C1%7C0%7C637363267745119455&amp;sdata=GdXrtwnYavfLpVIYqrOTrDx
> > U%2FOrpC7ayXhtZSuzM%2B2U%3D&amp;reserved=0
> >
> > _______________________________________________
> > tcpm mailing list
> > [e-mail]
> > https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> > ietf.org%2Fmailman%2Flistinfo%2Ftcpm&amp;data=02%7C01%7Cpravb%40micros
> > oft.com%7C4cc0adb558a64fedcfac08d85e83dca9%7C72f988bf86f141af91ab2d7cd
> > 011db47%7C1%7C0%7C637363267745119455&amp;sdata=GdXrtwnYavfLpVIYqrOTrDx
> > U%2FOrpC7ayXhtZSuzM%2B2U%3D&amp;reserved=0
>
> _______________________________________________
> tcpm mailing list
> [e-mail]
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ietf.org%2Fmailman%2Flistinfo%2Ftcpm&amp;data=02%7C01%7Cpravb%40microsoft.com%7C4cc0adb558a64fedcfac08d85e83dca9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637363267745119455&amp;sdata=GdXrtwnYavfLpVIYqrOTrDxU%2FOrpC7ayXhtZSuzM%2B2U%3D&amp;reserved=0