lkml.org 
[lkml]   [2008]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] Add IPv6 support to TCP SYN cookies
    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>


    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;
    \
     
     \ /
      Last update: 2008-02-07 10:43    [W:4.251 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site