after updating the value of the ICMP payload, inet_proto_csum_replace4() should
be called with zero pseudohdr.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/act_nat.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 24e614c..59f05ee 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -246,7 +246,7 @@ static int tcf_nat(struct sk_buff *skb, struct tc_action *a,
 			iph->saddr = new_addr;
 
 		inet_proto_csum_replace4(&icmph->checksum, skb, addr, new_addr,
-					 1);
+					 0);
 		break;
 	}
 	default:
--

[ message continues ]

No, the code is correct as is.  We need to update the checksum
even if the checksum is partial, which is what the 1 is for.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--

[ message continues ]

Is it really necessary, and I have noticed that netfilter doesn't call
inet_proto_csum_replace4 in this way.

static bool
icmp_manip_pkt(struct sk_buff *skb,
               unsigned int iphdroff,
               const struct nf_conntrack_tuple *tuple,
               enum nf_nat_manip_type maniptype)
{
        const struct iphdr *iph = (struct iphdr *)(skb->data + iphdroff);
        struct icmphdr *hdr;
        unsigned int hdroff = iphdroff + iph->ihl*4;

        if (!skb_make_writable(skb, hdroff + sizeof(*hdr)))
                return false;

        hdr = (struct icmphdr *)(skb->data + hdroff);
        inet_proto_csum_replace2(&hdr->checksum, skb,
                                 hdr->un.echo.id, tuple->src.u.icmp.id, 0);
        hdr->un.echo.id = tuple->src.u.icmp.id;
        return true;
}

Thanks.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)
--

[ message continues ]

The checksum update is for the inner IP header.  netfilter does
of course update the checksum, it just doesn't do it here which is
for the outer IP header.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--

[ message continues ]

I know we need to update the ICMP checksum if we alter the payload(the
inner IP header here) of ICMP. But I doubt if the update is really
necessary if the checksum is partial, as the   checksum will be done
later(by ether skb_checksum_help() or NIC hardware). In fact, as there
isn't any pseudo header, the icmph->checksum should be always ZERO,
otherwise skb_checksum_help() or NIC will give the wrong checksums,
when the checksum is partial.

-- 
Regards,
Changli Gao(xiaosuo@gmail.com)
--

[ message continues ]

Actually you are right.  I suppose the only reason this has never
shown up is because CHEKSUM_PARTIAL doesn't usually occur with
forwarded packets.

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--

[ message continues ]

From: Herbert Xu <herbert@gondor.apana.org.au>

Also applied, thanks.

--

[ message continues ]