So? And what does it more? They are equivalent, just that skb_queue_len is faster
(does not jump). But remember that 2.2 is critical bug fixes only, a unnecessary
jump is not a critical bug (actually once egcs will use CMOV again there will be
no jump anymore on x86, but skb_queue_len will be still faster)
>
> >
> >> }
> >> } else if (TCP_SKB_CB(skb)->ack_seq == tp->snd_una &&
> >> (sock_rspace(sk) ||
> >> - (!skb_queue_len(&sk->receive_queue) &&
> >> - !skb_queue_len(&tp->out_of_order_queue)))) {
> >> + !skb_queue_len(&sk->receive_queue))) {
> >> /* Bulk data transfer: receiver */
> >> __skb_pull(skb,th->doff*4);
> >
> >Still the unnecessary function call?
> >
> >!sock_rspace(sk) is equivalent to atomic_read(&rmem_alloc) <= sk->rcvbuf,
> >except that it is much slower.
>
> It produced cleaner code (more easy to read) according to me. But OK I
> agree with you.
>
> >I also think the change is wrong: such a deadlock you're trying to avoid
> >here is unusual. Unusual things are not handled in the fast path. If you
>
> I could agree to go in the slow path but it looks like the right place to
> put it according to me. Every time you see that the rcvbuf is empty you
> should also check that you are not going to deadlock before reject new
> incoming packets.
The basic idea behind header prediction is that you only turn it on if everything
is normal. If anything unnormal happens you turn header prediction off (pred_flags = 0)
and let it handle in the slow path. So for me it definitely does not look like the
right place for such a check.
Other than that I think doing these costly hack just to cope with too small receive
buffers is a bad idea. Just make sure the receive buffer is big enough
<tongue in cheek>
if the user makes it too small it is his fault ("It hurts when I do that. Then don't do that.")
Also as Alexey noted a way to make the TCP stack drop all new incoming packets is sometimes
useful.</>
ok, I know that 2.2 is not the right place for such experiments, best fix to me looks like to
always force a reasonable minimum buffer size.
> >Better make sure that rcvbuf is never < skb->truesize (which has the
> >advantage that it doesn't cost anything in critical paths too). Best would
> >be to make sure that the rcvbuf of every socket is always > maxmtu_of_devices
> >+ skb header size, but that is impractical. I think 4096 is good minimum value
> >and the ATM and HIPPIE people can tune it.
>
> What if I'll change the MTU during a transer? I just thought at the way
> you are proposing and it simply doesn't look like the right way to fix the
> bug to me.
I do not consider it a problem that needs fixing in 2.2. For common setups
(Ethernet, FDDI, ...) a minimum value of 4096 or 8192 should work reasonably,
for extreme setups with big MTUs (HIPPI, IP over ATM) this could be tuned - perhaps
by a sysctl.
-Andi
-- This is like TV. I don't like TV.- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/