Fixed bug of IPv4 option handling.

Signed-off-by: Hideyuki Tai <h-tai@cd.jp.nec.com>

diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/IPv4.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/IPv4.java
index 1e2f4277c1f4035b873c61502a43bdc1e6d6fe4c..a0ea67a91ac63aab6305209792d119f6e88d6d08 100644 (file)
--- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/IPv4.java
+++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/packet/IPv4.java
@@ -47,6 +47,10 @@ public class IPv4 extends Packet {
     private static final String DIP = "DestinationIPAddress";
     private static final String OPTIONS = "Options";
 
     private static final String DIP = "DestinationIPAddress";
     private static final String OPTIONS = "Options";
 

+    private static final int UNIT_SIZE_SHIFT = 2;
+    private static final int UNIT_SIZE = (1 << UNIT_SIZE_SHIFT);
+    private static final int MIN_HEADER_SIZE = 20;
+

     public static final Map<Byte, Class<? extends Packet>> protocolClassMap;
     static {
         protocolClassMap = new HashMap<Byte, Class<? extends Packet>>();
     public static final Map<Byte, Class<? extends Packet>> protocolClassMap;
     static {
         protocolClassMap = new HashMap<Byte, Class<? extends Packet>>();


@@ -145,12 +149,7 @@ public class IPv4 extends Packet {
     public int getHeaderSize() {
         int headerLen = this.getHeaderLen();
         if (headerLen == 0) {
     public int getHeaderSize() {
         int headerLen = this.getHeaderLen();
         if (headerLen == 0) {

-            headerLen = 20;
-        }
-
-        byte[] options = hdrFieldsMap.get(OPTIONS);
-        if (options != null) {
-            headerLen += options.length;
+            headerLen = MIN_HEADER_SIZE;

         }
 
         return headerLen * NetUtils.NumBitsInAByte;
         }
 
         return headerLen * NetUtils.NumBitsInAByte;


@@ -260,6 +259,10 @@ public class IPv4 extends Packet {
     public void setHeaderField(String headerField, byte[] readValue) {
         if (headerField.equals(PROTOCOL)) {
             payloadClass = protocolClassMap.get(readValue[0]);
     public void setHeaderField(String headerField, byte[] readValue) {
         if (headerField.equals(PROTOCOL)) {
             payloadClass = protocolClassMap.get(readValue[0]);

+        } else if (headerField.equals(OPTIONS) &&
+                   (readValue == null || readValue.length == 0)) {
+            hdrFieldsMap.remove(headerField);
+            return;

         }
         hdrFieldsMap.put(headerField, readValue);
     }
         }
         hdrFieldsMap.put(headerField, readValue);
     }


@@ -434,8 +437,23 @@ public class IPv4 extends Packet {
      * @return IPv4
      */
     public IPv4 setOptions(byte[] options) {
      * @return IPv4
      */
     public IPv4 setOptions(byte[] options) {

-        fieldValues.put(OPTIONS, options);
-        byte newIHL = (byte) (5 + options.length);
+        byte newIHL = (byte)(MIN_HEADER_SIZE >>> UNIT_SIZE_SHIFT);
+        if (options == null || options.length == 0) {
+            fieldValues.remove(OPTIONS);
+        } else {
+            int len = options.length;
+            int rlen = (len + (UNIT_SIZE - 1)) & ~(UNIT_SIZE - 1);
+            if (rlen > len) {
+                // Padding is required.
+                byte[] newopt = new byte[rlen];
+                System.arraycopy(options, 0, newopt, 0, len);
+                options = newopt;
+                len = rlen;
+            }
+            fieldValues.put(OPTIONS, options);
+            newIHL += (len >>> UNIT_SIZE_SHIFT);
+        }
+

         setHeaderLength(newIHL);
 
         return this;
         setHeaderLength(newIHL);
 
         return this;


@@ -477,15 +495,13 @@ public class IPv4 extends Packet {
     @Override
     /**
      * Gets the number of bits for the fieldname specified
     @Override
     /**
      * Gets the number of bits for the fieldname specified

-     * If the fieldname has variable length like "Options", then this value is computed using the
-     * options length and the header length
+     * If the fieldname has variable length like "Options", then this value is computed using the header length

      * @param fieldname - String
      * @return number of bits for fieldname - int
      */
     public int getfieldnumBits(String fieldName) {
         if (fieldName.equals(OPTIONS)) {
      * @param fieldname - String
      * @return number of bits for fieldname - int
      */
     public int getfieldnumBits(String fieldName) {
         if (fieldName.equals(OPTIONS)) {

-            byte[] options = getOptions();
-            return ((options == null) ? 0 : (options.length - getHeaderLen()));
+            return (getHeaderLen() - MIN_HEADER_SIZE) * NetUtils.NumBitsInAByte;

         }
         return hdrFieldCoordMap.get(fieldName).getRight();
     }
         }
         return hdrFieldCoordMap.get(fieldName).getRight();
     }

diff --git a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/packet/IPv4Test.java b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/packet/IPv4Test.java
index 429272f22769dc2ef36a17510cfe09910189a607..5afcd8be7dc4f47d79270dd00b6c3c8870c07631 100644 (file)
--- a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/packet/IPv4Test.java
+++ b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/packet/IPv4Test.java
@@ -224,6 +224,126 @@ public class IPv4Test {
         Assert.assertTrue(destinationAddress[3] == 110);
     }
 
         Assert.assertTrue(destinationAddress[3] == 110);
     }
 

+    @Test
+    public void testOptions() throws Exception {
+        IPv4 ip = new IPv4();
+        Assert.assertEquals(20, ip.getHeaderLen());
+        Assert.assertEquals(160, ip.getHeaderSize());
+        Assert.assertEquals(0, ip.getfieldnumBits("Options"));
+
+        byte[][] options = {
+            new byte[] {
+                (byte)0x01,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+            },
+            null,
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+                (byte)0x05,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+                (byte)0x05, (byte)0x06,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+                (byte)0x05, (byte)0x06, (byte)0x07,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+                (byte)0x05, (byte)0x06, (byte)0x07, (byte)0x08,
+            },
+            new byte[0],
+        };
+
+        byte[][] expected = {
+            new byte[] {
+                (byte)0x01, (byte)0x00, (byte)0x00, (byte)0x00,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x00, (byte)0x00,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x00,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+            },
+            null,
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+                (byte)0x05, (byte)0x00, (byte)0x00, (byte)0x00,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+                (byte)0x05, (byte)0x06, (byte)0x00, (byte)0x00,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+                (byte)0x05, (byte)0x06, (byte)0x07, (byte)0x00,
+            },
+            new byte[] {
+                (byte)0x01, (byte)0x02, (byte)0x03, (byte)0x04,
+                (byte)0x05, (byte)0x06, (byte)0x07, (byte)0x08,
+            },
+            null,
+        };
+
+        byte[] echo = {
+            (byte)0x11, (byte)0x22, (byte)0x33, (byte)0x44,
+            (byte)0x55, (byte)0x66, (byte)0x77, (byte)0x88,
+            (byte)0x99, (byte)0xaa,
+        };
+        ICMP icmp = new ICMP();
+        icmp.setType((byte)8);
+        icmp.setCode((byte)0);
+        icmp.setIdentifier((short)0xabcd);
+        icmp.setSequenceNumber((short)7777);
+        icmp.setRawPayload(echo);
+
+        ip.setSourceAddress(InetAddress.getByName("192.168.10.20"));
+        ip.setDestinationAddress(InetAddress.getByName("192.168.30.40"));
+        ip.setProtocol(IPProtocols.ICMP.byteValue());
+
+        for (int i = 0; i < options.length; i++) {
+            byte[] opts = options[i];
+            byte[] exp = expected[i];
+
+            // Set IPv4 options.
+            int hlen = 20;
+            int optlen;
+            if (exp != null) {
+                optlen = exp.length;
+                hlen += optlen;
+            } else {
+                optlen = 0;
+            }
+            ip.setOptions(opts);
+            Assert.assertTrue(Arrays.equals(exp, ip.getOptions()));
+            Assert.assertEquals(hlen, ip.getHeaderLen());
+            Assert.assertEquals(hlen * 8, ip.getHeaderSize());
+            Assert.assertEquals(optlen * 8, ip.getfieldnumBits("Options"));
+
+            // Serialize/Deserialize test.
+            ip.setPayload(icmp);
+
+            byte[] raw = ip.serialize();
+            IPv4 newip = new IPv4();
+            newip.deserialize(raw, 0, raw.length * 8);
+            Assert.assertEquals(ip, newip);
+            Assert.assertEquals(icmp, newip.getPayload());
+            Assert.assertTrue(Arrays.equals(exp, newip.getOptions()));
+        }
+    }
+

     @Test
     public void testChecksum() {
         byte header[] = { (byte) 0x45, 00, 00, (byte) 0x3c, (byte) 0x1c,
     @Test
     public void testChecksum() {
         byte header[] = { (byte) 0x45, 00, 00, (byte) 0x3c, (byte) 0x1c,