[SDFAB-82] Support policy update
Change-Id: I24aff46a0cea9b92f5c5e86c615d8280cba46f42
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/PolicyManager.java b/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/PolicyManager.java
index 78b8600..21cb8e3 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/PolicyManager.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/PolicyManager.java
@@ -573,13 +573,22 @@
trafficMatch.trafficMatchId(), trafficMatch.policyId());
}
TrafficMatchKey trafficMatchKey = new TrafficMatchKey(deviceId, trafficMatch.trafficMatchId());
- Operation trafficOperation = Versioned.valueOrNull(operations.get(trafficMatchKey.toString()));
- if (trafficOperation != null && trafficOperation.isInstall()) {
- if (log.isDebugEnabled()) {
- log.debug("There is already an install operation for traffic match {} associated to policy {} " +
- "for device {}", trafficMatch.trafficMatchId(), trafficMatch.policyId(), deviceId);
+ Operation oldTrafficOperation = Versioned.valueOrNull(operations.get(trafficMatchKey.toString()));
+ if (oldTrafficOperation != null && oldTrafficOperation.isInstall()) {
+ if (trafficMatch.equals(oldTrafficOperation.trafficMatch().orElse(null))) {
+ if (log.isDebugEnabled()) {
+ log.debug("There is already an install operation for traffic match {} associated to policy {} " +
+ "for device {}",
+ trafficMatch.trafficMatchId(), trafficMatch.policyId(), deviceId);
+ }
+ return;
+ } else {
+ if (log.isDebugEnabled()) {
+ log.debug("Starts updating traffic match {} associated to policy {} " +
+ "for device {}",
+ trafficMatch.trafficMatchId(), trafficMatch.policyId(), deviceId);
+ }
}
- return;
}
// For the DROP policy we need to set an ACL drop in the fwd objective. The other
// policies require to retrieve the next Id and sets the next step.
@@ -594,11 +603,11 @@
return;
}
// Updates the store and then send the versatile fwd objective to the pipeliner
- trafficOperation = Operation.builder()
+ Operation newTrafficOperation = Operation.builder()
.isInstall(true)
.trafficMatch(trafficMatch)
.build();
- operations.put(trafficMatchKey.toString(), trafficOperation);
+ operations.put(trafficMatchKey.toString(), newTrafficOperation);
Policy policy = policyOperation.policy().get();
ForwardingObjective.Builder builder = trafficMatchFwdObjective(trafficMatch, policy.policyType());
if (policy.policyType() == PolicyType.DROP) {
@@ -613,29 +622,65 @@
builder.nextStep(policyOperation.objectiveOperation().id());
}
// Once, the fwd objective has completed its execution, we update the policiesOps map
- CompletableFuture<Objective> future = new CompletableFuture<>();
+ CompletableFuture<Objective> addNewFuture = new CompletableFuture<>();
+ CompletableFuture<Objective> removeOldFuture = new CompletableFuture<>();
if (log.isDebugEnabled()) {
log.debug("Installing forwarding objective for dev: {}", deviceId);
}
- ObjectiveContext context = new DefaultObjectiveContext(
+ ObjectiveContext addNewContext = new DefaultObjectiveContext(
(objective) -> {
if (log.isDebugEnabled()) {
log.debug("Forwarding objective for policy {} installed", trafficMatch.policyId());
}
- future.complete(objective);
+ addNewFuture.complete(objective);
},
(objective, error) -> {
log.warn("Failed to install forwarding objective for policy {}: {}",
trafficMatch.policyId(), error);
- future.complete(null);
+ addNewFuture.complete(null);
});
+ ObjectiveContext removeOldContext = new DefaultObjectiveContext(
+ (objective) -> {
+ if (log.isDebugEnabled()) {
+ log.debug("Old forwarding objective for policy {} removed, update finished",
+ trafficMatch.policyId());
+ }
+ removeOldFuture.complete(objective);
+ },
+ (objective, error) -> {
+ log.warn("Failed to remove old forwarding objective for policy {}: {}",
+ trafficMatch.policyId(), error);
+ removeOldFuture.complete(null);
+ });
// Context is not serializable
ForwardingObjective serializableObjective = builder.add();
- flowObjectiveService.forward(deviceId, builder.add(context));
- future.whenComplete((objective, ex) -> {
+ flowObjectiveService.forward(deviceId, builder.add(addNewContext));
+ addNewFuture.whenComplete((objective, ex) -> {
if (ex != null) {
log.error("Exception installing forwarding objective", ex);
} else if (objective != null) {
+ // Remove existing flow with previous priority after new flow is installed
+ if (oldTrafficOperation != null && oldTrafficOperation.objectiveOperation() != null
+ && oldTrafficOperation.isInstall()
+ && oldTrafficOperation.objectiveOperation().priority() != serializableObjective.priority()) {
+ ForwardingObjective oldFwdObj = (ForwardingObjective) oldTrafficOperation.objectiveOperation();
+ ForwardingObjective.Builder oldBuilder = DefaultForwardingObjective.builder(oldFwdObj);
+ flowObjectiveService.forward(deviceId, oldBuilder.remove(removeOldContext));
+ } else {
+ operations.computeIfPresent(trafficMatchKey.toString(), (k, v) -> {
+ if (!v.isDone() && v.isInstall()) {
+ v.isDone(true);
+ v.objectiveOperation(serializableObjective);
+ }
+ return v;
+ });
+ }
+ }
+ });
+ removeOldFuture.whenComplete((objective, ex) -> {
+ if (ex != null) {
+ log.error("Exception removing old forwarding objective", ex);
+ } else if (objective != null) {
operations.computeIfPresent(trafficMatchKey.toString(), (k, v) -> {
if (!v.isDone() && v.isInstall()) {
v.isDone(true);
diff --git a/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/TrafficMatchRequest.java b/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/TrafficMatchRequest.java
index aaddc3d..65e39a0 100644
--- a/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/TrafficMatchRequest.java
+++ b/impl/src/main/java/org/onosproject/segmentrouting/policy/impl/TrafficMatchRequest.java
@@ -103,7 +103,7 @@
*
* @return the priority
*/
- public TrafficMatchPriority priority() {
+ public TrafficMatchPriority trafficMatchPriority() {
return trafficMatch.trafficMatchPriority();
}