Bug fix: Do not create groups as a side effect when revoking routes
Also changing pendingGroups to a cache that will automatically purge failed groups
Improving log messagesin several places
Change-Id: I6843a66d58e623259c7fd20ffe64d56a46d963f0
diff --git a/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index 95f07b2..850962a 100644
--- a/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -122,7 +122,8 @@
prefix, deviceId, outPort);
ForwardingObjective.Builder fwdBuilder;
try {
- fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac, hostVlanId, outPort);
+ fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac,
+ hostVlanId, outPort, false);
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Aborting populateIpRuleForHost.");
return;
@@ -156,7 +157,8 @@
prefix, deviceId, outPort);
ForwardingObjective.Builder fwdBuilder;
try {
- fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac, hostVlanId, outPort);
+ fwdBuilder = routingFwdObjBuilder(deviceId, prefix, hostMac,
+ hostVlanId, outPort, true);
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Aborting revokeIpRuleForHost.");
return;
@@ -184,12 +186,14 @@
* @param hostMac MAC address of the nexthop
* @param hostVlanId Vlan ID of the nexthop
* @param outPort port where the nexthop attaches to
+ * @param revoke true if forwarding objective is meant to revoke forwarding rule
* @return forwarding objective builder
* @throws DeviceConfigNotFoundException if given device is not configured
*/
private ForwardingObjective.Builder routingFwdObjBuilder(
DeviceId deviceId, IpPrefix prefix,
- MacAddress hostMac, VlanId hostVlanId, PortNumber outPort)
+ MacAddress hostMac, VlanId hostVlanId, PortNumber outPort,
+ boolean revoke)
throws DeviceConfigNotFoundException {
MacAddress deviceMac;
deviceMac = config.getDeviceMac(deviceId);
@@ -231,13 +235,14 @@
mbuilder.matchVlanId(INTERNAL_VLAN);
}
} else {
- log.warn("Tagged nexthop {}/{} is not allowed on {} without VLAN listed in tagged vlan",
- hostMac, hostVlanId, connectPoint);
+ log.warn("Tagged nexthop {}/{} is not allowed on {} without VLAN listed"
+ + " in tagged vlan", hostMac, hostVlanId, connectPoint);
return null;
}
-
+ // if the objective is to revoke an existing rule, and for some reason
+ // the next-objective does not exist, then a new one should not be created
int portNextObjId = srManager.getPortNextObjectiveId(deviceId, outPort,
- tbuilder.build(), mbuilder.build());
+ tbuilder.build(), mbuilder.build(), !revoke);
if (portNextObjId == -1) {
// Warning log will come from getPortNextObjective method
return null;
@@ -437,12 +442,13 @@
// Not-bos pop case (php for the current label). If MPLS-ECMP
// has been configured, the application we will request the
// installation for an MPLS-ECMP group.
- ForwardingObjective.Builder fwdObjNoBosBuilder = getMplsForwardingObjective(targetSwId,
- nextHops,
- true,
- isMplsBos,
- metabuilder.build(),
- routerIp);
+ ForwardingObjective.Builder fwdObjNoBosBuilder =
+ getMplsForwardingObjective(targetSwId,
+ nextHops,
+ true,
+ isMplsBos,
+ metabuilder.build(),
+ routerIp);
// Error case, we cannot handle, exit.
if (fwdObjNoBosBuilder == null) {
return Collections.emptyList();
@@ -456,12 +462,13 @@
// Not-bos pop case. If MPLS-ECMP has been configured, the
// application we will request the installation for an MPLS-ECMP
// group.
- ForwardingObjective.Builder fwdObjNoBosBuilder = getMplsForwardingObjective(targetSwId,
- nextHops,
- false,
- isMplsBos,
- metabuilder.build(),
- routerIp);
+ ForwardingObjective.Builder fwdObjNoBosBuilder =
+ getMplsForwardingObjective(targetSwId,
+ nextHops,
+ false,
+ isMplsBos,
+ metabuilder.build(),
+ routerIp);
// Error case, we cannot handle, exit.
if (fwdObjNoBosBuilder == null) {
return Collections.emptyList();
@@ -727,9 +734,6 @@
log.warn(e.getMessage() + " Processing SinglePortFilters aborted");
return null;
}
- // suppressed ports still have filtering rules pushed by SR using default vlan
- ConnectPoint connectPoint = new ConnectPoint(deviceId, portnum);
-
FilteringObjective.Builder fob = DefaultFilteringObjective.builder();
fob.withKey(Criteria.matchInPort(portnum))
.addCondition(Criteria.matchEthDst(deviceMac))