Remove default flow rules for ROADMs. Fix LinkResource NPE. Single instance provisions optical path.

Change-Id: Iacbd41403ecd0c0df240d09026253c4b761a1d6a

Remove default flow rules for ROADMs. Fix LinkResource NPE. Single instance provisions optical path.
Change-Id: Iacbd41403ecd0c0df240d09026253c4b761a1d6a
Marc De Leenheer · Gerrit Code Review
Commit 8b3e80b14e04501985ba303ce515168dbb05337f 8b3e80b1 1 parent e106e4b7
Showing 5 changed files with 61 additions and 29 deletions
apps/optical/src/main/java/org/onosproject/optical/OpticalPathProvisioner.java
core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
core/net/src/main/java/org/onosproject/net/resource/impl/LinkResourceManager.java
providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java
--- a/apps/optical/src/main/java/org/onosproject/optical/OpticalPathProvisioner.java
View file @8b3e80b
+++ b/apps/optical/src/main/java/org/onosproject/optical/OpticalPathProvisioner.java
View file @8b3e80b
@@ -20,8 +20,11 @@ import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
+import org.onosproject.cluster.ClusterService;
+import org.onosproject.cluster.NodeId;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
+import org.onosproject.mastership.MastershipService;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.Host;
 import org.onosproject.net.Link;
