Re: 2.4.4: Kernel crash, possibly tcp related

[Andrea Arcangeli <andrea@suse.de>]

On Tue, May 01, 2001 at 08:44:52PM +0400, kuznet@ms2.inr.ac.ru wrote:
> Hello!
> 
> > this is the strict fix:
> 
> Andrea, you caught the problem!
> 
> The fix is not right though (it is equivalent to straight
> tp->send_head=NULL, as you noticed. It also corrupts queue in
> an opposite manner.) Right fix is appended.
> 
> Explanation: in do_fault we must undo effect of enqueueing new segment
> in the case the segment remained empty. tp->send_head points to
> the first unsent skb in queue and it is NULL when and only when
> all the skbs are already sent. (Invariant is: tp->send_head==NULL ||
> tp->send_head->seq == tp->snd_nxt)
> I crapped this case except for the case when queue is completely empty,
> so that the last sent skb was accounted in packets_out twice...

I understsand the explanation but I don't think my patch is wrong, I
think it's simpler and faster instead.

My argument is very simple, if send_head points to skb and skb->len is
zero and we are running in such slow path, it is obvious the send_head
_was_ NULL when we entered the critical section, so it's perfectly fine
to set send_head back to null and to unlink the skb as the only actions
to undo the skb_entail. That's all. I don't see how my patch can fail.
If I'm missing something I'd love a further explanation indeed. Thanks!

> 
> Damn, what a silly mistake was it... shame.
> 
> Alexey
> 
> 
> --- ../vger3-010426/linux/net/ipv4/tcp.c	Wed Apr 25 21:02:18 2001
> +++ linux/net/ipv4/tcp.c	Tue May  1 20:38:44 2001
> @@ -1185,7 +1187,7 @@
>  	if (skb->len==0) {
>  		if (tp->send_head == skb) {
>  			tp->send_head = skb->prev;
> -			if (tp->send_head == (struct sk_buff*)&sk->write_queue)
> +			if (TCP_SKB_CB(skb)->seq == tp->snd_nxt)
>  				tp->send_head = NULL;
>  		}
>  		__skb_unlink(skb, skb->list);


Andrea