CORD-61 Dynamic XConnect support

- Add new XConnectConfig with unit test
- Gather XConnect features into XConnectHandler
- Introduce ObjectiveError.Type.GROUPREMOVALFAILED
- Rename
    - NetworkConfigEventHandler -> AppConfigHandler
    - XConnectNextObjectiveStoreKey -> XConnectStoreKey
    - Test json file
- Refactor

Change-Id: I8ca3176ed976c71ce9e28b7f3722ce80d49c816f

@@ -35,29 +35,29 @@ import java.util.HashSet;
 import java.util.Set;
 
 /**
- * Handles network config events.
+ * Handles Segment Routing app config events.
  */
-public class NetworkConfigEventHandler {
-    private static final Logger log = LoggerFactory.getLogger(NetworkConfigEventHandler.class);
+public class AppConfigHandler {
+    private static final Logger log = LoggerFactory.getLogger(AppConfigHandler.class);
     private final SegmentRoutingManager srManager;
     private final DeviceService deviceService;
 
     /**
-     * Constructs Network Config Event Handler.
+     * Constructs Segment Routing App Config Handler.
      *
      * @param srManager instance of {@link SegmentRoutingManager}
      */
-    public NetworkConfigEventHandler(SegmentRoutingManager srManager) {
+    public AppConfigHandler(SegmentRoutingManager srManager) {
         this.srManager = srManager;
         this.deviceService = srManager.deviceService;
     }
 
     /**
-     * Processes vRouter config added event.
+     * Processes Segment Routing App Config added event.
      *
      * @param event network config added event
      */
-    protected void processVRouterConfigAdded(NetworkConfigEvent event) {
+    protected void processAppConfigAdded(NetworkConfigEvent event) {
         log.info("Processing vRouter CONFIG_ADDED");
         SegmentRoutingAppConfig config = (SegmentRoutingAppConfig) event.config().get();
         deviceService.getAvailableDevices().forEach(device -> {
@@ -66,11 +66,11 @@ public class NetworkConfigEventHandler {
     }
 
     /**
-     * Processes vRouter config updated event.
+     * Processes Segment Routing App Config updated event.
      *
      * @param event network config updated event
      */
-    protected void processVRouterConfigUpdated(NetworkConfigEvent event) {
+    protected void processAppConfigUpdated(NetworkConfigEvent event) {
         log.info("Processing vRouter CONFIG_UPDATED");
         SegmentRoutingAppConfig config = (SegmentRoutingAppConfig) event.config().get();
         SegmentRoutingAppConfig prevConfig = (SegmentRoutingAppConfig) event.prevConfig().get();
@@ -91,11 +91,11 @@ public class NetworkConfigEventHandler {
     }
 
     /**
-     * Processes vRouter config removed event.
+     * Processes Segment Routing App Config removed event.
      *
      * @param event network config removed event
      */
-    protected void processVRouterConfigRemoved(NetworkConfigEvent event) {
+    protected void processAppConfigRemoved(NetworkConfigEvent event) {
         log.info("Processing vRouter CONFIG_REMOVED");
         SegmentRoutingAppConfig prevConfig = (SegmentRoutingAppConfig) event.prevConfig().get();
         deviceService.getAvailableDevices().forEach(device -> {

@@ -564,7 +564,6 @@ public class DefaultRoutingHandler {
      * @param deviceId Switch ID to set the rules
      */
     public void populatePortAddressingRules(DeviceId deviceId) {
-        rulePopulator.populateXConnectVlanFilters(deviceId);
         rulePopulator.populateRouterIpPunts(deviceId);
 
         // Although device is added, sometimes device store does not have the

@@ -79,8 +79,8 @@ public class McastHandler {
     private static final Logger log = LoggerFactory.getLogger(McastHandler.class);
     private final SegmentRoutingManager srManager;
     private final ApplicationId coreAppId;
-    private StorageService storageService;
-    private TopologyService topologyService;
+    private final StorageService storageService;
+    private final TopologyService topologyService;
     private final ConsistentMap<McastStoreKey, NextObjective> mcastNextObjStore;
     private final KryoNamespace.Builder mcastKryo;
     private final ConsistentMap<McastStoreKey, McastRole> mcastRoleStore;
@@ -132,7 +132,7 @@ public class McastHandler {
     /**
      * Read initial multicast from mcast store.
      */
-    public void init() {
+    protected void init() {
         srManager.multicastRouteService.getRoutes().forEach(mcastRoute -> {
             ConnectPoint source = srManager.multicastRouteService.fetchSource(mcastRoute);
             Set<ConnectPoint> sinks = srManager.multicastRouteService.fetchSinks(mcastRoute);
@@ -472,7 +472,7 @@ public class McastHandler {
                             log.warn("Failed to update {} on {}/{}, vlan {}: {}",
                                     mcastIp, deviceId, port.toLong(), assignedVlan, error));
             newNextObj = nextObjBuilder(mcastIp, assignedVlan, existingPorts).add();
-            fwdObj = fwdObjBuilder(mcastIp, assignedVlan, newNextObj.id()).add();
+            fwdObj = fwdObjBuilder(mcastIp, assignedVlan, newNextObj.id()).add(context);
             mcastNextObjStore.put(mcastStoreKey, newNextObj);
             srManager.flowObjectiveService.next(deviceId, newNextObj);
             srManager.flowObjectiveService.forward(deviceId, fwdObj);
@@ -779,11 +779,7 @@ public class McastHandler {
                 // Spine-facing port should have no subnet and no xconnect
                 if (srManager.deviceConfiguration != null &&
                         srManager.deviceConfiguration.getPortSubnet(ingressDevice, port) == null &&
-                        srManager.deviceConfiguration.getXConnects().values().stream()
-                                .allMatch(connectPoints ->
-                                        connectPoints.stream().noneMatch(connectPoint ->
-                                                connectPoint.port().equals(port))
-                                )) {
+                        !srManager.xConnectHandler.hasXConnect(new ConnectPoint(ingressDevice, port))) {
                     return port;
                 }
             }

@@ -50,7 +50,6 @@ import org.slf4j.LoggerFactory;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -694,85 +693,6 @@ public class RoutingRulePopulator {
         });
     }
 
-    /**
-     * Creates a filtering objective to permit VLAN cross-connect traffic.
-     *
-     * @param deviceId the DPID of the switch
-     */
-    public void populateXConnectVlanFilters(DeviceId deviceId) {
-        Map<VlanId, List<ConnectPoint>> xConnectsForDevice =
-                config.getXConnects();
-        xConnectsForDevice.forEach((vlanId, connectPoints) -> {
-            // Only proceed  the xConnect for given device
-            for (ConnectPoint connectPoint : connectPoints) {
-                if (!connectPoint.deviceId().equals(deviceId)) {
-                    return;
-                }
-            }
-
-            connectPoints.forEach(connectPoint -> {
-                FilteringObjective.Builder fob = DefaultFilteringObjective.builder();
-                fob.withKey(Criteria.matchInPort(connectPoint.port()))
-                        .addCondition(Criteria.matchVlanId(vlanId))
-                        .addCondition(Criteria.matchEthDst(MacAddress.NONE))
-                        .withPriority(SegmentRoutingService.XCONNECT_PRIORITY);
-                fob.permit().fromApp(srManager.appId);
-                ObjectiveContext context = new DefaultObjectiveContext(
-                        (objective) -> log.debug("XConnect filter for {} populated", connectPoint),
-                        (objective, error) ->
-                                log.warn("Failed to populate xconnect filter for {}: {}", connectPoint, error));
-                srManager.flowObjectiveService.filter(deviceId, fob.add(context));
-            });
-        });
-    }
-
-    /**
-     * Populates a forwarding objective that points the VLAN cross-connect
-     * packets to a broadcast group.
-     *
-     * @param deviceId switch ID to set the rules
-     */
-    public void populateXConnectBroadcastRule(DeviceId deviceId) {
-        Map<VlanId, List<ConnectPoint>> xConnects =
-                config.getXConnects();
-        xConnects.forEach((vlanId, connectPoints) -> {
-            // Only proceed  the xConnect for given device
-            for (ConnectPoint connectPoint : connectPoints) {
-                if (!connectPoint.deviceId().equals(deviceId)) {
-                    return;
-                }
-            }
-
-            int nextId = srManager.getXConnectNextObjectiveId(deviceId, vlanId);
-            if (nextId < 0) {
-                log.error("Cannot install cross-connect broadcast rule in dev:{} " +
-                        "due to missing nextId:{}", deviceId, nextId);
-                return;
-            }
-
-            /*
-             * Driver should treat objectives with MacAddress.NONE and !VlanId.NONE
-             * as the VLAN cross-connect broadcast rules
-             */
-            TrafficSelector.Builder sbuilder = DefaultTrafficSelector.builder();
-            sbuilder.matchVlanId(vlanId);
-            sbuilder.matchEthDst(MacAddress.NONE);
-
-            ForwardingObjective.Builder fob = DefaultForwardingObjective.builder();
-            fob.withFlag(Flag.SPECIFIC)
-                    .withSelector(sbuilder.build())
-                    .nextStep(nextId)
-                    .withPriority(SegmentRoutingService.DEFAULT_PRIORITY)
-                    .fromApp(srManager.appId)
-                    .makePermanent();
-            ObjectiveContext context = new DefaultObjectiveContext(
-                    (objective) -> log.debug("XConnect rule for {} populated", xConnects),
-                    (objective, error) ->
-                            log.warn("Failed to populate xconnect rule for {}: {}", xConnects, error));
-            srManager.flowObjectiveService.forward(deviceId, fob.add(context));
-        });
-    }
-
     private int getPriorityFromPrefix(IpPrefix prefix) {
         return (prefix.isIp4()) ?
                 2000 * prefix.prefixLength() + SegmentRoutingService.MIN_IP_PRIORITY :

@@ -39,7 +39,6 @@ import org.slf4j.LoggerFactory;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -58,7 +57,6 @@ public class DeviceConfiguration implements DeviceProperties {
     private static final Logger log = LoggerFactory.getLogger(DeviceConfiguration.class);
     private final List<Integer> allSegmentIds = new ArrayList<>();
     private final Map<DeviceId, SegmentRouterInfo> deviceConfigMap = new ConcurrentHashMap<>();
-    private final Map<VlanId, List<ConnectPoint>> xConnects = new ConcurrentHashMap<>();
     private ApplicationId appId;
     private NetworkConfigService cfgService;
 
@@ -148,28 +146,6 @@ public class DeviceConfiguration implements DeviceProperties {
                         }
                         info.subnets.put(port, interfaceAddress.subnetAddress().getIp4Prefix());
                     });
-
-                    // Extract VLAN cross-connect information
-                    // Do not setup cross-connect if VLAN is NONE
-                    if (vlanId.equals(VlanId.NONE)) {
-                        return;
-                    }
-                    List<ConnectPoint> connectPoints = xConnects.get(vlanId);
-                    if (connectPoints != null) {
-                        if (connectPoints.size() != 1) {
-                            log.warn("Cross-connect should only have two endpoints. Aborting.");
-                            return;
-                        }
-                        if (!connectPoints.get(0).deviceId().equals(connectPoint.deviceId())) {
-                            log.warn("Cross-connect endpoints must be on the same switch. Aborting.");
-                            return;
-                        }
-                        connectPoints.add(connectPoint);
-                    } else {
-                        connectPoints = new LinkedList<>();
-                        connectPoints.add(connectPoint);
-                        xConnects.put(vlanId, connectPoints);
-                    }
                 }
             });
         });
@@ -298,11 +274,6 @@ public class DeviceConfiguration implements DeviceProperties {
         return subnetPortMap;
     }
 
-    @Override
-    public Map<VlanId, List<ConnectPoint>> getXConnects() {
-        return xConnects;
-    }
-
     /**
      * Returns the device identifier or data plane identifier (dpid)
      * of a segment router given its segment id.

@@ -21,8 +21,6 @@ import java.util.Map;
 import org.onlab.packet.Ip4Address;
 import org.onlab.packet.Ip4Prefix;
 import org.onlab.packet.MacAddress;
-import org.onlab.packet.VlanId;
-import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.PortNumber;
 
@@ -97,11 +95,4 @@ public interface DeviceProperties {
      */
     Map<Ip4Prefix, List<PortNumber>> getSubnetPortsMap(DeviceId deviceId)
             throws DeviceConfigNotFoundException;
-
-    /**
-     * Returns the VLAN cross-connect configuration.
-     *
-     * @return A map of that maps VLAN ID to a list of cross-connect endpoints
-     */
-    Map<VlanId, List<ConnectPoint>> getXConnects();
 }

+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.segmentrouting.config;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.ImmutableSet;
+import org.onlab.packet.VlanId;
+import org.onosproject.core.ApplicationId;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.PortNumber;
+import org.onosproject.net.config.Config;
+import org.onosproject.segmentrouting.storekey.XConnectStoreKey;
+
+import java.util.Set;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Configuration object for cross-connect.
+ */
+public class XConnectConfig extends Config<ApplicationId> {
+    private static final String VLAN = "vlan";
+    private static final String PORTS = "ports";
+    private static final String NAME = "name"; // dummy field for naming
+
+    private static final String UNEXPECTED_FIELD_NAME = "Unexpected field name";
+
+    @Override
+    public boolean isValid() {
+        try {
+            getXconnects().forEach(this::getPorts);
+        } catch (IllegalArgumentException e) {
+            return false;
+        }
+        return true;
+    }
+
+    /**
+     * Returns all xconnect keys.
+     *
+     * @return all keys (device/vlan pairs)
+     * @throws IllegalArgumentException if wrong format
+     */
+    public Set<XConnectStoreKey> getXconnects() {
+        ImmutableSet.Builder<XConnectStoreKey> builder = ImmutableSet.builder();
+        object.fields().forEachRemaining(entry -> {
+            DeviceId deviceId = DeviceId.deviceId(entry.getKey());
+            builder.addAll(getXconnects(deviceId));
+        });
+        return builder.build();
+    }
+
+    /**
+     * Returns xconnect keys of given device.
+     *
+     * @param deviceId ID of the device from which we want to get XConnect info
+     * @return xconnect keys (device/vlan pairs) of given device
+     * @throws IllegalArgumentException if wrong format
+     */
+    public Set<XConnectStoreKey> getXconnects(DeviceId deviceId) {
+        ImmutableSet.Builder<XConnectStoreKey> builder = ImmutableSet.builder();
+        JsonNode vlanPortPair = object.get(deviceId.toString());
+        if (vlanPortPair != null) {
+            vlanPortPair.forEach(jsonNode -> {
+                if (!hasOnlyFields((ObjectNode) jsonNode, VLAN, PORTS, NAME)) {
+                    throw new IllegalArgumentException(UNEXPECTED_FIELD_NAME);
+                }
+                VlanId vlanId = VlanId.vlanId((short) jsonNode.get(VLAN).asInt());
+                builder.add(new XConnectStoreKey(deviceId, vlanId));
+            });
+        }
+        return builder.build();
+    }
+
+    /**
+     * Returns ports of given xconnect key.
+     *
+     * @param xconnect xconnect key
+     * @return set of two ports associated with given xconnect key
+     * @throws IllegalArgumentException if wrong format
+     */
+    public Set<PortNumber> getPorts(XConnectStoreKey xconnect) {
+        ImmutableSet.Builder<PortNumber> builder = ImmutableSet.builder();
+        object.get(xconnect.deviceId().toString()).forEach(vlanPortsPair -> {
+            if (xconnect.vlanId().toShort() == vlanPortsPair.get(VLAN).asInt()) {
+                int portCount = vlanPortsPair.get(PORTS).size();
+                checkArgument(portCount == 2,
+                        "Expect 2 ports but found " + portCount + " on " + xconnect);
+                vlanPortsPair.get(PORTS).forEach(portNode -> {
+                    builder.add(PortNumber.portNumber(portNode.asInt()));
+                });
+            }
+        });
+        return builder.build();
+    }
+}

@@ -35,7 +35,6 @@ import org.onlab.packet.MplsLabel;
 import org.onlab.packet.VlanId;
 import org.onlab.util.KryoNamespace;
 import org.onosproject.core.ApplicationId;
-import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Link;
 import org.onosproject.net.PortNumber;
@@ -55,7 +54,6 @@ import org.onosproject.segmentrouting.config.DeviceProperties;
 import org.onosproject.segmentrouting.storekey.NeighborSetNextObjectiveStoreKey;
 import org.onosproject.segmentrouting.storekey.PortNextObjectiveStoreKey;
 import org.onosproject.segmentrouting.storekey.SubnetNextObjectiveStoreKey;
-import org.onosproject.segmentrouting.storekey.XConnectNextObjectiveStoreKey;
 import org.onosproject.store.service.EventuallyConsistentMap;
 import org.slf4j.Logger;
 
@@ -89,8 +87,6 @@ public class DefaultGroupHandler {
             subnetNextObjStore = null;
     protected EventuallyConsistentMap<PortNextObjectiveStoreKey, Integer>
             portNextObjStore = null;
-    protected EventuallyConsistentMap<XConnectNextObjectiveStoreKey, Integer>
-            xConnectNextObjStore = null;
     private SegmentRoutingManager srManager;
 
     protected KryoNamespace.Builder kryo = new KryoNamespace.Builder()
@@ -123,7 +119,6 @@ public class DefaultGroupHandler {
         this.nsNextObjStore = srManager.nsNextObjStore;
         this.subnetNextObjStore = srManager.subnetNextObjStore;
         this.portNextObjStore = srManager.portNextObjStore;
-        this.xConnectNextObjStore = srManager.xConnectNextObjStore;
         this.srManager = srManager;
 
         populateNeighborMaps();
@@ -471,32 +466,6 @@ public class DefaultGroupHandler {
     }
 
     /**
-     * Returns the next objective ID of type broadcast associated with the VLAN
-     * cross-connection.
-     *
-     * @param vlanId VLAN ID for the cross-connection
-     * @return int if found or created, -1 if there are errors during the
-     *         creation of the next objective
-     */
-    public int getXConnectNextObjectiveId(VlanId vlanId) {
-        Integer nextId = xConnectNextObjStore
-                .get(new XConnectNextObjectiveStoreKey(deviceId, vlanId));
-        if (nextId == null) {
-            log.trace("getXConnectNextObjectiveId: Next objective id "
-                    + "not found for device {} and vlan {}. Creating", deviceId, vlanId);
-            createGroupsForXConnect(deviceId);
-            nextId = xConnectNextObjStore.get(
-                    new XConnectNextObjectiveStoreKey(deviceId, vlanId));
-            if (nextId == null) {
-                log.warn("getXConnectNextObjectiveId: Next objective id "
-                        + "not found for device {} and vlan {}.", deviceId, vlanId);
-                return -1;
-            }
-        }
-        return nextId;
-    }
-
-    /**
      * Checks if the next objective ID (group) for the neighbor set exists or not.
      *
      * @param ns neighbor set to check
@@ -743,55 +712,6 @@ public class DefaultGroupHandler {
     }
 
     /**
-     * Creates broadcast groups for VLAN cross-connect ports.
-     *
-     * @param deviceId the DPID of the switch
-     */
-    public void createGroupsForXConnect(DeviceId deviceId) {
-        Map<VlanId, List<ConnectPoint>> xConnectsForDevice = deviceConfig.getXConnects();
-
-        xConnectsForDevice.forEach((vlanId, connectPoints) -> {
-            // Only proceed  the xConnect for given device
-            for (ConnectPoint connectPoint : connectPoints) {
-                if (!connectPoint.deviceId().equals(deviceId)) {
-                    return;
-                }
-            }
-
-            // Check if the next obj is already in the store
-            XConnectNextObjectiveStoreKey key =
-                    new XConnectNextObjectiveStoreKey(deviceId, vlanId);
-            if (xConnectNextObjStore.containsKey(key)) {
-                log.debug("Cross-connect Broadcast group for device {} and vlanId {} exists",
-                        deviceId, vlanId);
-                return;
-            }
-
-            TrafficSelector metadata =
-                    DefaultTrafficSelector.builder().matchVlanId(vlanId).build();
-            int nextId = flowObjectiveService.allocateNextId();
-
-            NextObjective.Builder nextObjBuilder = DefaultNextObjective
-                    .builder().withId(nextId)
-                    .withType(NextObjective.Type.BROADCAST).fromApp(appId)
-                    .withMeta(metadata);
-
-            connectPoints.forEach(connectPoint -> {
-                TrafficTreatment.Builder tBuilder = DefaultTrafficTreatment.builder();
-                tBuilder.setOutput(connectPoint.port());
-                nextObjBuilder.addTreatment(tBuilder.build());
-            });
-
-            NextObjective nextObj = nextObjBuilder.add();
-            flowObjectiveService.next(deviceId, nextObj);
-            log.debug("createGroupsForXConnect: Submited next objective {} in device {}",
-                    nextId, deviceId);
-            xConnectNextObjStore.put(key, nextId);
-        });
-    }
-
-
-    /**
      * Create simple next objective for a single port. The treatments can include
      * all outgoing actions that need to happen on the packet.
      *

@@ -24,7 +24,7 @@ import java.util.Objects;
 /**
  * Key of VLAN cross-connect next objective store.
  */
-public class XConnectNextObjectiveStoreKey {
+public class XConnectStoreKey {
     private final DeviceId deviceId;
     private final VlanId vlanId;
 
@@ -34,7 +34,7 @@ public class XConnectNextObjectiveStoreKey {
      * @param deviceId device ID of the VLAN cross-connection
      * @param vlanId VLAN ID of the VLAN cross-connection
      */
-    public XConnectNextObjectiveStoreKey(DeviceId deviceId, VlanId vlanId) {
+    public XConnectStoreKey(DeviceId deviceId, VlanId vlanId) {
         this.deviceId = deviceId;
         this.vlanId = vlanId;
     }
@@ -62,11 +62,11 @@ public class XConnectNextObjectiveStoreKey {
         if (this == o) {
             return true;
         }
-        if (!(o instanceof XConnectNextObjectiveStoreKey)) {
+        if (!(o instanceof XConnectStoreKey)) {
             return false;
         }
-        XConnectNextObjectiveStoreKey that =
-                (XConnectNextObjectiveStoreKey) o;
+        XConnectStoreKey that =
+                (XConnectStoreKey) o;
         return (Objects.equals(this.deviceId, that.deviceId) &&
                 Objects.equals(this.vlanId, that.vlanId));
     }

@@ -41,9 +41,6 @@ import static org.junit.Assert.*;
  * Tests for class {@link SegmentRoutingAppConfig}.
  */
 public class SegmentRoutingAppConfigTest {
-    private static final ApplicationId APP_ID =
-            new TestApplicationId(SegmentRoutingManager.SR_APP_ID);
-
     private SegmentRoutingAppConfig config;
     private SegmentRoutingAppConfig invalidConfig;
 
@@ -67,12 +64,12 @@ public class SegmentRoutingAppConfigTest {
     @Before
     public void setUp() throws Exception {
         InputStream jsonStream = SegmentRoutingAppConfigTest.class
-                .getResourceAsStream("/sr-app-config.json");
+                .getResourceAsStream("/app.json");
         InputStream invalidJsonStream = SegmentRoutingAppConfigTest.class
-                .getResourceAsStream("/sr-app-config-invalid.json");
+                .getResourceAsStream("/app-invalid.json");
 
-        ApplicationId subject = APP_ID;
         String key = SegmentRoutingManager.SR_APP_ID;
+        ApplicationId subject = new TestApplicationId(key);
         ObjectMapper mapper = new ObjectMapper();
         JsonNode jsonNode = mapper.readTree(jsonStream);
         JsonNode invalidJsonNode = mapper.readTree(invalidJsonStream);

@@ -47,7 +47,7 @@ public class SegmentRoutingDeviceConfigTest {
     @Before
     public void setUp() throws Exception {
         InputStream jsonStream = SegmentRoutingDeviceConfigTest.class
-                .getResourceAsStream("/sr-device-config.json");
+                .getResourceAsStream("/device.json");
 
         adjacencySids1 = new HashMap<>();
         Set<Integer> ports1 = new HashSet<>();

+/*
+ * Copyright 2016-present Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.segmentrouting.config;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.junit.Before;
+import org.junit.Test;
+import org.onlab.packet.VlanId;
+import org.onosproject.TestApplicationId;
+import org.onosproject.core.ApplicationId;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.PortNumber;
+import org.onosproject.net.config.Config;
+import org.onosproject.net.config.ConfigApplyDelegate;
+import org.onosproject.segmentrouting.SegmentRoutingManager;
+import org.onosproject.segmentrouting.storekey.XConnectStoreKey;
+import java.io.InputStream;
+import java.util.Set;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+/**
+ * Tests for class {@link XConnectConfig}.
+ */
+public class XConnectConfigTest {
+    private static final DeviceId DEV1 = DeviceId.deviceId("of:0000000000000001");
+    private static final DeviceId DEV2 = DeviceId.deviceId("of:0000000000000002");
+    private static final VlanId VLAN10 = VlanId.vlanId((short) 10);
+    private static final VlanId VLAN20 = VlanId.vlanId((short) 20);
+    private static final PortNumber PORT3 = PortNumber.portNumber(3);
+    private static final PortNumber PORT4 = PortNumber.portNumber(4);
+    private static final PortNumber PORT5 = PortNumber.portNumber(5);
+    private static final XConnectStoreKey KEY1 = new XConnectStoreKey(DEV1, VLAN10);
+    private static final XConnectStoreKey KEY2 = new XConnectStoreKey(DEV2, VLAN10);
+    private static final XConnectStoreKey KEY3 = new XConnectStoreKey(DEV2, VLAN20);
+    private static final XConnectStoreKey KEY4 = new XConnectStoreKey(DEV2, VlanId.NONE);
+
+    private XConnectConfig config;
+    private XConnectConfig invalidConfig;
+
+    @Before
+    public void setUp() throws Exception {
+        InputStream jsonStream = SegmentRoutingAppConfigTest.class
+                .getResourceAsStream("/xconnect.json");
+        InputStream invalidJsonStream = SegmentRoutingAppConfigTest.class
+                .getResourceAsStream("/xconnect-invalid.json");
+
+        String key = SegmentRoutingManager.SR_APP_ID;
+        ApplicationId subject = new TestApplicationId(key);
+        ObjectMapper mapper = new ObjectMapper();
+        JsonNode jsonNode = mapper.readTree(jsonStream);
+        JsonNode invalidJsonNode = mapper.readTree(invalidJsonStream);
+        ConfigApplyDelegate delegate = new XConnectConfigTest.MockDelegate();
+
+        config = new XConnectConfig();
+        config.init(subject, key, jsonNode, mapper, delegate);
+        invalidConfig = new XConnectConfig();
+        invalidConfig.init(subject, key, invalidJsonNode, mapper, delegate);
+    }
+
+    /**
+     * Tests config validity.
+     */
+    @Test
+    public void testIsValid() {
+        assertTrue(config.isValid());
+        assertFalse(invalidConfig.isValid());
+    }
+
+    /**
+     * Tests getXconnects.
+     */
+    @Test
+    public void testGetXconnects() {
+        Set<XConnectStoreKey> xconnects = config.getXconnects();
+        assertThat(xconnects.size(), is(3));
+        assertTrue(xconnects.contains(KEY1));
+        assertTrue(xconnects.contains(KEY2));
+        assertTrue(xconnects.contains(KEY3));
+        assertFalse(xconnects.contains(KEY4));
+    }
+
+    /**
+     * Tests getPorts.
+     */
+    @Test
+    public void testGetPorts() {
+        Set<PortNumber> ports;
+
+        ports = config.getPorts(KEY1);
+        assertThat(ports.size(), is(2));
+        assertTrue(ports.contains(PORT3));
+        assertTrue(ports.contains(PORT4));
+
+        ports = config.getPorts(KEY2);
+        assertThat(ports.size(), is(2));
+        assertTrue(ports.contains(PORT3));
+        assertTrue(ports.contains(PORT4));
+
+        ports = config.getPorts(KEY3);
+        assertThat(ports.size(), is(2));
+        assertTrue(ports.contains(PORT4));
+        assertTrue(ports.contains(PORT5));
+    }
+
+    private class MockDelegate implements ConfigApplyDelegate {
+        @Override
+        public void onApply(Config config) {
+        }
+    }
+}
\ No newline at end of file

+{
+  "of:0000000000000001": [
+    {
+      "vlan": 10,
+      "ports": [3, 4]
+    }
+  ],
+  "of:0000000000000002": [
+    {
+      "vlan": 10,
+      "ports": [3, 4]
+    },
+    {
+      "vlan": 20,
+      "ports": [4, 5, 6]
+    }
+  ]
+}
\ No newline at end of file

+{
+  "of:0000000000000001": [
+    {
+      "vlan": 10,
+      "ports": [3, 4],
+      "name": "OLT1"
+    }
+  ],
+  "of:0000000000000002": [
+    {
+      "vlan": 10,
+      "ports": [3, 4]
+    },
+    {
+      "vlan": 20,
+      "ports": [4, 5]
+    }
+  ]
+}
\ No newline at end of file

@@ -34,11 +34,16 @@ public enum ObjectiveError {
     FLOWINSTALLATIONFAILED,
 
     /**
-     * THe group installation for this objective failed.
+     * The group installation for this objective failed.
      */
     GROUPINSTALLATIONFAILED,
 
     /**
+     * The group removal for this objective failed.
+     */
+    GROUPREMOVALFAILED,
+
+    /**
      * The group was reported as installed but is missing.
      */
     GROUPMISSING,

@@ -114,7 +114,8 @@ public class Ofdpa2GroupHandler {
 
     protected DeviceId deviceId;
     private FlowObjectiveStore flowObjectiveStore;
-    private Cache<GroupKey, List<OfdpaNextGroup>> pendingNextObjectives;
+    private Cache<GroupKey, List<OfdpaNextGroup>> pendingAddNextObjectives;
+    private Cache<NextObjective, List<GroupKey>> pendingRemoveNextObjectives;
     private ConcurrentHashMap<GroupKey, Set<GroupChainElem>> pendingGroups;
     private ScheduledExecutorService groupChecker =
             Executors.newScheduledThreadPool(2, groupedThreads("onos/pipeliner", "ofdpa2-%d", log));
@@ -134,7 +135,7 @@ public class Ofdpa2GroupHandler {
         this.storageService = serviceDirectory.get(StorageService.class);
         this.nextIndex = storageService.getAtomicCounter("group-id-index-counter");
 
-        pendingNextObjectives = CacheBuilder.newBuilder()
+        pendingAddNextObjectives = CacheBuilder.newBuilder()
                 .expireAfterWrite(20, TimeUnit.SECONDS)
                 .removalListener((
                         RemovalNotification<GroupKey, List<OfdpaNextGroup>> notification) -> {
@@ -142,7 +143,16 @@ public class Ofdpa2GroupHandler {
                         notification.getValue().forEach(ofdpaNextGrp ->
                                 Ofdpa2Pipeline.fail(ofdpaNextGrp.nextObj,
                                         ObjectiveError.GROUPINSTALLATIONFAILED));
+                    }
+                }).build();
 
+        pendingRemoveNextObjectives = CacheBuilder.newBuilder()
+                .expireAfterWrite(20, TimeUnit.SECONDS)
+                .removalListener((
+                        RemovalNotification<NextObjective, List<GroupKey>> notification) -> {
+                    if (notification.getCause() == RemovalCause.EXPIRED) {
+                            Ofdpa2Pipeline.fail(notification.getKey(),
+                                    ObjectiveError.GROUPREMOVALFAILED);
                     }
                 }).build();
         pendingGroups = new ConcurrentHashMap<>();
@@ -1012,6 +1022,11 @@ public class Ofdpa2GroupHandler {
      */
     protected void removeGroup(NextObjective nextObjective, NextGroup next) {
         List<Deque<GroupKey>> allgkeys = Ofdpa2Pipeline.appKryo.deserialize(next.data());
+
+        List<GroupKey> groupKeys = allgkeys.stream()
+                .map(Deque::getFirst).collect(Collectors.toList());
+        pendingRemoveNextObjectives.put(nextObjective, groupKeys);
+
         allgkeys.forEach(groupChain -> groupChain.forEach(groupKey ->
                 groupService.removeGroup(deviceId, groupKey, nextObjective.appId())));
         flowObjectiveStore.removeNextGroup(nextObjective.id());
@@ -1024,7 +1039,7 @@ public class Ofdpa2GroupHandler {
     private void updatePendingNextObjective(GroupKey key, OfdpaNextGroup value) {
         List<OfdpaNextGroup> nextList = new CopyOnWriteArrayList<OfdpaNextGroup>();
         nextList.add(value);
-        List<OfdpaNextGroup> ret = pendingNextObjectives.asMap()
+        List<OfdpaNextGroup> ret = pendingAddNextObjectives.asMap()
                 .putIfAbsent(key, nextList);
         if (ret != null) {
             ret.add(value);
@@ -1079,13 +1094,13 @@ public class Ofdpa2GroupHandler {
             Set<GroupKey> keys = pendingGroups.keySet().stream()
                     .filter(key -> groupService.getGroup(deviceId, key) != null)
                     .collect(Collectors.toSet());
-            Set<GroupKey> otherkeys = pendingNextObjectives.asMap().keySet().stream()
+            Set<GroupKey> otherkeys = pendingAddNextObjectives.asMap().keySet().stream()
                     .filter(otherkey -> groupService.getGroup(deviceId, otherkey) != null)
                     .collect(Collectors.toSet());
             keys.addAll(otherkeys);
 
             keys.stream().forEach(key ->
-                    processPendingGroupsOrNextObjectives(key, false));
+                    processPendingAddGroupsOrNextObjs(key, false));
         }
     }
 
@@ -1093,14 +1108,20 @@ public class Ofdpa2GroupHandler {
         @Override
         public void event(GroupEvent event) {
             log.trace("received group event of type {}", event.type());
-            if (event.type() == GroupEvent.Type.GROUP_ADDED) {
-                GroupKey key = event.subject().appCookie();
-                processPendingGroupsOrNextObjectives(key, true);
+            switch (event.type()) {
+                case GROUP_ADDED:
+                    processPendingAddGroupsOrNextObjs(event.subject().appCookie(), true);
+                    break;
+                case GROUP_REMOVED:
+                    processPendingRemoveNextObjs(event.subject().appCookie());
+                    break;
+                default:
+                    break;
             }
         }
     }
 
-    private void processPendingGroupsOrNextObjectives(GroupKey key, boolean added) {
+    private void processPendingAddGroupsOrNextObjs(GroupKey key, boolean added) {
         //first check for group chain
         Set<GroupChainElem> gceSet = pendingGroups.remove(key);
         if (gceSet != null) {
@@ -1114,9 +1135,9 @@ public class Ofdpa2GroupHandler {
             }
         } else {
             // otherwise chain complete - check for waiting nextObjectives
-            List<OfdpaNextGroup> nextGrpList = pendingNextObjectives.getIfPresent(key);
+            List<OfdpaNextGroup> nextGrpList = pendingAddNextObjectives.getIfPresent(key);
             if (nextGrpList != null) {
-                pendingNextObjectives.invalidate(key);
+                pendingAddNextObjectives.invalidate(key);
                 nextGrpList.forEach(nextGrp -> {
                     log.debug("Group service {} group key {} in device:{}. "
                                     + "Done implementing next objective: {} <<-->> gid:0x{}",
@@ -1137,6 +1158,17 @@ public class Ofdpa2GroupHandler {
         }
     }
 
+    private void processPendingRemoveNextObjs(GroupKey key) {
+        pendingRemoveNextObjectives.asMap().forEach((nextObjective, groupKeys) -> {
+            if (groupKeys.isEmpty()) {
+                pendingRemoveNextObjectives.invalidate(nextObjective);
+                Ofdpa2Pipeline.pass(nextObjective);
+            } else {
+                groupKeys.remove(key);
+            }
+        });
+    }
+
     protected int getNextAvailableIndex() {
         return (int) nextIndex.incrementAndGet();
     }