Fix bugs for dynamic interface configuration
- portNextObjStore is not updated when adding or removing portNextObjective
- Group keys for L2IG in flowObjectiveStore are deleted while modifying L2IG, which in turn causes an exception
- L3UG pointing to L2IG, which is already removed, is not removed
- Empty L2FG, with VLAN ID removed from the configuration, is not removed
- Bridging and unicast routing rules for hosts are not updated when changing port VLAN from untagged to tagged and vice versa
Change-Id: I9454fe553ae53e0fc8839a4ad629c0b5b9039a36
diff --git a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index dcc77f8..a21e327 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -707,15 +707,29 @@
MacAddress mac = host.mac();
VlanId hostVlanId = host.vlan();
- // Check whether the host vlan is valid for new interface configuration
- if ((!popVlan && hostVlanId.equals(vlanId)) ||
- (popVlan && hostVlanId.equals(VlanId.NONE))) {
- srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId, popVlan, install);
- // Update Forwarding objective and corresponding simple Next objective
- // for each host and IP address connected to given port
- host.ipAddresses().forEach(ipAddress -> srManager.defaultRoutingHandler.updateFwdObj(
- deviceId, portNum, ipAddress.toIpPrefix(), mac, vlanId, popVlan, install)
+ if (!install) {
+ // Do not check the host validity. Just remove all rules corresponding to the vlan id
+ // Revoke forwarding objective for bridging to the host
+ srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId, popVlan, false);
+
+ // Revoke forwarding objective and corresponding simple Next objective
+ // for each Host and IP address connected to given port
+ host.ipAddresses().forEach(ipAddress ->
+ srManager.routingRulePopulator.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
+ mac, vlanId, popVlan, false)
);
+ } else {
+ // Check whether the host vlan is valid for new interface configuration
+ if ((!popVlan && hostVlanId.equals(vlanId)) ||
+ (popVlan && hostVlanId.equals(VlanId.NONE))) {
+ srManager.defaultRoutingHandler.updateBridging(deviceId, portNum, mac, vlanId, popVlan, true);
+ // Update Forwarding objective and corresponding simple Next objective
+ // for each Host and IP address connected to given port
+ host.ipAddresses().forEach(ipAddress ->
+ srManager.routingRulePopulator.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
+ mac, vlanId, popVlan, true)
+ );
+ }
}
}
diff --git a/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index 9313f34..b5fcbcb 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -1685,12 +1685,15 @@
srManager.flowObjectiveService.forward(deviceId, install ? fob.add(context) : fob.remove(context));
if (!install) {
- DefaultGroupHandler grpHandler = srManager.getGroupHandler(deviceId);
- if (grpHandler == null) {
- log.warn("updateFwdObj: groupHandler for device {} not found", deviceId);
- } else {
- // Remove L3UG for the given port and host
- grpHandler.removeGroupFromPort(portNumber, tbuilder.build(), mbuilder.build());
+ if (!srManager.getVlanPortMap(deviceId).containsKey(vlanId) ||
+ !srManager.getVlanPortMap(deviceId).get(vlanId).contains(portNumber)) {
+ DefaultGroupHandler grpHandler = srManager.getGroupHandler(deviceId);
+ if (grpHandler == null) {
+ log.warn("updateFwdObj: groupHandler for device {} not found", deviceId);
+ } else {
+ // Remove L3UG for the given port and host
+ grpHandler.removeGroupFromPort(portNumber, tbuilder.build(), mbuilder.build());
+ }
}
}
}
diff --git a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 281a3c1..a948bfd 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -2149,6 +2149,11 @@
if (getVlanNextObjectiveId(deviceId, vlanId) != -1) {
// Update L2IG bucket of the port
grpHandler.updateL2InterfaceGroupBucket(portNum, vlanId, pushVlan);
+ // Update bridging and unicast routing rule for each host
+ hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
+ vlanId, !pushVlan, false));
+ hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
+ vlanId, pushVlan, true));
} else {
log.warn("Failed to retrieve next objective for vlan {} in device {}:{}", vlanId, deviceId, portNum);
}
@@ -2171,9 +2176,6 @@
int nextId = getVlanNextObjectiveId(deviceId, vlanId);
if (nextId != -1 && !install) {
- // Update next objective for a single port as an output port
- // Remove a single port from L2FG
- grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
// Remove L2 Bridging rule and L3 Unicast rule to the host
hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
vlanId, pushVlan, install));
@@ -2185,10 +2187,17 @@
// Remove L2FG for VLAN
grpHandler.removeBcastGroupFromVlan(deviceId, portNum, vlanId, pushVlan);
} else {
- // Remove L2IG of the port
- grpHandler.removePortNextObjective(deviceId, portNum, vlanId, pushVlan);
+ // Remove a single port from L2FG
+ grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
}
+ // Remove L2IG of the port
+ grpHandler.removePortNextObjective(deviceId, portNum, vlanId, pushVlan);
} else if (install) {
+ // Create L2IG of the port
+ grpHandler.createPortNextObjective(deviceId, portNum, vlanId, pushVlan);
+ // Create L2 Bridging rule and L3 Unicast rule to the host
+ hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
+ vlanId, pushVlan, install));
if (nextId != -1) {
// Add a single port to L2FG
grpHandler.updateGroupFromVlanConfiguration(vlanId, portNum, nextId, install);
@@ -2197,8 +2206,6 @@
grpHandler.createBcastGroupFromVlan(vlanId, Collections.singleton(portNum));
routingRulePopulator.updateSubnetBroadcastRule(deviceId, vlanId, install);
}
- hostEventExecutor.execute(() -> hostHandler.processIntfVlanUpdatedEvent(deviceId, portNum,
- vlanId, pushVlan, install));
} else {
log.warn("Failed to retrieve next objective for vlan {} in device {}:{}", vlanId, deviceId, portNum);
}
diff --git a/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 15513cf..f8d3053 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -1353,6 +1353,27 @@
}
/**
+ * Creates simple next objective for a single port.
+ *
+ * @param deviceId device id that has the port to deal with
+ * @param portNum the outgoing port on the device
+ * @param vlanId vlan id associated with the port
+ * @param popVlan true if POP_VLAN action is applied on the packets, false otherwise
+ */
+ public void createPortNextObjective(DeviceId deviceId, PortNumber portNum, VlanId vlanId, boolean popVlan) {
+ TrafficSelector.Builder mbuilder = DefaultTrafficSelector.builder();
+ mbuilder.matchVlanId(vlanId);
+
+ TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
+ tbuilder.immediate().setOutput(portNum);
+ if (popVlan) {
+ tbuilder.immediate().popVlan();
+ }
+
+ createGroupFromPort(portNum, tbuilder.build(), mbuilder.build());
+ }
+
+ /**
* Removes simple next objective for a single port.
*
* @param deviceId device id that has the port to deal with
@@ -1513,16 +1534,23 @@
* does not involve pop vlan tag action.
*/
public void updateL2InterfaceGroupBucket(PortNumber portNumber, VlanId vlanId, boolean pushVlan) {
+ TrafficTreatment.Builder oldTBuilder = DefaultTrafficTreatment.builder();
TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
+ tBuilder.setOutput(portNumber);
+ oldTBuilder.setOutput(portNumber);
if (pushVlan) {
tBuilder.popVlan();
+ } else {
+ oldTBuilder.popVlan();
}
- tBuilder.setOutput(portNumber);
TrafficSelector metadata =
DefaultTrafficSelector.builder().matchVlanId(vlanId).build();
- int nextId = getVlanNextObjectiveId(vlanId);
+ // Update portNextObjStore with new L2IG
+ int nextId = getPortNextObjectiveId(portNumber, oldTBuilder.build(), metadata, false);
+ portNextObjStore.remove(new PortNextObjectiveStoreKey(deviceId, portNumber, oldTBuilder.build(), metadata));
+ portNextObjStore.put(new PortNextObjectiveStoreKey(deviceId, portNumber, tBuilder.build(), metadata), nextId);
NextObjective.Builder nextObjBuilder = DefaultNextObjective
.builder().withId(nextId)