@@ -74,6 +77,12 @@ public class OpticalPathProvisioner {
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected HostService hostService;
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected MastershipService mastershipService;
+
+    @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
+    protected ClusterService clusterService;
+
     private ApplicationId appId;
     // TODO use a shared map for distributed operation
@@ -89,11 +98,10 @@ public class OpticalPathProvisioner {
     @Activate
     protected void activate() {
-        // TODO elect a leader and have one instance do the provisioning
         intentService.addListener(pathProvisioner);
         appId = coreService.registerApplication("org.onosproject.optical");
         initTport();
-        log.info("Starting optical path provisoning...");
+        log.info("Starting optical path provisioning...");
     }
     protected void initTport() {
@@ -178,17 +186,27 @@ public class OpticalPathProvisioner {
             // Low speed LLDP may cause multiple calls which are not expected
             if (!IntentState.FAILED.equals(intentService.getIntentState(intent.key()))) {
-                   return;
+                return;
             }
+            NodeId localNode = clusterService.getLocalNode().id();
+
             List<Intent> intents = Lists.newArrayList();
             if (intent instanceof HostToHostIntent) {
                 HostToHostIntent hostToHostIntent = (HostToHostIntent) intent;
+
                 Host one = hostService.getHost(hostToHostIntent.one());
                 Host two = hostService.getHost(hostToHostIntent.two());
                 if (one == null || two == null) {
                     return; //FIXME
                 }
+
+                // Ignore if we're not the master for the intent's origin device
+                NodeId sourceMaster = mastershipService.getMasterFor(one.location().deviceId());
+                if (!localNode.equals(sourceMaster)) {
+                    return;
+                }
+
                 // provision both directions
                 intents.addAll(getOpticalPath(one.location(), two.location()));
                 // note: bi-directional intent is set up
@@ -196,6 +214,13 @@ public class OpticalPathProvisioner {
                 //intents.addAll(getOpticalPath(two.location(), one.location()));
             } else if (intent instanceof PointToPointIntent) {
                 PointToPointIntent p2pIntent = (PointToPointIntent) intent;
+
+                // Ignore if we're not the master for the intent's origin device
+                NodeId sourceMaster = mastershipService.getMasterFor(p2pIntent.ingressPoint().deviceId());
+                if (!localNode.equals(sourceMaster)) {
+                    return;
+                }
+
                 intents.addAll(getOpticalPath(p2pIntent.ingressPoint(), p2pIntent.egressPoint()));
             } else {
                 log.info("Unsupported intent type: {}", intent.getClass());
--- a/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
View file @8b3e80b
+++ b/core/net/src/main/java/org/onosproject/net/packet/impl/PacketManager.java
View file @8b3e80b
@@ -220,6 +220,11 @@ implements PacketService, PacketProviderRegistry {
      * @param request the packet request
      */
     private void pushRule(Device device, PacketRequest request) {
+        // Everything is pre-provisioned on ROADMs
+        if (device.type().equals(Device.Type.ROADM)) {
+            return;
+        }
+
         TrafficTreatment treatment = DefaultTrafficTreatment.builder()
                                                             .punt()
                                                             .build();
--- a/core/net/src/main/java/org/onosproject/net/resource/impl/LinkResourceManager.java
View file @8b3e80b
+++ b/core/net/src/main/java/org/onosproject/net/resource/impl/LinkResourceManager.java
View file @8b3e80b
@@ -15,17 +15,6 @@
  */
 package org.onosproject.net.resource.impl;
-import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.common.base.Preconditions.checkNotNull;
-import static org.slf4j.LoggerFactory.getLogger;
-
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.Set;
-
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
@@ -57,6 +46,17 @@ import org.onosproject.net.resource.ResourceRequest;
 import org.onosproject.net.resource.ResourceType;
 import org.slf4j.Logger;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.slf4j.LoggerFactory.getLogger;
+
 /**
  * Provides basic implementation of link resources allocation.
  */
@@ -197,7 +197,10 @@ public class LinkResourceManager implements LinkResourceService {
         Map<Link, Set<ResourceAllocation>> allocations = new HashMap<>();
         for (Link link : req.links()) {
             allocations.put(link, new HashSet<ResourceAllocation>(allocs));
-            allocations.get(link).addAll(allocsPerLink.get(link));
+            Set<ResourceAllocation> linkAllocs = allocsPerLink.get(link);
+            if (linkAllocs != null) {
+                allocations.get(link).addAll(linkAllocs);
+            }
         }
         LinkResourceAllocations result =
                 new DefaultLinkResourceAllocations(req, allocations);
--- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
View file @8b3e80b
+++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
View file @8b3e80b
@@ -90,7 +90,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
     protected DeviceService deviceService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected PacketService packetSevice;
+    protected PacketService packetService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected MastershipService masterService;
@@ -143,7 +143,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
         providerService = providerRegistry.register(this);
         deviceService.addListener(listener);
-        packetSevice.addProcessor(listener, 0);
+        packetService.addProcessor(listener, 0);
         masterService.addListener(roleListener);
         LinkDiscovery ld;
@@ -152,7 +152,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
                 log.debug("LinkDiscovery from {} disabled by configuration", device.id());
                 continue;
             }
-            ld = new LinkDiscovery(device, packetSevice, masterService,
+            ld = new LinkDiscovery(device, packetService, masterService,
                               providerService, useBDDP);
             discoverers.put(device.id(), ld);
             for (Port p : deviceService.getPorts(device.id())) {
@@ -186,7 +186,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
         }
         providerRegistry.unregister(this);
         deviceService.removeListener(listener);
-        packetSevice.removeProcessor(listener);
+        packetService.removeProcessor(listener);
         masterService.removeListener(roleListener);
         providerService = null;
@@ -237,14 +237,14 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
     private void requestPackets() {
         TrafficSelector.Builder lldpSelector = DefaultTrafficSelector.builder();
         lldpSelector.matchEthType(Ethernet.TYPE_LLDP);
-        packetSevice.requestPackets(lldpSelector.build(),
+        packetService.requestPackets(lldpSelector.build(),
-                                    PacketPriority.CONTROL, appId);
+                PacketPriority.CONTROL, appId);
         if (useBDDP) {
             TrafficSelector.Builder bddpSelector = DefaultTrafficSelector.builder();
             bddpSelector.matchEthType(Ethernet.TYPE_BSN);
-            packetSevice.requestPackets(bddpSelector.build(),
+            packetService.requestPackets(bddpSelector.build(),
-                                        PacketPriority.CONTROL, appId);
+                    PacketPriority.CONTROL, appId);
         }
     }
@@ -273,7 +273,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
                     log.debug("Device mastership changed ({}) {}",
                             event.type(), deviceId);
                     discoverers.put(deviceId, new LinkDiscovery(device,
-                            packetSevice, masterService, providerService,
+                            packetService, masterService, providerService,
                             useBDDP));
                 }
             }
@@ -307,8 +307,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
                             log.debug("Device added ({}) {}", event.type(),
                                       deviceId);
                             discoverers.put(deviceId, new LinkDiscovery(device,
-                                                                        packetSevice, masterService, providerService,
+                                    packetService, masterService, providerService, useBDDP));
-                                                                        useBDDP));
                         } else {
                             if (ld.isStopped()) {
                                 log.debug("Device restarted ({}) {}", event.type(),
@@ -412,7 +411,7 @@ public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
                     DeviceId did = dev.id();
                     synchronized (discoverers) {
                         if (!discoverers.containsKey(did)) {
-                            ld = new LinkDiscovery(dev, packetSevice,
+                            ld = new LinkDiscovery(dev, packetService,
                                     masterService, providerService, useBDDP);
                             discoverers.put(did, ld);
                             for (Port p : deviceService.getPorts(did)) {
--- a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java
View file @8b3e80b
+++ b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LLDPLinkProviderTest.java
View file @8b3e80b
@@ -112,7 +112,7 @@ public class LLDPLinkProviderTest {
         provider.coreService = coreService;
         provider.deviceService = deviceService;
-        provider.packetSevice = packetService;
+        provider.packetService = packetService;
         provider.providerRegistry = linkService;
         provider.masterService = masterService;
@@ -204,7 +204,7 @@ public class LLDPLinkProviderTest {
         provider.coreService = null;
         provider.providerRegistry = null;
         provider.deviceService = null;
-        provider.packetSevice = null;
+        provider.packetService = null;
     }
     private DeviceEvent deviceEvent(DeviceEvent.Type type, DeviceId did) {