Routing/bridging rules on the same leaf pair should always be programmed by the same ONOS instance
Main change:
- Implement new logic for shouldHandleRouting, along with corresponding unit tests. Also rename it to shouldProgram
Side changes:
- Refactor revokeSubnet such that it is only invoked on the instance that should handle routing change
- Move the following methods to RoutingRulePopulator and follow the same design pattern as populate/revoke subnet/route
- populateBridging, revokeBridging, updateBridging
- updateFwdObj
- Make sure the following methods in RoutingRulePopulator are always invoked by DefaultRoutingHandler with shouldProgram check
- populateRoute, revokeRoute
- populateSubnet, revokeSubnet
- populateBridging, revokeBridging, updateBridging
- updateFwdObj
Change-Id: I903129271ede91c45ebf0d973e06faeae46c157a
diff --git a/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index ae20b3f..236121c 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -37,14 +37,18 @@
import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
import org.onosproject.segmentrouting.config.DeviceConfiguration;
import org.onosproject.segmentrouting.grouphandler.DefaultGroupHandler;
+import org.onosproject.store.serializers.KryoNamespaces;
+import org.onosproject.store.service.Serializer;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.time.Instant;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
@@ -82,6 +86,10 @@
= newScheduledThreadPool(1, groupedThreads("retryftr", "retry-%d", log));
private Instant lastRoutingChange;
+ // Keep track on which ONOS instance should program the device pair.
+ // There should be only one instance that programs the same pair.
+ Map<Set<DeviceId>, NodeId> shouldProgram;
+
/**
* Represents the default routing population status.
*/
@@ -105,12 +113,16 @@
*
* @param srManager SegmentRoutingManager object
*/
- public DefaultRoutingHandler(SegmentRoutingManager srManager) {
+ DefaultRoutingHandler(SegmentRoutingManager srManager) {
this.srManager = srManager;
this.rulePopulator = checkNotNull(srManager.routingRulePopulator);
this.config = checkNotNull(srManager.deviceConfiguration);
this.populationStatus = Status.IDLE;
this.currentEcmpSpgMap = Maps.newHashMap();
+ this.shouldProgram = srManager.storageService.<Set<DeviceId>, NodeId>consistentMapBuilder()
+ .withName("sr-should-program")
+ .withSerializer(Serializer.using(KryoNamespaces.API))
+ .build().asJavaMap();
}
/**
@@ -203,13 +215,12 @@
updatedEcmpSpgMap.put(pairDev.get(), ecmpSpgUpdated);
edgePairs.add(new EdgePair(dstSw, pairDev.get()));
}
- DeviceId ret = shouldHandleRouting(dstSw);
- if (ret == null) {
+
+ if (!shouldProgram(dstSw)) {
continue;
}
- Set<DeviceId> devsToProcess = Sets.newHashSet(dstSw, ret);
// To do a full reroute, assume all routes have changed
- for (DeviceId dev : devsToProcess) {
+ for (DeviceId dev : deviceAndItsPair(dstSw)) {
for (DeviceId targetSw : srManager.deviceConfiguration.getRouters()) {
if (targetSw.equals(dev)) {
continue;
@@ -301,8 +312,7 @@
log.warn("populateSubnet: no updated graph for dev:{}"
+ " ... creating", cp.deviceId());
}
- DeviceId retId = shouldHandleRouting(cp.deviceId());
- if (retId == null) {
+ if (!shouldProgram(cp.deviceId())) {
continue;
}
handleRouting = true;
@@ -317,9 +327,7 @@
log.warn("populateSubnet: no updated graph for dev:{}"
+ " ... creating", dstSw);
}
- if (srManager.mastershipService.isLocalMaster(dstSw)) {
- handleRouting = true;
- }
+ handleRouting = shouldProgram(dstSw);
}
if (!handleRouting) {
@@ -1066,7 +1074,10 @@
protected boolean revokeSubnet(Set<IpPrefix> subnets) {
statusLock.lock();
try {
- return srManager.routingRulePopulator.revokeIpRuleForSubnet(subnets);
+ return Sets.newHashSet(srManager.deviceService.getAvailableDevices()).stream()
+ .map(Device::id)
+ .filter(this::shouldProgram)
+ .allMatch(targetSw -> srManager.routingRulePopulator.revokeIpRuleForSubnet(targetSw, subnets));
} finally {
statusLock.unlock();
}
@@ -1084,7 +1095,7 @@
*/
void populateRoute(DeviceId deviceId, IpPrefix prefix,
MacAddress hostMac, VlanId hostVlanId, PortNumber outPort) {
- if (srManager.mastershipService.isLocalMaster(deviceId)) {
+ if (shouldProgram(deviceId)) {
srManager.routingRulePopulator.populateRoute(deviceId, prefix, hostMac, hostVlanId, outPort);
}
}
@@ -1101,11 +1112,38 @@
*/
void revokeRoute(DeviceId deviceId, IpPrefix prefix,
MacAddress hostMac, VlanId hostVlanId, PortNumber outPort) {
- if (srManager.mastershipService.isLocalMaster(deviceId)) {
+ if (shouldProgram(deviceId)) {
srManager.routingRulePopulator.revokeRoute(deviceId, prefix, hostMac, hostVlanId, outPort);
}
}
+ void populateBridging(DeviceId deviceId, PortNumber port, MacAddress mac, VlanId vlanId) {
+ if (shouldProgram(deviceId)) {
+ srManager.routingRulePopulator.populateBridging(deviceId, port, mac, vlanId);
+ }
+ }
+
+ void revokeBridging(DeviceId deviceId, PortNumber port, MacAddress mac, VlanId vlanId) {
+ if (shouldProgram(deviceId)) {
+ srManager.routingRulePopulator.revokeBridging(deviceId, port, mac, vlanId);
+ }
+ }
+
+ void updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
+ VlanId vlanId, boolean popVlan, boolean install) {
+ if (shouldProgram(deviceId)) {
+ srManager.routingRulePopulator.updateBridging(deviceId, portNum, hostMac, vlanId, popVlan, install);
+ }
+ }
+
+ void updateFwdObj(DeviceId deviceId, PortNumber portNumber, IpPrefix prefix, MacAddress hostMac,
+ VlanId vlanId, boolean popVlan, boolean install) {
+ if (shouldProgram(deviceId)) {
+ srManager.routingRulePopulator.updateFwdObj(deviceId, portNumber, prefix, hostMac,
+ vlanId, popVlan, install);
+ }
+ }
+
/**
* Remove ECMP graph entry for the given device. Typically called when
* device is no longer available.
@@ -1150,12 +1188,10 @@
for (Device sw : srManager.deviceService.getDevices()) {
log.debug("Computing the impacted routes for device {} due to link fail",
sw.id());
- DeviceId retId = shouldHandleRouting(sw.id());
- if (retId == null) {
+ if (!shouldProgram(sw.id())) {
continue;
}
- Set<DeviceId> devicesToProcess = Sets.newHashSet(retId, sw.id());
- for (DeviceId rootSw : devicesToProcess) {
+ for (DeviceId rootSw : deviceAndItsPair(sw.id())) {
EcmpShortestPathGraph ecmpSpg = currentEcmpSpgMap.get(rootSw);
if (ecmpSpg == null) {
log.warn("No existing ECMP graph for switch {}. Aborting optimized"
@@ -1219,12 +1255,10 @@
for (Device sw : srManager.deviceService.getDevices()) {
log.debug("Computing the impacted routes for device {}", sw.id());
- DeviceId retId = shouldHandleRouting(sw.id());
- if (retId == null) {
+ if (!shouldProgram(sw.id())) {
continue;
}
- Set<DeviceId> devicesToProcess = Sets.newHashSet(retId, sw.id());
- for (DeviceId rootSw : devicesToProcess) {
+ for (DeviceId rootSw : deviceAndItsPair(sw.id())) {
if (log.isTraceEnabled()) {
log.trace("Device links for dev: {}", rootSw);
for (Link link: srManager.linkService.getDeviceLinks(rootSw)) {
@@ -1375,47 +1409,87 @@
}
/**
- * Determines whether this controller instance should handle routing for the
+ * Determines whether this controller instance should program the
* given {@code deviceId}, based on mastership and pairDeviceId if one exists.
- * Returns null if this instance should not handle routing for given {@code deviceId}.
- * Otherwise the returned value could be the given deviceId itself, or the
- * deviceId for the paired edge device. In the latter case, this instance
- * should handle routing for both the given device and the paired device.
+ * <p>
+ * Once an instance is elected, it will be the only instance responsible for programming
+ * both devices in the pair until it goes down.
*
* @param deviceId device identifier to consider for routing
- * @return null or deviceId which could be the same as the given deviceId
- * or the deviceId of a paired edge device
+ * @return true if current instance should handle the routing for given device
*/
- private DeviceId shouldHandleRouting(DeviceId deviceId) {
- if (!srManager.mastershipService.isLocalMaster(deviceId)) {
- log.debug("Not master for dev:{} .. skipping routing, may get handled "
- + "elsewhere as part of paired devices", deviceId);
- return null;
- }
- NodeId myNode = srManager.mastershipService.getMasterFor(deviceId);
- Optional<DeviceId> pairDev = srManager.getPairDeviceId(deviceId);
+ boolean shouldProgram(DeviceId deviceId) {
+ Optional<DeviceId> pairDeviceId = srManager.getPairDeviceId(deviceId);
- if (pairDev.isPresent()) {
- if (!srManager.deviceService.isAvailable(pairDev.get())) {
- log.warn("pairedDev {} not available .. routing both this dev:{} "
- + "and pair without mastership check for pair",
- pairDev, deviceId);
- return pairDev.get(); // handle both temporarily
- }
- NodeId pairMasterNode = srManager.mastershipService.getMasterFor(pairDev.get());
- if (myNode.compareTo(pairMasterNode) <= 0) {
- log.debug("Handling routing for both dev:{} pair-dev:{}; myNode: {}"
- + " pairMaster:{} compare:{}", deviceId, pairDev,
- myNode, pairMasterNode,
- myNode.compareTo(pairMasterNode));
- return pairDev.get(); // handle both
- } else {
- log.debug("PairDev node: {} should handle routing for dev:{} and "
- + "pair-dev:{}", pairMasterNode, deviceId, pairDev);
- return null; // handle neither
- }
+ NodeId currentNodeId = srManager.clusterService.getLocalNode().id();
+ NodeId masterNodeId = srManager.mastershipService.getMasterFor(deviceId);
+ Optional<NodeId> pairMasterNodeId = pairDeviceId.map(srManager.mastershipService::getMasterFor);
+ log.debug("Evaluate shouldProgram {}/pair={}. current={}, master={}, pairMaster={}",
+ deviceId, pairDeviceId, currentNodeId, masterNodeId, pairMasterNodeId);
+
+ // No pair device configured. Only handle when current instance is the master of the device
+ if (!pairDeviceId.isPresent()) {
+ log.debug("No pair device. current={}, master={}", currentNodeId, masterNodeId);
+ return currentNodeId.equals(masterNodeId);
}
- return deviceId; // not paired, just handle given device
+
+ // Should not handle if current instance is not the master of either switch
+ if (!currentNodeId.equals(masterNodeId) &&
+ !(pairMasterNodeId.isPresent() && currentNodeId.equals(pairMasterNodeId.get()))) {
+ log.debug("Current node {} is neither the master of target device {} nor pair device {}",
+ currentNodeId, deviceId, pairDeviceId);
+ return false;
+ }
+
+ Set<DeviceId> key = Sets.newHashSet(deviceId, pairDeviceId.get());
+
+ NodeId king = shouldProgram.compute(key, ((k, v) -> {
+ if (v == null) {
+ // There is no value in the map. Elect a node
+ return elect(Lists.newArrayList(masterNodeId, pairMasterNodeId.orElse(null)));
+ } else {
+ if (v.equals(masterNodeId) || v.equals(pairMasterNodeId.orElse(null))) {
+ // Use the node in the map if it is still alive and is a master of any of the two switches
+ return v;
+ } else {
+ // Previously elected node is no longer the master of either switch. Re-elect a node.
+ return elect(Lists.newArrayList(masterNodeId, pairMasterNodeId.orElse(null)));
+ }
+ }
+ }));
+
+ if (king != null) {
+ log.debug("{} should handle routing for {}/pair={}", king, deviceId, pairDeviceId);
+ return king.equals(currentNodeId);
+ } else {
+ log.error("Fail to elect a king for {}/pair={}. Abort.", deviceId, pairDeviceId);
+ return false;
+ }
+ }
+
+ /**
+ * Elects a node who should take responsibility of programming devices.
+ * @param nodeIds list of candidate node ID
+ *
+ * @return NodeId of the node that gets elected, or null if none of the node can be elected
+ */
+ private NodeId elect(List<NodeId> nodeIds) {
+ // Remove all null elements. This could happen when some device has no master
+ nodeIds.removeAll(Collections.singleton(null));
+ nodeIds.sort(null);
+ return nodeIds.size() == 0 ? null : nodeIds.get(0);
+ }
+
+ /**
+ * Returns a set of device ID, containing given device and its pair device if exist.
+ *
+ * @param deviceId Device ID
+ * @return a set of device ID, containing given device and its pair device if exist.
+ */
+ private Set<DeviceId> deviceAndItsPair(DeviceId deviceId) {
+ Set<DeviceId> ret = Sets.newHashSet(deviceId);
+ srManager.getPairDeviceId(deviceId).ifPresent(ret::add);
+ return ret;
}
/**
diff --git a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
index 908a339..9590632 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/HostHandler.java
@@ -25,15 +25,6 @@
import org.onosproject.net.Host;
import org.onosproject.net.HostLocation;
import org.onosproject.net.PortNumber;
-import org.onosproject.net.flow.DefaultTrafficSelector;
-import org.onosproject.net.flow.DefaultTrafficTreatment;
-import org.onosproject.net.flow.TrafficSelector;
-import org.onosproject.net.flow.TrafficTreatment;
-import org.onosproject.net.flowobjective.DefaultForwardingObjective;
-import org.onosproject.net.flowobjective.DefaultObjectiveContext;
-import org.onosproject.net.flowobjective.FlowObjectiveService;
-import org.onosproject.net.flowobjective.ForwardingObjective;
-import org.onosproject.net.flowobjective.ObjectiveContext;
import org.onosproject.net.host.HostEvent;
import org.onosproject.net.host.HostLocationProbingService.ProbeMode;
import org.onosproject.net.host.HostService;
@@ -61,7 +52,6 @@
protected final SegmentRoutingManager srManager;
private HostService hostService;
- private FlowObjectiveService flowObjectiveService;
/**
* Constructs the HostHandler.
@@ -71,7 +61,6 @@
HostHandler(SegmentRoutingManager srManager) {
this.srManager = srManager;
hostService = srManager.hostService;
- flowObjectiveService = srManager.flowObjectiveService;
}
protected void init(DeviceId devId) {
@@ -108,19 +97,16 @@
Set<IpAddress> ips = host.ipAddresses();
log.info("Host {}/{} is added at {}", hostMac, hostVlanId, locations);
- if (srManager.isMasterOf(location)) {
- processBridgingRule(location.deviceId(), location.port(), hostMac, hostVlanId, false);
- ips.forEach(ip ->
- processRoutingRule(location.deviceId(), location.port(), hostMac, hostVlanId, ip, false)
- );
- }
+ processBridgingRule(location.deviceId(), location.port(), hostMac, hostVlanId, false);
+ ips.forEach(ip ->
+ processRoutingRule(location.deviceId(), location.port(), hostMac, hostVlanId, ip, false)
+ );
// Use the pair link temporarily before the second location of a dual-homed host shows up.
// This do not affect single-homed hosts since the flow will be blocked in
// processBridgingRule or processRoutingRule due to VLAN or IP mismatch respectively
srManager.getPairDeviceId(location.deviceId()).ifPresent(pairDeviceId -> {
- if (srManager.mastershipService.isLocalMaster(pairDeviceId) &&
- host.locations().stream().noneMatch(l -> l.deviceId().equals(pairDeviceId))) {
+ if (host.locations().stream().noneMatch(l -> l.deviceId().equals(pairDeviceId))) {
srManager.getPairLocalPorts(pairDeviceId).ifPresent(pairRemotePort -> {
// NOTE: Since the pairLocalPort is trunk port, use assigned vlan of original port
// when the host is untagged
@@ -150,18 +136,15 @@
log.info("Host {}/{} is removed from {}", hostMac, hostVlanId, locations);
locations.forEach(location -> {
- if (srManager.isMasterOf(location)) {
- processBridgingRule(location.deviceId(), location.port(), hostMac, hostVlanId, true);
- ips.forEach(ip ->
- processRoutingRule(location.deviceId(), location.port(), hostMac, hostVlanId, ip, true)
- );
- }
+ processBridgingRule(location.deviceId(), location.port(), hostMac, hostVlanId, true);
+ ips.forEach(ip ->
+ processRoutingRule(location.deviceId(), location.port(), hostMac, hostVlanId, ip, true)
+ );
// Also remove redirection flows on the pair device if exists.
Optional<DeviceId> pairDeviceId = srManager.getPairDeviceId(location.deviceId());
Optional<PortNumber> pairLocalPort = srManager.getPairLocalPorts(location.deviceId());
- if (pairDeviceId.isPresent() && pairLocalPort.isPresent() &&
- srManager.mastershipService.isLocalMaster(pairDeviceId.get())) {
+ if (pairDeviceId.isPresent() && pairLocalPort.isPresent()) {
// NOTE: Since the pairLocalPort is trunk port, use assigned vlan of original port
// when the host is untagged
VlanId vlanId = Optional.ofNullable(srManager.getInternalVlanId(location)).orElse(hostVlanId);
@@ -202,8 +185,7 @@
.collect(Collectors.toSet());
// For each old location
- Sets.difference(prevLocations, newLocations).stream().filter(srManager::isMasterOf)
- .forEach(prevLocation -> {
+ Sets.difference(prevLocations, newLocations).forEach(prevLocation -> {
// Remove routing rules for old IPs
Sets.difference(prevIps, newIps).forEach(ip ->
processRoutingRule(prevLocation.deviceId(), prevLocation.port(), hostMac, hostVlanId,
@@ -263,7 +245,7 @@
});
// For each new location, add all new IPs.
- Sets.difference(newLocations, prevLocations).stream().filter(srManager::isMasterOf)
+ Sets.difference(newLocations, prevLocations).stream()
.forEach(newLocation -> {
processBridgingRule(newLocation.deviceId(), newLocation.port(), hostMac, hostVlanId, false);
newIps.forEach(ip ->
@@ -273,7 +255,7 @@
});
// For each unchanged location, add new IPs and remove old IPs.
- Sets.intersection(newLocations, prevLocations).stream().filter(srManager::isMasterOf)
+ Sets.intersection(newLocations, prevLocations).stream()
.forEach(unchangedLocation -> {
Sets.difference(prevIps, newIps).forEach(ip ->
processRoutingRule(unchangedLocation.deviceId(), unchangedLocation.port(), hostMac,
@@ -305,7 +287,7 @@
Set<IpAddress> newIps = host.ipAddresses();
log.info("Host {}/{} is updated", hostMac, hostVlanId);
- locations.stream().filter(srManager::isMasterOf).forEach(location -> {
+ locations.forEach(location -> {
Sets.difference(prevIps, newIps).forEach(ip ->
processRoutingRule(location.deviceId(), location.port(), hostMac, hostVlanId, ip, true)
);
@@ -319,8 +301,7 @@
// processBridgingRule or processRoutingRule due to VLAN or IP mismatch respectively
locations.forEach(location ->
srManager.getPairDeviceId(location.deviceId()).ifPresent(pairDeviceId -> {
- if (srManager.mastershipService.isLocalMaster(pairDeviceId) &&
- locations.stream().noneMatch(l -> l.deviceId().equals(pairDeviceId))) {
+ if (locations.stream().noneMatch(l -> l.deviceId().equals(pairDeviceId))) {
Set<IpAddress> ipsToAdd = Sets.difference(newIps, prevIps);
Set<IpAddress> ipsToRemove = Sets.difference(prevIps, newIps);
@@ -408,81 +389,6 @@
}
/**
- * Generates a forwarding objective builder for bridging rules.
- * <p>
- * The forwarding objective bridges packets destined to a given MAC to
- * given port on given device.
- *
- * @param deviceId Device that host attaches to
- * @param mac MAC address of the host
- * @param hostVlanId VLAN ID of the host
- * @param outport Port that host attaches to
- * @param revoke true if forwarding objective is meant to revoke forwarding rule
- * @return Forwarding objective builder
- */
- ForwardingObjective.Builder bridgingFwdObjBuilder(
- DeviceId deviceId, MacAddress mac, VlanId hostVlanId,
- PortNumber outport, boolean revoke) {
- ConnectPoint connectPoint = new ConnectPoint(deviceId, outport);
- VlanId untaggedVlan = srManager.getUntaggedVlanId(connectPoint);
- Set<VlanId> taggedVlans = srManager.getTaggedVlanId(connectPoint);
- VlanId nativeVlan = srManager.getNativeVlanId(connectPoint);
-
- // Create host selector
- TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
- sbuilder.matchEthDst(mac);
-
- // Create host treatment
- TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
- tbuilder.immediate().setOutput(outport);
-
- // Create host meta
- TrafficSelector.Builder mbuilder = DefaultTrafficSelector.builder();
-
- // Adjust the selector, treatment and meta according to VLAN configuration
- if (taggedVlans.contains(hostVlanId)) {
- sbuilder.matchVlanId(hostVlanId);
- mbuilder.matchVlanId(hostVlanId);
- } else if (hostVlanId.equals(VlanId.NONE)) {
- if (untaggedVlan != null) {
- sbuilder.matchVlanId(untaggedVlan);
- mbuilder.matchVlanId(untaggedVlan);
- tbuilder.immediate().popVlan();
- } else if (nativeVlan != null) {
- sbuilder.matchVlanId(nativeVlan);
- mbuilder.matchVlanId(nativeVlan);
- tbuilder.immediate().popVlan();
- } else {
- log.warn("Untagged host {}/{} is not allowed on {} without untagged or native" +
- "vlan config", mac, hostVlanId, connectPoint);
- return null;
- }
- } else {
- log.warn("Tagged host {}/{} is not allowed on {} without VLAN listed in tagged vlan",
- mac, hostVlanId, connectPoint);
- return null;
- }
-
- // All forwarding is via Groups. Drivers can re-purpose to flow-actions if needed.
- // 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(), !revoke);
- if (portNextObjId == -1) {
- // Warning log will come from getPortNextObjective method
- return null;
- }
-
- return DefaultForwardingObjective.builder()
- .withFlag(ForwardingObjective.Flag.SPECIFIC)
- .withSelector(sbuilder.build())
- .nextStep(portNextObjId)
- .withPriority(100)
- .fromApp(srManager.appId)
- .makePermanent();
- }
-
- /**
* Populate or revoke a bridging rule on given deviceId that matches given mac, given vlan and
* output to given port.
*
@@ -494,21 +400,14 @@
*/
private void processBridgingRule(DeviceId deviceId, PortNumber port, MacAddress mac,
VlanId vlanId, boolean revoke) {
- log.debug("{} bridging entry for host {}/{} at {}:{}", revoke ? "Revoking" : "Populating",
+ log.info("{} bridging entry for host {}/{} at {}:{}", revoke ? "Revoking" : "Populating",
mac, vlanId, deviceId, port);
- ForwardingObjective.Builder fob = bridgingFwdObjBuilder(deviceId, mac, vlanId, port, revoke);
- if (fob == null) {
- log.warn("Fail to build fwd obj for host {}/{}. Abort.", mac, vlanId);
- return;
+ if (!revoke) {
+ srManager.defaultRoutingHandler.populateBridging(deviceId, port, mac, vlanId);
+ } else {
+ srManager.defaultRoutingHandler.revokeBridging(deviceId, port, mac, vlanId);
}
-
- ObjectiveContext context = new DefaultObjectiveContext(
- (objective) -> log.debug("Brigding rule for {}/{} {}", mac, vlanId,
- revoke ? "revoked" : "populated"),
- (objective, error) -> log.warn("Failed to {} bridging rule for {}/{}: {}",
- revoke ? "revoked" : "populated", mac, vlanId, error));
- flowObjectiveService.forward(deviceId, revoke ? fob.remove(context) : fob.add(context));
}
/**
@@ -538,59 +437,7 @@
}
}
- /**
- * Populate or revoke a bridging rule on given deviceId that matches given vlanId,
- * and hostMAC connected to given port, and output to given port only when
- * vlan information is valid.
- *
- * @param deviceId device ID that host attaches to
- * @param portNum port number that host attaches to
- * @param hostMac mac address of the host connected to the switch port
- * @param vlanId Vlan ID configured on the switch port
- * @param popVlan true to pop Vlan tag at TrafficTreatment, false otherwise
- * @param install true to populate the objective, false to revoke
- */
- private void updateBridgingRule(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
- VlanId vlanId, boolean popVlan, boolean install) {
- // Create host selector
- TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
- sbuilder.matchEthDst(hostMac);
- // Create host meta
- TrafficSelector.Builder mbuilder = DefaultTrafficSelector.builder();
-
- sbuilder.matchVlanId(vlanId);
- mbuilder.matchVlanId(vlanId);
-
- // Create host treatment
- TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
- tbuilder.immediate().setOutput(portNum);
-
- if (popVlan) {
- tbuilder.immediate().popVlan();
- }
-
- int portNextObjId = srManager.getPortNextObjectiveId(deviceId, portNum,
- tbuilder.build(), mbuilder.build(), install);
- if (portNextObjId != -1) {
- ForwardingObjective.Builder fob = DefaultForwardingObjective.builder()
- .withFlag(ForwardingObjective.Flag.SPECIFIC)
- .withSelector(sbuilder.build())
- .nextStep(portNextObjId)
- .withPriority(100)
- .fromApp(srManager.appId)
- .makePermanent();
-
- ObjectiveContext context = new DefaultObjectiveContext(
- (objective) -> log.debug("Brigding rule for {}/{} {}", hostMac, vlanId,
- install ? "populated" : "revoked"),
- (objective, error) -> log.warn("Failed to {} bridging rule for {}/{}: {}",
- install ? "populate" : "revoke", hostMac, vlanId, error));
- flowObjectiveService.forward(deviceId, install ? fob.add(context) : fob.remove(context));
- } else {
- log.warn("Failed to retrieve next objective for {}/{}", hostMac, vlanId);
- }
- }
/**
* Update forwarding objective for unicast bridging and unicast routing.
@@ -618,11 +465,11 @@
// Check whether the host vlan is valid for new interface configuration
if ((!popVlan && hostVlanId.equals(vlanId)) ||
(popVlan && hostVlanId.equals(VlanId.NONE))) {
- updateBridgingRule(deviceId, portNum, mac, vlanId, popVlan, install);
+ 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.routingRulePopulator.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
+ srManager.defaultRoutingHandler.updateFwdObj(deviceId, portNum, ipAddress.toIpPrefix(),
mac, vlanId, popVlan, install)
);
}
@@ -649,10 +496,10 @@
host.ipAddresses().forEach(hostIpAddress -> {
ipPrefixSet.forEach(ipPrefix -> {
if (install && ipPrefix.contains(hostIpAddress)) {
- srManager.routingRulePopulator.populateRoute(cp.deviceId(), hostIpAddress.toIpPrefix(),
+ srManager.defaultRoutingHandler.populateRoute(cp.deviceId(), hostIpAddress.toIpPrefix(),
host.mac(), host.vlan(), cp.port());
} else if (!install && ipPrefix.contains(hostIpAddress)) {
- srManager.routingRulePopulator.revokeRoute(cp.deviceId(), hostIpAddress.toIpPrefix(),
+ srManager.defaultRoutingHandler.revokeRoute(cp.deviceId(), hostIpAddress.toIpPrefix(),
host.mac(), host.vlan(), cp.port());
}
});
diff --git a/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java b/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
index a8de070..4b95bf3 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/RouteHandler.java
@@ -252,8 +252,7 @@
Set<HostLocation> newLocations = event.subject().locations();
// For each old location
- Sets.difference(prevLocations, newLocations).stream().filter(srManager::isMasterOf)
- .forEach(prevLocation -> {
+ Sets.difference(prevLocations, newLocations).forEach(prevLocation -> {
// Redirect the flows to pair link if configured
// Note: Do not continue removing any rule
Optional<DeviceId> pairDeviceId = srManager.getPairDeviceId(prevLocation.deviceId());
@@ -276,8 +275,7 @@
});
// For each new location, add all new IPs.
- Sets.difference(newLocations, prevLocations).stream().filter(srManager::isMasterOf)
- .forEach(newLocation -> {
+ Sets.difference(newLocations, prevLocations).forEach(newLocation -> {
log.debug("HostMoved. populateRoute {}, {}, {}, {}", newLocation, prefix, hostMac, hostVlanId);
srManager.defaultRoutingHandler.populateRoute(newLocation.deviceId(), prefix,
hostMac, hostVlanId, newLocation.port());
diff --git a/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index 06f7ef0..bb9378e 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -117,8 +117,182 @@
}
/**
- * Populates IP rules for a route that has direct connection to the
- * switch.
+ * Populate a bridging rule on given deviceId that matches given mac, given vlan and
+ * output to given port.
+ *
+ * @param deviceId device ID
+ * @param port port
+ * @param mac mac address
+ * @param vlanId VLAN ID
+ */
+ void populateBridging(DeviceId deviceId, PortNumber port, MacAddress mac, VlanId vlanId) {
+ ForwardingObjective.Builder fob = bridgingFwdObjBuilder(deviceId, mac, vlanId, port, false);
+ if (fob == null) {
+ log.warn("Fail to build fwd obj for host {}/{}. Abort.", mac, vlanId);
+ return;
+ }
+
+ ObjectiveContext context = new DefaultObjectiveContext(
+ (objective) -> log.debug("Brigding rule for {}/{} revoked", mac, vlanId),
+ (objective, error) -> log.warn("Failed to revoke bridging rule for {}/{}: {}", mac, vlanId, error));
+ srManager.flowObjectiveService.forward(deviceId, fob.add(context));
+ }
+
+ /**
+ * Revoke a bridging rule on given deviceId that matches given mac, given vlan and
+ * output to given port.
+ *
+ * @param deviceId device ID
+ * @param port port
+ * @param mac mac address
+ * @param vlanId VLAN ID
+ */
+ void revokeBridging(DeviceId deviceId, PortNumber port, MacAddress mac, VlanId vlanId) {
+ ForwardingObjective.Builder fob = bridgingFwdObjBuilder(deviceId, mac, vlanId, port, true);
+ if (fob == null) {
+ log.warn("Fail to build fwd obj for host {}/{}. Abort.", mac, vlanId);
+ return;
+ }
+
+ ObjectiveContext context = new DefaultObjectiveContext(
+ (objective) -> log.debug("Brigding rule for {}/{} populated", mac, vlanId),
+ (objective, error) -> log.warn("Failed to populate bridging rule for {}/{}: {}", mac, vlanId, error));
+ srManager.flowObjectiveService.forward(deviceId, fob.remove(context));
+ }
+
+ /**
+ * Generates a forwarding objective builder for bridging rules.
+ * <p>
+ * The forwarding objective bridges packets destined to a given MAC to
+ * given port on given device.
+ *
+ * @param deviceId Device that host attaches to
+ * @param mac MAC address of the host
+ * @param hostVlanId VLAN ID of the host
+ * @param outport Port that host attaches to
+ * @param revoke true if forwarding objective is meant to revoke forwarding rule
+ * @return Forwarding objective builder
+ */
+ private ForwardingObjective.Builder bridgingFwdObjBuilder(
+ DeviceId deviceId, MacAddress mac, VlanId hostVlanId, PortNumber outport, boolean revoke) {
+ ConnectPoint connectPoint = new ConnectPoint(deviceId, outport);
+ VlanId untaggedVlan = srManager.getUntaggedVlanId(connectPoint);
+ Set<VlanId> taggedVlans = srManager.getTaggedVlanId(connectPoint);
+ VlanId nativeVlan = srManager.getNativeVlanId(connectPoint);
+
+ // Create host selector
+ TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
+ sbuilder.matchEthDst(mac);
+
+ // Create host treatment
+ TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
+ tbuilder.immediate().setOutput(outport);
+
+ // Create host meta
+ TrafficSelector.Builder mbuilder = DefaultTrafficSelector.builder();
+
+ // Adjust the selector, treatment and meta according to VLAN configuration
+ if (taggedVlans.contains(hostVlanId)) {
+ sbuilder.matchVlanId(hostVlanId);
+ mbuilder.matchVlanId(hostVlanId);
+ } else if (hostVlanId.equals(VlanId.NONE)) {
+ if (untaggedVlan != null) {
+ sbuilder.matchVlanId(untaggedVlan);
+ mbuilder.matchVlanId(untaggedVlan);
+ tbuilder.immediate().popVlan();
+ } else if (nativeVlan != null) {
+ sbuilder.matchVlanId(nativeVlan);
+ mbuilder.matchVlanId(nativeVlan);
+ tbuilder.immediate().popVlan();
+ } else {
+ log.warn("Untagged host {}/{} is not allowed on {} without untagged or native" +
+ "vlan config", mac, hostVlanId, connectPoint);
+ return null;
+ }
+ } else {
+ log.warn("Tagged host {}/{} is not allowed on {} without VLAN listed in tagged vlan",
+ mac, hostVlanId, connectPoint);
+ return null;
+ }
+
+ // All forwarding is via Groups. Drivers can re-purpose to flow-actions if needed.
+ // 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(), !revoke);
+ if (portNextObjId == -1) {
+ // Warning log will come from getPortNextObjective method
+ return null;
+ }
+
+ return DefaultForwardingObjective.builder()
+ .withFlag(ForwardingObjective.Flag.SPECIFIC)
+ .withSelector(sbuilder.build())
+ .nextStep(portNextObjId)
+ .withPriority(100)
+ .fromApp(srManager.appId)
+ .makePermanent();
+ }
+
+ /**
+ * Populate or revoke a bridging rule on given deviceId that matches given vlanId,
+ * and hostMAC connected to given port, and output to given port only when
+ * vlan information is valid.
+ *
+ * @param deviceId device ID that host attaches to
+ * @param portNum port number that host attaches to
+ * @param hostMac mac address of the host connected to the switch port
+ * @param vlanId Vlan ID configured on the switch port
+ * @param popVlan true to pop Vlan tag at TrafficTreatment, false otherwise
+ * @param install true to populate the objective, false to revoke
+ */
+ // TODO Refactor. There are a lot of duplications between this method, populateBridging,
+ // revokeBridging and bridgingFwdObjBuilder.
+ void updateBridging(DeviceId deviceId, PortNumber portNum, MacAddress hostMac,
+ VlanId vlanId, boolean popVlan, boolean install) {
+ // Create host selector
+ TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
+ sbuilder.matchEthDst(hostMac);
+
+ // Create host meta
+ TrafficSelector.Builder mbuilder = DefaultTrafficSelector.builder();
+
+ sbuilder.matchVlanId(vlanId);
+ mbuilder.matchVlanId(vlanId);
+
+ // Create host treatment
+ TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
+ tbuilder.immediate().setOutput(portNum);
+
+ if (popVlan) {
+ tbuilder.immediate().popVlan();
+ }
+
+ int portNextObjId = srManager.getPortNextObjectiveId(deviceId, portNum,
+ tbuilder.build(), mbuilder.build(), install);
+ if (portNextObjId != -1) {
+ ForwardingObjective.Builder fob = DefaultForwardingObjective.builder()
+ .withFlag(ForwardingObjective.Flag.SPECIFIC)
+ .withSelector(sbuilder.build())
+ .nextStep(portNextObjId)
+ .withPriority(100)
+ .fromApp(srManager.appId)
+ .makePermanent();
+
+ ObjectiveContext context = new DefaultObjectiveContext(
+ (objective) -> log.debug("Brigding rule for {}/{} {}", hostMac, vlanId,
+ install ? "populated" : "revoked"),
+ (objective, error) -> log.warn("Failed to {} bridging rule for {}/{}: {}",
+ install ? "populate" : "revoke", hostMac, vlanId, error));
+ srManager.flowObjectiveService.forward(deviceId, install ? fob.add(context) : fob.remove(context));
+ } else {
+ log.warn("Failed to retrieve next objective for {}/{}", hostMac, vlanId);
+ }
+ }
+
+ /**
+ * Populates IP rules for a route that has direct connection to the switch.
+ * This method should not be invoked directly without going through DefaultRoutingHandler.
*
* @param deviceId device ID of the device that next hop attaches to
* @param prefix IP prefix of the route
@@ -157,6 +331,7 @@
/**
* Removes IP rules for a route when the next hop is gone.
+ * This method should not be invoked directly without going through DefaultRoutingHandler.
*
* @param deviceId device ID of the device that next hop attaches to
* @param prefix IP prefix of the route
@@ -293,14 +468,15 @@
}
/**
- * Revokes IP flow rules for the subnets in each edge switch.
+ * Revokes IP flow rules for the subnets from given device.
*
+ * @param targetSw target switch from which the subnets need to be removed
* @param subnets subnet being removed
* @return true if all rules are removed successfully, false otherwise
*/
- boolean revokeIpRuleForSubnet(Set<IpPrefix> subnets) {
+ boolean revokeIpRuleForSubnet(DeviceId targetSw, Set<IpPrefix> subnets) {
for (IpPrefix subnet : subnets) {
- if (!revokeIpRuleForRouter(subnet)) {
+ if (!revokeIpRuleForRouter(targetSw, subnet)) {
return false;
}
}
@@ -419,12 +595,13 @@
}
/**
- * Revokes IP flow rules for the router IP address.
+ * Revokes IP flow rules for the router IP address from given device.
*
+ * @param targetSw target switch from which the ipPrefix need to be removed
* @param ipPrefix the IP address of the destination router
* @return true if all rules are removed successfully, false otherwise
*/
- private boolean revokeIpRuleForRouter(IpPrefix ipPrefix) {
+ private boolean revokeIpRuleForRouter(DeviceId targetSw, IpPrefix ipPrefix) {
TrafficSelector.Builder sbuilder = buildIpSelectorFromIpPrefix(ipPrefix);
TrafficSelector selector = sbuilder.build();
TrafficTreatment dummyTreatment = DefaultTrafficTreatment.builder().build();
@@ -438,17 +615,11 @@
.withPriority(getPriorityFromPrefix(ipPrefix))
.withFlag(ForwardingObjective.Flag.SPECIFIC);
- srManager.deviceService.getAvailableDevices().forEach(device -> {
- if (srManager.mastershipService.isLocalMaster(device.id())) {
- ObjectiveContext context = new DefaultObjectiveContext(
- (objective) -> log.debug("IP rule for router {} revoked from {}", ipPrefix, device.id()),
- (objective, error) -> log.warn("Failed to revoke IP rule for router {} from {}: {}",
- ipPrefix, device.id(), error));
- srManager.flowObjectiveService.forward(device.id(), fwdBuilder.remove(context));
- } else {
- log.debug("Not the master of {}. Abort route {} removal", device.id(), ipPrefix);
- }
- });
+ ObjectiveContext context = new DefaultObjectiveContext(
+ (objective) -> log.debug("IP rule for router {} revoked from {}", ipPrefix, targetSw),
+ (objective, error) -> log.warn("Failed to revoke IP rule for router {} from {}: {}",
+ ipPrefix, targetSw, error));
+ srManager.flowObjectiveService.forward(targetSw, fwdBuilder.remove(context));
return true;
}
diff --git a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index fe01fd3..c17df68 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -1215,7 +1215,6 @@
if (mastershipService.isLocalMaster(deviceId)) {
defaultRoutingHandler.populatePortAddressingRules(deviceId);
- hostHandler.init(deviceId);
xConnectHandler.init(deviceId);
DefaultGroupHandler groupHandler = groupHandlerMap.get(deviceId);
groupHandler.createGroupsFromVlanConfig();
@@ -1223,6 +1222,7 @@
}
appCfgHandler.init(deviceId);
+ hostHandler.init(deviceId);
routeHandler.init(deviceId);
}
@@ -1556,12 +1556,6 @@
DeviceId deviceId = intf.connectPoint().deviceId();
PortNumber portNum = intf.connectPoint().port();
- if (!mastershipService.isLocalMaster(deviceId)) {
- log.debug("CONFIG_UPDATED event for interfaces should be " +
- "handled by master node for device {}", deviceId);
- return;
- }
-
removeSubnetConfig(prevIntf.connectPoint(),
Sets.difference(new HashSet<>(prevIntf.ipAddressesList()),
new HashSet<>(intf.ipAddressesList())));