1. 13 6月, 2008 1 次提交
    • D
      tcp: Revert 'process defer accept as established' changes. · ec0a1966
      David S. Miller 提交于
      This reverts two changesets, ec3c0982
      ("[TCP]: TCP_DEFER_ACCEPT updates - process as established") and
      the follow-on bug fix 9ae27e0a
      ("tcp: Fix slab corruption with ipv6 and tcp6fuzz").
      
      This change causes several problems, first reported by Ingo Molnar
      as a distcc-over-loopback regression where connections were getting
      stuck.
      
      Ilpo Järvinen first spotted the locking problems.  The new function
      added by this code, tcp_defer_accept_check(), only has the
      child socket locked, yet it is modifying state of the parent
      listening socket.
      
      Fixing that is non-trivial at best, because we can't simply just grab
      the parent listening socket lock at this point, because it would
      create an ABBA deadlock.  The normal ordering is parent listening
      socket --> child socket, but this code path would require the
      reverse lock ordering.
      
      Next is a problem noticed by Vitaliy Gusev, he noted:
      
      ----------------------------------------
      >--- a/net/ipv4/tcp_timer.c
      >+++ b/net/ipv4/tcp_timer.c
      >@@ -481,6 +481,11 @@ static void tcp_keepalive_timer (unsigned long data)
      > 		goto death;
      > 	}
      >
      >+	if (tp->defer_tcp_accept.request && sk->sk_state == TCP_ESTABLISHED) {
      >+		tcp_send_active_reset(sk, GFP_ATOMIC);
      >+		goto death;
      
      Here socket sk is not attached to listening socket's request queue. tcp_done()
      will not call inet_csk_destroy_sock() (and tcp_v4_destroy_sock() which should
      release this sk) as socket is not DEAD. Therefore socket sk will be lost for
      freeing.
      ----------------------------------------
      
      Finally, Alexey Kuznetsov argues that there might not even be any
      real value or advantage to these new semantics even if we fix all
      of the bugs:
      
      ----------------------------------------
      Hiding from accept() sockets with only out-of-order data only
      is the only thing which is impossible with old approach. Is this really
      so valuable? My opinion: no, this is nothing but a new loophole
      to consume memory without control.
      ----------------------------------------
      
      So revert this thing for now.
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      ec0a1966
  2. 05 6月, 2008 2 次提交
    • I
      tcp: fix skb vs fack_count out-of-sync condition · a6604471
      Ilpo Järvinen 提交于
      This bug is able to corrupt fackets_out in very rare cases.
      In order for this to cause corruption:
        1) DSACK in the middle of previous SACK block must be generated.
        2) In order to take that particular branch, part or all of the
           DSACKed segment must already be SACKed so that we have that
           in cache in the first place.
        3) The new info must be top enough so that fackets_out will be
           updated on this iteration.
      ...then fack_count is updated while skb wasn't, then we walk again
      that particular segment thus updating fack_count twice for
      a single skb and finally that value is assigned to fackets_out
      by tcp_sacktag_one.
      
      It is safe to call tcp_sacktag_one just once for a segment (at
      DSACK), no need to call again for plain SACK.
      
      Potential problem of the miscount are limited to premature entry
      to recovery and to inflated reordering metric (which could even
      cancel each other out in the most the luckiest scenarios :-)).
      Both are quite insignificant in worst case too and there exists
      also code to reset them (fackets_out once sacked_out becomes zero
      and reordering metric on RTO).
      
      This has been reported by a number of people, because it occurred
      quite rarely, it has been very evasive. Andy Furniss was able to
      get it to occur couple of times so that a bit more info was
      collected about the problem using a debug patch, though it still
      required lot of checking around. Thanks also to others who have
      tried to help here.
      
      This is listed as Bugzilla #10346. The bug was introduced by
      me in commit 68f8353b ([TCP]: Rewrite SACK block processing & 
      sack_recv_cache use), I probably thought back then that there's
      need to scan that entry twice or didn't dare to make it go
      through it just once there. Going through twice would have
      required restoring fack_count after the walk but as noted above,
      I chose to drop the additional walk step altogether here.
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a6604471
    • I
      tcp: Fix inconsistency source (CA_Open only when !tcp_left_out(tp)) · 8aca6cb1
      Ilpo Järvinen 提交于
      It is possible that this skip path causes TCP to end up into an
      invalid state where ca_state was left to CA_Open while some
      segments already came into sacked_out. If next valid ACK doesn't
      contain new SACK information TCP fails to enter into
      tcp_fastretrans_alert(). Thus at least high_seq is set
      incorrectly to a too high seqno because some new data segments
      could be sent in between (and also, limited transmit is not
      being correctly invoked there). Reordering in both directions
      can easily cause this situation to occur.
      
      I guess we would want to use tcp_moderate_cwnd(tp) there as well
      as it may be possible to use this to trigger oversized burst to
      network by sending an old ACK with huge amount of SACK info, but
      I'm a bit unsure about its effects (mainly to FlightSize), so to
      be on the safe side I just currently fixed it minimally to keep
      TCP's state consistent (obviously, such nasty ACKs have been
      possible this far). Though it seems that FlightSize is already
      underestimated by some amount, so probably on the long term we
      might want to trigger recovery there too, if appropriate, to make
      FlightSize calculation to resemble reality at the time when the
      losses where discovered (but such change scares me too much now
      and requires some more thinking anyway how to do that as it
      likely involves some code shuffling).
      
      This bug was found by Brian Vowell while running my TCP debug
      patch to find cause of another TCP issue (fackets_out
      miscount).
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      8aca6cb1
  3. 13 5月, 2008 2 次提交
    • I
      tcp FRTO: work-around inorder receivers · 79d44516
      Ilpo Järvinen 提交于
      If receiver consumes segments successfully only in-order, FRTO
      fallback to conventional recovery produces RTO loop because
      FRTO's forward transmissions will always get dropped and need to
      be resent, yet by default they're not marked as lost (which are
      the only segments we will retransmit in CA_Loss).
      
      Price to pay about this is occassionally unnecessarily
      retransmitting the forward transmission(s). SACK blocks help
      a bit to avoid this, so it's mainly a concern for NewReno case
      though SACK is not fully immune either.
      
      This change has a side-effect of fixing SACKFRTO problem where
      it didn't have snd_nxt of the RTO time available anymore when
      fallback become necessary (this problem would have only occured
      when RTO would occur for two or more segments and ECE arrives
      in step 3; no need to figure out how to fix that unless the
      TODO item of selective behavior is considered in future).
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Reported-by: NDamon L. Chesser <damon@damtek.com>
      Tested-by: NDamon L. Chesser <damon@damtek.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      79d44516
    • I
      tcp FRTO: Fix fallback to conventional recovery · a1c1f281
      Ilpo Järvinen 提交于
      It seems that commit 009a2e3e ("[TCP] FRTO: Improve
      interoperability with other undo_marker users") run into
      another land-mine which caused fallback to conventional
      recovery to break:
      
      1. Cumulative ACK arrives after FRTO retransmission
      2. tcp_try_to_open sees zero retrans_out, clears retrans_stamp
         which should be kept like in CA_Loss state it would be
      3. undo_marker change allowed tcp_packet_delayed to return
         true because of the cleared retrans_stamp once FRTO is
         terminated causing LossUndo to occur, which means all loss
         markings FRTO made are reverted.
      
      This means that the conventional recovery basically recovered
      one loss per RTT, which is not that efficient. It was quite
      unobvious that the undo_marker change broken something like
      this, I had a quite long session to track it down because of
      the non-intuitiviness of the bug (luckily I had a trivial
      reproducer at hand and I was also able to learn to use kprobes
      in the process as well :-)).
      
      This together with the NewReno+FRTO fix and FRTO in-order
      workaround this fixes Damon's problems, this and the first
      mentioned are enough to fix Bugzilla #10063.
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Reported-by: NDamon L. Chesser <damon@damtek.com>
      Tested-by: NDamon L. Chesser <damon@damtek.com>
      Tested-by: NSebastian Hyrwall <zibbe@cisko.org>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      a1c1f281
  4. 08 5月, 2008 1 次提交
    • I
      tcp FRTO: SACK variant is errorneously used with NewReno · 62ab2227
      Ilpo Järvinen 提交于
      Note: there's actually another bug in FRTO's SACK variant, which
      is the causing failure in NewReno case because of the error
      that's fixed here. I'll fix the SACK case separately (it's
      a separate bug really, though related, but in order to fix that
      I need to audit tp->snd_nxt usage a bit).
      
      There were two places where SACK variant of FRTO is getting
      incorrectly used even if SACK wasn't negotiated by the TCP flow.
      This leads to incorrect setting of frto_highmark with NewReno
      if a previous recovery was interrupted by another RTO.
      
      An eventual fallback to conventional recovery then incorrectly
      considers one or couple of segments as forward transmissions
      though they weren't, which then are not LOST marked during
      fallback making them "non-retransmittable" until the next RTO.
      In a bad case, those segments are really lost and are the only
      one left in the window. Thus TCP needs another RTO to continue.
      The next FRTO, however, could again repeat the same events
      making the progress of the TCP flow extremely slow.
      
      In order for these events to occur at all, FRTO must occur
      again in FRTOs step 3 while the key segments must be lost as
      well, which is not too likely in practice. It seems to most
      frequently with some small devices such as network printers
      that *seem* to accept TCP segments only in-order. In cases
      were key segments weren't lost, things get automatically
      resolved because those wrongly marked segments don't need to be
      retransmitted in order to continue.
      
      I found a reproducer after digging up relevant reports (few
      reports in total, none at netdev or lkml I know of), some
      cases seemed to indicate middlebox issues which seems now
      to be a false assumption some people had made. Bugzilla
      #10063 _might_ be related. Damon L. Chesser <damon@damtek.com>
      had a reproducable case and was kind enough to tcpdump it
      for me. With the tcpdump log it was quite trivial to figure
      out.
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      62ab2227
  5. 05 5月, 2008 1 次提交
  6. 03 5月, 2008 1 次提交
  7. 28 4月, 2008 1 次提交
    • E
      tcp: Fix slab corruption with ipv6 and tcp6fuzz · 9ae27e0a
      Evgeniy Polyakov 提交于
      From: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
      
      This fixes a regression added by ec3c0982
      ("[TCP]: TCP_DEFER_ACCEPT updates - process as established")
      
      tcp_v6_do_rcv()->tcp_rcv_established(), the latter goes to step5, where
      eventually skb can be freed via tcp_data_queue() (drop: label), then if
      check for tcp_defer_accept_check() returns true and thus
      tcp_rcv_established() returns -1, which forces tcp_v6_do_rcv() to jump
      to reset: label, which in turn will pass through discard: label and free
      the same skb again.
      
      Tested by Eric Sesterhenn.
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      Acked-By: NPatrick McManus <mcmanus@ducksong.com>
      9ae27e0a
  8. 22 4月, 2008 1 次提交
  9. 16 4月, 2008 1 次提交
  10. 15 4月, 2008 1 次提交
  11. 14 4月, 2008 3 次提交
  12. 08 4月, 2008 4 次提交
    • I
      [TCP]: Don't allow FRTO to take place while MTU is being probed · 6adb4f73
      Ilpo Järvinen 提交于
      MTU probe can cause some remedies for FRTO because the normal
      packet ordering may be violated allowing FRTO to make a wrong
      decision (it might not be that serious threat for anything
      though). Thus it's safer to not run FRTO while MTU probe is
      underway.
      
      It seems that the basic FRTO variant should also look for an
      skb at probe_seq.start to check if that's retransmitted one
      but I didn't implement it now (plain seqno in window check
      isn't robust against wraparounds).
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      6adb4f73
    • I
      [TCP]: tcp_simple_retransmit can cause S+L · 882bebaa
      Ilpo Järvinen 提交于
      This fixes Bugzilla #10384
      
      tcp_simple_retransmit does L increment without any checking
      whatsoever for overflowing S+L when Reno is in use.
      
      The simplest scenario I can currently think of is rather
      complex in practice (there might be some more straightforward
      cases though). Ie., if mss is reduced during mtu probing, it
      may end up marking everything lost and if some duplicate ACKs
      arrived prior to that sacked_out will be non-zero as well,
      leading to S+L > packets_out, tcp_clean_rtx_queue on the next
      cumulative ACK or tcp_fastretrans_alert on the next duplicate
      ACK will fix the S counter.
      
      More straightforward (but questionable) solution would be to
      just call tcp_reset_reno_sack() in tcp_simple_retransmit but
      it would negatively impact the probe's retransmission, ie.,
      the retransmissions would not occur if some duplicate ACKs
      had arrived.
      
      So I had to add reno sacked_out reseting to CA_Loss state
      when the first cumulative ACK arrives (this stale sacked_out
      might actually be the explanation for the reports of left_out
      overflows in kernel prior to 2.6.23 and S+L overflow reports
      of 2.6.24). However, this alone won't be enough to fix kernel
      before 2.6.24 because it is building on top of the commit
      1b6d427b ([TCP]: Reduce sacked_out with reno when purging
      write_queue) to keep the sacked_out from overflowing.
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Reported-by: NAlessandro Suardi <alessandro.suardi@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      882bebaa
    • I
      [TCP]: Fix NewReno's fast rexmit/recovery problems with GSOed skb · c137f3dd
      Ilpo Järvinen 提交于
      Fixes a long-standing bug which makes NewReno recovery crippled.
      With GSO the whole head skb was marked as LOST which is in
      violation of NewReno procedure that only wants to mark one packet
      and ended up breaking our TCP code by causing counter overflow
      because our code was built on top of assumption about valid
      NewReno procedure. This manifested as triggering a WARN_ON for
      the overflow in a number of places.
      
      It seems relatively safe alternative to just do nothing if
      tcp_fragment fails due to oom because another duplicate ACK is
      likely to be received soon and the fragmentation will be retried.
      
      Special thanks goes to Soeren Sonnenburg <kernel@nn7.de> who was
      lucky enough to be able to reproduce this so that the warning
      for the overflow was hit. It's not as easy task as it seems even
      if this bug happens quite often because the amount of outstanding
      data is pretty significant for the mismarkings to lead to an
      overflow.
      
      Because it's very late in 2.6.25-rc cycle (if this even makes in
      time), I didn't want to touch anything with SACK enabled here.
      Fragmenting might be useful for it as well but it's more or less
      a policy decision rather than mandatory fix. Thus there's no need
      to rush and we can postpone considering tcp_fragment with SACK
      for 2.6.26.
      
      In 2.6.24 and earlier, this very same bug existed but the effect
      is slightly different because of a small changes in the if
      conditions that fit to the patch's context. With them nothing
      got lost marker and thus no retransmissions happened.
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      c137f3dd
    • I
      [TCP]: Restore 2.6.24 mark_head_lost behavior for newreno/fack · 1b69d745
      Ilpo Järvinen 提交于
      The fast retransmission can be forced locally to the rfc3517
      branch in tcp_update_scoreboard instead of making such fragile
      constructs deeper in tcp_mark_head_lost.
      
      This is necessary for the next patch which must not have
      loopholes for cnt > packets check. As one can notice,
      readability got some improvements too because of this :-).
      Signed-off-by: NIlpo Järvinen <ilpo.jarvinen@helsinki.fi>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      1b69d745
  13. 22 3月, 2008 1 次提交
    • P
      [TCP]: TCP_DEFER_ACCEPT updates - process as established · ec3c0982
      Patrick McManus 提交于
      Change TCP_DEFER_ACCEPT implementation so that it transitions a
      connection to ESTABLISHED after handshake is complete instead of
      leaving it in SYN-RECV until some data arrvies. Place connection in
      accept queue when first data packet arrives from slow path.
      
      Benefits:
        - established connection is now reset if it never makes it
         to the accept queue
      
       - diagnostic state of established matches with the packet traces
         showing completed handshake
      
       - TCP_DEFER_ACCEPT timeouts are expressed in seconds and can now be
         enforced with reasonable accuracy instead of rounding up to next
         exponential back-off of syn-ack retry.
      Signed-off-by: NPatrick McManus <mcmanus@ducksong.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      ec3c0982
  14. 06 3月, 2008 1 次提交
  15. 04 3月, 2008 2 次提交
  16. 01 2月, 2008 1 次提交
  17. 29 1月, 2008 16 次提交