Message ID | 1299894051-13820-1-git-send-email-ycheng@google.com |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
Hi, On Saturday 12 March 2011, Yuchung Cheng wrote: > In the current undo logic, cwnd is moderated after it was restored > to the value prior entering fast-recovery. It was moderated first > in tcp_try_undo_recovery then again in tcp_complete_cwr. > > Since the undo indicates recovery was false, these moderations > are not necessary. If the undo is triggered when most of the > outstanding data have been acknowledged, the (restored) cwnd is > falsely pulled down to a small value. > > This patch removes these cwnd moderations if cwnd is undone during > the fast-recovery. The moderation is in place to avoid gigantic segment bursts, which could cause unnecessary pressure on buffers. In my eyes it's already suboptimal that the moderation is weakened in the presence of (detected) reordering, let alone removing it completely. More importantly, the prior ssthresh is restored and not affected by moderation. This means, if moderation reduces cwnd to a small value, then cwnd < ssthresh and TCP will quickly slow-start back to the previous state, without sending a big burst of segments. Also, you intended to remove cwnd moderation only from an undo during recovery, but I think your patch also removes cwnd moderation when the undo is caused by D-SACK, i.e. most likely after recovery already ended. Carsten
On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > The moderation is in place to avoid gigantic segment bursts, which could cause > unnecessary pressure on buffers. In my eyes it's already suboptimal that the > moderation is weakened in the presence of (detected) reordering, let alone > removing it completely. In the presence of reordering, cwnd is already moderated in Disorder state before entering the (false) recovery. > > More importantly, the prior ssthresh is restored and not affected by > moderation. This means, if moderation reduces cwnd to a small value, then cwnd > < ssthresh and TCP will quickly slow-start back to the previous state, without > sending a big burst of segments. > > Also, you intended to remove cwnd moderation only from an undo during > recovery, but I think your patch also removes cwnd moderation when the undo is > caused by D-SACK, i.e. most likely after recovery already ended. Thanks. I will update my patch description. But the same principle applies that cwnd should not be moderated on false events. Whether it should be moderated on reordering or other events is another (complex) design issue. But this patch does not touch that. Yuchung -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Mar 14, 2011 at 3:10 PM, Yuchung Cheng <ycheng@google.com> wrote: > On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote: >> The moderation is in place to avoid gigantic segment bursts, which could cause >> unnecessary pressure on buffers. In my eyes it's already suboptimal that the >> moderation is weakened in the presence of (detected) reordering, let alone >> removing it completely. > > In the presence of reordering, cwnd is already moderated in Disorder > state before > entering the (false) recovery. I've always been somewhat skeptical of the usefulness of cwnd moderation. First, I don't know that its behavior is well defined. When *should* tcp_moderate_cwnd() actually be called, and why? Second, I've never liked the idea in general. Reducing cwnd has an effect lasting many RTTs, so reducing it in response to a transient event like reordering seems dubious. And it does not address many causes of bursts, such as ack compression or stretch acks. -John -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Monday 14 March 2011, Yuchung Cheng wrote: > On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > > The moderation is in place to avoid gigantic segment bursts, which could > > cause unnecessary pressure on buffers. In my eyes it's already > > suboptimal that the moderation is weakened in the presence of (detected) > > reordering, let alone removing it completely. > > In the presence of reordering, cwnd is already moderated in Disorder > state before > entering the (false) recovery. Sure, cwnd moderation to in_flight + 1 segment is applied in disorder state, because this is implementing a form of extended limited transmit. Nevertheless, after a reordering event that caused a spurious fast retransmit, there can be an undo of congestion state changes (either after recovery or interrupting recovery, depending on the options enabled in the connection). I just wanted to point out, that the moderation step happening upon an undo may allow a larger burst, if a previous reordering event was detected and caused tp->reordering to be increased. > > More importantly, the prior ssthresh is restored and not affected by > > moderation. This means, if moderation reduces cwnd to a small value, then > > cwnd < ssthresh and TCP will quickly slow-start back to the previous > > state, without sending a big burst of segments. This is actually the more important point, because it means that the moderation does not negate the effects of the undo operation, as suggested by your patch-description. > > Also, you intended to remove cwnd moderation only from an undo during > > recovery, but I think your patch also removes cwnd moderation when the > > undo is caused by D-SACK, i.e. most likely after recovery already ended. > > Thanks. I will update my patch description. But the same principle > applies that cwnd > should not be moderated on false events. Whether it should be moderated on > reordering or other events is another (complex) design issue. But this > patch does not touch that. False fast retransmits are mostly caused by reordering, spurious RTOs can also be caused by delay variations that do not exhibit reordering. Your patch touches all cases of spurious events. Anyway, I just mentioned reordering, because it is the event in which Linux already allows larger bursts of size tp->reordering in the moderation function (i.e. tp->reordering might be increased). It's also not important to me if the undo is happening duringor after recovery, the important question is, if burst protection in general is an important goal, or not (and I think it's there for a reason). Carsten
On Monday 14 March 2011, John Heffner wrote: > On Mon, Mar 14, 2011 at 3:10 PM, Yuchung Cheng <ycheng@google.com> wrote: > > On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > >> The moderation is in place to avoid gigantic segment bursts, which could > >> cause unnecessary pressure on buffers. In my eyes it's already > >> suboptimal that the moderation is weakened in the presence of > >> (detected) reordering, let alone removing it completely. > > > > In the presence of reordering, cwnd is already moderated in Disorder > > state before > > entering the (false) recovery. > > I've always been somewhat skeptical of the usefulness of cwnd > moderation. First, I don't know that its behavior is well defined. > When *should* tcp_moderate_cwnd() actually be called, and why? > > Second, I've never liked the idea in general. Reducing cwnd has an > effect lasting many RTTs, so reducing it in response to a transient > event like reordering seems dubious. And it does not address many > causes of bursts, such as ack compression or stretch acks. I think the undo operations are a special case where it absolutely makes sense, because the cwnd _already_ has been reduced (down to 1 segment by RTO recovery, or, with its special mixture of ratehalving and newreno, Linux sometimes reduces cwnd far below half of it even in fast recovery.). The undo re-opens the window all at once, which may allow _really huge_ bursts. I don't know enough about the other causes of TCPs burstiness, I'm just concerned about allowing this special cause. Carsten
Anecdotally, I've seen more harm than good from cwnd moderation but I don't have any data to back that up. It's hard to categorically say anything. But I don't know what makes the cases where cwnd moderation is applied special. Some other sources of bursts include - Send-limited applications that stall for some time but less than an RTO and do a large write - Receive-limited applications that stall for some time but less than an RTO and do a large read - "Ack compression," when acks are queued somewhere in the network and flood out at line rate - "Stretch acks," when the receiving TCP does not ack every segment (for efficiency, etc.), or acks are lost in the network. Since all of these may occur in the Open state, cwnd moderation is not applied. While I'm adding to my wish list ;-), I will also say that the undo logic seems unnecessarily complicated. If you expect you might have to undo what you just did, why not wait a bit? Something like rfc4653 seems like it might be a better approach and might make the cwnd moderation discussion moot. Thanks, -John On Tue, Mar 15, 2011 at 6:20 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > On Monday 14 March 2011, John Heffner wrote: >> On Mon, Mar 14, 2011 at 3:10 PM, Yuchung Cheng <ycheng@google.com> wrote: >> > On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff <carsten@wolffcarsten.de> > wrote: >> >> The moderation is in place to avoid gigantic segment bursts, which could >> >> cause unnecessary pressure on buffers. In my eyes it's already >> >> suboptimal that the moderation is weakened in the presence of >> >> (detected) reordering, let alone removing it completely. >> > >> > In the presence of reordering, cwnd is already moderated in Disorder >> > state before >> > entering the (false) recovery. >> >> I've always been somewhat skeptical of the usefulness of cwnd >> moderation. First, I don't know that its behavior is well defined. >> When *should* tcp_moderate_cwnd() actually be called, and why? >> >> Second, I've never liked the idea in general. Reducing cwnd has an >> effect lasting many RTTs, so reducing it in response to a transient >> event like reordering seems dubious. And it does not address many >> causes of bursts, such as ack compression or stretch acks. > > I think the undo operations are a special case where it absolutely makes > sense, because the cwnd _already_ has been reduced (down to 1 segment by RTO > recovery, or, with its special mixture of ratehalving and newreno, Linux > sometimes reduces cwnd far below half of it even in fast recovery.). The undo > re-opens the window all at once, which may allow _really huge_ bursts. I don't > know enough about the other causes of TCPs burstiness, I'm just concerned > about allowing this special cause. > > Carsten > -- > /\-´-/\ > ( @ @ ) > ________o0O___^___O0o________ > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi John, Am 15.03.2011 um 11:50 schrieb John Heffner: > > While I'm adding to my wish list ;-), I will also say that the undo > logic seems unnecessarily complicated. If you expect you might have > to undo what you just did, why not wait a bit? Something like rfc4653 > seems like it might be a better approach and might make the cwnd > moderation discussion moot. > actually, our group (including Carsten) implemented RFC4653 for Linux. At the moment, we run our final measurements. However, since I leave the university, the work is not making much progress. Nevertheless I I hope that we can finish this in the next couple of months. Besides the Linux patch, we will bring an rfc4653bis as well as a reordering detection draft to tcpm. Alex // // Dipl.-Inform. Alexander Zimmermann // Department of Computer Science, Informatik 4 // RWTH Aachen University // Ahornstr. 55, 52056 Aachen, Germany // phone: (49-241) 80-21422, fax: (49-241) 80-22222 // email: zimmermann@cs.rwth-aachen.de // web: http://www.umic-mesh.net // -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.17 (Darwin) Comment: GPGTools - http://gpgtools.org iEYEARECAAYFAk1/tQsACgkQdyiq39b9uS7cFACeLaNsGVWoiVzAnUjXOnz8Y/Lk fp0An0IlprotGMbtl1nXC62oyVEHcHv1 =zxT9 -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 15, 2011 at 3:07 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > > Hi, > > On Monday 14 March 2011, Yuchung Cheng wrote: > > On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff <carsten@wolffcarsten.de> > wrote: > > In the presence of reordering, cwnd is already moderated in Disorder > > state before > > entering the (false) recovery. > > Sure, cwnd moderation to in_flight + 1 segment is applied in disorder state, it's in_flight + 3 usually. the moderation first happens tcp_try_to_open() instead of tcp_cwnd_down() > > because this is implementing a form of extended limited transmit. > Nevertheless, after a reordering event that caused a spurious fast retransmit, > there can be an undo of congestion state changes (either after recovery or > interrupting recovery, depending on the options enabled in the connection). I > just wanted to point out, that the moderation step happening upon an undo may > allow a larger burst, if a previous reordering event was detected and caused > tp->reordering to be increased. Your point is that cwnd should be moderated on reordering (in undo or other events). Point taken. My point is that cwnd does not need to be moderated on false recoveries. Do you agree? To implement your design, tcp_update_reordering should do tcp_cwnd_moderation(). To implement my point, the moderations should be avoided in undo operations. The two aren't in conflict. But there are cases that have both undo and reordering. Are we on the same page? > > > > More importantly, the prior ssthresh is restored and not affected by > > > moderation. This means, if moderation reduces cwnd to a small value, then > > > cwnd < ssthresh and TCP will quickly slow-start back to the previous > > > state, without sending a big burst of segments. > > This is actually the more important point, because it means that the > moderation does not negate the effects of the undo operation, as suggested by > your patch-description. It's a double-edge sword. Why slow-start if there is no real loss? It hurts short request-response type traffic performance badly b/c each undo makes cwnd = 3. > > False fast retransmits are mostly caused by reordering, spurious RTOs can also > be caused by delay variations that do not exhibit reordering. Your patch > touches all cases of spurious events. Anyway, I just mentioned reordering, > because it is the event in which Linux already allows larger bursts of size > tp->reordering in the moderation function (i.e. tp->reordering might be > increased). It's also not important to me if the undo is happening duringor > after recovery, the important question is, if burst protection in general is > an important goal, or not (and I think it's there for a reason). I am hoping my previous explanation make sense to you (these two points are not in conflict). -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi again, On Wednesday 16 March 2011, Yuchung Cheng wrote: > On Tue, Mar 15, 2011 at 3:07 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > > Hi, > > > > On Monday 14 March 2011, Yuchung Cheng wrote: > > > On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff > > > <carsten@wolffcarsten.de> > > > > wrote: > > > In the presence of reordering, cwnd is already moderated in Disorder > > > state before > > > entering the (false) recovery. > > > > Sure, cwnd moderation to in_flight + 1 segment is applied in disorder > > state, > > it's in_flight + 3 usually. the moderation first happens > tcp_try_to_open() instead of tcp_cwnd_down() In disorder state, tcp_try_to_open() calls tcp_cwnd_down() which clamps cwnd to in_flight + 1 for dupacks (where tcp_packets_in_flight() is not to be confused with the IN_FLIGHT variable in IETF documents, which is called packets_out in Linux ...). Otherwise, Linux would be violating RFC3042, which allows to send one SMSS of data on each dupack before recovery (actually, just the first two, but since the DupThresh can be larger than 3 in linux, it extends Limited Transmit to more than just the first two dupacks). This is mostly equivalent to the aggressive variant of extended limited transmit in RFC4653. > > because this is implementing a form of extended limited transmit. > > Nevertheless, after a reordering event that caused a spurious fast > > retransmit, there can be an undo of congestion state changes (either > > after recovery or interrupting recovery, depending on the options > > enabled in the connection). I just wanted to point out, that the > > moderation step happening upon an undo may allow a larger burst, if a > > previous reordering event was detected and caused tp->reordering to be > > increased. > > Your point is that cwnd should be moderated on reordering (in undo or > other events). Point taken. > My point is that cwnd does not need to be moderated on false > recoveries. Do you agree? > To implement your design, tcp_update_reordering should do > tcp_cwnd_moderation(). > To implement my point, the moderations should be avoided in undo > operations. > > The two aren't in conflict. But there are cases that have both undo > and reordering. > Are we on the same page? Unfortunately, no. ;-) My point is, that cwnd should be moderated when the congestion state changes are undone after a spurious recovery has been detected. Reordering is only one possible reason for a false recovery. And I stick to that point because of the thoughts I pointed out in my mail to john, i.e. undo typically leading to exceptionally large segment bursts. As for cwnd moderation upon the detection of a reordering event (that's a different thing at a differnt point in time than detection of a false recovery!): This wouldn't make sense to me. The detection of the reordering event together with a metric that measures the extent of the reordering can be used to try and prevent false recoverys in future reordering events, by delaying the congestion reaction (i.e. fast retransmit) then. Reordering can be a cause of spurious recovery. But undo mechanisms and mechanisms to prevent false recovery(s) are orthogonal. Your patch touches all undos, while reordering is just an example for a cause of false recovery. > > > > More importantly, the prior ssthresh is restored and not affected by > > > > moderation. This means, if moderation reduces cwnd to a small value, > > > > then cwnd < ssthresh and TCP will quickly slow-start back to the > > > > previous state, without sending a big burst of segments. > > > > This is actually the more important point, because it means that the > > moderation does not negate the effects of the undo operation, as > > suggested by your patch-description. > > It's a double-edge sword. Why slow-start if there is no real loss? Its a timing thing. I mean, it is an undo operation: the harm has been done, some opportunity to send new data has been lost. Trying to send all that data at once now without an ACK-clock will cause more harm when buffers are under pressure. The undo operation should not try to make up for lost opportunity, only try to reduce further loss of opportunity to send new data. For this, the segment bursts have to be moderated. > It > hurts short > request-response type traffic performance badly b/c each undo makes cwnd = > 3. > > > False fast retransmits are mostly caused by reordering, spurious RTOs can > > also be caused by delay variations that do not exhibit reordering. Your > > patch touches all cases of spurious events. Anyway, I just mentioned > > reordering, because it is the event in which Linux already allows larger > > bursts of size tp->reordering in the moderation function (i.e. > > tp->reordering might be increased). It's also not important to me if the > > undo is happening duringor after recovery, the important question is, if > > burst protection in general is an important goal, or not (and I think > > it's there for a reason). > > I am hoping my previous explanation make sense to you (these two points are > not in conflict). I hope the same for my explanations. :-) Cheers Carsten
On Wed, Mar 16, 2011 at 12:18 PM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > Unfortunately, no. ;-) My point is, that cwnd should be moderated when the > congestion state changes are undone after a spurious recovery has been > detected. Reordering is only one possible reason for a false recovery. And I > stick to that point because of the thoughts I pointed out in my mail to john, > i.e. undo typically leading to exceptionally large segment bursts. There's a more general discussion here as to how much it's worth avoiding bursts at all. What research on the subject I'm aware of is somewhat inconclusive. It's possible to construct scenarios where bursting, burst suppression, or pacing each win or lose badly. If you can choose only one approach as one-size-fits all it's difficult with the information at hand to pick only one. However, I really do wonder why it's so important to suppress bursts on congestion state undo when other, likely far more common, sources of bursts are not suppressed. Carsten, do you have any specific examples of cases you're concerned about? FWIW, there are exactly two causes for spurious retransmits: spurious fast retransmit due to reordering, and spurious timeouts due to a delay spike. Are you particularly concerned with one more than the other? Thanks, -John -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 16 Mar 2011, John Heffner wrote: > On Wed, Mar 16, 2011 at 12:18 PM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > > Unfortunately, no. ;-) My point is, that cwnd should be moderated when the > > congestion state changes are undone after a spurious recovery has been > > detected. Reordering is only one possible reason for a false recovery. And I > > stick to that point because of the thoughts I pointed out in my mail to john, > > i.e. undo typically leading to exceptionally large segment bursts. > > There's a more general discussion here as to how much it's worth > avoiding bursts at all. What research on the subject I'm aware of is > somewhat inconclusive. It's possible to construct scenarios where > bursting, burst suppression, or pacing each win or lose badly. If you > can choose only one approach as one-size-fits all it's difficult with > the information at hand to pick only one. However, I really do wonder > why it's so important to suppress bursts on congestion state undo when > other, likely far more common, sources of bursts are not suppressed. > > Carsten, do you have any specific examples of cases you're concerned > about? FWIW, there are exactly two causes for spurious retransmits: > spurious fast retransmit due to reordering, and spurious timeouts due > to a delay spike. Are you particularly concerned with one more than > the other? The latter is handled with FRTO (defaults on in Linux) so nicely that no burstiness can be seen.
Hi Carsten, Thanks for the detailed explanation. This funnels down to a) is cwnd moderation always a good idea? b) if (a) is true, cwnd should be moderated all the time to suppress any possible burst. Clearly, Linux code supports (a) and (b). Like John's email, I am not aware of too much scientific data supporting that. But I do have a real problem on web server. The HTTP response has normally a handful of packets. Moderating cwnd at the end or after the (false) recovery often makes cwnd=3-4. This causes slow-start on the subsequent HTTP response and throws away some benefit of persistent HTTP connections. That slow-start is not necessary and adds some extra round-trips to serve the tiny object. On Wed, Mar 16, 2011 at 9:18 AM, Carsten Wolff <carsten@wolffcarsten.de> wrote: > Hi again, > > On Wednesday 16 March 2011, Yuchung Cheng wrote: >> On Tue, Mar 15, 2011 at 3:07 AM, Carsten Wolff <carsten@wolffcarsten.de> > wrote: >> > Hi, >> > >> > On Monday 14 March 2011, Yuchung Cheng wrote: >> > > On Mon, Mar 14, 2011 at 3:06 AM, Carsten Wolff >> > > <carsten@wolffcarsten.de> >> > >> > wrote: >> > > In the presence of reordering, cwnd is already moderated in Disorder >> > > state before >> > > entering the (false) recovery. >> > >> > Sure, cwnd moderation to in_flight + 1 segment is applied in disorder >> > state, >> >> it's in_flight + 3 usually. the moderation first happens >> tcp_try_to_open() instead of tcp_cwnd_down() > > In disorder state, tcp_try_to_open() calls tcp_cwnd_down() which clamps cwnd > to in_flight + 1 for dupacks (where tcp_packets_in_flight() is not to be > confused with the IN_FLIGHT variable in IETF documents, which is called > packets_out in Linux ...). Otherwise, Linux would be violating RFC3042, which > allows to send one SMSS of data on each dupack before recovery (actually, just > the first two, but since the DupThresh can be larger than 3 in linux, it > extends Limited Transmit to more than just the first two dupacks). This is > mostly equivalent to the aggressive variant of extended limited transmit in > RFC4653. > >> > because this is implementing a form of extended limited transmit. >> > Nevertheless, after a reordering event that caused a spurious fast >> > retransmit, there can be an undo of congestion state changes (either >> > after recovery or interrupting recovery, depending on the options >> > enabled in the connection). I just wanted to point out, that the >> > moderation step happening upon an undo may allow a larger burst, if a >> > previous reordering event was detected and caused tp->reordering to be >> > increased. >> >> Your point is that cwnd should be moderated on reordering (in undo or >> other events). Point taken. >> My point is that cwnd does not need to be moderated on false >> recoveries. Do you agree? >> To implement your design, tcp_update_reordering should do >> tcp_cwnd_moderation(). >> To implement my point, the moderations should be avoided in undo >> operations. >> >> The two aren't in conflict. But there are cases that have both undo >> and reordering. >> Are we on the same page? > > Unfortunately, no. ;-) My point is, that cwnd should be moderated when the > congestion state changes are undone after a spurious recovery has been > detected. Reordering is only one possible reason for a false recovery. And I > stick to that point because of the thoughts I pointed out in my mail to john, > i.e. undo typically leading to exceptionally large segment bursts. > > As for cwnd moderation upon the detection of a reordering event (that's a > different thing at a differnt point in time than detection of a false > recovery!): This wouldn't make sense to me. The detection of the reordering > event together with a metric that measures the extent of the reordering can be > used to try and prevent false recoverys in future reordering events, by > delaying the congestion reaction (i.e. fast retransmit) then. > > Reordering can be a cause of spurious recovery. But undo mechanisms and > mechanisms to prevent false recovery(s) are orthogonal. > > Your patch touches all undos, while reordering is just an example for a cause > of false recovery. > >> > > > More importantly, the prior ssthresh is restored and not affected by >> > > > moderation. This means, if moderation reduces cwnd to a small value, >> > > > then cwnd < ssthresh and TCP will quickly slow-start back to the >> > > > previous state, without sending a big burst of segments. >> > >> > This is actually the more important point, because it means that the >> > moderation does not negate the effects of the undo operation, as >> > suggested by your patch-description. >> >> It's a double-edge sword. Why slow-start if there is no real loss? > > Its a timing thing. I mean, it is an undo operation: the harm has been done, > some opportunity to send new data has been lost. Trying to send all that data > at once now without an ACK-clock will cause more harm when buffers are under > pressure. The undo operation should not try to make up for lost opportunity, > only try to reduce further loss of opportunity to send new data. For this, the > segment bursts have to be moderated. > >> It >> hurts short >> request-response type traffic performance badly b/c each undo makes cwnd = >> 3. >> >> > False fast retransmits are mostly caused by reordering, spurious RTOs can >> > also be caused by delay variations that do not exhibit reordering. Your >> > patch touches all cases of spurious events. Anyway, I just mentioned >> > reordering, because it is the event in which Linux already allows larger >> > bursts of size tp->reordering in the moderation function (i.e. >> > tp->reordering might be increased). It's also not important to me if the >> > undo is happening duringor after recovery, the important question is, if >> > burst protection in general is an important goal, or not (and I think >> > it's there for a reason). >> >> I am hoping my previous explanation make sense to you (these two points are >> not in conflict). > > I hope the same for my explanations. :-) > > Cheers > Carsten > -- > /\-´-/\ > ( @ @ ) > ________o0O___^___O0o________ > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 08ea735..1bd0f38 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -2659,7 +2659,7 @@ static void DBGUNDO(struct sock *sk, const char *msg) #define DBGUNDO(x...) do { } while (0) #endif -static void tcp_undo_cwr(struct sock *sk, const int undo) +static void tcp_undo_cwr(struct sock *sk, const int undo_ssthresh) { struct tcp_sock *tp = tcp_sk(sk); @@ -2671,14 +2671,13 @@ static void tcp_undo_cwr(struct sock *sk, const int undo) else tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh << 1); - if (undo && tp->prior_ssthresh > tp->snd_ssthresh) { + if (undo_ssthresh && tp->prior_ssthresh > tp->snd_ssthresh) { tp->snd_ssthresh = tp->prior_ssthresh; TCP_ECN_withdraw_cwr(tp); } } else { tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh); } - tcp_moderate_cwnd(tp); tp->snd_cwnd_stamp = tcp_time_stamp; } @@ -2822,8 +2821,11 @@ static int tcp_try_undo_loss(struct sock *sk) static inline void tcp_complete_cwr(struct sock *sk) { struct tcp_sock *tp = tcp_sk(sk); - tp->snd_cwnd = min(tp->snd_cwnd, tp->snd_ssthresh); - tp->snd_cwnd_stamp = tcp_time_stamp; + /* Do not moderate cwnd if it's already undone in cwr or recovery */ + if (tp->undo_marker && tp->snd_cwnd > tp->snd_ssthresh) { + tp->snd_cwnd = tp->snd_ssthresh; + tp->snd_cwnd_stamp = tcp_time_stamp; + } tcp_ca_event(sk, CA_EVENT_COMPLETE_CWR); }
In the current undo logic, cwnd is moderated after it was restored to the value prior entering fast-recovery. It was moderated first in tcp_try_undo_recovery then again in tcp_complete_cwr. Since the undo indicates recovery was false, these moderations are not necessary. If the undo is triggered when most of the outstanding data have been acknowledged, the (restored) cwnd is falsely pulled down to a small value. This patch removes these cwnd moderations if cwnd is undone during the fast-recovery. Signed-off-by: Yuchung Cheng <ycheng@google.com> --- net/ipv4/tcp_input.c | 12 +++++++----- 1 files changed, 7 insertions(+), 5 deletions(-)