netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Dumazet <eric.dumazet@gmail.com>
To: William Allen Simpson <william.allen.simpson@gmail.com>
Cc: "Linux Kernel Developers" <linux-kernel@vger.kernel.org>,
	"Linux Kernel Network Developers" <netdev@vger.kernel.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>,
	"Andi Kleen" <andi@firstfloor.org>
Subject: Re: [PATCH] tcp: harmonize tcp_vx_rcv header length assumptions
Date: Tue, 12 Jan 2010 11:46:02 +0100	[thread overview]
Message-ID: <4B4C52EA.6070705@gmail.com> (raw)
In-Reply-To: <4B4C4962.8040207@gmail.com>

Le 12/01/2010 11:05, William Allen Simpson a écrit :
> Harmonize tcp_v4_rcv() and tcp_v6_rcv() -- better document tcp doff and
> header length assumptions.
> 
> Reduces multiply/shifts, marginally improving speed.
> 
> Retains redundant tcp header length checks before checksumming.
> 
> Stand-alone patch, originally developed for TCPCT.
> 
> Requires:
>   net: tcp_header_len_th and tcp_option_len_th
> 
> Signed-off-by: William.Allen.Simpson@gmail.com
> ---
>  net/ipv4/tcp_ipv4.c |   36 +++++++++++++++++++-------------
>  net/ipv6/tcp_ipv6.c |   56
> ++++++++++++++++++++++++++++----------------------
>  2 files changed, 52 insertions(+), 40 deletions(-)

Seems fine, but :

1) What means the "Transformed ?" you wrote several times ?

2) This part :

-	th = tcp_hdr(skb);
-
-	if (th->doff < sizeof(struct tcphdr)/4)
+	/* Check bad doff, compare doff directly to constant value */
+	tcp_header_len = tcp_hdr(skb)->doff;
+	if (tcp_header_len < (sizeof(struct tcphdr) / 4))
 		goto bad_packet;
-	if (!pskb_may_pull(skb, th->doff*4))
+
+	/* Check too short header and options */
+	tcp_header_len *= 4;
+	if (!pskb_may_pull(skb, tcp_header_len))
 		goto discard_it;


could be :  (no need for 4 multiplies/divides)

	tcp_header_len = tcp_header_len_th(tcp_hdr(skb));
	if (tcp_header_len < sizeof(struct tcphdr))
		goto bad_packet;
	/* Check too short header and options */
	if (!pskb_may_pull(skb, tcp_header_len))
		goto discard_it;

	 

  reply	other threads:[~2010-01-12 10:46 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-01-10 13:02 query: redundant tcp header length checks? William Allen Simpson
2010-01-12 10:05 ` [PATCH] tcp: harmonize tcp_vx_rcv header length assumptions William Allen Simpson
2010-01-12 10:46   ` Eric Dumazet [this message]
2010-01-12 17:11     ` William Allen Simpson
2010-01-13  9:50       ` William Allen Simpson
2010-01-12 17:14     ` William Allen Simpson
2010-01-13 10:48 ` [PATCH v4] " William Allen Simpson
2010-01-13 11:56   ` Andi Kleen
2010-01-13 15:36     ` William Allen Simpson
2010-01-13 15:53       ` Andi Kleen
2010-01-13 16:40         ` [PATCH] Makefile: Document ability to make file.lst and file.S Joe Perches
2010-01-13 17:14           ` Andi Kleen
2010-01-13 17:31             ` Joe Perches
2010-01-13 19:51               ` William Allen Simpson
2010-01-14  3:26               ` Américo Wang
2010-01-18 12:29               ` Michal Marek
2010-01-13 19:49         ` [PATCH v4] tcp: harmonize tcp_vx_rcv header length assumptions William Allen Simpson
2010-01-13 20:19           ` Andi Kleen
2010-01-13 21:13           ` William Allen Simpson
2010-01-14  1:03             ` Joe Perches
2010-01-14  8:39               ` Patrick McHardy
2010-01-14 15:02                 ` William Allen Simpson
2010-01-14 15:10 ` [PATCH v5] " 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=4B4C52EA.6070705@gmail.com \
    --to=eric.dumazet@gmail.com \
    --cc=andi@firstfloor.org \
    --cc=ilpo.jarvinen@helsinki.fi \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=william.allen.simpson@gmail.com \
    /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).