Calculate IGMP checksum and use more reasonble max response time.

Also made IGMP properties configurable at runtime. Change-Id: I98b40a43a0c17b7bf21f1bd622032c64d7434214

Calculate IGMP checksum and use more reasonble max response time.
Also made IGMP properties configurable at runtime. Change-Id: I98b40a43a0c17b7bf21f1bd622032c64d7434214
Jonathan Hart · Gerrit Code Review
Commit 81d73102c3fade60bb45b651e52e0cd4532308b2 81d73102 1 parent 0c3d0e02
Showing 4 changed files with 95 additions and 14 deletions
apps/cordmcast/src/main/java/org/onosproject/cordmcast/CordMcast.java
apps/igmp/src/main/java/org/onosproject/igmp/IgmpSnoop.java
core/api/src/main/java/org/onosproject/cfg/ConfigProperty.java
utils/misc/src/main/java/org/onlab/packet/IGMP.java
--- a/apps/cordmcast/src/main/java/org/onosproject/cordmcast/CordMcast.java
View file @81d7310
+++ b/apps/cordmcast/src/main/java/org/onosproject/cordmcast/CordMcast.java
View file @81d7310
@@ -138,7 +138,9 @@ public class CordMcast {
     private String fabricOnosUrl;
     @Activate
-    public void activate() {
+    public void activate(ComponentContext context) {
+        modified(context);
+
         appId = coreService.registerApplication("org.onosproject.cordmcast");
         componentConfigService.registerProperties(getClass());
         mcastService.addListener(listener);
--- a/apps/igmp/src/main/java/org/onosproject/igmp/IgmpSnoop.java
View file @81d7310
+++ b/apps/igmp/src/main/java/org/onosproject/igmp/IgmpSnoop.java
View file @81d7310
@@ -18,6 +18,7 @@ package org.onosproject.igmp;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
+import org.apache.felix.scr.annotations.Modified;
 import org.apache.felix.scr.annotations.Property;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
@@ -31,6 +32,8 @@ import org.onlab.packet.Ip4Address;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
 import org.onlab.util.SafeRecurringTask;
+import org.onlab.util.Tools;
+import org.onosproject.cfg.ComponentConfigService;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
 import org.onosproject.net.ConnectPoint;
@@ -63,12 +66,15 @@ import org.onosproject.net.packet.PacketProcessor;
 import org.onosproject.net.packet.PacketService;
 import org.onosproject.olt.AccessDeviceConfig;
 import org.onosproject.olt.AccessDeviceData;
+import org.osgi.service.component.ComponentContext;
 import org.slf4j.Logger;
 import java.nio.ByteBuffer;
 import java.util.Collection;
+import java.util.Dictionary;
 import java.util.List;
 import java.util.Map;
+import java.util.Properties;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
@@ -90,7 +96,7 @@ public class IgmpSnoop {
     private static final String DEST_IP = "224.0.0.1";
     private static final int DEFAULT_QUERY_PERIOD_SECS = 60;
-    private static final byte DEFAULT_IGMP_RESP_CODE = 0;
+    private static final byte DEFAULT_IGMP_RESP_CODE = 100;
     private static final String DEFAULT_MCAST_ADDR = "224.0.0.0/4";
     @Property(name = "multicastAddress",
@@ -118,6 +124,9 @@ public class IgmpSnoop {
     protected NetworkConfigRegistry networkConfig;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected ComponentConfigService componentConfigService;
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected MulticastRouteService multicastService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
@@ -169,9 +178,13 @@ public class IgmpSnoop {
     @Activate
-    public void activate() {
+    public void activate(ComponentContext context) {
+        modified(context);
+
         appId = coreService.registerApplication("org.onosproject.igmp");
+        componentConfigService.registerProperties(getClass());
+
         packetService.addProcessor(processor, PacketProcessor.director(1));
         networkConfig.registerConfigFactory(configFactory);
@@ -208,13 +221,7 @@ public class IgmpSnoop {
         deviceService.addListener(deviceListener);
-        queryPacket = buildQueryPacket();
+        restartQueryTask();
-
-        queryTask = queryService.scheduleWithFixedDelay(
-                SafeRecurringTask.wrap(this::querySubscribers),
-                0,
-                queryPeriod,
-                TimeUnit.SECONDS);
         log.info("Started");
     }
@@ -229,9 +236,49 @@ public class IgmpSnoop {
         networkConfig.unregisterConfigFactory(ssmTranslateConfigFactory);
         queryTask.cancel(true);
         queryService.shutdownNow();
+        componentConfigService.unregisterProperties(getClass(), false);
         log.info("Stopped");
     }
+    @Modified
+    protected void modified(ComponentContext context) {
+        Dictionary<?, ?> properties = context != null ? context.getProperties() : new Properties();
+
+        String strQueryPeriod = Tools.get(properties, "queryPeriod");
+        String strResponseCode = Tools.get(properties, "maxRespCode");
+        try {
+            byte newMaxRespCode = Byte.parseByte(strResponseCode);
+            if (maxRespCode != newMaxRespCode) {
+                maxRespCode = newMaxRespCode;
+                queryPacket = buildQueryPacket();
+            }
+
+            int newQueryPeriod = Integer.parseInt(strQueryPeriod);
+            if (newQueryPeriod != queryPeriod) {
+                queryPeriod = newQueryPeriod;
+                restartQueryTask();
+            }
+
+        } catch (NumberFormatException e) {
+            log.warn("Error parsing config input", e);
+        }
+
+        log.info("queryPeriod set to {}", queryPeriod);
+        log.info("maxRespCode set to {}", maxRespCode);
+    }
+
+    private void restartQueryTask() {
+        if (queryTask != null) {
+            queryTask.cancel(true);
+        }
+        queryPacket = buildQueryPacket();
+        queryTask = queryService.scheduleWithFixedDelay(
+                SafeRecurringTask.wrap(this::querySubscribers),
+                0,
+                queryPeriod,
+                TimeUnit.SECONDS);
+    }
+
     private void processFilterObjective(DeviceId devId, Port port, boolean remove) {
         //TODO migrate to packet requests when packet service uses filtering objectives
--- a/core/api/src/main/java/org/onosproject/cfg/ConfigProperty.java
View file @81d7310
+++ b/core/api/src/main/java/org/onosproject/cfg/ConfigProperty.java
View file @81d7310
@@ -44,6 +44,11 @@ public final class ConfigProperty {
         STRING,
         /**
+         * Indicates the value is a byte.
+         */
+        BYTE,
+
+        /**
          * Indicates the value is an integer.
          */
         INTEGER,
@@ -194,6 +199,16 @@ public final class ConfigProperty {
     }
     /**
+     * Returns the property value as a byte.
+     *
+     * @return byte value
+     */
+    public byte asByte() {
+        checkState(type == Type.BYTE, "Value is not a byte");
+        return Byte.parseByte(value);
+    }
+
+    /**
      * Returns the property value as an integer.
      *
      * @return integer value
--- a/utils/misc/src/main/java/org/onlab/packet/IGMP.java
View file @81d7310
+++ b/utils/misc/src/main/java/org/onlab/packet/IGMP.java
View file @81d7310
@@ -15,6 +15,8 @@
  */
 package org.onlab.packet;
+import org.slf4j.Logger;
+
 import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -22,8 +24,6 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import org.slf4j.Logger;
-
 import static com.google.common.base.MoreObjects.toStringHelper;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.onlab.packet.PacketUtils.checkInput;
@@ -33,7 +33,7 @@ import static org.slf4j.LoggerFactory.getLogger;
  * Implements IGMP control packet format.
  */
 public class IGMP extends BasePacket {
-    private final Logger log = getLogger(getClass());
+    private static final Logger log = getLogger(IGMP.class);
     public static final byte TYPE_IGMPV3_MEMBERSHIP_QUERY = 0x11;
     public static final byte TYPE_IGMPV1_MEMBERSHIP_REPORT = 0x12;
@@ -169,6 +169,8 @@ public class IGMP extends BasePacket {
         // Must calculate checksum
         bb.putShort((short) 0);
+
+
         switch (this.igmpType) {
             case IGMP.TYPE_IGMPV3_MEMBERSHIP_REPORT:
@@ -191,6 +193,21 @@ public class IGMP extends BasePacket {
         }
         int size = bb.position();
+
+        // compute checksum if needed
+        if (this.checksum == 0) {
+            bb.rewind();
+            int accumulation = 0;
+            for (int i = 0; i < size * 2; ++i) {
+                accumulation += 0xffff & bb.getShort();
+            }
+            accumulation = (accumulation >> 16 & 0xffff)
+                    + (accumulation & 0xffff);
+            this.checksum = (short) (~accumulation & 0xffff);
+            bb.putShort(2, this.checksum);
+        }
+
+
         bb.position(0);
         byte[] rdata = new byte[size];
         bb.get(rdata, 0, size);
@@ -238,7 +255,7 @@ public class IGMP extends BasePacket {
             igmp.igmpType = bb.get();
             igmp.resField = bb.get();
             igmp.checksum = bb.getShort();
-            int len = MINIMUM_HEADER_LEN;
+
             String msg;
             switch (igmp.igmpType) {