[PATCHv2 net-2.6 1/4] [TCP]: Restore 2.6.24 mark_head_lost behavior for newreno/fack - Kernel

This is a discussion on [PATCHv2 net-2.6 1/4] [TCP]: Restore 2.6.24 mark_head_lost behavior for newreno/fack - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 2 of 2

Thread: [PATCHv2 net-2.6 1/4] [TCP]: Restore 2.6.24 mark_head_lost behavior for newreno/fack

  1. [PATCHv2 net-2.6 1/4] [TCP]: Restore 2.6.24 mark_head_lost behavior for newreno/fack

    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: Ilpo Järvinen
    ---
    net/ipv4/tcp_input.c | 16 ++++++++--------
    1 files changed, 8 insertions(+), 8 deletions(-)

    diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
    index 6e46b4c..f82b573 100644
    --- a/net/ipv4/tcp_input.c
    +++ b/net/ipv4/tcp_input.c
    @@ -2134,7 +2134,7 @@ static void tcp_verify_retransmit_hint(struct tcp_sock *tp, struct sk_buff *skb)
    /* Mark head of queue up as lost. With RFC3517 SACK, the packets is
    * is against sacked "cnt", otherwise it's against facked "cnt"
    */
    -static void tcp_mark_head_lost(struct sock *sk, int packets, int fast_rexmit)
    +static void tcp_mark_head_lost(struct sock *sk, int packets)
    {
    struct tcp_sock *tp = tcp_sk(sk);
    struct sk_buff *skb;
    @@ -2161,7 +2161,7 @@ static void tcp_mark_head_lost(struct sock *sk, int packets, int fast_rexmit)
    (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED))
    cnt += tcp_skb_pcount(skb);

    - if (((!fast_rexmit || (tp->lost_out > 0)) && (cnt > packets)) ||
    + if ((cnt > packets) ||
    after(TCP_SKB_CB(skb)->end_seq, tp->high_seq))
    break;
    if (!(TCP_SKB_CB(skb)->sacked & (TCPCB_SACKED_ACKED|TCPCB_LOST))) {
    @@ -2180,17 +2180,17 @@ static void tcp_update_scoreboard(struct sock *sk, int fast_rexmit)
    struct tcp_sock *tp = tcp_sk(sk);

    if (tcp_is_reno(tp)) {
    - tcp_mark_head_lost(sk, 1, fast_rexmit);
    + tcp_mark_head_lost(sk, 1);
    } else if (tcp_is_fack(tp)) {
    int lost = tp->fackets_out - tp->reordering;
    if (lost <= 0)
    lost = 1;
    - tcp_mark_head_lost(sk, lost, fast_rexmit);
    + tcp_mark_head_lost(sk, lost);
    } else {
    int sacked_upto = tp->sacked_out - tp->reordering;
    - if (sacked_upto < 0)
    - sacked_upto = 0;
    - tcp_mark_head_lost(sk, sacked_upto, fast_rexmit);
    + if (sacked_upto < fast_rexmit)
    + sacked_upto = fast_rexmit;
    + tcp_mark_head_lost(sk, sacked_upto);
    }

    /* New heuristics: it is possible only after we switched
    @@ -2524,7 +2524,7 @@ static void tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
    before(tp->snd_una, tp->high_seq) &&
    icsk->icsk_ca_state != TCP_CA_Open &&
    tp->fackets_out > tp->reordering) {
    - tcp_mark_head_lost(sk, tp->fackets_out - tp->reordering, 0);
    + tcp_mark_head_lost(sk, tp->fackets_out - tp->reordering);
    NET_INC_STATS_BH(LINUX_MIB_TCPLOSS);
    }

    --
    1.5.2.2

    --
    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/

  2. [PATCHv2 net-2.6 2/4] [TCP]: Fix NewReno's fast rexmit/recovery problems with GSOed skb

    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 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: Ilpo Järvinen
    ---
    net/ipv4/tcp_input.c | 22 ++++++++++++++++++----
    1 files changed, 18 insertions(+), 4 deletions(-)

    diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
    index f82b573..f053bfe 100644
    --- a/net/ipv4/tcp_input.c
    +++ b/net/ipv4/tcp_input.c
    @@ -2138,7 +2138,9 @@ static void tcp_mark_head_lost(struct sock *sk, int packets)
    {
    struct tcp_sock *tp = tcp_sk(sk);
    struct sk_buff *skb;
    - int cnt;
    + int cnt, oldcnt;
    + int err;
    + unsigned int mss;

    BUG_TRAP(packets <= tp->packets_out);
    if (tp->lost_skb_hint) {
    @@ -2157,13 +2159,25 @@ static void tcp_mark_head_lost(struct sock *sk, int packets)
    tp->lost_skb_hint = skb;
    tp->lost_cnt_hint = cnt;

    + if (after(TCP_SKB_CB(skb)->end_seq, tp->high_seq))
    + break;
    +
    + oldcnt = cnt;
    if (tcp_is_fack(tp) || tcp_is_reno(tp) ||
    (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED))
    cnt += tcp_skb_pcount(skb);

    - if ((cnt > packets) ||
    - after(TCP_SKB_CB(skb)->end_seq, tp->high_seq))
    - break;
    + if (cnt > packets) {
    + if (tcp_is_sack(tp) || (oldcnt >= packets))
    + break;
    +
    + mss = skb_shinfo(skb)->gso_size;
    + err = tcp_fragment(sk, skb, (packets - oldcnt) * mss, mss);
    + if (err < 0)
    + break;
    + cnt = packets;
    + }
    +
    if (!(TCP_SKB_CB(skb)->sacked & (TCPCB_SACKED_ACKED|TCPCB_LOST))) {
    TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
    tp->lost_out += tcp_skb_pcount(skb);
    --
    1.5.2.2

    --
    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/

+ Reply to Thread