Date:	Tue, 10 Nov 2009 15:29:39 +0100
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	William Allen Simpson <william.allen.simpson@...il.com>
CC:	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>,
	Joe Perches <joe@...ches.com>
Subject: Re: [net-next-2.6 PATCH v5 5/5 RFC] TCPCT part1e: initial SYN exchange
 with SYNACK data

William Allen Simpson a écrit :
> This is a significantly revised implementation of an earlier (year-old)
> patch that no longer applies cleanly, with permission of the original
> author (Adam Langley).  That patch was previously reviewed:
> 
>    http://thread.gmane.org/gmane.linux.network/102586
> 
> The principle difference is using a TCP option to carry the cookie nonce,
> instead of a user configured offset in the data.  This is more flexible and
> less subject to user configuration error.  Such a cookie option has been
> suggested for many years, and is also useful without SYN data, allowing
> several related concepts to use the same extension option.
> 
>    "Re: SYN floods (was: does history repeat itself?)", September 9, 1996.
>    http://www.merit.net/mail.archives/nanog/1996-09/msg00235.html
> 
>    "Re: what a new TCP header might look like", May 12, 1998.
>    ftp://ftp.isi.edu/end2end/end2end-interest-1998.mail
> 
> Data structures are carefully composed to require minimal additions.
> For example, the struct tcp_options_received cookie_plus variable fits
> between existing 16-bit and 8-bit variables, requiring no additional
> space (taking alignment into consideration).  There are no additions to
> tcp_request_sock, and only 1 pointer in tcp_sock.
> 
> Allocations have been rearranged to avoid requiring GFP_ATOMIC, with
> only one unavoidable exception in tcp_create_openreq_child(), where the
> tcp_sock itself is created GFP_ATOMIC.
> 
> These functions will also be used in subsequent patches that implement
> additional features.
> 
> Requires:
>   TCPCT part 1a: add request_values parameter for sending SYNACK
>   TCPCT part 1b: TCP_MSS_DEFAULT, TCP_MSS_DESIRED
>   TCPCT part 1c: sysctl_tcp_cookie_size, socket option
> TCP_COOKIE_TRANSACTIONS, functions
>   TCPCT part 1d: generate Responder Cookie
> 
> Signed-off-by: William.Allen.Simpson@...il.com
> ---
>  include/linux/tcp.h      |   29 ++++-
>  include/net/tcp.h        |   72 +++++++++++++
>  net/ipv4/syncookies.c    |    5 +-
>  net/ipv4/tcp.c           |  127 ++++++++++++++++++++++-
>  net/ipv4/tcp_input.c     |   86 +++++++++++++--
>  net/ipv4/tcp_ipv4.c      |   69 ++++++++++++-
>  net/ipv4/tcp_minisocks.c |   59 ++++++++---
>  net/ipv4/tcp_output.c    |  255
> +++++++++++++++++++++++++++++++++++++++++-----
>  net/ipv6/syncookies.c    |    5 +-
>  net/ipv6/tcp_ipv6.c      |   65 +++++++++++-
>  10 files changed, 701 insertions(+), 71 deletions(-)
> 

I really tried hard to understand what was going on, and failed, because I dont
have much time these days...

Lack of documentation maybe ? Some DATA flow could help...

Please please, cook up elementatry patches to perform one action at a time,
even if they are not fully functionnal ?

One patch to be able to send SYN + COOKIE (if we are the client)

One patch to be able to receive this SYN + COOKIE and answer a SYNACK + cookies (we are the server)

One patch to ... Receive the ACK from client (if we are the server) and check cookies

One patch to ...  Send the ACK (if we are the client)

Patches to receive FIN + cookies

One patch to ... enable the whole thing and setsockopt() bits

That way we could review your patches step by step, and not 770 lines in one block.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists