Charles Chan

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
......@@ -513,7 +513,6 @@ public class DefaultRoutingHandler {
public void populatePortAddressingRules(DeviceId deviceId) {
rulePopulator.populateRouterMacVlanFilters(deviceId);
rulePopulator.populateRouterIpPunts(deviceId);
rulePopulator.populateArpPunts(deviceId);
}
/**
......
......@@ -303,6 +303,7 @@ public class RoutingRulePopulator {
// 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 @@ public class RoutingRulePopulator {
}
/**
* 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.
......
......@@ -51,6 +51,7 @@ import org.onosproject.net.flowobjective.ObjectiveContext;
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.Collections;
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 @@ public class SegmentRoutingManager implements SegmentRoutingService {
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 @@ public class SegmentRoutingManager implements SegmentRoutingService {
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 @@ public class SegmentRoutingManager implements SegmentRoutingService {
flowObjectiveService,
nsNextObjStore,
subnetNextObjStore,
portNextObjStore);
portNextObjStore,
this);
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Aborting processDeviceAdded.");
return;
......@@ -766,7 +779,8 @@ public class SegmentRoutingManager implements SegmentRoutingService {
flowObjectiveService,
nsNextObjStore,
subnetNextObjStore,
portNextObjStore);
portNextObjStore,
segmentRoutingManager);
} catch (DeviceConfigNotFoundException e) {
log.warn(e.getMessage() + " Aborting configureNetwork.");
return;
......@@ -836,23 +850,33 @@ public class SegmentRoutingManager implements SegmentRoutingService {
private ForwardingObjective.Builder getForwardingObjectiveBuilder(
DeviceId deviceId, MacAddress mac, VlanId vlanId,
PortNumber outport) {
// Get assigned VLAN for the subnet
VlanId outvlan = null;
Ip4Prefix subnet = deviceConfiguration.getPortSubnet(deviceId, outport);
if (subnet == null) {
outvlan = VlanId.vlanId(ASSIGNED_VLAN_NO_SUBNET);
} else {
outvlan = getSubnetAssignedVlanId(deviceId, subnet);
}
// match rule
TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
sbuilder.matchEthDst(mac);
sbuilder.matchVlanId(vlanId);
/*
* 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
VlanId outvlan = null;
Ip4Prefix subnet = deviceConfiguration.getPortSubnet(deviceId, outport);
if (subnet == null) {
outvlan = VlanId.vlanId(ASSIGNED_VLAN_NO_SUBNET);
} else {
outvlan = getSubnetAssignedVlanId(deviceId, subnet);
}
TrafficSelector meta = DefaultTrafficSelector.builder()
.matchVlanId(outvlan).build();
......
......@@ -24,6 +24,7 @@ import org.onosproject.net.DeviceId;
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 @@ import org.onosproject.store.service.EventuallyConsistentMap;
* 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 @@ public class DefaultEdgeGroupHandler extends DefaultGroupHandler {
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
......
......@@ -87,6 +87,7 @@ public class DefaultGroupHandler {
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 @@ public class DefaultGroupHandler {
.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 @@ public class DefaultGroupHandler {
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 @@ public class DefaultGroupHandler {
this.nsNextObjStore = nsNextObjStore;
this.subnetNextObjStore = subnetNextObjStore;
this.portNextObjStore = portNextObjStore;
this.srManager = srManager;
populateNeighborMaps();
}
......@@ -153,7 +157,8 @@ public class DefaultGroupHandler {
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 @@ public class DefaultGroupHandler {
flowObjService,
nsNextObjStore,
subnetNextObjStore,
portNextObjStore);
portNextObjStore,
srManager
);
} else {
return new DefaultTransitGroupHandler(deviceId, appId, config,
linkService,
flowObjService,
nsNextObjStore,
subnetNextObjStore,
portNextObjStore);
portNextObjStore,
srManager);
}
}
......@@ -663,11 +671,17 @@ public class DefaultGroupHandler {
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();
......
......@@ -23,6 +23,7 @@ import org.onosproject.net.DeviceId;
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 @@ import org.onosproject.store.service.EventuallyConsistentMap;
* 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 @@ public class DefaultTransitGroupHandler extends DefaultGroupHandler {
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
......
......@@ -27,6 +27,7 @@ import java.util.List;
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 @@ public class PolicyGroupHandler extends DefaultGroupHandler {
* @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 @@ public class PolicyGroupHandler extends DefaultGroupHandler {
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,
......
......@@ -106,13 +106,13 @@ public class CpqdOFDPA2Pipeline extends OFDPA2Pipeline {
for (PortNumber pnum : portnums) {
// update storage
port2Vlan.put(pnum, storeVlan);
Set<PortNumber> vlanPorts = vlan2Port.get(storeVlan);
ofdpa2GroupHandler.port2Vlan.put(pnum, storeVlan);
Set<PortNumber> vlanPorts = ofdpa2GroupHandler.vlan2Port.get(storeVlan);
if (vlanPorts == null) {
vlanPorts = Collections.newSetFromMap(
new ConcurrentHashMap<PortNumber, Boolean>());
vlanPorts.add(pnum);
vlan2Port.put(storeVlan, vlanPorts);
ofdpa2GroupHandler.vlan2Port.put(storeVlan, vlanPorts);
} else {
vlanPorts.add(pnum);
}
......
......@@ -51,6 +51,7 @@ import org.onosproject.net.flow.criteria.Criterion.Type;
import org.onosproject.net.flow.criteria.EthCriterion;
import org.onosproject.net.flow.criteria.EthTypeCriterion;
import org.onosproject.net.flow.criteria.IPCriterion;
import org.onosproject.net.flow.criteria.MplsBosCriterion;
import org.onosproject.net.flow.criteria.MplsCriterion;
import org.onosproject.net.flow.criteria.PortCriterion;
import org.onosproject.net.flow.criteria.VlanIdCriterion;
......@@ -593,9 +594,10 @@ public class SpringOpenTTP extends AbstractHandlerBehaviour
.matchEthType(Ethernet.MPLS_UNICAST)
.matchMplsLabel(((MplsCriterion)
selector.getCriterion(Criterion.Type.MPLS_LABEL)).label());
//TODO: Add Match for BoS
//if (selector.getCriterion(Criterion.Type.MPLS_BOS) != null) {
//}
if (selector.getCriterion(Criterion.Type.MPLS_BOS) != null) {
filteredSelectorBuilder.matchMplsBos(((MplsBosCriterion)
selector.getCriterion(Type.MPLS_BOS)).mplsBos());
}
forTableId = mplsTableId;
log.debug("processing MPLS specific forwarding objective:{} in dev:{}",
fwd.id(), deviceId);
......
......@@ -34,6 +34,7 @@ import org.onosproject.net.flow.criteria.Criterion;
import org.onosproject.net.flow.criteria.EthCriterion;
import org.onosproject.net.flow.criteria.EthTypeCriterion;
import org.onosproject.net.flow.criteria.IPCriterion;
import org.onosproject.net.flow.criteria.MplsBosCriterion;
import org.onosproject.net.flow.criteria.MplsCriterion;
import org.onosproject.net.flow.criteria.VlanIdCriterion;
import org.onosproject.net.flow.instructions.Instruction;
......@@ -116,9 +117,10 @@ public class SpringOpenTTPDell extends SpringOpenTTP {
.matchEthType(Ethernet.MPLS_UNICAST)
.matchMplsLabel(((MplsCriterion)
selector.getCriterion(Criterion.Type.MPLS_LABEL)).label());
//TODO: Add Match for BoS
//if (selector.getCriterion(Criterion.Type.MPLS_BOS) != null) {
//}
if (selector.getCriterion(Criterion.Type.MPLS_BOS) != null) {
filteredSelectorBuilder.matchMplsBos(((MplsBosCriterion)
selector.getCriterion(Criterion.Type.MPLS_BOS)).mplsBos());
}
forTableId = mplsTableId;
log.debug("processing MPLS specific forwarding objective");
}
......