aboutsummaryrefslogtreecommitdiffstats
path: root/net/ipv4
diff options
context:
space:
mode:
authorIlpo Järvinen <ilpo.jarvinen@helsinki.fi>2008-04-08 01:32:38 -0400
committerDavid S. Miller <davem@davemloft.net>2008-04-08 01:32:38 -0400
commitc137f3dda04b0aee1bc6889cdc69185f53df8a82 (patch)
tree75031e829df932aae697b92ee1ef490bdf71c762 /net/ipv4
parent1b69d745397eac12b3f8a2eb6b799cd476aef282 (diff)
[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 <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: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/ipv4')
-rw-r--r--net/ipv4/tcp_input.c22
1 files changed, 18 insertions, 4 deletions
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 5573202f0861..7d0958785bfb 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)
2138{ 2138{
2139 struct tcp_sock *tp = tcp_sk(sk); 2139 struct tcp_sock *tp = tcp_sk(sk);
2140 struct sk_buff *skb; 2140 struct sk_buff *skb;
2141 int cnt; 2141 int cnt, oldcnt;
2142 int err;
2143 unsigned int mss;
2142 2144
2143 BUG_TRAP(packets <= tp->packets_out); 2145 BUG_TRAP(packets <= tp->packets_out);
2144 if (tp->lost_skb_hint) { 2146 if (tp->lost_skb_hint) {
@@ -2157,13 +2159,25 @@ static void tcp_mark_head_lost(struct sock *sk, int packets)
2157 tp->lost_skb_hint = skb; 2159 tp->lost_skb_hint = skb;
2158 tp->lost_cnt_hint = cnt; 2160 tp->lost_cnt_hint = cnt;
2159 2161
2162 if (after(TCP_SKB_CB(skb)->end_seq, tp->high_seq))
2163 break;
2164
2165 oldcnt = cnt;
2160 if (tcp_is_fack(tp) || tcp_is_reno(tp) || 2166 if (tcp_is_fack(tp) || tcp_is_reno(tp) ||
2161 (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED)) 2167 (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED))
2162 cnt += tcp_skb_pcount(skb); 2168 cnt += tcp_skb_pcount(skb);
2163 2169
2164 if ((cnt > packets) || 2170 if (cnt > packets) {
2165 after(TCP_SKB_CB(skb)->end_seq, tp->high_seq)) 2171 if (tcp_is_sack(tp) || (oldcnt >= packets))
2166 break; 2172 break;
2173
2174 mss = skb_shinfo(skb)->gso_size;
2175 err = tcp_fragment(sk, skb, (packets - oldcnt) * mss, mss);
2176 if (err < 0)
2177 break;
2178 cnt = packets;
2179 }
2180
2167 if (!(TCP_SKB_CB(skb)->sacked & (TCPCB_SACKED_ACKED|TCPCB_LOST))) { 2181 if (!(TCP_SKB_CB(skb)->sacked & (TCPCB_SACKED_ACKED|TCPCB_LOST))) {
2168 TCP_SKB_CB(skb)->sacked |= TCPCB_LOST; 2182 TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
2169 tp->lost_out += tcp_skb_pcount(skb); 2183 tp->lost_out += tcp_skb_pcount(skb);