| Message ID | 4AFCE13B.4060006@gmail.com |
|---|---|
| State | RFC, archived |
| Headers | show |
On Thu, 2009-11-12 at 23:31 -0500, William Allen Simpson wrote: > Define sysctl (tcp_cookie_size) to turn on and off the cookie option > default globally, instead of a compiled configuration option. [] > +#define TCP_COOKIE_MIN 8 /* 64-bits */ > +#define TCP_COOKIE_MAX 16 /* 128-bits */ perhaps something like: static const int TCP_COOKIE_MIN = 8; static const int TCP_COOKIE_MAX = 16; [] > --- a/net/ipv4/sysctl_net_ipv4.c > +++ b/net/ipv4/sysctl_net_ipv4.c > @@ -714,6 +714,14 @@ static struct ctl_table ipv4_table[] = { > }, > { > .ctl_name = CTL_UNNUMBERED, > + .procname = "tcp_cookie_size", > + .data = &sysctl_tcp_cookie_size, > + .maxlen = sizeof(int), > + .mode = 0644, > + .proc_handler = proc_dointvec with proc_dointvec_minmax .extra1 = &TCP_COOKIE_MIN, .extra2 = &TCP_COOKIE_MAX, or even adding proc_dointvec_minmax_even might save some cycles during cookie handling. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Joe Perches wrote: > On Thu, 2009-11-12 at 23:31 -0500, William Allen Simpson wrote: >> Define sysctl (tcp_cookie_size) to turn on and off the cookie option >> default globally, instead of a compiled configuration option. > [] > >> +#define TCP_COOKIE_MIN 8 /* 64-bits */ >> +#define TCP_COOKIE_MAX 16 /* 128-bits */ > > perhaps something like: > > static const int TCP_COOKIE_MIN = 8; > static const int TCP_COOKIE_MAX = 16; > > [] > >> --- a/net/ipv4/sysctl_net_ipv4.c >> +++ b/net/ipv4/sysctl_net_ipv4.c >> @@ -714,6 +714,14 @@ static struct ctl_table ipv4_table[] = { >> }, >> { >> .ctl_name = CTL_UNNUMBERED, >> + .procname = "tcp_cookie_size", >> + .data = &sysctl_tcp_cookie_size, >> + .maxlen = sizeof(int), >> + .mode = 0644, >> + .proc_handler = proc_dointvec > > with proc_dointvec_minmax > > .extra1 = &TCP_COOKIE_MIN, > .extra2 = &TCP_COOKIE_MAX, > > or even adding proc_dointvec_minmax_even > might save some cycles during cookie handling. > Well, that would have to be proc_dointvec_minmax_even_zero(), as the valid values can be 0, 8, 10, 12, 14, or 16. But my guess (based on experience owning an ISP and teaching student operators) is that folks will remember to turn it off (0) or on (1) or "bigger" (255), and not really care about the finer gradations. Error messages have a tendency to scroll off the screen and be forgotten. And it seems to me a case of "premature optimization" -- it might save 1 or 2 tests in the order I've listed them in tcp_cookie_size_check(), those tests only happen on the SYN the first time the _client_ calls, and the result is saved for the future. See part 1f: + u8 cookie_size = (!tp->rx_opt.cookie_out_never && cvp != NULL) + ? tcp_cookie_size_check(cvp->cookie_desired) + : 0; [] + /* Remember for future incarnations. */ + cvp->cookie_desired = cookie_size; On the server side, we have to test during parsing anyway. I never trust any data that comes over the network.... -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
William Allen Simpson wrote: > Joe Perches wrote: >> or even adding proc_dointvec_minmax_even >> might save some cycles during cookie handling. >> > Well, that would have to be proc_dointvec_minmax_even_zero(), as the > valid values can be 0, 8, 10, 12, 14, or 16. ... > > And it seems to me a case of "premature optimization" -- it might save > 1 or 2 tests in the order I've listed them in tcp_cookie_size_check(), > ... > > On the server side, we have to test during parsing anyway. I never trust > any data that comes over the network.... > I looked at this again today, and proc_dointvec_minmax_even_zero() still seems to be overkill. I couldn't find anybody else that does such things at sysctl parsing time. I did find we could eliminate a test for odd (in part 1f), + if (unlikely(0x1 & cookie_size)) { + /* 8-bit multiple, illegal, ignore */ + cookie_size = 0; by using a better test for the network data (in part 1g), and it should be faster, too (assuming gcc is smart enough): Was: + if (TCPOLEN_COOKIE_MAX >= opsize + && TCPOLEN_COOKIE_MIN <= opsize) { Now: + switch (opsize) { + case TCPOLEN_COOKIE_MIN+0: + case TCPOLEN_COOKIE_MIN+2: + case TCPOLEN_COOKIE_MIN+4: + case TCPOLEN_COOKIE_MIN+6: + case TCPOLEN_COOKIE_MAX: (This division into so many parts for review is driving me crazy....) Saved 2 if's for every cookie! Of course, there's a cpu intensive SHA1 for each cookie, so this pales in comparison. Premature optimization or not, thanks for the ideas! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt index a0e134d..d47c000 100644 --- a/Documentation/networking/ip-sysctl.txt +++ b/Documentation/networking/ip-sysctl.txt @@ -164,6 +164,14 @@ tcp_congestion_control - STRING additional choices may be available based on kernel configuration. Default is set as part of kernel configuration. +tcp_cookie_size - INTEGER + Default size of TCP Cookie Transactions (TCPCT) option, that may be + overridden on a per socket basis by the TCPCT socket option. + Values greater than the maximum (16) are interpreted as the maximum. + Values greater than zero and less than the minimum (8) are interpreted + as the minimum. + Default: 0 (off). + tcp_dsack - BOOLEAN Allows TCP to send "duplicate" SACKs. diff --git a/include/linux/tcp.h b/include/linux/tcp.h index 32d7d77..eaa3113 100644 --- a/include/linux/tcp.h +++ b/include/linux/tcp.h @@ -102,7 +102,9 @@ enum { #define TCP_QUICKACK 12 /* Block/reenable quick acks */ #define TCP_CONGESTION 13 /* Congestion control algorithm */ #define TCP_MD5SIG 14 /* TCP MD5 Signature (RFC2385) */ +#define TCP_COOKIE_TRANSACTIONS 15 /* TCP Cookie Transactions */ +/* for TCP_INFO socket option */ #define TCPI_OPT_TIMESTAMPS 1 #define TCPI_OPT_SACK 2 #define TCPI_OPT_WSCALE 4 @@ -174,6 +176,30 @@ struct tcp_md5sig { __u8 tcpm_key[TCP_MD5SIG_MAXKEYLEN]; /* key (binary) */ }; +/* for TCP_COOKIE_TRANSACTIONS (TCPCT) socket option */ +#define TCP_COOKIE_MIN 8 /* 64-bits */ +#define TCP_COOKIE_MAX 16 /* 128-bits */ +#define TCP_COOKIE_PAIR_SIZE (2*TCP_COOKIE_MAX) + +/* Flags for both getsockopt and setsockopt */ +#define TCP_COOKIE_IN_ALWAYS (1 << 0) /* Discard SYN without cookie */ +#define TCP_COOKIE_OUT_NEVER (1 << 1) /* Prohibit outgoing cookies, + * supercedes everything. */ + +/* Flags for getsockopt */ +#define TCP_S_DATA_IN (1 << 2) /* Was data received? */ +#define TCP_S_DATA_OUT (1 << 3) /* Was data sent? */ + +/* TCP_COOKIE_TRANSACTIONS data */ +struct tcp_cookie_transactions { + __u16 tcpct_flags; /* see above */ + __u8 __tcpct_pad1; /* zero */ + __u8 tcpct_cookie_desired; /* bytes */ + __u16 tcpct_s_data_desired; /* bytes of variable data */ + __u16 tcpct_used; /* bytes in value */ + __u8 tcpct_value[TCP_MSS_DEFAULT]; +}; + #ifdef __KERNEL__ #include <linux/skbuff.h> @@ -227,6 +253,11 @@ struct tcp_options_received { u16 mss_clamp; /* Maximal mss, negotiated at connection setup */ }; +static inline void tcp_clear_options(struct tcp_options_received *rx_opt) +{ + rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0; +} + /* This is the max number of SACKS that we'll generate and process. It's safe * to increse this, although since: * size = TCPOLEN_SACK_BASE_ALIGNED (4) + n * TCPOLEN_SACK_PERBLOCK (8) @@ -435,6 +466,6 @@ static inline struct tcp_timewait_sock *tcp_twsk(const struct sock *sk) return (struct tcp_timewait_sock *)sk; } -#endif +#endif /* __KERNEL__ */ #endif /* _LINUX_TCP_H */ diff --git a/include/net/tcp.h b/include/net/tcp.h index 4a99a8e..738b65f 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -234,6 +234,7 @@ extern int sysctl_tcp_base_mss; extern int sysctl_tcp_workaround_signed_windows; extern int sysctl_tcp_slow_start_after_idle; extern int sysctl_tcp_max_ssthresh; +extern int sysctl_tcp_cookie_size; extern atomic_t tcp_memory_allocated; extern struct percpu_counter tcp_sockets_allocated; @@ -340,11 +341,6 @@ static inline void tcp_dec_quickack_mode(struct sock *sk, extern void tcp_enter_quickack_mode(struct sock *sk); -static inline void tcp_clear_options(struct tcp_options_received *rx_opt) -{ - rx_opt->tstamp_ok = rx_opt->sack_ok = rx_opt->wscale_ok = rx_opt->snd_wscale = 0; -} - #define TCP_ECN_OK 1 #define TCP_ECN_QUEUE_CWR 2 #define TCP_ECN_DEMAND_CWR 4 diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index 2dcf04d..3422c54 100644 --- a/net/ipv4/sysctl_net_ipv4.c +++ b/net/ipv4/sysctl_net_ipv4.c @@ -714,6 +714,14 @@ static struct ctl_table ipv4_table[] = { }, { .ctl_name = CTL_UNNUMBERED, + .procname = "tcp_cookie_size", + .data = &sysctl_tcp_cookie_size, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_dointvec + }, + { + .ctl_name = CTL_UNNUMBERED, .procname = "udp_mem", .data = &sysctl_udp_mem, .maxlen = sizeof(sysctl_udp_mem), diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 1151cb8..e59fa5a 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -59,6 +59,14 @@ int sysctl_tcp_base_mss __read_mostly = 512; /* By default, RFC2861 behavior. */ int sysctl_tcp_slow_start_after_idle __read_mostly = 1; +#ifdef CONFIG_SYSCTL +/* By default, let the user enable it. */ +int sysctl_tcp_cookie_size __read_mostly = 0; +#else +int sysctl_tcp_cookie_size __read_mostly = TCP_COOKIE_MAX; +#endif + + /* Account for new data that has been sent to the network. */ static void tcp_event_new_data_sent(struct sock *sk, struct sk_buff *skb) {
Define sysctl (tcp_cookie_size) to turn on and off the cookie option default globally, instead of a compiled configuration option. Define per socket option (TCP_COOKIE_TRANSACTIONS) for setting constant data values, retrieving variable cookie values, and other facilities. Move inline tcp_clear_options() unchanged from net/tcp.h to linux/tcp.h, near its corresponding struct tcp_options_received (prior to changes). This is a straightforward re-implementation of an earlier (year-old) patch that no longer applies cleanly, with permission of the original author (Adam Langley): http://thread.gmane.org/gmane.linux.network/102586 These functions will also be used in subsequent patches that implement additional features. Requires: net: TCP_MSS_DEFAULT, TCP_MSS_DESIRED Signed-off-by: William.Allen.Simpson@gmail.com --- Documentation/networking/ip-sysctl.txt | 8 +++++++ include/linux/tcp.h | 33 +++++++++++++++++++++++++++++++- include/net/tcp.h | 6 +---- net/ipv4/sysctl_net_ipv4.c | 8 +++++++ net/ipv4/tcp_output.c | 8 +++++++ 5 files changed, 57 insertions(+), 6 deletions(-)