Miscellaneous fixes for reading Linc-OE port types.

- Keep track of created PortDescriptions so that they can be replayed when configurations don't stick - Push topology configs to all cluster members (Temporary hack until Configs are made Mastership-aware) - Port type consistency for Optical ports - default to FIBER port type Change-Id: Ib2c9e2839c212d2998206bd0106490b2b38446a9

Miscellaneous fixes for reading Linc-OE port types.
- Keep track of created PortDescriptions so that they can be replayed when configurations don't stick - Push topology configs to all cluster members (Temporary hack until Configs are made Mastership-aware) - Port type consistency for Optical ports - default to FIBER port type Change-Id: Ib2c9e2839c212d2998206bd0106490b2b38446a9
Ayaka Koshibe · Gerrit Code Review
Commit 74b5527081c4d52c0166ee6f235665edca41fdc6 74b55270 1 parent d24420f4
Showing 5 changed files with 69 additions and 47 deletions
core/api/src/main/java/org/onosproject/net/DefaultAnnotations.java
core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
tools/test/topos/opticalUtils.py
web/api/src/main/java/org/onosproject/rest/resources/ConfigProvider.java
--- a/core/api/src/main/java/org/onosproject/net/DefaultAnnotations.java
View file @74b5527
+++ b/core/api/src/main/java/org/onosproject/net/DefaultAnnotations.java
View file @74b5527
@@ -189,6 +189,11 @@ public final class DefaultAnnotations implements SparseAnnotations {
         throw new IllegalArgumentException("Expecting HashMap instance");
     }
+    @Override
+    public String toString() {
+        return (map == null) ? "null" : map.toString();
+    }
+
     /**
      * Facility for gradually building model annotations.
      */
--- a/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
View file @74b5527
+++ b/core/store/dist/src/main/java/org/onosproject/store/device/impl/GossipDeviceStore.java
View file @74b5527
@@ -1023,11 +1023,12 @@ public class GossipDeviceStore
                     continue;
                 }
                 annotations = merge(annotations, otherPortDesc.value().annotations());
-                switch (otherPortDesc.value().type()) {
+                PortDescription other = otherPortDesc.value();
+                switch (other.type()) {
                     case OMS:
                         OmsPortDescription omsPortDesc = (OmsPortDescription) otherPortDesc.value();
                         updated = new OmsPort(device, number, isEnabled, omsPortDesc.minFrequency(),
-                                omsPortDesc.maxFrequency(), omsPortDesc.grid());
+                                omsPortDesc.maxFrequency(), omsPortDesc.grid(), annotations);
                         break;
                     case OCH:
                         OchPortDescription ochPortDesc = (OchPortDescription) otherPortDesc.value();
@@ -1039,7 +1040,8 @@ public class GossipDeviceStore
                         updated = new OduCltPort(device, number, isEnabled, oduCltPortDesc.signalType(), annotations);
                         break;
                     default:
-                        updated = new DefaultPort(device, number, isEnabled, annotations);
+                        updated = new DefaultPort(
+                                device, number, isEnabled, other.type(), other.portSpeed(), annotations);
                 }
                 newest = otherPortDesc.timestamp();
             }
@@ -1047,7 +1049,10 @@ public class GossipDeviceStore
         if (portDesc == null) {
             return updated == null ? new DefaultPort(device, number, false, annotations) : updated;
         }
