LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay.com>
To: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Cc: Glenn Griffin <ggriffin.kernel@gmail.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Andi Kleen <andi@firstfloor.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Add IPv6 support to TCP SYN cookies
Date: Thu, 07 Feb 2008 10:40:19 +0100	[thread overview]
Message-ID: <47AAD203.4030706@cosmosbay.com> (raw)
In-Reply-To: <20080207072415.GA13782@2ka.mipt.ru>

[-- Attachment #1: Type: text/plain, Size: 2349 bytes --]

Evgeniy Polyakov a écrit :
> On Wed, Feb 06, 2008 at 10:30:24AM -0800, Glenn Griffin (ggriffin.kernel@gmail.com) wrote:
>   
>>>> +static u32 cookie_hash(struct in6_addr *saddr, struct in6_addr *daddr,
>>>> +		       __be16 sport, __be16 dport, u32 count, int c)
>>>> +{
>>>> +	__u32 tmp[16 + 5 + SHA_WORKSPACE_WORDS];
>>>>         
>>> This huge buffer should not be allocated on stack.
>>>       
>> I can replace it will a kmalloc, but for my benefit what's the practical
>> size we try and limit the stack to?  It seemed at first glance to me
>> that 404 bytes plus the arguments, etc. was not such a large buffer for
>> a non-recursive function.  Plus the alternative with a kmalloc requires
>>     
>
> Well, maybe for connection establishment path it is not, but it is
> absolutely the case in the sending and sometimes receiving pathes for 4k
> stacks. The main problem is that bugs which happen because of stack
> overflow are so much obscure, that it is virtually impossible to detect
> where overflow happend. 'Debug stack overflow' somehow does not help to
> detect it.
>
> Usually there is about 1-1.5 kb of free stack for each process, so this
> change will cut one third of the free stack, getting into account that
> something can store ipv6 addresses on stack too, this can end up badly.
>
>   
>> propogating the possible error status back up to tcp_ipv6.c in the event
>> we are unable to allocate enough memory, so it can simply drop the
>> connection.  Not an impossible task by any means but it does
>> significantly complicate things and I would like to know it's worth the
>> effort.  Also would it be worth it to provide a supplemental patch for
>> the ipv4 implementation as it allocates the same buffer?
>>     
>
> One can reorganize syncookie support to work with request hash tables
> too, so that we could allocate per hash-bucket space and use it as a
> scratchpad for cookies.
>
>   
Or maybe use percpu storage for that...

I am not sure if cookie_hash() is always called with preemption disabled.
(If not, we have to use get_cpu_var()/put_cpu_var())

[NET] IPV4: lower stack usage in cookie_hash() function

400 bytes allocated on stack might be a litle bit too much. Using a 
per_cpu var is more friendly.

Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>



[-- Attachment #2: cookie_scratch.patch --]
[-- Type: text/plain, Size: 681 bytes --]

diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
index f470fe4..177da14 100644
--- a/net/ipv4/syncookies.c
+++ b/net/ipv4/syncookies.c
@@ -35,10 +35,12 @@ module_init(init_syncookies);
 #define COOKIEBITS 24	/* Upper bits store count */
 #define COOKIEMASK (((__u32)1 << COOKIEBITS) - 1)
 
+static DEFINE_PER_CPU(__u32, cookie_scratch)[16 + 5 + SHA_WORKSPACE_WORDS];
+
 static u32 cookie_hash(__be32 saddr, __be32 daddr, __be16 sport, __be16 dport,
 		       u32 count, int c)
 {
-	__u32 tmp[16 + 5 + SHA_WORKSPACE_WORDS];
+	__u32 *tmp = __get_cpu_var(cookie_scratch);
 
 	memcpy(tmp + 3, syncookie_secret[c], sizeof(syncookie_secret[c]));
 	tmp[0] = (__force u32)saddr;

  reply	other threads:[~2008-02-07  9:40 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-04 23:01 Glenn Griffin
2008-02-05 15:55 ` Andi Kleen
2008-02-05 15:42   ` Alan Cox
2008-02-05 16:39     ` Andi Kleen
2008-02-05 16:03       ` Alan Cox
2008-02-05 16:48         ` Andi Kleen
2008-02-05 16:14           ` Alan Cox
2008-02-05 20:50       ` Willy Tarreau
2008-02-05 18:29   ` Glenn Griffin
2008-02-05 19:25     ` Ross Vandegrift
2008-02-05 20:11       ` Andi Kleen
2008-02-05 21:23         ` Ross Vandegrift
2008-02-06  8:53           ` Andi Kleen
2008-02-07 19:44             ` Ross Vandegrift
2008-02-08 12:07               ` Andi Kleen
2008-02-12 20:38                 ` Ross Vandegrift
2008-02-05 20:02     ` Andi Kleen
2008-02-05 20:39       ` Evgeniy Polyakov
2008-02-05 20:53         ` Andi Kleen
2008-02-05 21:50           ` Evgeniy Polyakov
2008-02-05 21:20         ` Alan Cox
2008-02-05 21:52           ` Evgeniy Polyakov
2008-02-05 21:20             ` Willy Tarreau
2008-02-05 22:05             ` Alan Cox
2008-02-06  1:52               ` Glenn Griffin
2008-02-06  7:50                 ` Andi Kleen
2008-02-06 17:36                   ` Glenn Griffin
2008-02-06 18:45                     ` Andi Kleen
2008-02-06 23:03                       ` Glenn Griffin
2008-02-06  9:13                 ` Evgeniy Polyakov
2008-02-06 18:30                   ` Glenn Griffin
2008-02-07  7:24                     ` Evgeniy Polyakov
2008-02-07  9:40                       ` Eric Dumazet [this message]
2008-02-08  5:32                         ` Glenn Griffin
2008-02-08  5:49                           ` Glenn Griffin
2008-02-11 16:07                             ` YOSHIFUJI Hideaki / 吉藤英明
2008-02-18 23:45                               ` Glenn Griffin
2008-02-13  7:31                         ` YOSHIFUJI Hideaki / 吉藤英明
2008-02-05 19:57   ` Jan Engelhardt
2008-02-05 21:25     ` Alan Cox
2008-02-04 23:01 Glenn Griffin

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=47AAD203.4030706@cosmosbay.com \
    --to=dada1@cosmosbay.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=andi@firstfloor.org \
    --cc=ggriffin.kernel@gmail.com \
    --cc=johnpol@2ka.mipt.ru \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH] Add IPv6 support to TCP SYN cookies' \
    /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

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).