IPv4: skip loopback checksums in ip_rcv()

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

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

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

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 ?
--
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

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.

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

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 ?
--
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

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