netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: William Allen Simpson <william.allen.simpson@gmail.com>
To: Joe Perches <joe@perches.com>
Cc: Linux Kernel Network Developers <netdev@vger.kernel.org>
Subject: Re: [net-next-2.6 PATCH v6 3/7 RFC] TCPCT part 1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS
Date: Sat, 14 Nov 2009 10:43:33 -0500	[thread overview]
Message-ID: <4AFED025.1060202@gmail.com> (raw)
In-Reply-To: <4AFDB740.3060509@gmail.com>

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!


  reply	other threads:[~2009-11-14 15:43 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-13  4:03 [net-next-2.6 PATCH v6 0/7 RFC] TCPCT part 1: cookie option exchange William Allen Simpson
2009-11-13  4:07 ` [net-next-2.6 PATCH v6 1/7 RFC] TCPCT part 1a: add request_values parameter for sending SYNACK William Allen Simpson
2009-11-13  4:54   ` Ilpo Järvinen
2009-11-13  4:17 ` [net-next-2.6 PATCH v6 2/7 RFC] TCPCT part 1b: generate Responder Cookie William Allen Simpson
2009-11-13  6:21   ` Eric Dumazet
2009-11-13 14:35     ` William Allen Simpson
2009-11-13  6:26   ` Joe Perches
2009-11-13 14:51     ` William Allen Simpson
2009-11-13 18:04       ` Joe Perches
2009-11-16 14:39         ` William Allen Simpson
2009-11-16 15:34           ` Eric Dumazet
2009-11-16 20:06             ` William Allen Simpson
2009-11-13  4:31 ` [net-next-2.6 PATCH v6 3/7 RFC] TCPCT part 1c: sysctl_tcp_cookie_size, socket option TCP_COOKIE_TRANSACTIONS William Allen Simpson
2009-11-13 18:37   ` Joe Perches
2009-11-13 19:45     ` William Allen Simpson
2009-11-14 15:43       ` William Allen Simpson [this message]
2009-11-16 20:40         ` William Allen Simpson
2009-11-13  4:53 ` [net-next-2.6 PATCH v6 4/7 RFC] TCPCT part 1d: define TCP cookie option, extend existing struct's William Allen Simpson
2009-11-13  6:32   ` Eric Dumazet
2009-11-13 16:06     ` William Allen Simpson
2009-11-16 20:50       ` William Allen Simpson
2009-11-16 21:08         ` Eric Dumazet
2009-11-16 22:09           ` William Allen Simpson
2009-11-16 22:26             ` Eric Dumazet
2009-11-17  3:15               ` David Miller
2009-11-17 10:41                 ` William Allen Simpson
2009-11-17 12:18                 ` Ilpo Järvinen
2009-11-17 12:22                   ` David Miller
2009-11-17 12:38                     ` Ilpo Järvinen
2009-11-17 12:48                       ` David Miller
2009-11-17 12:07               ` Ilpo Järvinen
2009-11-18 13:55                 ` William Allen Simpson
2009-11-18 14:08                   ` Ilpo Järvinen
2009-11-18 14:42               ` William Allen Simpson
2009-11-13  5:10 ` [net-next-2.6 PATCH v6 5/7 RFC] TCPCT part 1e: implement socket option TCP_COOKIE_TRANSACTIONS William Allen Simpson
2009-11-13 14:11   ` Andi Kleen
2009-11-13 16:32     ` William Allen Simpson
2009-11-18 15:03   ` William Allen Simpson
2009-11-13  5:40 ` [net-next-2.6 PATCH v6 6/7 RFC] TCPCT part 1f: Initiator Cookie => Responder William Allen Simpson
2009-11-13 16:51   ` William Allen Simpson
2009-11-16 21:35     ` William Allen Simpson
2009-11-13  5:53 ` [net-next-2.6 PATCH v6 7/7 RFC] TCPCT part 1g: Responder Cookie => Initiator William Allen Simpson

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=4AFED025.1060202@gmail.com \
    --to=william.allen.simpson@gmail.com \
    --cc=joe@perches.com \
    --cc=netdev@vger.kernel.org \
    /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).