-        return updated == null ? new DefaultPort(device, number, isEnabled, annotations) : updated;
+        PortDescription current = portDesc.value();
+        return updated == null
+                ? new DefaultPort(device, number, isEnabled, current.type(), current.portSpeed(), annotations)
+                : updated;
     }
     /**
--- a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
View file @74b5527
+++ b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
View file @74b5527
@@ -342,7 +342,6 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
             if (sw.isOptical()) {
                 OpenFlowOpticalSwitch opsw = (OpenFlowOpticalSwitch) sw;
                 opsw.getPortTypes().forEach(type -> {
-                    LOG.debug("ports: {}", opsw.getPortsOf(type));
                     opsw.getPortsOf(type).forEach(
                         op -> {
                             portDescs.add(buildPortDescription(type, (OFPortOptical) op));
@@ -398,10 +397,10 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
         private PortDescription buildPortDescription(PortDescPropertyType ptype, OFPortOptical port) {
             // Minimally functional fixture. This needs to be fixed as we add better support.
             PortNumber portNo = PortNumber.portNumber(port.getPortNo().getPortNumber());
+
             boolean enabled = !port.getState().contains(OFPortState.LINK_DOWN)
                     && !port.getConfig().contains(OFPortConfig.PORT_DOWN);
             SparseAnnotations annotations = makePortNameAnnotation(port.getName());
-            Port.Type type = FIBER;
             if (port.getVersion() == OFVersion.OF_13
                     && ptype == PortDescPropertyType.OPTICAL_TRANSPORT) {
@@ -411,7 +410,7 @@ public class OpenFlowDeviceProvider extends AbstractProvider implements DevicePr
                 // removable once 1.4+ support complete.
                 LOG.debug("Unsupported optical port properties");
             }
-            return new DefaultPortDescription(portNo, enabled, type, 0, annotations);
+            return new DefaultPortDescription(portNo, enabled, FIBER, 0, annotations);
         }
         private PortDescription buildPortDescription(OFPortStatus status) {
--- a/tools/test/topos/opticalUtils.py
View file @74b5527
+++ b/tools/test/topos/opticalUtils.py
View file @74b5527
@@ -445,12 +445,12 @@ class LINCSwitch(OpticalSwitch):
             sleep(SLEEP_TIME)
         info('*** Pushing Topology.json to ONOS\n')
-        output = quietRun('%s/tools/test/bin/onos-topo-cfg %s Topology.json' % (LINCSwitch.onosDir, LINCSwitch.controllers[ 0 ].ip), shell=True)
+        for index in range(len(LINCSwitch.controllers)):
-
+            output = quietRun('%s/tools/test/bin/onos-topo-cfg %s Topology.json &' % (LINCSwitch.onosDir, LINCSwitch.controllers[ index ].ip), shell=True)
-        # successful output contains the two characters '{}'
+            # successful output contains the two characters '{}'
-        # if there is more output than this, there is an issue
+            # if there is more output than this, there is an issue
-        if output.strip('{}'):
+            if output.strip('{}'):
-            warn('***WARNING: Could not push topology file to ONOS: %s\n' % output)
+                warn('***WARNING: Could not push topology file to ONOS: %s\n' % output)
     @staticmethod
     def waitStarted(net, timeout=TIMEOUT):
--- a/web/api/src/main/java/org/onosproject/rest/resources/ConfigProvider.java
View file @74b5527
+++ b/web/api/src/main/java/org/onosproject/rest/resources/ConfigProvider.java
View file @74b5527
@@ -16,13 +16,16 @@
 package org.onosproject.rest.resources;
 import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.base.Strings;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import org.onlab.packet.ChassisId;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.VlanId;
 import org.onlab.util.Frequency;
+import org.onosproject.net.AnnotationKeys;
 import org.onosproject.net.ChannelSpacing;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DefaultAnnotations;
@@ -40,6 +43,7 @@ import org.onosproject.net.OduCltPort;
 import org.onosproject.net.OduSignalType;
 import org.onosproject.net.OmsPort;
 import org.onosproject.net.Port;
+import org.onosproject.net.PortNumber;
 import org.onosproject.net.SparseAnnotations;
 import org.onosproject.net.device.DefaultDeviceDescription;
 import org.onosproject.net.device.DefaultPortDescription;
@@ -71,6 +75,7 @@ import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -113,6 +118,7 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
     private DeviceListener deviceEventCounter = new DeviceEventCounter();
     private List<ConnectPoint> connectPoints = Lists.newArrayList();
+    private Map<ConnectPoint, PortDescription> descriptions = Maps.newHashMap();
     /**
      * Creates a new configuration provider.
@@ -216,24 +222,29 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
     // Parses the given node with port information.
     private PortDescription parsePort(JsonNode node) {
         Port.Type type = Port.Type.valueOf(node.path("type").asText("COPPER"));
+        PortNumber port = portNumber(node.path("port").asLong(0));
+        String portName = Strings.emptyToNull(port.name());
+        SparseAnnotations annotations  = null;
+        if (portName != null) {
+            annotations = DefaultAnnotations.builder()
+                    .set(AnnotationKeys.PORT_NAME, portName).build();
+        }
         switch (type) {
             case COPPER:
-                return new DefaultPortDescription(portNumber(node.path("port").asLong(0)),
+                return new DefaultPortDescription(port, node.path("enabled").asBoolean(true),
-                        node.path("enabled").asBoolean(true),
+                                                  type, node.path("speed").asLong(1_000),
-                        type, node.path("speed").asLong(1_000));
+                                                  annotations);
             case FIBER:
                 // Currently, assume OMS when FIBER. Provide sane defaults.
-                return new OmsPortDescription(portNumber(node.path("port").asLong(0)),
+                return new OmsPortDescription(port, node.path("enabled").asBoolean(true),
-                        node.path("enabled").asBoolean(true),
+                                              CENTER, CENTER.add(TOTAL),
-                        CENTER,
+                                              Frequency.ofGHz(100), annotations);
-                        CENTER.add(TOTAL),
-                        Frequency.ofGHz(100));
             default:
                 log.warn("{}: Unsupported Port Type");
         }
-        return new DefaultPortDescription(portNumber(node.path("port").asLong(0)),
+        return new DefaultPortDescription(port, node.path("enabled").asBoolean(true),
-                                          node.path("enabled").asBoolean(true),
+                                          type, node.path("speed").asLong(1_000),
-                                          type, node.path("speed").asLong(1_000));
+                                          annotations);
     }
     // Parses the given JSON and provides links as configured.
@@ -267,17 +278,12 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
     }
     private void updatePorts(ConnectPoint src, ConnectPoint dst, SparseAnnotations annotations) {
-        DeviceId srcId = src.deviceId();
-        DeviceId dstId = dst.deviceId();
-        Port srcPort = deviceService.getPort(srcId, src.port());
-        Port dstPort = deviceService.getPort(dstId, dst.port());
-
         final String linkType = annotations.value("optical.type");
         if ("cross-connect".equals(linkType)) {
             String value = annotations.value("bandwidth").trim();
             try {
                 double bw = Double.parseDouble(value);
-                updateOchPort(bw, srcPort, dstPort, srcId, dstId);
+                updateOchPort(bw, src, dst);
             } catch (NumberFormatException e) {
                 log.warn("Invalid bandwidth ({}), can't configure port(s)", value);
                 return;
@@ -286,7 +292,7 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
             String value = annotations.value("optical.waves").trim();
             try {
                 int numChls = Integer.parseInt(value);
-                updateOMSPorts(numChls, srcPort, dstPort, srcId, dstId);
+                updateOMSPorts(numChls, src, dst);
             } catch (NumberFormatException e) {
                 log.warn("Invalid channel ({}), can't configure port(s)", value);
                 return;
@@ -295,10 +301,10 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
     }
     // uses 'bandwidth' annotation to determine the channel spacing.
-    private void updateOchPort(double bw, Port srcPort, Port dstPort, DeviceId srcId, DeviceId dstId) {
+    private void updateOchPort(double bw, ConnectPoint srcCp, ConnectPoint dstCp) {
-        Device src = deviceService.getDevice(srcId);
+        Device src = deviceService.getDevice(srcCp.deviceId());
-        Device dst = deviceService.getDevice(dstId);
+        Device dst = deviceService.getDevice(dstCp.deviceId());
-        // bandwidth in MHz (assuming Hz - linc is not clear if Hz or b).
+        // bandwidth in MHz (assuming Hz - linc is not clear if that or Mb).
         Frequency spacing = Frequency.ofMHz(bw);
         // channel bandwidth is smaller than smallest standard channel spacing.
         ChannelSpacing chsp = null;
@@ -319,20 +325,21 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
         }
         OchSignal signal = new OchSignal(GridType.DWDM, chsp, 1, 1);
         if (src.type() == Device.Type.ROADM) {
-            PortDescription portDesc = new OchPortDescription(srcPort.number(), srcPort.isEnabled(),
+            PortDescription portDesc = new OchPortDescription(srcCp.port(), true,
                     OduSignalType.ODU4, true, signal);
-            deviceProviderService.portStatusChanged(srcId, portDesc);
+            descriptions.put(srcCp, portDesc);
+            deviceProviderService.portStatusChanged(srcCp.deviceId(), portDesc);
         }
         if (dst.type() == Device.Type.ROADM) {
-            PortDescription portDesc = new OchPortDescription(dstPort.number(), dstPort.isEnabled(),
+            PortDescription portDesc = new OchPortDescription(dstCp.port(), true,
                     OduSignalType.ODU4, true, signal);
-            deviceProviderService.portStatusChanged(dstId, portDesc);
+            descriptions.put(dstCp, portDesc);
+            deviceProviderService.portStatusChanged(dstCp.deviceId(), portDesc);
         }
     }
-    private void updateOMSPorts(int numChls, Port srcPort, Port dstPort, DeviceId srcId, DeviceId dstId) {
+    private void updateOMSPorts(int numChls, ConnectPoint srcCp, ConnectPoint dstCp) {
-        // round down to largest slot that allows numChl channels to fit into C
+        // round down to largest slot that allows numChl channels to fit into C band range
-        // band range
         ChannelSpacing chl = null;
         Frequency perChl = TOTAL.floorDivision(numChls);
         for (int i = 0; i < ChannelSpacing.values().length; i++) {
@@ -352,10 +359,12 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
         Frequency min = CENTER.add(grid);
         Frequency max = CENTER.add(grid.multiply(numChls));
-        PortDescription srcPortDesc = new OmsPortDescription(srcPort.number(), srcPort.isEnabled(), min, max, grid);
+        PortDescription srcPortDesc = new OmsPortDescription(srcCp.port(), true, min, max, grid);
-        PortDescription dstPortDesc = new OmsPortDescription(dstPort.number(), dstPort.isEnabled(), min, max, grid);
+        PortDescription dstPortDesc = new OmsPortDescription(dstCp.port(), true, min, max, grid);
-        deviceProviderService.portStatusChanged(srcId, srcPortDesc);
+        descriptions.put(srcCp, srcPortDesc);
-        deviceProviderService.portStatusChanged(dstId, dstPortDesc);
+        descriptions.put(dstCp, dstPortDesc);
+        deviceProviderService.portStatusChanged(srcCp.deviceId(), srcPortDesc);
+        deviceProviderService.portStatusChanged(dstCp.deviceId(), dstPortDesc);
     }
     // Parses the given JSON and provides hosts as configured.
@@ -440,8 +449,12 @@ class ConfigProvider implements DeviceProvider, LinkProvider, HostProvider {
         }
     }
-    // Creates a port description from the specified connection point.
+    // Creates a port description from the specified connection point if none created earlier.
     private PortDescription description(ConnectPoint cp) {
+        PortDescription saved = descriptions.get(cp);
+        if (saved != null) {
+            return saved;
+        }
         Port p = deviceService.getPort(cp.deviceId(), cp.port());
         if (p == null) {
             return new DefaultPortDescription(cp.port(), true);