Messages in this thread Patch in this message | | | Date | Mon, 3 Mar 2008 15:53:12 +0200 (EET) | From | "Ilpo Järvinen" <> | Subject | [PATCH net-2.6] [TCP]: Must count fack_count also when skipping |
| |
On Mon, 3 Mar 2008, Ilpo Järvinen wrote:
> On Mon, 3 Mar 2008, Ilpo Järvinen wrote: > > > On Sun, 2 Mar 2008, Ilpo Järvinen wrote: > > > > > On Sun, 2 Mar 2008, Guillaume Chazarain wrote: > > > > > > >> On Sun, Mar 2, 2008 at 1:38 PM, Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> wrote: > > > >>> It does not contain any "KERNEL: assertion (packets <= > > > >> > tp->packets_out) failed at" line, so I'm afraid it's just noise. > > > > > > Doh, you were right in this one... > > I did some filtering among those and found out that some still point out > bug (there might be indication of tcp_mark_head_lost inconsistency as > well but it's nearly impossible to track with all the incorporated noise > which is causing which). ...I'll post the patch separately right after > this.
Dave, at least this is needed for TCP packet counter correctness, there could be other one hiding still because this only makes fackets_out too small (there could be second order effect though), while people are seeing a result of too large fackets_out in tcp_mark_head_lost.
It's a bit shame that I didn't notice this when I verified time-seqno graphs after that sacktag rewrite (it mostly results just a substle difference in the slope).
With this & the debug patch I didn't get anything into my logs but others might be more successful if there are other bugs still to solve that require more sophisticated network conditions to occur.
-- i.
[PATCH net-2.6] [TCP]: Must count fack_count also when skipping
It makes fackets_out to grow too slowly compared with the real write queue.
This shouldn't cause those BUG_TRAP(packets <= tp->packets_out) to trigger but how knows how such inconsistent fackets_out affects here and there around TCP when everything is nowadays assuming accurate fackets_out. So lets see if this silences them all.
Reported by Guillaume Chazarain <guichaz@gmail.com>.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> --- net/ipv4/tcp_input.c | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 19c449f..7facdb0 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -1367,7 +1367,7 @@ static struct sk_buff *tcp_sacktag_walk(struct sk_buff *skb, struct sock *sk, * a normal way */ static struct sk_buff *tcp_sacktag_skip(struct sk_buff *skb, struct sock *sk, - u32 skip_to_seq) + u32 skip_to_seq, int *fack_count) { tcp_for_write_queue_from(skb, sk) { if (skb == tcp_send_head(sk)) @@ -1375,6 +1375,8 @@ static struct sk_buff *tcp_sacktag_skip(struct sk_buff *skb, struct sock *sk, if (!before(TCP_SKB_CB(skb)->end_seq, skip_to_seq)) break; + + *fack_count += tcp_skb_pcount(skb); } return skb; } @@ -1390,7 +1392,7 @@ static struct sk_buff *tcp_maybe_skipping_dsack(struct sk_buff *skb, return skb; if (before(next_dup->start_seq, skip_to_seq)) { - skb = tcp_sacktag_skip(skb, sk, next_dup->start_seq); + skb = tcp_sacktag_skip(skb, sk, next_dup->start_seq, fack_count); tcp_sacktag_walk(skb, sk, NULL, next_dup->start_seq, next_dup->end_seq, 1, fack_count, reord, flag); @@ -1537,7 +1539,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, /* Head todo? */ if (before(start_seq, cache->start_seq)) { - skb = tcp_sacktag_skip(skb, sk, start_seq); + skb = tcp_sacktag_skip(skb, sk, start_seq, + &fack_count); skb = tcp_sacktag_walk(skb, sk, next_dup, start_seq, cache->start_seq, @@ -1565,7 +1568,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, goto walk; } - skb = tcp_sacktag_skip(skb, sk, cache->end_seq); + skb = tcp_sacktag_skip(skb, sk, cache->end_seq, + &fack_count); /* Check overlap against next cached too (past this one already) */ cache++; continue; @@ -1577,7 +1581,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, break; fack_count = tp->fackets_out; } - skb = tcp_sacktag_skip(skb, sk, start_seq); + skb = tcp_sacktag_skip(skb, sk, start_seq, &fack_count); walk: skb = tcp_sacktag_walk(skb, sk, next_dup, start_seq, end_seq, -- 1.5.2.2 | |