netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] IPv4: skip loopback checksums in ip_rcv()
@ 2009-10-19 19:34 Torsten Schmidt
  2009-10-19 20:42 ` Eric Dumazet
  0 siblings, 1 reply; 6+ messages in thread
From: Torsten Schmidt @ 2009-10-19 19:34 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

Skip loopback checksum in ip_rcv() for speed optimisation.

Signed-off-by: Torsten Schmidt <schmto@hrz.tu-chemnitz.de>
---
 net/ipv4/ip_input.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index 6c98b43..dc72286 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -406,7 +406,7 @@ int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
 	 *
 	 *	1.	Length at least the size of an ip header
 	 *	2.	Version of 4
-	 *	3.	Checksums correctly. [Speed optimisation for later, skip loopback checksums]
+	 *	3.	Checksums correctly. [Speed optimisation: skip loopback checksums]
 	 *	4.	Doesn't have a bogus length
 	 */
 
@@ -418,8 +418,9 @@ int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
 
 	iph = ip_hdr(skb);
 
-	if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
-		goto inhdr_error;
+	if (!ipv4_is_loopback(iph->daddr))
+		if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
+			goto inhdr_error;
 
 	len = ntohs(iph->tot_len);
 	if (skb->len < len) {
-- 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPv4: skip loopback checksums in ip_rcv()
  2009-10-19 19:34 [PATCH] IPv4: skip loopback checksums in ip_rcv() Torsten Schmidt
@ 2009-10-19 20:42 ` Eric Dumazet
  2009-10-19 20:56   ` Torsten Schmidt
  2009-10-20  0:24   ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2009-10-19 20:42 UTC (permalink / raw)
  To: Torsten Schmidt; +Cc: David S. Miller, Linux Netdev List

Torsten Schmidt a écrit :
> Skip loopback checksum in ip_rcv() for speed optimisation.
> 
> Signed-off-by: Torsten Schmidt <schmto@hrz.tu-chemnitz.de>
> ---
>  net/ipv4/ip_input.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
> index 6c98b43..dc72286 100644
> --- a/net/ipv4/ip_input.c
> +++ b/net/ipv4/ip_input.c
> @@ -406,7 +406,7 @@ int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
>  	 *
>  	 *	1.	Length at least the size of an ip header
>  	 *	2.	Version of 4
> -	 *	3.	Checksums correctly. [Speed optimisation for later, skip loopback checksums]
> +	 *	3.	Checksums correctly. [Speed optimisation: skip loopback checksums]
>  	 *	4.	Doesn't have a bogus length
>  	 */
>  
> @@ -418,8 +418,9 @@ int ip_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt,
>  
>  	iph = ip_hdr(skb);
>  
> -	if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
> -		goto inhdr_error;
> +	if (!ipv4_is_loopback(iph->daddr))
> +		if (unlikely(ip_fast_csum((u8 *)iph, iph->ihl)))
> +			goto inhdr_error;
>  
>  	len = ntohs(iph->tot_len);
>  	if (skb->len < len) {

This is bogus IMHO.

One bit could be corrupted in iph, and ntohl(iph->daddr) becomes 0x7fxxyyzz,
we then accept a bogus frame. This is a RFC violation.

This also slows down non loopback devices, adding an extra test.

ip_fast_csum() is really fast (about 16 instructions).


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPv4: skip loopback checksums in ip_rcv()
  2009-10-19 20:42 ` Eric Dumazet
@ 2009-10-19 20:56   ` Torsten Schmidt
  2009-10-19 21:10     ` Eric Dumazet
  2009-10-20  0:24   ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Torsten Schmidt @ 2009-10-19 20:56 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Linux Netdev List

Eric Dumazet wrotes:
> This is bogus IMHO.
> 
> One bit could be corrupted in iph, and ntohl(iph->daddr) becomes 0x7fxxyyzz,
> we then accept a bogus frame. This is a RFC violation.
> 
> This also slows down non loopback devices, adding an extra test.
> 
> ip_fast_csum() is really fast (about 16 instructions).

