Re: 2.6.24-rc4-mm1

Ilpo Järvinen
2007-12-10T12:24:46Z

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3239 bytes --]

On Wed, 5 Dec 2007, Andrew Morton wrote:

> On Thu, 06 Dec 2007 17:59:37 +1100 Reuben Farrelly <reuben-linuxkernel@reub.net> wrote:
>
> > This non fatal oops which I have just noticed may be related to this change then
> > - certainly looks networking related.
>
> yep, but it isn't e1000. It's core TCP.
>
> > WARNING: at net/ipv4/tcp_input.c:2518 tcp_fastretrans_alert()
> > Pid: 0, comm: swapper Not tainted 2.6.24-rc4-mm1 #1
>
> Ilpo, Reuben's kernel is talking to you ;)

...Please try the patch below. Andrew, this probably fixes your problem
(the packets <= tp->packets_out) as well.

Dave, please include this one to net-2.6.25.

--
i.

--
[PATCH] [TCP]: Fix fack_count miscountings (multiple places)

1) Fack_count is set incorrectly if the highest sent skb is
already sacked (the skb->prev won't return it because it's on
the other list already). These manifest as fackets_out counting
error later on, the second-order effects are very hard to track,
so it may fix all out-standing TCP bug reports.

2) Prev == NULL check was wrong way around

3) Last skb's fack count was incorrectly skipped while() {} loop

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
include/net/tcp.h | 22 ++++++++++++++++------
1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 9dbed0b..11a7e3e 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1337,10 +1337,20 @@ static inline struct sk_buff *tcp_send_head(struct sock *sk)
static inline void tcp_advance_send_head(struct sock *sk, struct sk_buff *skb)
{
struct sk_buff *prev = tcp_write_queue_prev(sk, skb);
+ unsigned int fc = 0;
+
+ if (prev == (struct sk_buff *)&sk->sk_write_queue)
+ prev = NULL;
+ else if (!tcp_skb_adjacent(sk, prev, skb))
+ prev = NULL;

- if (prev != (struct sk_buff *)&sk->sk_write_queue)
- TCP_SKB_CB(skb)->fack_count = TCP_SKB_CB(prev)->fack_count +
- tcp_skb_pcount(prev);
+ if ((prev == NULL) && !__tcp_write_queue_empty(sk, TCP_WQ_SACKED))
+ prev = __tcp_write_queue_tail(sk, TCP_WQ_SACKED);
+
+ if (prev != NULL)
+ fc = TCP_SKB_CB(prev)->fack_count + tcp_skb_pcount(prev);
+
+ TCP_SKB_CB(skb)->fack_count = fc;

sk->sk_send_head = tcp_write_queue_next(sk, skb);
if (sk->sk_send_head == (struct sk_buff *)&sk->sk_write_queue)
@@ -1464,7 +1474,7 @@ static inline struct sk_buff *__tcp_reset_fack_counts(struct sock *sk,
{
unsigned int fc = 0;

- if (prev == NULL)
+ if (prev != NULL)
fc = TCP_SKB_CB(*prev)->fack_count + tcp_skb_pcount(*prev);

BUG_ON((*prev != NULL) && !tcp_skb_adjacent(sk, *prev, skb));
@@ -1521,7 +1531,7 @@ static inline void tcp_reset_fack_counts(struct sock *sk, struct sk_buff *inskb)
skb[otherq] = prev->next;
}

- while (skb[queue] != __tcp_write_queue_tail(sk, queue)) {
+ do {
/* Lazy find for the other queue */
if (skb[queue] == NULL) {
skb[queue] = tcp_write_queue_find(sk, TCP_SKB_CB(prev)->seq,
@@ -1535,7 +1545,7 @@ static inline void tcp_reset_fack_counts(struct sock *sk, struct sk_buff *inskb)
break;

queue ^= TCP_WQ_SACKED;
- }
+ } while (skb[queue] != __tcp_write_queue_tail(sk, queue));
}

static inline void __tcp_insert_write_queue_after(struct sk_buff *skb,
--
1.5.0.6