Segment Routing bug fix and enhancement
Bugfix:
- Add MPLS BOS matching
- Fix NPE caused by race between filter objective and broadcast next objective
Enhancement:
- Move group handler out from OFDPA pipeline
- Move ARP request from rule populator to packet request
Change-Id: I0ba40e10f7cb7f97277df86725fbd2546a62e890
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 b3e432c..1a37efd 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -513,7 +513,6 @@
public void populatePortAddressingRules(DeviceId deviceId) {
rulePopulator.populateRouterMacVlanFilters(deviceId);
rulePopulator.populateRouterIpPunts(deviceId);
- rulePopulator.populateArpPunts(deviceId);
}
/**
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 d4aa770..8543c86 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -303,6 +303,7 @@
// TODO Handle the case of Bos == false
sbuilder.matchEthType(Ethernet.MPLS_UNICAST);
sbuilder.matchMplsLabel(MplsLabel.mplsLabel(segmentId));
+ sbuilder.matchMplsBos(true);
TrafficSelector selector = sbuilder.build();
// setup metadata to pass to nextObjective - indicate the vlan on egress
@@ -525,39 +526,6 @@
}
/**
- * Creates a forwarding objective to punt all IP packets, destined to the
- * router's port IP addresses, to the controller. Note that the input
- * port should not be matched on, as these packets can come from any input.
- * Furthermore, these are applied only by the master instance.
- *
- * @param deviceId the switch dpid for the router
- */
- public void populateArpPunts(DeviceId deviceId) {
- if (!srManager.mastershipService.isLocalMaster(deviceId)) {
- log.debug("Not installing port-IP punts - not the master for dev:{} ",
- deviceId);
- return;
- }
-
- ForwardingObjective.Builder puntArp = DefaultForwardingObjective.builder();
- TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
- TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
- sbuilder.matchEthType(Ethernet.TYPE_ARP);
- tbuilder.setOutput(PortNumber.CONTROLLER);
- puntArp.withSelector(sbuilder.build());
- puntArp.withTreatment(tbuilder.build());
- puntArp.withFlag(Flag.VERSATILE)
- .withPriority(HIGHEST_PRIORITY)
- .makePermanent()
- .fromApp(srManager.appId);
- log.debug("Installing forwarding objective to punt ARPs");
- srManager.flowObjectiveService.
- forward(deviceId,
- puntArp.add(new SRObjectiveContext(deviceId,
- SRObjectiveContext.ObjectiveType.FORWARDING)));
- }
-
- /**
* Populates a forwarding objective to send packets that miss other high
* priority Bridging Table entries to a group that contains all ports of
* its subnet.
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 7bcdfeb..0f20451 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -51,6 +51,7 @@
import org.onosproject.net.flowobjective.ObjectiveError;
import org.onosproject.net.host.HostEvent;
import org.onosproject.net.host.HostListener;
+import org.onosproject.net.packet.PacketPriority;
import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
import org.onosproject.segmentrouting.config.DeviceConfiguration;
import org.onosproject.segmentrouting.config.SegmentRoutingConfig;
@@ -90,6 +91,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedQueue;
@@ -297,6 +299,11 @@
linkService.addListener(linkListener);
deviceService.addListener(deviceListener);
+ // Request ARP packet-in
+ TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
+ selector.matchEthType(Ethernet.TYPE_ARP);
+ packetService.requestPackets(selector.build(), PacketPriority.CONTROL, appId, Optional.empty());
+
cfgListener.configureNetwork();
log.info("Started");
@@ -307,6 +314,11 @@
cfgService.removeListener(cfgListener);
cfgService.unregisterConfigFactory(cfgFactory);
+ // Withdraw ARP packet-in
+ TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
+ selector.matchEthType(Ethernet.TYPE_ARP);
+ packetService.cancelPackets(selector.build(), PacketPriority.CONTROL, appId, Optional.empty());
+
packetService.removeProcessor(processor);
linkService.removeListener(linkListener);
deviceService.removeListener(deviceListener);
@@ -697,7 +709,8 @@
flowObjectiveService,
nsNextObjStore,
subnetNextObjStore,
- portNextObjStore);
+ portNextObjStore,
+ this);
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Aborting processDeviceAdded.");
return;
@@ -766,7 +779,8 @@
flowObjectiveService,
nsNextObjStore,
subnetNextObjStore,
- portNextObjStore);
+ portNextObjStore,
+ segmentRoutingManager);
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Aborting configureNetwork.");
return;
@@ -836,16 +850,7 @@
private ForwardingObjective.Builder getForwardingObjectiveBuilder(
DeviceId deviceId, MacAddress mac, VlanId vlanId,
PortNumber outport) {
- // match rule
- TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
- sbuilder.matchEthDst(mac);
- sbuilder.matchVlanId(vlanId);
-
- TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
- tbuilder.immediate().popVlan();
- tbuilder.immediate().setOutput(outport);
-
- // for switch pipelines that need it, provide outgoing vlan as metadata
+ // Get assigned VLAN for the subnet
VlanId outvlan = null;
Ip4Prefix subnet = deviceConfiguration.getPortSubnet(deviceId, outport);
if (subnet == null) {
@@ -853,6 +858,25 @@
} else {
outvlan = getSubnetAssignedVlanId(deviceId, subnet);
}
+
+ // match rule
+ TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
+ sbuilder.matchEthDst(mac);
+ /*
+ * Note: for untagged packets, match on the assigned VLAN.
+ * for tagged packets, match on its incoming VLAN.
+ */
+ if (vlanId.equals(VlanId.NONE)) {
+ sbuilder.matchVlanId(outvlan);
+ } else {
+ sbuilder.matchVlanId(vlanId);
+ }
+
+ TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
+ tbuilder.immediate().popVlan();
+ tbuilder.immediate().setOutput(outport);
+
+ // for switch pipelines that need it, provide outgoing vlan as metadata
TrafficSelector meta = DefaultTrafficSelector.builder()
.matchVlanId(outvlan).build();
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultEdgeGroupHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultEdgeGroupHandler.java
index 32c5365..4866b82 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultEdgeGroupHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultEdgeGroupHandler.java
@@ -24,6 +24,7 @@
import org.onosproject.net.Link;
import org.onosproject.net.flowobjective.FlowObjectiveService;
import org.onosproject.net.link.LinkService;
+import org.onosproject.segmentrouting.SegmentRoutingManager;
import org.onosproject.segmentrouting.config.DeviceProperties;
import org.onosproject.store.service.EventuallyConsistentMap;
@@ -46,7 +47,7 @@
* 8) what about ecmp no label case
*/
public class DefaultEdgeGroupHandler extends DefaultGroupHandler {
-
+ // TODO Access stores through srManager
protected DefaultEdgeGroupHandler(DeviceId deviceId,
ApplicationId appId,
DeviceProperties config,
@@ -58,9 +59,10 @@
EventuallyConsistentMap<SubnetNextObjectiveStoreKey,
Integer> subnetNextObjStore,
EventuallyConsistentMap<PortNextObjectiveStoreKey,
- Integer> portNextObjStore) {
+ Integer> portNextObjStore,
+ SegmentRoutingManager srManager) {
super(deviceId, appId, config, linkService, flowObjService,
- nsNextObjStore, subnetNextObjStore, portNextObjStore);
+ nsNextObjStore, subnetNextObjStore, portNextObjStore, srManager);
}
@Override
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 2986c50..a4b89c1 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -87,6 +87,7 @@
SubnetNextObjectiveStoreKey, Integer> subnetNextObjStore = null;
protected EventuallyConsistentMap<
PortNextObjectiveStoreKey, Integer> portNextObjStore = null;
+ private SegmentRoutingManager srManager;
protected KryoNamespace.Builder kryo = new KryoNamespace.Builder()
.register(URI.class).register(HashSet.class)
@@ -96,6 +97,7 @@
.register(GroupBucketIdentifier.class)
.register(GroupBucketIdentifier.BucketOutputType.class);
+ // TODO Access stores through srManager
protected DefaultGroupHandler(DeviceId deviceId, ApplicationId appId,
DeviceProperties config,
LinkService linkService,
@@ -105,7 +107,8 @@
EventuallyConsistentMap<SubnetNextObjectiveStoreKey,
Integer> subnetNextObjStore,
EventuallyConsistentMap<PortNextObjectiveStoreKey,
- Integer> portNextObjStore) {
+ Integer> portNextObjStore,
+ SegmentRoutingManager srManager) {
this.deviceId = checkNotNull(deviceId);
this.appId = checkNotNull(appId);
this.deviceConfig = checkNotNull(config);
@@ -123,6 +126,7 @@
this.nsNextObjStore = nsNextObjStore;
this.subnetNextObjStore = subnetNextObjStore;
this.portNextObjStore = portNextObjStore;
+ this.srManager = srManager;
populateNeighborMaps();
}
@@ -153,7 +157,8 @@
EventuallyConsistentMap<SubnetNextObjectiveStoreKey,
Integer> subnetNextObjStore,
EventuallyConsistentMap<PortNextObjectiveStoreKey,
- Integer> portNextObjStore)
+ Integer> portNextObjStore,
+ SegmentRoutingManager srManager)
throws DeviceConfigNotFoundException {
// handle possible exception in the caller
if (config.isEdgeDevice(deviceId)) {
@@ -162,14 +167,17 @@
flowObjService,
nsNextObjStore,
subnetNextObjStore,
- portNextObjStore);
+ portNextObjStore,
+ srManager
+ );
} else {
return new DefaultTransitGroupHandler(deviceId, appId, config,
linkService,
flowObjService,
nsNextObjStore,
subnetNextObjStore,
- portNextObjStore);
+ portNextObjStore,
+ srManager);
}
}
@@ -663,11 +671,17 @@
return;
}
+ VlanId assignedVlanId =
+ srManager.getSubnetAssignedVlanId(this.deviceId, subnet);
+ TrafficSelector metadata =
+ DefaultTrafficSelector.builder().matchVlanId(assignedVlanId).build();
+
int nextId = flowObjectiveService.allocateNextId();
NextObjective.Builder nextObjBuilder = DefaultNextObjective
.builder().withId(nextId)
- .withType(NextObjective.Type.BROADCAST).fromApp(appId);
+ .withType(NextObjective.Type.BROADCAST).fromApp(appId)
+ .withMeta(metadata);
ports.forEach(port -> {
TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultTransitGroupHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultTransitGroupHandler.java
index 7a43e73..5bc7ede 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultTransitGroupHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultTransitGroupHandler.java
@@ -23,6 +23,7 @@
import org.onosproject.net.Link;
import org.onosproject.net.flowobjective.FlowObjectiveService;
import org.onosproject.net.link.LinkService;
+import org.onosproject.segmentrouting.SegmentRoutingManager;
import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
import org.onosproject.segmentrouting.config.DeviceProperties;
import org.onosproject.store.service.EventuallyConsistentMap;
@@ -40,7 +41,7 @@
* 2) all ports to D3 + with no label push,
*/
public class DefaultTransitGroupHandler extends DefaultGroupHandler {
-
+ // TODO Access stores through srManager
protected DefaultTransitGroupHandler(DeviceId deviceId,
ApplicationId appId,
DeviceProperties config,
@@ -52,9 +53,10 @@
EventuallyConsistentMap<SubnetNextObjectiveStoreKey,
Integer> subnetNextObjStore,
EventuallyConsistentMap<PortNextObjectiveStoreKey,
- Integer> portNextObjStore) {
+ Integer> portNextObjStore,
+ SegmentRoutingManager srManager) {
super(deviceId, appId, config, linkService, flowObjService,
- nsNextObjStore, subnetNextObjStore, portNextObjStore);
+ nsNextObjStore, subnetNextObjStore, portNextObjStore, srManager);
}
@Override
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/PolicyGroupHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/PolicyGroupHandler.java
index ef143dc..4b0d518 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/PolicyGroupHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/grouphandler/PolicyGroupHandler.java
@@ -27,6 +27,7 @@
import org.onlab.packet.MacAddress;
import org.onlab.packet.MplsLabel;
import org.onosproject.core.ApplicationId;
+import org.onosproject.segmentrouting.SegmentRoutingManager;
import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
import org.onosproject.segmentrouting.config.DeviceProperties;
import org.onosproject.segmentrouting.grouphandler.GroupBucketIdentifier.BucketOutputType;
@@ -60,6 +61,7 @@
* @param nsNextObjStore NeighborSet next objective store map
* @param subnetNextObjStore subnet next objective store map
*/
+ // TODO Access stores through srManager
public PolicyGroupHandler(DeviceId deviceId,
ApplicationId appId,
DeviceProperties config,
@@ -70,9 +72,10 @@
EventuallyConsistentMap<SubnetNextObjectiveStoreKey,
Integer> subnetNextObjStore,
EventuallyConsistentMap<PortNextObjectiveStoreKey,
- Integer> portNextObjStore) {
+ Integer> portNextObjStore,
+ SegmentRoutingManager srManager) {
super(deviceId, appId, config, linkService, flowObjService,
- nsNextObjStore, subnetNextObjStore, portNextObjStore);
+ nsNextObjStore, subnetNextObjStore, portNextObjStore, srManager);
}
public PolicyGroupIdentifier createPolicyGroupChain(String id,