Merge "Bug 1805: Fixed 2 bugs in ICMP.computeChecksum()."

diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/ICMP.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/ICMP.java
index 35ae71d0019ed2b0b1d4893c7e47901be6e906ea..987394402d7157f44a011c3d16ab77308855d8a6 100644 (file)
--- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/ICMP.java
+++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/ICMP.java
@@ -1,6 +1,6 @@
 
 /*
- * Copyright (c) 2013 Cisco Systems, Inc. and others.  All rights reserved.
+ * Copyright (c) 2013-2014 Cisco Systems, Inc. and others.  All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
@@ -190,8 +190,9 @@ public class ICMP extends Packet {
             end += rawPayload.length;
         }
         int checksumStartByte = start + getfieldOffset(CHECKSUM) / NetUtils.NumBitsInAByte;
+        int even = end & ~1;
 
-        for (int i = start; i <= (end - 1); i = i + 2) {
+        for (int i = start; i < even; i = i + 2) {
             // Skip, if the current bytes are checkSum bytes
             if (i == checksumStartByte) {
                 continue;
@@ -199,7 +200,13 @@ public class ICMP extends Packet {
             wordData = ((data[i] << 8) & 0xFF00) + (data[i + 1] & 0xFF);
             sum = sum + wordData;
         }
-        carry = (sum >> 16) & 0xFF;
+        if (even < end) {
+            // Add the last octet with zero padding.
+            wordData = (data[even] << 8) & 0xFF00;
+            sum = sum + wordData;
+        }
+
+        carry = sum >>> 16;
         finalSum = (sum & 0xFFFF) + carry;
         return (short) ~((short) finalSum & 0xFFFF);
     }

diff --git a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/packet/ICMPTest.java b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/packet/ICMPTest.java
index e81fbf02cfafc4550ebfb5e5558d144e55d0696d..287b73ae3c22fda416fe21a8f003bc9d6e451312 100644 (file)
--- a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/packet/ICMPTest.java
+++ b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/packet/ICMPTest.java
@@ -1,6 +1,6 @@
 
 /*
- * Copyright (c) 2013 Cisco Systems, Inc. and others.  All rights reserved.
+ * Copyright (c) 2013-2014 Cisco Systems, Inc. and others.  All rights reserved.
  *
  * This program and the accompanying materials are made available under the
  * terms of the Eclipse Public License v1.0 which accompanies this distribution,
@@ -9,6 +9,8 @@
 
 package org.opendaylight.controller.sal.packet;
 
+import java.util.Arrays;
+
 import junit.framework.Assert;
 
 import org.junit.Test;
@@ -74,28 +76,58 @@ public class ICMPTest {
                 (byte) 0x2b, (byte) 0x2c, (byte) 0x2d, (byte) 0x2e,
                 (byte) 0x2f, (byte) 0x30, (byte) 0x31, (byte) 0x32,
                 (byte) 0x33, (byte) 0x34, (byte) 0x35, (byte) 0x36, (byte) 0x37 };
+        serializeTest(icmpRawPayload, (short)0xe553);
+
+        serializeTest(null, (short)0xb108);
+        serializeTest(new byte[0], (short)0xb108);
+
+        byte[] odd = {
+            (byte)0xba, (byte)0xd4, (byte)0xc7, (byte)0x53,
+            (byte)0xf8, (byte)0x59, (byte)0x68, (byte)0x77,
+            (byte)0xfd, (byte)0x27, (byte)0xe0, (byte)0x5b,
+            (byte)0xd0, (byte)0x2e, (byte)0x28, (byte)0x41,
+            (byte)0xa3, (byte)0x48, (byte)0x5d, (byte)0x2e,
+            (byte)0x7d, (byte)0x5b, (byte)0xd3, (byte)0x60,
+            (byte)0xb3, (byte)0x88, (byte)0x8d, (byte)0x0f,
+            (byte)0x1d, (byte)0x87, (byte)0x51, (byte)0x0f,
+            (byte)0x6a, (byte)0xff, (byte)0xf7, (byte)0xd4,
+            (byte)0x40, (byte)0x35, (byte)0x4e, (byte)0x01,
+            (byte)0x36,
+        };
+        serializeTest(odd, (short)0xd0ad);
+
+        // Large payload that causes 16-bit checksum overflow more than
+        // 255 times.
+        byte[] largeEven = new byte[1024];
+        Arrays.fill(largeEven, (byte)0xff);
+        serializeTest(largeEven, (short)0xb108);
+
+        byte[] largeOdd = new byte[1021];
+        Arrays.fill(largeOdd, (byte)0xff);
+        serializeTest(largeOdd, (short)0xb207);
+    }
 
-        short checksum = (short)0xe553;
-
-        // Create ICMP object
+    private void serializeTest(byte[] payload, short checksum)
+        throws PacketException {
         ICMP icmp = new ICMP();
-        icmp.setType((byte)8);
-        icmp.setCode((byte)0);
-        icmp.setIdentifier((short) 0x46f5);
-        icmp.setSequenceNumber((short) 2);
-        icmp.setRawPayload(icmpRawPayload);
-        //icmp.setChecksum(checksum);
+        icmp.setType((byte)8).setCode((byte)0).
+            setIdentifier((short)0x46f5).setSequenceNumber((short)2);
+        int payloadSize = 0;
+        if (payload != null) {
+            icmp.setRawPayload(payload);
+            payloadSize = payload.length;
+        }
 
         // Serialize
-        byte[] stream = icmp.serialize();
-        Assert.assertTrue(stream.length == 64);
+        byte[] data = icmp.serialize();
+        Assert.assertEquals(payloadSize + 8, data.length);
 
         // Deserialize
         ICMP icmpDes = new ICMP();
-        icmpDes.deserialize(stream, 0, stream.length);
+        icmpDes.deserialize(data, 0, data.length);
 
         Assert.assertFalse(icmpDes.isCorrupted());
-        Assert.assertTrue(icmpDes.getChecksum() == checksum);
-        Assert.assertTrue(icmp.equals(icmpDes));
+        Assert.assertEquals(checksum, icmpDes.getChecksum());
+        Assert.assertEquals(icmp, icmpDes);
     }
 }