netdev - RE: [PATCH V3 net-next] TLP: Don't reschedule PTO when there's one outstanding TLP retransmission

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 29 Jul 2017 01:39:01 +0000
From:   maowenan <maowenan@...wei.com>
To:     Neal Cardwell <ncardwell@...gle.com>
CC:     Netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Yuchung Cheng <ycheng@...gle.com>,
        Nandita Dukkipati <nanditad@...gle.com>,
        "weiyongjun (A)" <weiyongjun1@...wei.com>,
        Chenweilong <chenweilong@...wei.com>,
        "Wangkefeng (Kevin)" <wangkefeng.wang@...wei.com>
Subject: RE: [PATCH V3 net-next] TLP: Don't reschedule PTO when there's one
 outstanding TLP retransmission



> -----Original Message-----
> From: Neal Cardwell [mailto:ncardwell@...gle.com]
> Sent: Saturday, July 29, 2017 6:48 AM
> To: maowenan
> Cc: Netdev; David Miller; Yuchung Cheng; Nandita Dukkipati; weiyongjun (A);
> Chenweilong; Wangkefeng (Kevin)
> Subject: Re: [PATCH V3 net-next] TLP: Don't reschedule PTO when there's one
> outstanding TLP retransmission
> 
> On Thu, Jul 27, 2017 at 8:08 AM, Mao Wenan <maowenan@...wei.com>
> wrote:
> > If there is one TLP probe went out(TLP use the write_queue_tail packet
> > as TLP probe, we assume this first TLP probe named A), and this TLP
> > probe was not acked by receive side.
> >
> > Then the transmit side sent the next two packetes out(named B,C), but
> > unfortunately these two packets are also not acked by receive side.
> >
> > And then there is one data packet with ack_seq A arrive at transmit
> > side, in tcp_ack() will call tcp_schedule_loss_probe() to rearm PTO,
> > the handler tcp_send_loss_probe() is to check
> > if(tp->tlp_high_seq) then go to rearm_timer(because there is one
> > outstanding TLP named A), so the new TLP probe can't be sent out and
> > it needs to rearm the RTO timer(timeout is relative to the transmit
> > time of the write queue head).
> >
> > After that, there is another data packet with ack_seq A is received,
> > if the tlp_time_stamp is greater than rto_time_stamp, it will reset
> > the TLP timeout, which is before previous RTO timeout, so PTO is rearm
> > and previous RTO is cleared. Because there is no retransmission packet
> > was sent or no TLP sack receive,
> > tp->tlp_high_seq can't be reset to zero and the next TLP probe also
> > can't be sent out, so there is no way(or very long time) to retransmit
> > the lost packet.
> >
> > This fix is to check(tp->tlp_high_seq) in tcp_schedule_loss_probe()
> > when TLP PTO is after RTO, It is not needed to reschedule PTO when
> > there is one outstanding TLP retransmission, so if the TLP A is lost
> > RTO can retransmit lost packet, then tp->tlp_high_seq will be set to
> > 0, and TLP will go to the normal work process.
> >
> > v1->v2
> >         refine some words of code and patch comments.
> > v2->v3
> >         delete senseless "{" and "}" in if clause.
> >
> > Signed-off-by: Mao Wenan <maowenan@...wei.com>
> 
> Thanks for posting this patch with a detailed problem description, as well as a
> trace in the thread for v1 of the patch. This was very helpful!
> 
> Thinking about the problem you describe, and looking at the trace, AFAICT I
> don't think this is the patch we want.
> 
> We can still have this problem of improperly/repeatedly rescheduling a PTO
> even when the TLPs are new data. When the TLPs are new data
> tp->tlp_high_seq is not set, and so the patch above will not help.
[Mao Wenan]ok, We have reproduced this issue with packetdrill yesterday, 
there is no the same issue when TLP send new data packet, RTO will be fired and 
retransmit packet.
> 
> I think the broader problem is hinted at in this part of your commit
> description:
> 
> > After that, there is another data packet with ack_seq A is received,
> > if the tlp_time_stamp is greater than rto_time_stamp, it will reset
> > the TLP timeout
> 
> The broader problem here is that an incoming data packet (with no new
> ACK/SACK info) affected the TLP for our outbound data. That is a problem
> because such incoming data can cause us to delay the TLP when there is no
> reason to.
> 
> I think this is basically the same as the TLP issue from the "TCP fast retransmit
> issues" thread on netdev from July 26. Our TCP team at Google has a proposed
> fix for this more general issue that we have tested and reviewed. I will post a
> quick summary of the proposed patch in the "TCP fast retransmit issues"
> thread. Once the patch has undergone a little more testing we will send it to
> the list, hopefully next week.
[Mao Wenan]OK.
> 
> Thanks!
> neal