Eliminate code duplication
Change-Id: I79129b1dbcbba169b143730f89a38db64f5e460f
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index 1873530..678d8a0 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -27,7 +27,6 @@
import org.onosproject.net.Device;
import org.onosproject.net.DeviceId;
import org.onosproject.net.Link;
-import org.onosproject.net.PortNumber;
import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
import org.onosproject.segmentrouting.config.DeviceConfiguration;
import org.slf4j.Logger;
@@ -659,28 +658,6 @@
}
/**
- * Populates filtering rules for a port that has been enabled.
- * Should only be called by the master instance for this device/port.
- *
- * @param devId device identifier
- * @param portnum port identifier
- */
- public void populateSinglePortFilteringRules(DeviceId devId, PortNumber portnum) {
- rulePopulator.populateSinglePortFilters(devId, portnum);
- }
-
- /**
- * Revokes filtering rules for a port that has been disabled.
- * Should only be called by the master instance for this device/port.
- *
- * @param devId device identifier
- * @param portnum port identifier
- */
- public void revokeSinglePortFilteringRules(DeviceId devId, PortNumber portnum) {
- rulePopulator.revokeSinglePortFilters(devId, portnum);
- }
-
- /**
* Start the flow rule population process if it was never started. The
* process finishes successfully when all flow rules are set and stops with
* ABORTED status when any groups required for flows is not set yet.
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index 06a9dbc..6b5aba7 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -642,7 +642,7 @@
disabledPorts++;
continue;
}
- if (populateSinglePortFilters(deviceId, port.number())) {
+ if (processSinglePortFilters(deviceId, port.number(), true)) {
filteredPorts++;
} else {
errorPorts++;
@@ -655,14 +655,15 @@
}
/**
- * Creates filtering objectives for a single port. Should only be called
- * by the master for a switch.
+ * Creates or removes filtering objectives for a single port. Should only be
+ * called by the master for a switch.
*
* @param deviceId device identifier
* @param portnum port identifier for port to be filtered
+ * @param install true to install the filtering objective, false to remove
* @return true if no errors occurred during the build of the filtering objective
*/
- public boolean populateSinglePortFilters(DeviceId deviceId, PortNumber portnum) {
+ public boolean processSinglePortFilters(DeviceId deviceId, PortNumber portnum, boolean install) {
ConnectPoint connectPoint = new ConnectPoint(deviceId, portnum);
VlanId untaggedVlan = srManager.getUntaggedVlanId(connectPoint);
Set<VlanId> taggedVlans = srManager.getTaggedVlanId(connectPoint);
@@ -671,97 +672,48 @@
if (taggedVlans.size() != 0) {
// Filter for tagged vlans
if (!srManager.getTaggedVlanId(connectPoint).stream().allMatch(taggedVlanId ->
- populateSinglePortFiltersInternal(deviceId, portnum, false, taggedVlanId))) {
+ processSinglePortFiltersInternal(deviceId, portnum, false, taggedVlanId, install))) {
return false;
}
if (nativeVlan != null) {
// Filter for native vlan
- if (!populateSinglePortFiltersInternal(deviceId, portnum, true, nativeVlan)) {
+ if (!processSinglePortFiltersInternal(deviceId, portnum, true, nativeVlan, install)) {
return false;
}
}
} else if (untaggedVlan != null) {
// Filter for untagged vlan
- if (!populateSinglePortFiltersInternal(deviceId, portnum, true, untaggedVlan)) {
+ if (!processSinglePortFiltersInternal(deviceId, portnum, true, untaggedVlan, install)) {
return false;
}
} else {
// Unconfigure port, use INTERNAL_VLAN
- if (!populateSinglePortFiltersInternal(deviceId, portnum, true, INTERNAL_VLAN)) {
+ if (!processSinglePortFiltersInternal(deviceId, portnum, true, INTERNAL_VLAN, install)) {
return false;
}
}
return true;
}
- private boolean populateSinglePortFiltersInternal(DeviceId deviceId, PortNumber portnum,
- boolean pushVlan, VlanId vlanId) {
+ private boolean processSinglePortFiltersInternal(DeviceId deviceId, PortNumber portnum,
+ boolean pushVlan, VlanId vlanId, boolean install) {
FilteringObjective.Builder fob = buildFilteringObjective(deviceId, portnum, pushVlan, vlanId);
if (fob == null) {
// error encountered during build
return false;
}
- log.info("Sending filtering objectives for dev/port:{}/{}", deviceId, portnum);
+ log.info("{} filtering objectives for dev/port:{}/{}", (install ? "Installing" : "Removing"),
+ deviceId, portnum);
ObjectiveContext context = new DefaultObjectiveContext(
- (objective) -> log.debug("Filter for {}/{} populated", deviceId, portnum),
- (objective, error) ->
- log.warn("Failed to populate filter for {}/{}: {}",
- deviceId, portnum, error));
- srManager.flowObjectiveService.filter(deviceId, fob.add(context));
- return true;
- }
-
- /**
- * Removes filtering objectives for a single port. Should only be called
- * by the master for a switch.
- *
- * @param deviceId device identifier
- * @param portnum port identifier
- * @return true if no errors occurred during the destruction of the filtering objective
- */
- public boolean revokeSinglePortFilters(DeviceId deviceId, PortNumber portnum) {
- ConnectPoint connectPoint = new ConnectPoint(deviceId, portnum);
- VlanId untaggedVlan = srManager.getUntaggedVlanId(connectPoint);
- Set<VlanId> taggedVlans = srManager.getTaggedVlanId(connectPoint);
- VlanId nativeVlan = srManager.getNativeVlanId(connectPoint);
-
- if (taggedVlans.size() != 0) {
- // Filter for tagged vlans
- if (!srManager.getTaggedVlanId(connectPoint).stream().allMatch(taggedVlanId ->
- revokeSinglePortFiltersInternal(deviceId, portnum, false, taggedVlanId))) {
- return false;
- }
- // Filter for native vlan
- if (nativeVlan == null ||
- !revokeSinglePortFiltersInternal(deviceId, portnum, true, nativeVlan)) {
- return false;
- }
- // Filter for untagged vlan
- } else if (untaggedVlan == null ||
- !revokeSinglePortFiltersInternal(deviceId, portnum, true, untaggedVlan)) {
- return false;
- // Unconfigure port, use INTERNAL_VLAN
- } else if (!revokeSinglePortFiltersInternal(deviceId, portnum, true, INTERNAL_VLAN)) {
- return false;
+ (objective) -> log.debug("Filter for {}/{} {}", deviceId, portnum,
+ (install ? "installed" : "removed")),
+ (objective, error) -> log.warn("Failed to {} filter for {}/{}: {}",
+ (install ? "install" : "remove"), deviceId, portnum, error));
+ if (install) {
+ srManager.flowObjectiveService.filter(deviceId, fob.add(context));
+ } else {
+ srManager.flowObjectiveService.filter(deviceId, fob.remove(context));
}
-
- return true;
- }
-
- private boolean revokeSinglePortFiltersInternal(DeviceId deviceId, PortNumber portnum,
- boolean pushVlan, VlanId vlanId) {
- FilteringObjective.Builder fob = buildFilteringObjective(deviceId, portnum, pushVlan, vlanId);
- if (fob == null) {
- // error encountered during build
- return false;
- }
- log.info("Removing filtering objectives for dev/port:{}/{}", deviceId, portnum);
- ObjectiveContext context = new DefaultObjectiveContext(
- (objective) -> log.debug("Filter for {}/{} removed", deviceId, portnum),
- (objective, error) ->
- log.warn("Failed to remove filter for {}/{}: {}",
- deviceId, portnum, error));
- srManager.flowObjectiveService.filter(deviceId, fob.remove(context));
return true;
}
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 3c801b7..0e444fd 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -1003,13 +1003,11 @@
if (port.isEnabled()) {
log.info("Switchport {}/{} enabled..programming filters",
device.id(), port.number());
- defaultRoutingHandler.populateSinglePortFilteringRules(device.id(),
- port.number());
+ routingRulePopulator.processSinglePortFilters(device.id(), port.number(), true);
} else {
log.info("Switchport {}/{} disabled..removing filters",
device.id(), port.number());
- defaultRoutingHandler.revokeSinglePortFilteringRules(device.id(),
- port.number());
+ routingRulePopulator.processSinglePortFilters(device.id(), port.number(), false);
}
// portUpdated calls are for ports that have gone down or up. For switch