Bug-fix: Remove flows from voltha for deleted subscriber, even if sadis cannot
fetch information about the deleted subscriber.
Other changes:
- Added a cli command that details subscribers that have actually been programmed in the data plane
- Changed all cli commands to start with volt-* as all app cli commands should
- removed DeviceIdCompleter from the 'olts' cli command as it is not needed
- removed unused method, renamed another one, fixed some logs
- S-tags are no longer 'DeviceVlans', so changed variable name
Change-Id: I7cfde850669ed5b581fbac3195f68da28c4514c3
diff --git a/api/pom.xml b/api/pom.xml
index 74e93d8..87c67a4 100644
--- a/api/pom.xml
+++ b/api/pom.xml
@@ -32,6 +32,9 @@
<url>http://opencord.org</url>
<description>OLT application API</description>
+ <properties>
+ <sadis.api.version>2.1.0</sadis.api.version>
+ </properties>
<dependencies>
<dependency>
@@ -46,6 +49,12 @@
<artifactId>onos-core-serializers</artifactId>
<version>${onos.version}</version>
</dependency>
+
+ <dependency>
+ <groupId>org.opencord</groupId>
+ <artifactId>sadis-api</artifactId>
+ <version>${sadis.api.version}</version>
+ </dependency>
</dependencies>
<build>
diff --git a/api/src/main/java/org/opencord/olt/AccessDeviceService.java b/api/src/main/java/org/opencord/olt/AccessDeviceService.java
index 9b1e574..6387a03 100644
--- a/api/src/main/java/org/opencord/olt/AccessDeviceService.java
+++ b/api/src/main/java/org/opencord/olt/AccessDeviceService.java
@@ -16,14 +16,17 @@
package org.opencord.olt;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+
import org.onlab.packet.VlanId;
import org.onosproject.event.ListenerService;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.DeviceId;
+import org.opencord.sadis.SubscriberAndDeviceInformation;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
+import com.google.common.collect.ImmutableMap;
/**
* Service for interacting with an access device (OLT).
@@ -77,4 +80,12 @@
*/
List<DeviceId> fetchOlts();
+ /**
+ * Returns information about subscribers that have been programmed in the
+ * dataplane.
+ *
+ * @return an immutable map of locations and subscriber information
+ */
+ ImmutableMap<ConnectPoint, SubscriberAndDeviceInformation> getProgSubs();
+
}
diff --git a/app/src/main/java/org/opencord/olt/cli/ShowOltCommand.java b/app/src/main/java/org/opencord/olt/cli/ShowOltCommand.java
index a256ccd..d01e8e2 100644
--- a/app/src/main/java/org/opencord/olt/cli/ShowOltCommand.java
+++ b/app/src/main/java/org/opencord/olt/cli/ShowOltCommand.java
@@ -18,14 +18,13 @@
import org.apache.karaf.shell.commands.Command;
import org.onosproject.cli.AbstractShellCommand;
-
import org.opencord.olt.AccessDeviceService;
/**
* Shows configured OLTs.
*/
-@Command(scope = "onos", name = "olts",
- description = "Shows OLTs connected to ONOS")
+@Command(scope = "onos", name = "volt-olts",
+ description = "Shows vOLTs connected to ONOS")
public class ShowOltCommand extends AbstractShellCommand {
@Override
diff --git a/app/src/main/java/org/opencord/olt/cli/ShowProgrammedSubscribersCommand.java b/app/src/main/java/org/opencord/olt/cli/ShowProgrammedSubscribersCommand.java
new file mode 100644
index 0000000..2a4409e
--- /dev/null
+++ b/app/src/main/java/org/opencord/olt/cli/ShowProgrammedSubscribersCommand.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2016-present Open Networking Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.opencord.olt.cli;
+
+import java.util.Map;
+
+import org.apache.karaf.shell.commands.Command;
+import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.net.ConnectPoint;
+import org.opencord.olt.AccessDeviceService;
+import org.opencord.sadis.SubscriberAndDeviceInformation;
+
+/**
+ * Shows subscriber information for those subscriber which have been programmed
+ * in the data-plane.
+ */
+@Command(scope = "onos", name = "volt-programmed-subscribers",
+ description = "Shows subscribers programmed in the dataplane")
+public class ShowProgrammedSubscribersCommand extends AbstractShellCommand {
+
+ @Override
+ protected void execute() {
+ AccessDeviceService service = AbstractShellCommand.get(AccessDeviceService.class);
+ Map<ConnectPoint, SubscriberAndDeviceInformation> info = service.getProgSubs();
+ info.forEach(this::display);
+ }
+
+ private void display(ConnectPoint cp, SubscriberAndDeviceInformation sub) {
+ print("location=%s subscriber=%s", cp, sub);
+ }
+}
diff --git a/app/src/main/java/org/opencord/olt/cli/ShowSubscribersCommand.java b/app/src/main/java/org/opencord/olt/cli/ShowSubscribersCommand.java
index 4298cd6..711d1fa 100644
--- a/app/src/main/java/org/opencord/olt/cli/ShowSubscribersCommand.java
+++ b/app/src/main/java/org/opencord/olt/cli/ShowSubscribersCommand.java
@@ -16,19 +16,20 @@
package org.opencord.olt.cli;
+import java.util.Map;
+
import org.apache.karaf.shell.commands.Command;
import org.onlab.packet.VlanId;
import org.onosproject.cli.AbstractShellCommand;
import org.onosproject.net.ConnectPoint;
import org.opencord.olt.AccessDeviceService;
-import java.util.Map;
-
/**
- * Shows provisioned subscribers.
+ * Shows provisioned (configured) subscribers. The data plane flows for the
+ * subscribers may or may not have been programmed.
*/
-@Command(scope = "onos", name = "subscribers",
- description = "Shows provisioned subscribers")
+@Command(scope = "onos", name = "volt-subscribers",
+ description = "Shows pre-provisioned subscribers")
public class ShowSubscribersCommand extends AbstractShellCommand {
private static final String FORMAT = "port=%s, svlan=%s, cvlan=%s";
diff --git a/app/src/main/java/org/opencord/olt/cli/SubscriberAddCommand.java b/app/src/main/java/org/opencord/olt/cli/SubscriberAddCommand.java
index d5fa0da..f6db81e 100644
--- a/app/src/main/java/org/opencord/olt/cli/SubscriberAddCommand.java
+++ b/app/src/main/java/org/opencord/olt/cli/SubscriberAddCommand.java
@@ -27,7 +27,7 @@
/**
* Adds a subscriber to an access device.
*/
-@Command(scope = "onos", name = "add-subscriber-access",
+@Command(scope = "onos", name = "volt-add-subscriber-access",
description = "Adds a subscriber to an access device")
public class SubscriberAddCommand extends AbstractShellCommand {
diff --git a/app/src/main/java/org/opencord/olt/cli/SubscriberRemoveCommand.java b/app/src/main/java/org/opencord/olt/cli/SubscriberRemoveCommand.java
index 4d6ddc7..a6d901d 100644
--- a/app/src/main/java/org/opencord/olt/cli/SubscriberRemoveCommand.java
+++ b/app/src/main/java/org/opencord/olt/cli/SubscriberRemoveCommand.java
@@ -27,7 +27,7 @@
/**
* Adds a subscriber to an access device.
*/
-@Command(scope = "onos", name = "remove-subscriber-access",
+@Command(scope = "onos", name = "volt-remove-subscriber-access",
description = "Adds a subscriber to an access device")
public class SubscriberRemoveCommand extends AbstractShellCommand {
diff --git a/app/src/main/java/org/opencord/olt/impl/Olt.java b/app/src/main/java/org/opencord/olt/impl/Olt.java
index 3c5108e..e1d1dac 100644
--- a/app/src/main/java/org/opencord/olt/impl/Olt.java
+++ b/app/src/main/java/org/opencord/olt/impl/Olt.java
@@ -15,6 +15,25 @@
*/
package org.opencord.olt.impl;
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
+import static org.onlab.util.Tools.get;
+import static org.onlab.util.Tools.groupedThreads;
+import static org.slf4j.LoggerFactory.getLogger;
+
+import java.util.AbstractMap;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Dictionary;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Properties;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -64,24 +83,8 @@
import org.osgi.service.component.ComponentContext;
import org.slf4j.Logger;
-import java.util.AbstractMap;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Dictionary;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Properties;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-
-import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.common.base.Strings.isNullOrEmpty;
-import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
-import static org.onlab.util.Tools.get;
-import static org.onlab.util.Tools.groupedThreads;
-import static org.slf4j.LoggerFactory.getLogger;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
/**
* Provisions rules on access devices.
@@ -131,18 +134,21 @@
private ApplicationId appId;
- private ExecutorService oltInstallers = Executors.newFixedThreadPool(4,
- groupedThreads("onos/olt-service",
- "olt-installer-%d"));
+ private ExecutorService oltInstallers = Executors
+ .newFixedThreadPool(4, groupedThreads("onos/olt-service",
+ "olt-installer-%d"));
protected ExecutorService eventExecutor;
+ private Map<ConnectPoint, SubscriberAndDeviceInformation> programmedSubs;
+
@Activate
public void activate(ComponentContext context) {
eventExecutor = newSingleThreadScheduledExecutor(groupedThreads("onos/olt", "events-%d", log));
modified(context);
appId = coreService.registerApplication(APP_NAME);
componentConfigService.registerProperties(getClass());
+ programmedSubs = Maps.newConcurrentMap();
eventDispatcher.addSink(AccessDeviceEvent.class, listenerRegistry);
@@ -210,14 +216,16 @@
if (enableDhcpOnProvisioning) {
processDhcpFilteringObjectives(port.deviceId(), port.port(), true);
}
-
+ log.info("Programming vlans for subscriber: {}", sub);
Optional<VlanId> defaultVlan = Optional.empty();
- provisionVlans(port.deviceId(), uplinkPort.number(), port.port(), sub.cTag(), sub.sTag(),
- defaultVlan);
+ provisionVlans(port.deviceId(), uplinkPort.number(), port.port(),
+ sub.cTag(), sub.sTag(), defaultVlan);
if (enableIgmpOnProvisioning) {
processIgmpFilteringObjectives(port.deviceId(), port.port(), true);
}
+ // cache subscriber info
+ programmedSubs.put(port, sub);
return true;
}
@@ -226,8 +234,13 @@
// Get the subscriber connected to this port from Sadis
SubscriberAndDeviceInformation subscriber = getSubscriber(port);
if (subscriber == null) {
- log.warn("Subscriber on port {} not found", port);
- return false;
+ log.warn("Subscriber on port {} not found in sadis .. checking "
+ + "local cache", port);
+ subscriber = programmedSubs.get(port);
+ if (subscriber == null) {
+ log.warn("Subscriber on port {} was not previously programmed", port);
+ return false;
+ }
}
// Get the uplink port
@@ -241,13 +254,15 @@
processDhcpFilteringObjectives(port.deviceId(), port.port(), false);
}
+ log.info("Removing programmed vlans for subscriber: {}", subscriber);
Optional<VlanId> defaultVlan = Optional.empty();
- unprovisionSubscriber(port.deviceId(), uplinkPort.number(), port.port(), subscriber.cTag(),
- subscriber.sTag(), defaultVlan);
+ unprovisionSubscriber(port.deviceId(), uplinkPort.number(), port.port(),
+ subscriber.cTag(), subscriber.sTag(), defaultVlan);
if (enableIgmpOnProvisioning) {
processIgmpFilteringObjectives(port.deviceId(), port.port(), false);
}
+ programmedSubs.remove(port);
return true;
}
@@ -279,9 +294,12 @@
public Collection<Map.Entry<ConnectPoint, Map.Entry<VlanId, VlanId>>> getSubscribers() {
ArrayList<Map.Entry<ConnectPoint, Map.Entry<VlanId, VlanId>>> subs = new ArrayList<>();
- // Get the subscribers for all the devices
+ // Get the subscribers for all the devices configured in sadis
// If the port is UNI, is enabled and exists in Sadis then copy it
for (Device d : deviceService.getDevices()) {
+ if (getOltInfo(d) == null) {
+ continue; // not an olt, or not configured in sadis
+ }
for (Port p: deviceService.getPorts(d.id())) {
if (isUniPort(d, p) && p.isEnabled()) {
ConnectPoint cp = new ConnectPoint(d.id(), p.number());
@@ -299,16 +317,18 @@
}
@Override
+ public ImmutableMap<ConnectPoint, SubscriberAndDeviceInformation> getProgSubs() {
+ return ImmutableMap.copyOf(programmedSubs);
+ }
+
+ @Override
public List<DeviceId> fetchOlts() {
// look through all the devices and find the ones that are OLTs as per Sadis
List<DeviceId> olts = new ArrayList<>();
Iterable<Device> devices = deviceService.getDevices();
for (Device d : devices) {
- String devSerialNo = d.serialNumber();
- SubscriberAndDeviceInformation deviceInfo = subsService.get(devSerialNo);
-
- if (deviceInfo != null) {
- // So this is indeed a OLT device
+ if (getOltInfo(d) != null) {
+ // So this is indeed an OLT device
olts.add(d.id());
}
}
@@ -336,31 +356,20 @@
return null;
}
- private void initializeUni(Port port) {
- DeviceId deviceId = (DeviceId) port.element().id();
-
- post(new AccessDeviceEvent(AccessDeviceEvent.Type.UNI_ADDED, deviceId, port));
-
- if (port.isEnabled()) {
- processFilteringObjectives(deviceId, port.number(), true);
- }
- }
-
private void unprovisionSubscriber(DeviceId deviceId, PortNumber uplink,
- PortNumber subscriberPort, VlanId subscriberVlan,
- VlanId deviceVlan, Optional<VlanId> defaultVlan) {
+ PortNumber subscriberPort, VlanId cVlan,
+ VlanId sVlan, Optional<VlanId> defaultVlan) {
CompletableFuture<ObjectiveError> downFuture = new CompletableFuture();
CompletableFuture<ObjectiveError> upFuture = new CompletableFuture();
ForwardingObjective.Builder upFwd = upBuilder(uplink, subscriberPort,
- subscriberVlan, deviceVlan,
+ cVlan, sVlan,
defaultVlan);
ForwardingObjective.Builder downFwd = downBuilder(uplink, subscriberPort,
- subscriberVlan, deviceVlan,
+ cVlan, sVlan,
defaultVlan);
-
flowObjectiveService.forward(deviceId, upFwd.remove(new ObjectiveContext() {
@Override
public void onSuccess(Objective objective) {
@@ -389,16 +398,16 @@
if (upStatus == null && downStatus == null) {
post(new AccessDeviceEvent(AccessDeviceEvent.Type.SUBSCRIBER_UNREGISTERED,
deviceId,
- deviceVlan,
- subscriberVlan));
+ sVlan,
+ cVlan));
} else if (downStatus != null) {
log.error("Subscriber with vlan {} on device {} " +
"on port {} failed downstream uninstallation: {}",
- subscriberVlan, deviceId, subscriberPort, downStatus);
+ cVlan, deviceId, subscriberPort, downStatus);
} else if (upStatus != null) {
log.error("Subscriber with vlan {} on device {} " +
"on port {} failed upstream uninstallation: {}",
- subscriberVlan, deviceId, subscriberPort, upStatus);
+ cVlan, deviceId, subscriberPort, upStatus);
}
}, oltInstallers);
@@ -519,7 +528,7 @@
.withTreatment(upstreamTreatment);
}
- private void processFilteringObjectives(DeviceId devId, PortNumber port, boolean install) {
+ private void processEapolFilteringObjectives(DeviceId devId, PortNumber port, boolean install) {
if (!mastershipService.isLocalMaster(devId)) {
return;
}
@@ -535,14 +544,15 @@
.add(new ObjectiveContext() {
@Override
public void onSuccess(Objective objective) {
- log.info("Eapol filter for {} on {} installed.",
- devId, port);
+ log.info("Eapol filter for {} on {} {}.",
+ devId, port, (install) ? "installed" : "removed");
}
@Override
public void onError(Objective objective, ObjectiveError error) {
- log.info("Eapol filter for {} on {} failed because {}",
- devId, port, error);
+ log.info("Eapol filter for {} on {} failed {} because {}",
+ devId, port, (install) ? "installation" : "removal",
+ error);
}
});
@@ -579,14 +589,15 @@
.add(new ObjectiveContext() {
@Override
public void onSuccess(Objective objective) {
- log.info("LLDP filter for {} on {} installed.",
- devId, port);
+ log.info("LLDP filter for {} on {} {}.",
+ devId, port, (install) ? "installed" : "removed");
}
@Override
public void onError(Objective objective, ObjectiveError error) {
- log.info("LLDP filter for {} on {} failed because {}",
- devId, port, error);
+ log.info("LLDP filter for {} on {} failed {} because {}",
+ devId, port, (install) ? "installation" : "removal",
+ error);
}
});
@@ -613,14 +624,15 @@
.add(new ObjectiveContext() {
@Override
public void onSuccess(Objective objective) {
- log.info("DHCP filter for {} on {} installed.",
- devId, port);
+ log.info("DHCP filter for {} on {} {}.",
+ devId, port, (install) ? "installed" : "removed");
}
@Override
public void onError(Objective objective, ObjectiveError error) {
- log.info("DHCP filter for {} on {} failed because {}",
- devId, port, error);
+ log.info("DHCP filter for {} on {} failed {} because {}",
+ devId, port, (install) ? "installation" : "removal",
+ error);
}
});
@@ -647,14 +659,15 @@
.add(new ObjectiveContext() {
@Override
public void onSuccess(Objective objective) {
- log.info("Igmp filter for {} on {} installed.",
- devId, port);
+ log.info("Igmp filter for {} on {} {}.",
+ devId, port, (install) ? "installed" : "removed");
}
@Override
public void onError(Objective objective, ObjectiveError error) {
- log.info("Igmp filter for {} on {} failed because {}.",
- devId, port, error);
+ log.info("Igmp filter for {} on {} failed {} because {}.",
+ devId, port, (install) ? "installation" : "removal",
+ error);
}
});
@@ -681,7 +694,7 @@
// This is an OLT device as per Sadis, we create flows for UNI and NNI ports
for (Port p : deviceService.getPorts(dev.id())) {
if (isUniPort(dev, p)) {
- processFilteringObjectives(dev.id(), p.number(), true);
+ processEapolFilteringObjectives(dev.id(), p.number(), true);
} else {
processNniFilteringObjectives(dev.id(), p.number(), true);
}
@@ -704,18 +717,20 @@
String devSerialNo = dev.serialNumber();
SubscriberAndDeviceInformation deviceInfo = subsService.get(devSerialNo);
log.debug("getUplinkPort: deviceInfo {}", deviceInfo);
-
- if (deviceInfo != null) {
- // Return the port that has been configured as the uplink port of this OLT in Sadis
- for (Port p: deviceService.getPorts(dev.id())) {
- if (p.number().toLong() == deviceInfo.uplinkPort()) {
- log.debug("getUplinkPort: Found port {}", p);
- return p;
- }
+ if (deviceInfo == null) {
+ log.warn("Device {} is not configured in SADIS .. cannot fetch device"
+ + " info", dev.id());
+ return null;
+ }
+ // Return the port that has been configured as the uplink port of this OLT in Sadis
+ for (Port p: deviceService.getPorts(dev.id())) {
+ if (p.number().toLong() == deviceInfo.uplinkPort()) {
+ log.debug("getUplinkPort: Found port {}", p);
+ return p;
}
}
- log.debug("getUplinkPort: No uplink port found for OLT {}", dev);
+ log.debug("getUplinkPort: No uplink port found for OLT {}", dev.id());
return null;
}
@@ -772,7 +787,7 @@
post(new AccessDeviceEvent(AccessDeviceEvent.Type.UNI_ADDED, devId, event.port()));
if (event.port().isEnabled()) {
- processFilteringObjectives(devId, event.port().number(), true);
+ processEapolFilteringObjectives(devId, event.port().number(), true);
}
} else {
checkAndCreateDeviceFlows(dev);
@@ -781,7 +796,7 @@
case PORT_REMOVED:
if (isUniPort(dev, event.port())) {
if (event.port().isEnabled()) {
- processFilteringObjectives(devId, event.port().number(), false);
+ processEapolFilteringObjectives(devId, event.port().number(), false);
removeSubscriber(new ConnectPoint(devId, event.port().number()));
}
@@ -795,10 +810,10 @@
}
if (event.port().isEnabled()) {
- processFilteringObjectives(devId, event.port().number(), true);
+ processEapolFilteringObjectives(devId, event.port().number(), true);
post(new AccessDeviceEvent(AccessDeviceEvent.Type.UNI_ADDED, devId, event.port()));
} else {
- processFilteringObjectives(devId, event.port().number(), false);
+ processEapolFilteringObjectives(devId, event.port().number(), false);
post(new AccessDeviceEvent(AccessDeviceEvent.Type.UNI_REMOVED, devId, event.port()));
}
break;
diff --git a/app/src/main/resources/OSGI-INF/blueprint/shell-config.xml b/app/src/main/resources/OSGI-INF/blueprint/shell-config.xml
index 8cde67c..ace9bb6 100644
--- a/app/src/main/resources/OSGI-INF/blueprint/shell-config.xml
+++ b/app/src/main/resources/OSGI-INF/blueprint/shell-config.xml
@@ -32,14 +32,13 @@
</command>
<command>
<action class="org.opencord.olt.cli.ShowOltCommand"/>
- <completers>
- <ref component-id="deviceIdCompleter"/>
- <null/>
- </completers>
</command>
<command>
<action class="org.opencord.olt.cli.ShowSubscribersCommand"/>
</command>
+ <command>
+ <action class="org.opencord.olt.cli.ShowProgrammedSubscribersCommand"/>
+ </command>
</command-bundle>
<bean id="deviceIdCompleter" class="org.onosproject.cli.net.DeviceIdCompleter"/>