Message ID | 200910192134.02125.schmto@hrz.tu-chemnitz.de |
---|---|
State | Rejected, archived |
Delegated to: | David Miller |
Headers | show |
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
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) {
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(-)