netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: William Allen Simpson <william.allen.simpson@gmail.com>
Cc: Linux Kernel Developers <linux-kernel@vger.kernel.org>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>
Subject: Re: [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie
Date: Mon, 02 Nov 2009 18:42:02 +0100	[thread overview]
Message-ID: <4AEF19EA.6020003@gmail.com> (raw)
In-Reply-To: <4AEF1534.4090506@gmail.com>

William Allen Simpson a écrit :
> Eric Dumazet wrote:
>> Large part of network code is run by softirq handler, and a softirq
>> handler
>> is not preemptable with another softirq (including itself).
>>
> Thank you.  That's helpful to know, as some existing locks have a "bh".
> I've never figured out the ip_local_deliver_finish() context.
> 
> Knowing that there can only be one instance of the tcp stack running at
> any one time, and the cpu never changes even after being interrupted, will
> make it much easier to code.

Correction :

There is one instance of sofirq handler running per cpu.

So TCP (or UDP) stack can run simultaneously on several (and eventually all online) cpus.

This is why we still need some locks in various places.


>>
> (No, I've not yet added locks; obviously, I'm still asking about them.)
> 
> Unlikely, as it was easy to reproduce by changing one line, without
> *any* of
> my code present.  Usually works, but doesn't work with tcpdump running on
> the interface:

Yes, tcpdump has the nasty habit to consume a lot of ram, queuing a copy of
all network traffic on an af_packet socket. (or using a mmap buffer, it depends
on libpcap version you use)

> 
>  struct sock *tcp_create_openreq_child(struct sock *sk, struct
> request_sock *req, struct sk_buff *skb)
>  {
> -    struct sock *newsk = inet_csk_clone(sk, req, GFP_ATOMIC);
> +    struct sock *newsk = inet_csk_clone(sk, req, GFP_KERNEL);

Here you want a GFP_KERNEL allocation, that is allowed to sleep if there is not
enough available memory. (It's allowed to sleep to let some processes to free
bit of ram, eventually)

But as the caller of tcp_create_openreq_child() runs from {soft}irq context,
its not allowed to sleep at all, even 10 usecs.

Therefore, linux kernel kindly warns you that's its illegal and deadlockable.

Dont change GFP_ATOMIC here, its really there for a valid reason.
And be prepared to get a NULL result from allocation.

If your machine has problems when running tcpdump, maybe it lacks some ram, maybe you could
tune tcpdump socket to not exhaust all LOWMEM.
I see your kernel is 32bits, so you have only 860 MB of kernel memory, called LOWMEM.
I believe last kernels might have some problems in OOM situations... 

  reply	other threads:[~2009-11-02 17:42 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-30 11:00 [net-next-2.6 PATCH RFC] TCPCT part 1d: generate Responder Cookie William Allen Simpson
2009-10-30 18:11 ` William Allen Simpson
2009-11-01 13:01 ` William Allen Simpson
2009-11-01 18:03   ` Eric Dumazet
2009-11-02 10:39     ` William Allen Simpson
2009-11-02 10:50       ` David Miller
2009-11-02 10:56       ` Eric Dumazet
2009-11-02 12:36         ` William Allen Simpson
2009-11-02 13:16           ` Eric Dumazet
2009-11-02 17:21             ` William Allen Simpson
2009-11-02 17:42               ` Eric Dumazet [this message]
2009-11-03 22:38     ` William Allen Simpson
2009-11-03 23:03       ` Eric Dumazet
2009-11-04 21:48       ` Paul E. McKenney
2009-11-05 12:17         ` William Allen Simpson
2009-11-05 12:45           ` William Allen Simpson
2009-11-05 13:34             ` Eric Dumazet
2009-11-05 13:19           ` Eric Dumazet
2009-11-05 19:44             ` William Allen Simpson
2009-11-05 14:59           ` Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4AEF19EA.6020003@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=william.allen.simpson@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).