Yes, you are right. So it would be better to only skip csum if *dev is 
our loopback interface ? Right ?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPv4: skip loopback checksums in ip_rcv()
  2009-10-19 20:56   ` Torsten Schmidt
@ 2009-10-19 21:10     ` Eric Dumazet
  2009-10-19 21:34       ` Torsten Schmidt
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2009-10-19 21:10 UTC (permalink / raw)
  To: Torsten Schmidt; +Cc: Linux Netdev List

Torsten Schmidt a écrit :
> Eric Dumazet wrotes:
>> This is bogus IMHO.
>>
>> One bit could be corrupted in iph, and ntohl(iph->daddr) becomes 0x7fxxyyzz,
>> we then accept a bogus frame. This is a RFC violation.
>>
>> This also slows down non loopback devices, adding an extra test.
>>
>> ip_fast_csum() is really fast (about 16 instructions).
> 
> Yes, you are right. So it would be better to only skip csum if *dev is 
> our loopback interface ? Right ?

An application could send a bogus IP packet using RAW interface, and we still should
check IP checksum before delivering this packet, even on loopback device.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPv4: skip loopback checksums in ip_rcv()
  2009-10-19 21:10     ` Eric Dumazet
@ 2009-10-19 21:34       ` Torsten Schmidt
  0 siblings, 0 replies; 6+ messages in thread
From: Torsten Schmidt @ 2009-10-19 21:34 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Linux Netdev List

Eric Dumazet:
> Torsten Schmidt a écrit :
>> Eric Dumazet wrotes:
>>> This is bogus IMHO.
>>>
>>> One bit could be corrupted in iph, and ntohl(iph->daddr) becomes 0x7fxxyyzz,
>>> we then accept a bogus frame. This is a RFC violation.
>>>
>>> This also slows down non loopback devices, adding an extra test.
>>>
>>> ip_fast_csum() is really fast (about 16 instructions).
>> 
>> Yes, you are right. So it would be better to only skip csum if *dev is 
>> our loopback interface ? Right ?
> 
> An application could send a bogus IP packet using RAW interface, and we still should
> check IP checksum before delivering this packet, even on loopback device.

Yes, then we should fix this comment in ip_rcv():

	RFC1122: 3.2.1.2 MUST silently discard any IP frame that fails the checksum.
	Is the datagram acceptable?
	1.	Length at least the size of an ip header
	2.	Version of 4
->	3.	Checksums correctly. [Speed optimisation for later, skip loopback checksums]
	4.	Doesn't have a bogus length

Right ?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] IPv4: skip loopback checksums in ip_rcv()
  2009-10-19 20:42 ` Eric Dumazet
  2009-10-19 20:56   ` Torsten Schmidt
@ 2009-10-20  0:24   ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2009-10-20  0:24 UTC (permalink / raw)
  To: eric.dumazet; +Cc: torsten.schmidt, netdev

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 19 Oct 2009 22:42:02 +0200

> One bit could be corrupted in iph, and ntohl(iph->daddr) becomes 0x7fxxyyzz,
> we then accept a bogus frame. This is a RFC violation.
> 
> This also slows down non loopback devices, adding an extra test.
> 
> ip_fast_csum() is really fast (about 16 instructions).

Also loopback doesn't mean anything.  That packet could
be mirrored and sent externally via packet scheduler
rules and actions.

And for this specific case, the savings are absolutely zero.

We've brought the whole damn IP header into the CPU cache and
that is the real cost.  The calculation is something like 12
instructions, maybe 6 cycles on a modern cpu, which is nothing.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2009-10-20  0:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-19 19:34 [PATCH] IPv4: skip loopback checksums in ip_rcv() Torsten Schmidt
2009-10-19 20:42 ` Eric Dumazet
2009-10-19 20:56   ` Torsten Schmidt
2009-10-19 21:10     ` Eric Dumazet
2009-10-19 21:34       ` Torsten Schmidt
2009-10-20  0:24   ` David Miller

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).