lkml.org 
[lkml]   [2007]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [2.6.20.21 review 12/35] TCP: Fix TCP handling of SACK in bidirectional flows.
    Hi Ilpo,

    On Sat, Oct 13, 2007 at 08:15:52PM +0300, Ilpo Järvinen wrote:
    > On Sat, 13 Oct 2007, Willy Tarreau wrote:
    >
    > > It's possible that new SACK blocks that should trigger new LOST
    > > markings arrive with new data (which previously made is_dupack
    > > false). In addition, I think this fixes a case where we get
    > > a cumulative ACK with enough SACK blocks to trigger the fast
    > > recovery (is_dupack would be false there too).
    > >
    > > I'm not completely pleased with this solution because readability
    > > of the code is somewhat questionable as 'is_dupack' in SACK case
    > > is no longer about dupacks only but would mean something like
    > > 'lost_marker_work_todo' too... But because of Eifel stuff done
    > > in CA_Recovery, the FLAG_DATA_SACKED check cannot be placed to
    > > the if statement which seems attractive solution. Nevertheless,
    > > I didn't like adding another variable just for that either... :-)
    > >
    > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
    > > Signed-off-by: David S. Miller <davem@davemloft.net>
    > > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
    > > ---
    > > net/ipv4/tcp_input.c | 5 ++++-
    > > 1 files changed, 4 insertions(+), 1 deletions(-)
    > >
    > > Index: 2.6/net/ipv4/tcp_input.c
    > > ===================================================================
    > > --- 2.6.orig/net/ipv4/tcp_input.c
    > > +++ 2.6/net/ipv4/tcp_input.c
    > > @@ -1951,7 +1951,10 @@ tcp_fastretrans_alert(struct sock *sk, u
    > > {
    > > struct inet_connection_sock *icsk = inet_csk(sk);
    > > struct tcp_sock *tp = tcp_sk(sk);
    > > - int is_dupack = (tp->snd_una == prior_snd_una && !(flag&FLAG_NOT_DUP));
    > > + int is_dupack = (tp->snd_una == prior_snd_una &&
    > > + (!(flag&FLAG_NOT_DUP) ||
    > > + ((flag&FLAG_DATA_SACKED) &&
    > > + (tp->fackets_out > tp->reordering))));
    > >
    > > /* Some technical things:
    > > * 1. Reno does not count dupacks (sacked_out) automatically. */
    >
    > FYI,
    >
    > This ended up being a non complete fix. Day after these two patches
    > (11-12) I submitted two other patches to complete this fix series (got
    > bitten by release-early-release-often, fixed day-after-submission
    > thoughts in those two later patches). For some reason these two keep
    > floating around as separate ones from those two later ones.
    >
    > To make things even more complicated, eb7bdad82e8 (see stable-2.6.22)
    > could have been split more logically to do_lost addition and
    > FLAG_SND_UNA_ADVANCED parts (but that didn't occur to me back then).
    >
    > All of them listed here (from stable-2.6.22 since one of them is
    > reduced from mainline version):
    >
    > 6d742fb6e2b8913457e1282e1be77d6f4e45af00 Fix TCP DSACK cwnd handling
    > eb7bdad82e8af48e1ed1b650268dc85ca7e9ff39 Handle snd_una in tcp_cwnd_down()
    > 8385cffd22359ad561a173accefeb354bd606ce4 TCP: Fix TCP handling of SACK in
    > 783366ad4b212cde069c50903494eb6a6b83958c TCP: Fix TCP rate-halving on

    Thanks for your help, I really appreciate it. In fact, I've reviewed them
    four, but two of them did not apply and the code looked somewhat different,
    so I considered them irrelevant to 2.6.20. I didn't understand that they
    were all related, so maybe I checked them in a wrong order.

    I'll recheck all that in the right sequence and will merge them four, or
    get back to you if something still puzzles me.

    Thanks!
    Willy

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-10-13 19:33    [W:0.061 / U:60.784 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site