Re: [Bug #14301] WARNING: at net/ipv4/af_inet.c:154

Eric Dumazet
2009-10-07T15:41:50Z

Eric Dumazet a écrit :
> Eric Dumazet a écrit :
>> Eric Dumazet a écrit :
>>> Rafael J. Wysocki a écrit :
>>>> This message has been generated automatically as a part of a report
>>>> of regressions introduced between 2.6.30 and 2.6.31.
>>>>
>>>> The following bug entry is on the current list of known regressions
>>>> introduced between 2.6.30 and 2.6.31. Please verify if it still should
>>>> be listed and let me know (either way).
>>>>
>>>>
>>>> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=14301
>>>> Subject : WARNING: at net/ipv4/af_inet.c:154
>>>> Submitter : Ralf Hildebrandt <Ralf.Hildebrandt-jq1tPX9l7E6ELgA04lAiVw@public.gmane.org>
>>>> Date : 2009-09-30 12:24 (2 days old)
>>>> References : http://marc.info/?l=linux-kernel&m=125431350218137&w=4
>>>>
>
> Investigation still needed...
>

OK, my last (buggy ???) feeling is about commit 95766fff6b9a78d1

[UDP]: Add memory accounting.

(Its a two years old patch, oh well...)

Problem is the udp_poll() :

We check the first frame to be dequeued from sk_receive_queue has a good checksum.

If it doesnt, we drop the frame ( calling kfree_skb(skb); )

Problem is now we perform memory accounting on UDP, this kfree_skb()
should be done with socket locked, but we are allowed to
call lock_sock() from this udp_poll() context

unsigned int udp_poll(struct file *file, struct socket *sock, poll_table *wait)
{
unsigned int mask = datagram_poll(file, sock, wait);
struct sock *sk = sock->sk;
int is_lite = IS_UDPLITE(sk);

/* Check for false positives due to checksum errors */
if ((mask & POLLRDNORM) &&
!(file->f_flags & O_NONBLOCK) &&
!(sk->sk_shutdown & RCV_SHUTDOWN)) {
struct sk_buff_head *rcvq = &sk->sk_receive_queue;
struct sk_buff *skb;

spin_lock_bh(&rcvq->lock);
while ((skb = skb_peek(rcvq)) != NULL &&
udp_lib_checksum_complete(skb)) {
UDP_INC_STATS_BH(sock_net(sk),
UDP_MIB_INERRORS, is_lite);
__skb_unlink(skb, rcvq);
<<HERE>> kfree_skb(skb);
}
spin_unlock_bh(&rcvq->lock);

David, Herbert, any idea how to solve this problem ?

1) Allow false positives

Or

2) Maybe we should finally convert sk_forward_alloc to an atomic_t after all...
This would make things easier, and speedup UDP (no more need to lock_sock())

Or

3) ???