[PATCH] nf_nat_snmp: fix checksum calculation (v3)

[PATCH] nf_nat_snmp: fix checksum calculation (v3)

[Stephen Hemminger]

Revised version of the original patch in the bug
  https://bugzilla.kernel.org/show_bug.cgi?id=17622
from clark wang <wtweeker@163.com>

I took the opportunity to do some cleanup here.
 * reorder the assignment to make the byte order clear
 * get rid of unnecessary ref/deref and just pass the bytes
 * use sizeof() instead of hard coding size

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

---


--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c	2010-09-20 09:05:11.752067965 -0700
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c	2010-09-20 09:38:48.518661840 -0700
@@ -885,24 +885,23 @@ static unsigned char snmp_request_decode
  * Fast checksum update for possibly oddly-aligned UDP byte, from the
  * code example in the draft.
  */
-static void fast_csum(__sum16 *csum,
-		      const unsigned char *optr,
-		      const unsigned char *nptr,
-		      int offset)
+static void fast_csum(__sum16 *csum, __u8 from, __u8 to, int offset)
 {
-	unsigned char s[4];
+	__be16 diff[2];
 
 	if (offset & 1) {
-		s[0] = s[2] = 0;
-		s[1] = ~*optr;
-		s[3] = *nptr;
+		s[0] = ~0;
+		s[1] = ~from;
+		s[2] = 0;
+		s[3] = to;
 	} else {
-		s[1] = s[3] = 0;
-		s[0] = ~*optr;
-		s[2] = *nptr;
+		s[0] = ~from;
+		s[1] = ~0;
+		s[2] = to;
+		s[3] = 0;
 	}
 
-	*csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum)));
+	*csum = csum_fold(csum_partial(s, sizeof(s), ~csum_unfold(*csum)));
 }
 
 /*
@@ -924,11 +923,8 @@ static inline void mangle_address(unsign
 		*addr = map->to;
 
 		/* Update UDP checksum if being used */
-		if (*check) {
-			fast_csum(check,
-				  &map->from, &map->to, addr - begin);
-
-		}
+		if (*check)
+			fast_csum(check, map->from, map->to, addr - begin);
 
 		if (debug)
 			printk(KERN_DEBUG "bsalg: mapped %pI4 to %pI4\n",

Re: [PATCH] nf_nat_snmp: fix checksum calculation (v3)

[Patrick McHardy]

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

On 20.09.2010 18:44, Stephen Hemminger wrote:
> Revised version of the original patch in the bug
>   https://bugzilla.kernel.org/show_bug.cgi?id=17622
> from clark wang <wtweeker@163.com>
> 
> I took the opportunity to do some cleanup here.
>  * reorder the assignment to make the byte order clear
>  * get rid of unnecessary ref/deref and just pass the bytes
>  * use sizeof() instead of hard coding size
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Thanks Stephen, but this patch didn't compile:

net/ipv4/netfilter/nf_nat_snmp_basic.c: In function 'fast_csum':
net/ipv4/netfilter/nf_nat_snmp_basic.c:893: error: 's' undeclared (first
use in this function)
net/ipv4/netfilter/nf_nat_snmp_basic.c:893: error: (Each undeclared
identifier is reported only once
net/ipv4/netfilter/nf_nat_snmp_basic.c:893: error: for each function it
appears in.)
net/ipv4/netfilter/nf_nat_snmp_basic.c:890: warning: unused variable 'diff'

Since I prefer to keep this fix to the minimal size at this point,
I've committed this patch based on Clark's and your patches:


[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1031 bytes --]

commit 8d70d82cdcc6da0a77440c6d860957a1f50b8089
Author: Patrick McHardy <kaber@trash.net>
Date:   Mon Sep 20 19:29:40 2010 +0200

    netfilter: nf_nat_snmp: fix checksum calculation (v4)
    
    Fix checksum calculation in nf_nat_snmp_basic.
    
    Based on patches by Clark Wang <wtweeker@163.com> and
    Stephen Hemminger <shemminger@vyatta.com>.
    
    https://bugzilla.kernel.org/show_bug.cgi?id=17622
    
    Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/ipv4/netfilter/nf_nat_snmp_basic.c b/net/ipv4/netfilter/nf_nat_snmp_basic.c
index 1679e2c..ee5f419 100644
--- a/net/ipv4/netfilter/nf_nat_snmp_basic.c
+++ b/net/ipv4/netfilter/nf_nat_snmp_basic.c
@@ -893,13 +893,15 @@ static void fast_csum(__sum16 *csum,
 	unsigned char s[4];
 
 	if (offset & 1) {
-		s[0] = s[2] = 0;
+		s[0] = ~0;
 		s[1] = ~*optr;
+		s[2] = 0;
 		s[3] = *nptr;
 	} else {
-		s[1] = s[3] = 0;
 		s[0] = ~*optr;
+		s[1] = ~0;
 		s[2] = *nptr;
+		s[3] = 0;
 	}
 
 	*csum = csum_fold(csum_partial(s, 4, ~csum_unfold(*csum)));

Re: [PATCH] nf_nat_snmp: fix checksum calculation (v3)

[Patrick McHardy]

On 30.09.2010 10:25, 王韬 wrote:
> Dear Patrick
> 
>                   I am Clark Wang.  I have one question is that which
> version of kernel will contain this patch.

Its currently contained in the net-2.6.git tree, so it should be
in final .36 release.