Prev: [git pull] Please pull powerpc.git merge branch
Next: msi-wmi: make needlessly global symbols static
From: Ilpo Järvinen on 11 Jul 2010 15:30
On Sun, 11 Jul 2010, Eric Dumazet wrote:

> Le dimanche 11 juillet 2010 à 19:46 +0200, Eric Dumazet a écrit :
> > Le dimanche 11 juillet 2010 à 19:06 +0200, Eric Dumazet a écrit :
> > > Le dimanche 11 juillet 2010 à 19:09 +0300, Ilpo Järvinen a écrit :
> > > > On Thu, 8 Jul 2010, Tejun Heo wrote:
> > > >
> > > > > We've been seeing oops in tcp_xmit_retransmit_queue() w/ 2.6.32.15.
> > > > > Please see the attached photoshoot. This is happening on a HPC
> > > > > cluster and very interestingly caused by one particular job. How long
> > > > > it takes isn't clear yet (at least more than a day) but when it
> > > > > happens it happens on a lot of machines in relatively short time.
> > > > >
> > > > > With a bit of disassemblying, I've found that the oops is happening
> > > > > during tcp_for_write_queue_from() because the skb->next points to
> > > > > NULL.
> > > > >
> > > > > void tcp_xmit_retransmit_queue(struct sock *sk)
> > > > > {
> > > > > ...
> > > > > if (tp->retransmit_skb_hint) {
> > > > > skb = tp->retransmit_skb_hint;
> > > > > last_lost = TCP_SKB_CB(skb)->end_seq;
> > > > > if (after(last_lost, tp->retransmit_high))
> > > > > last_lost = tp->retransmit_high;
> > > > > } else {
> > > > > skb = tcp_write_queue_head(sk);
> > > > > last_lost = tp->snd_una;
> > > > > }
> > > > >
> > > > > => tcp_for_write_queue_from(skb, sk) {
> > > > > __u8 sacked = TCP_SKB_CB(skb)->sacked;
> > > > >
> > > > > if (skb == tcp_send_head(sk))
> > > > > break;
> > > > > /* we could do better than to assign each time */
> > > > > if (hole == NULL)
> > > > >
> > > > > This can happen for one of the following reasons,
> > > > >
> > > > > 1. tp->retransmit_skb_hint is NULL and tcp_write_queue_head() is NULL
> > > > > too. ie. tcp_xmit_retransmit_queue() is called on an empty write
> > > > > queue for some reason.
> > > > >
> > > > > 2. tp->retransmit_skb_hint is pointing to a skb which is not on the
> > > > > write_queue. ie. somebody forgot to update hint while removing the
> > > > > skb from the write queue.
> > > >
> > > > Once again I've read the unlinkers through, and only thing that could
> > > > cause this is tcp_send_synack (others do deal with the hints) but I think
> > > > Eric already proposed a patch to that but we never got anywhere due to
> > > > some counterargument why it wouldn't take place (too far away for me to
> > > > remember, see archives about the discussions). ...But if you want be dead
> > > > sure some WARN_ON there might not hurt. Also the purging of the whole
> > > > queue was a similar suspect I then came across (but that would only
> > > > materialize with sk reuse happening e.g., with nfs which the other guys
> > > > weren't using).
> > > >
> > >
> > > Hmm.
> > >
> > > This sounds familiar to me, but I cannot remember the discussion you
> > > mention or the patch.
> > >
> > > Or maybe it was the TCP transaction thing ? (including data in SYN or
> > > SYN-ACK packet)

No. That's another thing. ...I've already found it with google today but
cannot seem to find it again. I thought I used tcp_make_synack eric but
for some reason I only get these transaction fix hits. I'll keep looking.

> > Hmm, I cannot find where we reset restransmit_skb_hint in
> > tcp_mtu_probe(), if we call tcp_unlink_write_queue().
> >
> > if (skb->len <= copy) {
> > /* We've eaten all the data from this skb.
> > * Throw it away. */
> > TCP_SKB_CB(nskb)->flags |= TCP_SKB_CB(skb)->flags;
> > <<>> tcp_unlink_write_queue(skb, sk);
> > sk_wmem_free_skb(sk, skb);
> > } else {
> >
> >
> > Sorry if this was already discussed. We might add a comment here in anycase ;)
> >
>
> Just in case, here is a patch for this issue, if Tejun wants to try it.
>
> Thanks
>
> [PATCH] tcp: tcp_mtu_probe() and retransmit hints
>
> When removing an skb from tcp write queue, we must take care of various
> hints that could be kept on this skb. tcp_mtu_probe() misses this
> cleanup.
>
> lkml reference : http://lkml.org/lkml/2010/7/8/63
>
> Reported-by: Tejun Heo <tj(a)kernel.org>
> Signed-off-by: Eric Dumazet <eric.dumazet(a)gmail.com>
> ---
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index b4ed957..187453f 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1666,6 +1666,9 @@ static int tcp_mtu_probe(struct sock *sk)
> * Throw it away. */
> TCP_SKB_CB(nskb)->flags |= TCP_SKB_CB(skb)->flags;
> tcp_unlink_write_queue(skb, sk);
> + tcp_clear_retrans_hints_partial(tp);
> + if (skb == tp->retransmit_skb_hint)
> + tp->retransmit_skb_hint = nskb;

....I think we've not sent that skb just yet, so this'll never be true (and
the rexmit loop terminates at send_head without setting it so we should
be safe, I'll still need to check that no other code can accidently move
it to the send_head but I doubt it happens).

> sk_wmem_free_skb(sk, skb);
> } else {
> TCP_SKB_CB(nskb)->flags |= TCP_SKB_CB(skb)->flags &
>
>
>

--
i.
From: Ilpo Järvinen on 11 Jul 2010 15:50
On Sun, 11 Jul 2010, Ilpo Järvinen wrote:

> On Sun, 11 Jul 2010, Ilpo Järvinen wrote:
>
> > On Sun, 11 Jul 2010, Eric Dumazet wrote:
> >
> > > Le dimanche 11 juillet 2010 à 19:46 +0200, Eric Dumazet a écrit :
> > > > Le dimanche 11 juillet 2010 à 19:06 +0200, Eric Dumazet a écrit :
> > > > > Le dimanche 11 juillet 2010 à 19:09 +0300, Ilpo Järvinen a écrit :
> > > > > > On Thu, 8 Jul 2010, Tejun Heo wrote:
> > > > > >
> > > > > > > We've been seeing oops in tcp_xmit_retransmit_queue() w/ 2.6.32.15.
> > > > > > > Please see the attached photoshoot. This is happening on a HPC
> > > > > > > cluster and very interestingly caused by one particular job. How long
> > > > > > > it takes isn't clear yet (at least more than a day) but when it
> > > > > > > happens it happens on a lot of machines in relatively short time.
> > > > > > >
> > > > > > > With a bit of disassemblying, I've found that the oops is happening
> > > > > > > during tcp_for_write_queue_from() because the skb->next points to
> > > > > > > NULL.
> > > > > > >
> > > > > > > void tcp_xmit_retransmit_queue(struct sock *sk)
> > > > > > > {
> > > > > > > ...
> > > > > > > if (tp->retransmit_skb_hint) {
> > > > > > > skb = tp->retransmit_skb_hint;
> > > > > > > last_lost = TCP_SKB_CB(skb)->end_seq;
> > > > > > > if (after(last_lost, tp->retransmit_high))
> > > > > > > last_lost = tp->retransmit_high;
> > > > > > > } else {
> > > > > > > skb = tcp_write_queue_head(sk);
> > > > > > > last_lost = tp->snd_una;
> > > > > > > }
> > > > > > >
> > > > > > > => tcp_for_write_queue_from(skb, sk) {
> > > > > > > __u8 sacked = TCP_SKB_CB(skb)->sacked;
> > > > > > >
> > > > > > > if (skb == tcp_send_head(sk))
> > > > > > > break;
> > > > > > > /* we could do better than to assign each time */
> > > > > > > if (hole == NULL)
> > > > > > >
> > > > > > > This can happen for one of the following reasons,
> > > > > > >
> > > > > > > 1. tp->retransmit_skb_hint is NULL and tcp_write_queue_head() is NULL
> > > > > > > too. ie. tcp_xmit_retransmit_queue() is called on an empty write
> > > > > > > queue for some reason.
> > > > > > >
> > > > > > > 2. tp->retransmit_skb_hint is pointing to a skb which is not on the
> > > > > > > write_queue. ie. somebody forgot to update hint while removing the
> > > > > > > skb from the write queue.
> > > > > >
> > > > > > Once again I've read the unlinkers through, and only thing that could
> > > > > > cause this is tcp_send_synack (others do deal with the hints) but I think
> > > > > > Eric already proposed a patch to that but we never got anywhere due to
> > > > > > some counterargument why it wouldn't take place (too far away for me to
> > > > > > remember, see archives about the discussions). ...But if you want be dead
> > > > > > sure some WARN_ON there might not hurt. Also the purging of the whole
> > > > > > queue was a similar suspect I then came across (but that would only
> > > > > > materialize with sk reuse happening e.g., with nfs which the other guys
> > > > > > weren't using).
> > > > > >
> > > > >
> > > > > Hmm.
> > > > >
> > > > > This sounds familiar to me, but I cannot remember the discussion you
> > > > > mention or the patch.
> > > > >
> > > > > Or maybe it was the TCP transaction thing ? (including data in SYN or
> > > > > SYN-ACK packet)
> >
> > No. That's another thing. ...I've already found it with google today but
> > cannot seem to find it again. I thought I used tcp_make_synack eric but
> > for some reason I only get these transaction fix hits. I'll keep looking.
>
> Right, this one:
>
> http://kerneltrap.org/mailarchive/linux-netdev/2009/10/29/6259073

Hmm, another idea... It might be useful to try to disable
tcp_retrans_try_collapse in tcp_retransmit_skb as a test. I think that it
might be possible that tcp_xmit_retransmit_queue might end up holding
a stale reference either in hole or skb. Kind of shot into the dark still,
no actual theory on how that could happen but that
tcp_xmit_retransmit_queue logic is rather tricky because of returning to
the first hole if such exists so that I couldn't immediately rule out the
possibility.

--
i.
From: Eric Dumazet on 15 Jul 2010 08:10
Le jeudi 15 juillet 2010 à 13:58 +0200, Lennart Schulte a écrit :
> I'm testing new reordering algorithms in a virtual testbed, that is the
> nodes are emulated with xen and all the network parameters can be tuned
> with queues.
> With one of the algorithms I also got tracebacks which include
> tcp_xmit_retransmit_queue. It only happens with ACK loss. The kernel
> version however is 2.6.31.
> When I read this thread I tried the debug patch and got the following:
>
> [ 2754.413150] NULL head, pkts 0
> [ 2754.413156] Errors caught so far 1
>
> Hope that is of any help.

Not sure I understand.

Are you saying you reproduce same tcp_xmit_retransmit_queue() bug, with
a special tc qdisc/class droppping some ACK frames ?

Could it be some sched problem and incorrect return codes in case of
congestion ?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Lennart Schulte on 15 Jul 2010 08:10
I'm testing new reordering algorithms in a virtual testbed, that is the
nodes are emulated with xen and all the network parameters can be tuned
with queues.
With one of the algorithms I also got tracebacks which include
tcp_xmit_retransmit_queue. It only happens with ACK loss. The kernel
version however is 2.6.31.
When I read this thread I tried the debug patch and got the following:

[ 2754.413150] NULL head, pkts 0
[ 2754.413156] Errors caught so far 1

Hope that is of any help.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Lennart Schulte on 15 Jul 2010 09:00
Since tcp_xmit_retransmit_queue also gets skb == NULL I'm pretty sure it
is the same bug.
Up to now I only experienced the problem with ACK loss (without ACK loss
the test ran about 30min without problems, with ACK loss it had paniced
within 10min).
The data sender only has a HTB queue for traffic shaping (set to 20
Mbit/s). The ACK loss is done by another router.
The setup looks like this. This way it seems to be the most realistic.

o sender with HTB
|
|
o netem queue for forward path delay
|
o netem queue for a queue limit
|
o netem queue for backward path delay
|
o netem queue for ACK loss
|
|
o receiver with HTB

Perhaps now it is a little big clearer.


On 15.07.2010 14:05, Eric Dumazet wrote:
> Le jeudi 15 juillet 2010 à 13:58 +0200, Lennart Schulte a écrit :
>
>> I'm testing new reordering algorithms in a virtual testbed, that is the
>> nodes are emulated with xen and all the network parameters can be tuned
>> with queues.
>> With one of the algorithms I also got tracebacks which include
>> tcp_xmit_retransmit_queue. It only happens with ACK loss. The kernel
>> version however is 2.6.31.
>> When I read this thread I tried the debug patch and got the following:
>>
>> [ 2754.413150] NULL head, pkts 0
>> [ 2754.413156] Errors caught so far 1
>>
>> Hope that is of any help.
>>
> Not sure I understand.
>
> Are you saying you reproduce same tcp_xmit_retransmit_queue() bug, with
> a special tc qdisc/class droppping some ACK frames ?
>
> Could it be some sched problem and incorrect return codes in case of
> congestion ?
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: [git pull] Please pull powerpc.git merge branch
Next: msi-wmi: make needlessly global symbols static