ONOS-1930 : Tunnel and policy failover with multi-instances

 - Prevents tunnel delete from removing groups used for default flows
 - Removes SegmentRoutingManager reference from Tunnel and Policy class
 - Adds some error checks such as duplicates tunnel IDs or duplicate polices

Change-Id: I0e7d5e2eff0aea6dad13137a872fee58e083b11c

@@ -16,16 +16,8 @@
 
 package org.onosproject.segmentrouting;
 
-import org.onosproject.net.DeviceId;
-import org.onosproject.net.Link;
-import org.onosproject.segmentrouting.grouphandler.NeighborSet;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
+import java.util.Objects;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -34,31 +26,11 @@ import static com.google.common.base.Preconditions.checkNotNull;
  */
 public class DefaultTunnel implements Tunnel {
 
-    private static final Logger log = LoggerFactory
-            .getLogger(DefaultTunnel.class);
-
-    private final String tunnelId;
+    private final String id;
     private final List<Integer> labelIds;
-    private final SegmentRoutingManager srManager;
-    private final DeviceConfiguration config;
 
     private int groupId;
-
-    /**
-     * Creates a Tunnel reference.
-     *
-     * @param srm SegmentRoutingManager object
-     * @param tid  Tunnel ID
-     * @param labelIds Label stack of the tunnel
-     */
-    public DefaultTunnel(SegmentRoutingManager srm, String tid,
-                         List<Integer> labelIds) {
-        this.srManager = checkNotNull(srm);
-        this.tunnelId = checkNotNull(tid);
-        this.labelIds = Collections.unmodifiableList(labelIds);
-        this.config = srManager.deviceConfiguration;
-        this.groupId = -1;
-    }
+    private boolean allowedToRemoveGroup;
 
     /**
      * Creates a Tunnel reference.
@@ -67,10 +39,10 @@ public class DefaultTunnel implements Tunnel {
      * @param labelIds Label stack of the tunnel
      */
     public DefaultTunnel(String tid, List<Integer> labelIds) {
-        this.srManager = null;
-        this.tunnelId = checkNotNull(tid);
-        this.labelIds = Collections.unmodifiableList(labelIds);
-        this.config = null;
+        this.id = checkNotNull(tid);
+        this.labelIds = labelIds;
+        //TODO: need to register the class in Kryo for this
+        //this.labelIds = Collections.unmodifiableList(labelIds);
         this.groupId = -1;
     }
 
@@ -80,16 +52,14 @@ public class DefaultTunnel implements Tunnel {
      * @param tunnel DefaultTunnel reference
      */
     public DefaultTunnel(DefaultTunnel tunnel) {
-        this.srManager = tunnel.srManager;
-        this.tunnelId = tunnel.tunnelId;
+        this.id = tunnel.id;
         this.labelIds = tunnel.labelIds;
-        this.config = tunnel.config;
         this.groupId = tunnel.groupId;
     }
 
     @Override
     public String id() {
-        return this.tunnelId;
+        return this.id;
     }
 
     @Override
@@ -98,71 +68,44 @@ public class DefaultTunnel implements Tunnel {
     }
 
     @Override
-    public boolean create() {
-
-        if (labelIds.isEmpty() || labelIds.size() < 3) {
-            log.error("More than one router needs to specified to created a tunnel");
-            return false;
-        }
-
-        groupId = createGroupsForTunnel();
-        if (groupId < 0) {
-            log.error("Failed to create groups for the tunnel");
-            return false;
-        }
+    public int groupId() {
+        return this.groupId;
+    }
 
-        return true;
+    @Override
+    public void setGroupId(int id) {
+        this.groupId = id;
     }
 
     @Override
-    public boolean remove() {
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
 
-        DeviceId deviceId = config.getDeviceId(labelIds.get(0));
-        srManager.removeNextObjective(deviceId, groupId);
+        if (o instanceof DefaultTunnel) {
+            DefaultTunnel tunnel = (DefaultTunnel) o;
+            // We compare only the tunnel paths.
+            if (tunnel.labelIds.equals(this.labelIds)) {
+                return true;
+            }
+        }
 
-        return true;
+        return false;
     }
 
     @Override
-    public int groupId() {
-        return this.groupId;
+    public int hashCode() {
+        return Objects.hash(labelIds);
     }
 
     @Override
-    public DeviceId source() {
-        return config.getDeviceId(labelIds.get(0));
+    public boolean isAllowedToRemoveGroup() {
+        return this.allowedToRemoveGroup;
     }
 
-    private int createGroupsForTunnel() {
-
-        List<Integer> portNumbers;
-
-        int groupId;
-
-        DeviceId deviceId = config.getDeviceId(labelIds.get(0));
-        if (deviceId == null) {
-            log.warn("No device found for SID {}", labelIds.get(0));
-            return -1;
-        }
-        Set<DeviceId> deviceIds = new HashSet<>();
-        int sid = labelIds.get(1);
-        if (config.isAdjacencySid(deviceId, sid)) {
-            portNumbers = config.getPortsForAdjacencySid(deviceId, sid);
-            for (Link link: srManager.linkService.getDeviceEgressLinks(deviceId)) {
-                for (Integer port: portNumbers) {
-                    if (link.src().port().toLong() == port) {
-                        deviceIds.add(link.dst().deviceId());
-                    }
-                }
-            }
-        } else {
-            deviceIds.add(config.getDeviceId(sid));
-        }
-
-        NeighborSet ns = new NeighborSet(deviceIds, labelIds.get(2));
-        groupId = srManager.getNextObjectiveId(deviceId, ns);
-
-        return groupId;
+    @Override
+    public void allowToRemoveGroup(boolean b) {
+        this.allowedToRemoveGroup = b;
     }
-
 }

@@ -16,8 +16,6 @@
 
 package org.onosproject.segmentrouting;
 
-import org.onosproject.net.flow.TrafficSelector;
-
 /**
  * Interface for Segment Routing Policy.
  */
@@ -47,13 +45,6 @@ public interface Policy {
     String id();
 
     /**
-     * Returns the traffic selector object.
-     *
-     * @return TrafficSelector object
-     */
-    TrafficSelector selector();
-
-    /**
      * Returns the priority of the policy.
      *
      * @return priority
@@ -68,16 +59,38 @@ public interface Policy {
     Type type();
 
     /**
-     * Creates a policy.
+     * Returns the source IP address of the policy.
+     *
+     * @return source IP address
+     */
+    String srcIp();
+
+    /**
+     * Returns the destination IP address of the policy.
      *
-     * @return true if succeeds, false otherwise
+     * @return destination IP address
      */
-    boolean create();
+    String dstIp();
 
     /**
-     * Removes the policy.
+     * Returns the IP protocol of the policy.
      *
-     * @return true if succeeds, false otherwise
+     * @return IP protocol
      */
-    boolean remove();
+    String ipProto();
+
+    /**
+     * Returns the source port of the policy.
+     *
+     * @return source port
+     */
+    short srcPort();
+
+    /**
+     * Returns the destination of the policy.
+     *
+     * @return destination port
+     */
+    short dstPort();
+
 }

@@ -16,10 +16,18 @@
 
 package org.onosproject.segmentrouting;
 
+import org.onlab.packet.Ethernet;
+import org.onlab.packet.IpPrefix;
+import org.onosproject.cli.net.IpProtocol;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.flow.DefaultTrafficSelector;
+import org.onosproject.net.flow.TrafficSelector;
+import org.onosproject.net.flowobjective.DefaultForwardingObjective;
+import org.onosproject.net.flowobjective.ForwardingObjective;
+import org.onosproject.store.service.EventuallyConsistentMap;
 import org.slf4j.Logger;
 
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 
 import static org.slf4j.LoggerFactory.getLogger;
@@ -31,13 +39,17 @@ public class PolicyHandler {
 
     protected final Logger log = getLogger(getClass());
 
-    private final HashMap<String, Policy> policyMap;
+    private final SegmentRoutingManager srManager;
+    private final EventuallyConsistentMap<String, Policy> policyStore;
 
     /**
      * Creates a reference.
+     * @param policyStore
      */
-    public PolicyHandler() {
-        policyMap = new HashMap<>();
+    public PolicyHandler(SegmentRoutingManager srManager,
+                         EventuallyConsistentMap<String, Policy> policyStore) {
+        this.srManager = srManager;
+        this.policyStore = policyStore;
     }
 
     /**
@@ -47,7 +59,7 @@ public class PolicyHandler {
      */
     public List<Policy> getPolicies() {
         List<Policy> policies = new ArrayList<>();
-        policyMap.values().forEach(policy -> policies.add(
+        policyStore.values().forEach(policy -> policies.add(
                 new TunnelPolicy((TunnelPolicy) policy)));
 
         return policies;
@@ -59,8 +71,43 @@ public class PolicyHandler {
      * @param policy policy reference to create
      */
     public void createPolicy(Policy policy) {
-        policy.create();
-        policyMap.put(policy.id(), policy);
+
+        if (policyStore.containsKey(policy.id())) {
+            log.warn("The policy id {} exists already", policy.id());
+            return;
+        }
+
+        if (policyStore.containsValue(policy)) {
+            log.warn("The same policy exists already");
+            return;
+        }
+
+        if (policy.type() == Policy.Type.TUNNEL_FLOW) {
+
+            TunnelPolicy tunnelPolicy = (TunnelPolicy) policy;
+            Tunnel tunnel = srManager.getTunnel(tunnelPolicy.tunnelId());
+            if (tunnel == null) {
+                log.error("Tunnel {} does not exists");
+                return;
+            }
+
+            ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
+                    .builder()
+                    .fromApp(srManager.appId)
+                    .makePermanent()
+                    .nextStep(tunnel.groupId())
+                    .withPriority(tunnelPolicy.priority())
+                    .withSelector(buildSelector(policy))
+                    .withFlag(ForwardingObjective.Flag.VERSATILE);
+
+            DeviceId source = srManager.deviceConfiguration.getDeviceId(tunnel.labelIds().get(0));
+            srManager.flowObjectiveService.forward(source, fwdBuilder.add());
+
+        } else {
+            log.warn("Policy type {} is not supported yet.", policy.type());
+        }
+
+        policyStore.put(policy.id(), policy);
     }
 
     /**
@@ -69,15 +116,64 @@ public class PolicyHandler {
      * @param policyInfo policy information to remove
      */
     public void removePolicy(Policy policyInfo) {
-        if (policyMap.get(policyInfo.id()) != null) {
-            if (policyMap.get(policyInfo.id()).remove()) {
-                policyMap.remove(policyInfo.id());
-            } else {
-                log.error("Failed to remove the policy {}", policyInfo.id());
+
+        if (policyStore.get(policyInfo.id()) != null) {
+            Policy policy = policyStore.get(policyInfo.id());
+            if (policy.type() == Policy.Type.TUNNEL_FLOW) {
+                TunnelPolicy tunnelPolicy = (TunnelPolicy) policy;
+                Tunnel tunnel = srManager.getTunnel(tunnelPolicy.tunnelId());
+
+                ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
+                        .builder()
+                        .fromApp(srManager.appId)
+                        .makePermanent()
+                        .withSelector(buildSelector(policy))
+                        .withPriority(tunnelPolicy.priority())
+                        .nextStep(tunnel.groupId())
+                        .withFlag(ForwardingObjective.Flag.VERSATILE);
+
+                DeviceId source = srManager.deviceConfiguration.getDeviceId(tunnel.labelIds().get(0));
+                srManager.flowObjectiveService.forward(source, fwdBuilder.remove());
+
+                policyStore.remove(policyInfo.id());
             }
         } else {
             log.warn("Policy {} was not found", policyInfo.id());
         }
     }
 
+
+    private TrafficSelector buildSelector(Policy policy) {
+
+        TrafficSelector.Builder tsb = DefaultTrafficSelector.builder();
+        tsb.matchEthType(Ethernet.TYPE_IPV4);
+        if (policy.dstIp() != null && !policy.dstIp().isEmpty()) {
+            tsb.matchIPDst(IpPrefix.valueOf(policy.dstIp()));
+        }
+        if (policy.srcIp() != null && !policy.srcIp().isEmpty()) {
+            tsb.matchIPSrc(IpPrefix.valueOf(policy.srcIp()));
+        }
+        if (policy.ipProto() != null && !policy.ipProto().isEmpty()) {
+            Short ipProto = Short.valueOf(IpProtocol.valueOf(policy.ipProto()).value());
+            tsb.matchIPProtocol(ipProto.byteValue());
+            if (IpProtocol.valueOf(policy.ipProto()).equals(IpProtocol.TCP)) {
+                if (policy.srcPort() != 0) {
+                    tsb.matchTcpSrc(policy.srcPort());
+                }
+                if (policy.dstPort() != 0) {
+                    tsb.matchTcpDst(policy.dstPort());
+                }
+            } else if (IpProtocol.valueOf(policy.ipProto()).equals(IpProtocol.UDP)) {
+                if (policy.srcPort() != 0) {
+                    tsb.matchUdpSrc(policy.srcPort());
+                }
+                if (policy.dstPort() != 0) {
+                    tsb.matchUdpDst(policy.dstPort());
+                }
+            }
+        }
+
+        return tsb.build();
+    }
+
 }

@@ -128,6 +128,9 @@ public class SegmentRoutingManager implements SegmentRoutingService {
     // Per device next objective ID store with (device id + neighbor set) as key
     private EventuallyConsistentMap<NeighborSetNextObjectiveStoreKey,
         Integer> nsNextObjStore = null;
+    private EventuallyConsistentMap<String, Tunnel> tunnelStore = null;
+    private EventuallyConsistentMap<String, Policy> policyStore = null;
+
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected StorageService storageService;
 
@@ -148,13 +151,18 @@ public class SegmentRoutingManager implements SegmentRoutingService {
 
         kryoBuilder = new KryoNamespace.Builder()
             .register(NeighborSetNextObjectiveStoreKey.class,
-                  NeighborSet.class,
-                  DeviceId.class,
-                  URI.class,
-                  WallClockTimestamp.class,
-                  org.onosproject.cluster.NodeId.class,
-                  HashSet.class
-                );
+                    NeighborSet.class,
+                    DeviceId.class,
+                    URI.class,
+                    WallClockTimestamp.class,
+                    org.onosproject.cluster.NodeId.class,
+                    HashSet.class,
+                    Tunnel.class,
+                    DefaultTunnel.class,
+                    Policy.class,
+                    TunnelPolicy.class,
+                    Policy.Type.class
+            );
 
         log.debug("Creating EC map nsnextobjectivestore");
         EventuallyConsistentMapBuilder<NeighborSetNextObjectiveStoreKey, Integer>
@@ -167,6 +175,24 @@ public class SegmentRoutingManager implements SegmentRoutingService {
                 .build();
         log.trace("Current size {}", nsNextObjStore.size());
 
+        EventuallyConsistentMapBuilder<String, Tunnel> tunnelMapBuilder =
+                storageService.eventuallyConsistentMapBuilder();
+
+        tunnelStore = tunnelMapBuilder
+                .withName("tunnelstore")
+                .withSerializer(kryoBuilder)
+                .withClockService(new WallclockClockManager<>())
+                .build();
+
+        EventuallyConsistentMapBuilder<String, Policy> policyMapBuilder =
+                storageService.eventuallyConsistentMapBuilder();
+
+        policyStore = policyMapBuilder
+                .withName("policystore")
+                .withSerializer(kryoBuilder)
+                .withClockService(new WallclockClockManager<>())
+                .build();
+
         networkConfigService.init();
         deviceConfiguration = new DeviceConfiguration(networkConfigService);
         arpHandler = new ArpHandler(this);
@@ -174,8 +200,8 @@ public class SegmentRoutingManager implements SegmentRoutingService {
         ipHandler = new IpHandler(this);
         routingRulePopulator = new RoutingRulePopulator(this);
         defaultRoutingHandler = new DefaultRoutingHandler(this);
-        tunnelHandler = new TunnelHandler();
-        policyHandler = new PolicyHandler();
+        tunnelHandler = new TunnelHandler(this, tunnelStore);
+        policyHandler = new PolicyHandler(this, policyStore);
 
         packetService.addProcessor(processor, PacketProcessor.ADVISOR_MAX + 2);
         linkService.addListener(new InternalLinkListener());
@@ -295,6 +321,24 @@ public class SegmentRoutingManager implements SegmentRoutingService {
     }
 
     /**
+     * Checks if the next objective ID (group) for the neighbor set exists or not in the device.
+     *
+     * @param deviceId Device ID to check
+     * @param ns neighbor set to check
+     * @return true if it exists, false otherwise
+     */
+    public boolean hasNextObjectiveId(DeviceId deviceId, NeighborSet ns) {
+        if (groupHandlerMap.get(deviceId) != null) {
+            log.trace("getNextObjectiveId query in device {}", deviceId);
+            return groupHandlerMap
+                    .get(deviceId).hasNextObjectiveId(ns);
+        } else {
+            log.warn("getNextObjectiveId query in device {} not found", deviceId);
+            return false;
+        }
+    }
+
+    /**
      * Removes the next objective ID.
      *
      * @param deviceId Device ID

@@ -16,8 +16,6 @@
 
 package org.onosproject.segmentrouting;
 
-import org.onosproject.net.DeviceId;
-
 import java.util.List;
 
 /**
@@ -40,30 +38,29 @@ public interface Tunnel {
     List<Integer> labelIds();
 
     /**
-     * Creates a tunnel.
+     * Returns the group ID for the tunnel.
      *
-     * @return true if succeeds, false otherwise
+     * @return group ID
      */
-    boolean create();
+    int groupId();
 
     /**
-     * Removes the tunnel.
+     * Sets group ID for the tunnel.
      *
-     * @return true if succeeds, false otherwise.
      */
-    boolean remove();
+    void setGroupId(int groupId);
 
     /**
-     * Returns the group ID for the tunnel.
+     * Sets the flag to allow to remove the group or not.
      *
-     * @return group ID
+     * @param ok the flag; true - allow to remove
      */
-    int groupId();
+    void allowToRemoveGroup(boolean ok);
 
     /**
-     * Returns the source device Id of the tunnel.
+     * Checks if it is allowed to remove the group for the tunnel.
      *
-     * @return source device Id
+     * @return true if allowed, false otherwise
      */
-    DeviceId source();
+    boolean isAllowedToRemoveGroup();
 }

@@ -15,12 +15,18 @@
  */
 package org.onosproject.segmentrouting;
 
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.Link;
+import org.onosproject.segmentrouting.grouphandler.NeighborSet;
+import org.onosproject.store.service.EventuallyConsistentMap;
 import org.slf4j.Logger;
 
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
-import java.util.HashMap;
+import java.util.Set;
 
+import static com.google.common.base.Preconditions.checkNotNull;
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
@@ -29,10 +35,15 @@ import static org.slf4j.LoggerFactory.getLogger;
 public class TunnelHandler {
     protected final Logger log = getLogger(getClass());
 
-    private final HashMap<String, Tunnel> tunnelMap;
+    private final SegmentRoutingManager srManager;
+    private final DeviceConfiguration config;
+    private final EventuallyConsistentMap<String, Tunnel> tunnelStore;
 
-    public TunnelHandler() {
-        tunnelMap = new HashMap<>();
+    public TunnelHandler(SegmentRoutingManager srm,
+                         EventuallyConsistentMap<String, Tunnel> tunnelStore) {
+        this.srManager = checkNotNull(srm);
+        this.config = srm.deviceConfiguration;
+        this.tunnelStore = tunnelStore;
     }
 
     /**
@@ -40,9 +51,33 @@ public class TunnelHandler {
      *
      * @param tunnel tunnel reference to create a tunnel
      */
-    public void createTunnel(Tunnel tunnel) {
-        tunnel.create();
-        tunnelMap.put(tunnel.id(), tunnel);
+    public boolean createTunnel(Tunnel tunnel) {
+
+        if (tunnel.labelIds().isEmpty() || tunnel.labelIds().size() < 3) {
+            log.error("More than one router needs to specified to created a tunnel");
+            return false;
+        }
+
+        if (tunnelStore.containsKey(tunnel.id())) {
+            log.warn("The same tunnel ID exists already");
+            return false;
+        }
+
+        if (tunnelStore.containsValue(tunnel)) {
+            log.warn("The same tunnel exists already");
+            return false;
+        }
+
+        int groupId = createGroupsForTunnel(tunnel);
+        if (groupId < 0) {
+            log.error("Failed to create groups for the tunnel");
+            return false;
+        }
+
+        tunnel.setGroupId(groupId);
+        tunnelStore.put(tunnel.id(), tunnel);
+
+        return true;
     }
 
     /**
@@ -52,10 +87,19 @@ public class TunnelHandler {
      */
     public void removeTunnel(Tunnel tunnelInfo) {
 
-        Tunnel tunnel = tunnelMap.get(tunnelInfo.id());
+        Tunnel tunnel = tunnelStore.get(tunnelInfo.id());
         if (tunnel != null) {
-            tunnel.remove();
-            tunnelMap.remove(tunnel.id());
+            DeviceId deviceId = config.getDeviceId(tunnel.labelIds().get(0));
+            if (tunnel.isAllowedToRemoveGroup()) {
+                if (srManager.removeNextObjective(deviceId, tunnel.groupId())) {
+                    tunnelStore.remove(tunnel.id());
+                } else {
+                    log.error("Failed to remove the tunnel {}", tunnelInfo.id());
+                }
+            } else {
+                log.debug("The group is not removed because it is being used.");
+                tunnelStore.remove(tunnel.id());
+            }
         } else {
             log.warn("No tunnel found for tunnel ID {}", tunnelInfo.id());
         }
@@ -68,7 +112,7 @@ public class TunnelHandler {
      * @return Tunnel reference
      */
     public Tunnel getTunnel(String tid) {
-        return tunnelMap.get(tid);
+        return tunnelStore.get(tid);
     }
 
     /**
@@ -78,9 +122,50 @@ public class TunnelHandler {
      */
     public List<Tunnel> getTunnels() {
         List<Tunnel> tunnels = new ArrayList<>();
-        tunnelMap.values().forEach(tunnel -> tunnels.add(
+        tunnelStore.values().forEach(tunnel -> tunnels.add(
                 new DefaultTunnel((DefaultTunnel) tunnel)));
 
         return tunnels;
     }
+
+    private int createGroupsForTunnel(Tunnel tunnel) {
+
+        List<Integer> portNumbers;
+
+        int groupId;
+
+        DeviceId deviceId = config.getDeviceId(tunnel.labelIds().get(0));
+        if (deviceId == null) {
+            log.warn("No device found for SID {}", tunnel.labelIds().get(0));
+            return -1;
+        }
+        Set<DeviceId> deviceIds = new HashSet<>();
+        int sid = tunnel.labelIds().get(1);
+        if (config.isAdjacencySid(deviceId, sid)) {
+            portNumbers = config.getPortsForAdjacencySid(deviceId, sid);
+            for (Link link: srManager.linkService.getDeviceEgressLinks(deviceId)) {
+                for (Integer port: portNumbers) {
+                    if (link.src().port().toLong() == port) {
+                        deviceIds.add(link.dst().deviceId());
+                    }
+                }
+            }
+        } else {
+            deviceIds.add(config.getDeviceId(sid));
+        }
+
+        NeighborSet ns = new NeighborSet(deviceIds, tunnel.labelIds().get(2));
+
+        // If the tunnel reuses any existing groups, then tunnel handler
+        // should not remove the group.
+        if (srManager.hasNextObjectiveId(deviceId, ns)) {
+            tunnel.allowToRemoveGroup(false);
+        } else {
+            tunnel.allowToRemoveGroup(true);
+        }
+        groupId = srManager.getNextObjectiveId(deviceId, ns);
+
+        return groupId;
+    }
+
 }

@@ -16,9 +16,7 @@
 
 package org.onosproject.segmentrouting;
 
-import org.onosproject.net.flow.TrafficSelector;
-import org.onosproject.net.flowobjective.DefaultForwardingObjective;
-import org.onosproject.net.flowobjective.ForwardingObjective;
+import java.util.Objects;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
@@ -27,53 +25,45 @@ import static com.google.common.base.Preconditions.checkNotNull;
  */
 public final class TunnelPolicy implements Policy {
 
-    // FIXME: We should avoid passing around references to implementation objects
-    // Instead, if some operational context is required, we should abstract it to
-    // a bare minimum.
-    private final SegmentRoutingManager srManager;
     private final Type type;
     private final String id;
-    private final TrafficSelector selector;
     private final int priority;
     private final String tunnelId;
-
-    private TunnelPolicy(SegmentRoutingManager srm, String policyId, Type type,
-                         TrafficSelector selector, int priority, String tunnelId) {
-        this.srManager = srm;
+    private String dstIp;
+    private String srcIp;
+    private String ipProto;
+    private short srcPort;
+    private short dstPort;
+
+    private TunnelPolicy(String policyId, Type type, int priority, String tunnelId, String srcIp,
+                         String dstIp, String ipProto, short srcPort, short dstPort) {
         this.id = checkNotNull(policyId);
         this.type = type;
         this.tunnelId = tunnelId;
         this.priority = priority;
-        this.selector = selector;
-    }
+        this.dstIp = dstIp;
+        this.srcIp = srcIp;
+        this.ipProto = ipProto;
+        this.srcPort = srcPort;
+        this.dstPort = dstPort;
 
-    /**
-     * Creates a TunnelPolicy reference.
-     *
-     * @param p TunnelPolicy reference
-     */
-    public TunnelPolicy(TunnelPolicy p) {
-        this.srManager = p.srManager;
-        this.id = p.id;
-        this.type = p.type;
-        this.tunnelId = p.tunnelId;
-        this.priority = p.priority;
-        this.selector = p.selector;
     }
 
     /**
      * Creates a TunnelPolicy reference.
      *
-     * @param srm reference to the segment routing component
      * @param p TunnelPolicy reference
      */
-    public TunnelPolicy(SegmentRoutingManager srm, TunnelPolicy p) {
-        this.srManager = srm;
+    public TunnelPolicy(TunnelPolicy p) {
         this.id = p.id;
         this.type = p.type;
         this.tunnelId = p.tunnelId;
         this.priority = p.priority;
-        this.selector = p.selector;
+        this.srcIp = p.srcIp;
+        this.dstIp = p.dstIp;
+        this.ipProto = p.ipProto;
+        this.srcPort = p.srcPort;
+        this.dstPort = p.dstPort;
     }
 
     /**
@@ -91,11 +81,6 @@ public final class TunnelPolicy implements Policy {
     }
 
     @Override
-    public TrafficSelector selector() {
-        return selector;
-    }
-
-    @Override
     public int priority() {
         return priority;
     }
@@ -106,41 +91,58 @@ public final class TunnelPolicy implements Policy {
     }
 
     @Override
-    public boolean create() {
-
-        Tunnel tunnel = srManager.getTunnel(tunnelId);
+    public String srcIp() {
+        return srcIp;
+    }
 
-        ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
-                .builder()
-                .fromApp(srManager.appId)
-                .makePermanent()
-                .nextStep(tunnel.groupId())
-                .withPriority(priority)
-                .withSelector(selector)
-                .withFlag(ForwardingObjective.Flag.VERSATILE);
+    @Override
+    public String dstIp() {
+        return dstIp;
+    }
 
-        srManager.flowObjectiveService.forward(tunnel.source(), fwdBuilder.add());
+    @Override
+    public String ipProto() {
+        return ipProto;
+    }
 
-        return true;
+    @Override
+    public short srcPort() {
+        return srcPort;
     }
 
     @Override
-    public boolean remove() {
+    public short dstPort() {
+        return dstPort;
+    }
 
-        Tunnel tunnel = srManager.getTunnel(tunnelId);
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
 
-        ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
-                .builder()
-                .fromApp(srManager.appId)
-                .makePermanent()
-                .withSelector(selector)
-                .withPriority(priority)
-                .nextStep(tunnel.groupId())
-                .withFlag(ForwardingObjective.Flag.VERSATILE);
+        if (o instanceof TunnelPolicy) {
+            TunnelPolicy that = (TunnelPolicy) o;
+            // We do not compare the policy ID
+            if (this.type.equals(that.type) &&
+                    this.tunnelId.equals(that.tunnelId) &&
+                    this.priority == that.priority &&
+                    this.srcIp.equals(that.srcIp) &&
+                    this.dstIp.equals(that.dstIp) &&
+                    this.srcPort == that.srcPort &&
+                    this.dstPort == that.dstPort &&
+                    this.ipProto.equals(that.ipProto)) {
+                return true;
+            }
+        }
 
-        srManager.flowObjectiveService.forward(tunnel.source(), fwdBuilder.remove());
+        return false;
+    }
 
-        return true;
+    @Override
+    public int hashCode() {
+        return Objects.hash(type, tunnelId, srcIp, dstIp, ipProto,
+                srcPort, dstPort, priority);
     }
 
     /**
@@ -152,17 +154,21 @@ public final class TunnelPolicy implements Policy {
         return this.tunnelId;
     }
 
+
     /**
      * Tunnel Policy Builder.
      */
     public static final class Builder {
 
-        private SegmentRoutingManager srManager;
         private String id;
         private Type type;
-        private TrafficSelector selector;
         private int priority;
         private String tunnelId;
+        private String dstIp;
+        private String srcIp;
+        private String ipProto;
+        private short srcPort;
+        private short dstPort;
 
         /**
          * Sets the policy Id.
@@ -189,49 +195,85 @@ public final class TunnelPolicy implements Policy {
         }
 
         /**
-         * Sets the TrafficSelector.
+         * Sets the source IP address.
          *
-         * @param selector TrafficSelector
+         * @param srcIp source IP address
          * @return Builder object
          */
-        public Builder setSelector(TrafficSelector selector) {
-            this.selector = selector;
+        public Builder setSrcIp(String srcIp) {
+            this.srcIp = srcIp;
 
             return this;
         }
 
         /**
-         * Sets the priority of the policy.
+         * Sets the destination IP address.
          *
-         * @param p priority
+         * @param dstIp destination IP address
          * @return Builder object
          */
-        public Builder setPriority(int p) {
-            this.priority = p;
+        public Builder setDstIp(String dstIp) {
+            this.dstIp = dstIp;
 
             return this;
         }
 
         /**
-         * Sets the tunnel Id.
+         * Sets the IP protocol.
          *
-         * @param tunnelId tunnel Id
+         * @param proto IP protocol
          * @return Builder object
          */
-        public Builder setTunnelId(String tunnelId) {
-            this.tunnelId = tunnelId;
+        public Builder setIpProto(String proto) {
+            this.ipProto = proto;
+
+            return this;
+        }
+
+        /**
+         * Sets the source port.
+         *
+         * @param srcPort source port
+         * @return Builder object
+         */
+        public Builder setSrcPort(short srcPort) {
+            this.srcPort = srcPort;
 
             return this;
         }
 
         /**
-         * Sets the Segment Routing Manager reference.
+         * Sets the destination port.
          *
-         * @param srm Segment Routing Manager reference
+         * @param dstPort destination port
          * @return Builder object
          */
-        public Builder setManager(SegmentRoutingManager srm) {
-            this.srManager = srm;
+        public Builder setDstPort(short dstPort) {
+            this.dstPort = dstPort;
+
+            return this;
+        }
+
+        /**
+         * Sets the priority of the policy.
+         *
+         * @param p priority
+         * @return Builder object
+         */
+        public Builder setPriority(int p) {
+            this.priority = p;
+
+            return this;
+        }
+
+        /**
+         * Sets the tunnel Id.
+         *
+         * @param tunnelId tunnel Id
+         * @return Builder object
+         */
+        public Builder setTunnelId(String tunnelId) {
+            this.tunnelId = tunnelId;
 
             return this;
         }
@@ -242,7 +284,8 @@ public final class TunnelPolicy implements Policy {
          * @return Tunnel Policy reference
          */
         public Policy build() {
-            return new TunnelPolicy(srManager, id, type, selector, priority, tunnelId);
+            return new TunnelPolicy(id, type, priority, tunnelId, srcIp, dstIp,
+                    ipProto, srcPort, dstPort);
         }
     }
 }

@@ -181,8 +181,8 @@ public class DefaultGroupHandler {
                         .contains(newLink.dst().deviceId())))
                 .collect(Collectors.toSet());
         log.trace("linkUp: nsNextObjStore contents for device {}:",
-                  deviceId,
-                  nsSet);
+                deviceId,
+                nsSet);
         for (NeighborSet ns : nsSet) {
             // Create the new bucket to be updated
             TrafficTreatment.Builder tBuilder =
@@ -317,6 +317,22 @@ public class DefaultGroupHandler {
         return nextId.intValue();
     }
 
+    /**
+     * Checks if the next objective ID (group) for the neighbor set exists or not.
+     *
+     * @param ns neighbor set to check
+     * @return true if it exists, false otherwise
+     */
+    public boolean hasNextObjectiveId(NeighborSet ns) {
+        Integer nextId = nsNextObjStore.
+                get(new NeighborSetNextObjectiveStoreKey(deviceId, ns));
+        if (nextId == null) {
+            return false;
+        }
+
+        return true;
+    }
+
     // Empty implementation
     protected void newNeighbor(Link newLink) {
     }
@@ -489,6 +505,7 @@ public class DefaultGroupHandler {
                     break;
                 }
             }
+            return true;
         }
 
         return false;

@@ -15,19 +15,9 @@
  */
 package org.onosproject.segmentrouting.web;
 
-import org.onlab.packet.Ethernet;
-import org.onlab.packet.IpPrefix;
-import org.onosproject.cli.net.IpProtocol;
 import org.onosproject.codec.CodecContext;
 import org.onosproject.codec.JsonCodec;
 import com.fasterxml.jackson.databind.node.ObjectNode;
-import org.onosproject.net.flow.DefaultTrafficSelector;
-import org.onosproject.net.flow.TrafficSelector;
-import org.onosproject.net.flow.criteria.Criterion;
-import org.onosproject.net.flow.criteria.IPCriterion;
-import org.onosproject.net.flow.criteria.IPProtocolCriterion;
-import org.onosproject.net.flow.criteria.TcpPortCriterion;
-import org.onosproject.net.flow.criteria.UdpPortCriterion;
 import org.onosproject.segmentrouting.Policy;
 import org.onosproject.segmentrouting.TunnelPolicy;
 
@@ -52,45 +42,24 @@ public final class PolicyCodec extends JsonCodec<Policy> {
         result.put(PRIORITY, policy.priority());
         result.put(TYPE, policy.type().toString());
 
-        if (policy.selector().getCriterion(Criterion.Type.IPV4_DST) != null) {
-            IPCriterion criterion = (IPCriterion) policy.selector().getCriterion(
-                    Criterion.Type.IPV4_DST);
-            result.put(DST_IP, criterion.ip().toString());
+        if (policy.dstIp() != null) {
+            result.put(DST_IP, policy.dstIp());
         }
-        if (policy.selector().getCriterion(Criterion.Type.IPV4_SRC) != null) {
-            IPCriterion criterion = (IPCriterion) policy.selector().getCriterion(
-                    Criterion.Type.IPV4_SRC);
-            result.put(SRC_IP, criterion.ip().toString());
+        if (policy.srcIp() != null) {
+            result.put(SRC_IP, policy.srcIp());
         }
-        if (policy.selector().getCriterion(Criterion.Type.IP_PROTO) != null) {
-            IPProtocolCriterion protocolCriterion =
-                    (IPProtocolCriterion) policy.selector().getCriterion(Criterion.Type.IP_PROTO);
-            result.put(PROTO_TYPE, protocolCriterion.protocol());
+        if (policy.ipProto() != null) {
+            result.put(PROTO_TYPE, policy.ipProto());
         }
-        if (policy.selector().getCriterion(Criterion.Type.TCP_SRC) != null) {
-            TcpPortCriterion tcpPortCriterion =
-                    (TcpPortCriterion) policy.selector().getCriterion(Criterion.Type.TCP_SRC);
-            result.put(SRC_PORT, tcpPortCriterion.toString());
-        } else if (policy.selector().getCriterion(Criterion.Type.UDP_SRC) != null) {
-            UdpPortCriterion udpPortCriterion =
-                    (UdpPortCriterion) policy.selector().getCriterion(Criterion.Type.UDP_SRC);
-            result.put(SRC_PORT, udpPortCriterion.toString());
-        }
-        if (policy.selector().getCriterion(Criterion.Type.TCP_DST) != null) {
-            TcpPortCriterion tcpPortCriterion =
-                    (TcpPortCriterion) policy.selector().getCriterion(Criterion.Type.TCP_DST);
-            result.put(DST_PORT, tcpPortCriterion.toString());
-        } else if (policy.selector().getCriterion(Criterion.Type.UDP_DST) != null) {
-            UdpPortCriterion udpPortCriterion =
-                    (UdpPortCriterion) policy.selector().getCriterion(Criterion.Type.UDP_DST);
-            result.put(DST_PORT, udpPortCriterion.toString());
+
+        int srcPort = policy.srcPort() & 0xffff;
+        if (policy.srcPort() != 0) {
+            result.put(SRC_PORT, srcPort);
         }
-        if (policy.selector().getCriterion(Criterion.Type.IP_PROTO) != null) {
-            IPProtocolCriterion protocolCriterion =
-                    (IPProtocolCriterion) policy.selector().getCriterion(Criterion.Type.IP_PROTO);
-            result.put(PROTO_TYPE, protocolCriterion.toString());
+        int dstPort = policy.dstPort() & 0xffff;
+        if (policy.dstPort() != 0) {
+            result.put(DST_PORT, dstPort);
         }
-
         if (policy.type() == Policy.Type.TUNNEL_FLOW) {
             result.put(TUNNEL_ID, ((TunnelPolicy) policy).tunnelId());
         }
@@ -111,53 +80,43 @@ public final class PolicyCodec extends JsonCodec<Policy> {
         short srcPort = json.path(SRC_PORT).shortValue();
         short dstPort = json.path(DST_PORT).shortValue();
 
-        if (tunnelId != null) {
-            TrafficSelector.Builder tsb = DefaultTrafficSelector.builder();
-            tsb.matchEthType(Ethernet.TYPE_IPV4);
-            if (dstIp != null && !dstIp.isEmpty()) {
-                tsb.matchIPDst(IpPrefix.valueOf(dstIp));
+        if (json.path(POLICY_ID).isMissingNode() || pid == null) {
+            // TODO: handle errors
+            return null;
+        }
+
+        TunnelPolicy.Builder tpb = TunnelPolicy.builder().setPolicyId(pid);
+        if (!json.path(TYPE).isMissingNode() && type != null &&
+                Policy.Type.valueOf(type).equals(Policy.Type.TUNNEL_FLOW)) {
+
+            if (json.path(TUNNEL_ID).isMissingNode() || tunnelId == null) {
+                return null;
             }
-            if (srcIp != null && !srcIp.isEmpty()) {
-                tsb.matchIPSrc(IpPrefix.valueOf(srcIp));
+
+            tpb.setTunnelId(tunnelId);
+            tpb.setType(Policy.Type.valueOf(type));
+
+            if (!json.path(PRIORITY).isMissingNode()) {
+                tpb.setPriority(priority);
             }
-            if (protoType != null && !protoType.isEmpty()) {
-                Short ipProto = Short.valueOf(IpProtocol.valueOf(protoType).value());
-                tsb.matchIPProtocol(ipProto.byteValue());
-                if (IpProtocol.valueOf(protoType).equals(IpProtocol.TCP)) {
-                    if (srcPort != 0) {
-                        tsb.matchTcpSrc(srcPort);
-                    }
-                    if (dstPort != 0) {
-                        tsb.matchTcpDst(dstPort);
-                    }
-                } else if (IpProtocol.valueOf(protoType).equals(IpProtocol.UDP)) {
-                    if (srcPort != 0) {
-                        tsb.matchUdpSrc(srcPort);
-                    }
-                    if (dstPort != 0) {
-                        tsb.matchUdpDst(dstPort);
-                    }
-                }
+            if (dstIp != null) {
+                tpb.setDstIp(dstIp);
             }
-            TunnelPolicy.Builder tpb = TunnelPolicy.builder().setPolicyId(pid);
-            if (tunnelId != null) {
-                tpb.setTunnelId(tunnelId);
+            if (srcIp != null) {
+                tpb.setSrcIp(srcIp);
             }
-            if (!json.path(PRIORITY).isMissingNode()) {
-                tpb.setPriority(priority);
+            if (protoType != null) {
+                tpb.setIpProto(protoType);
             }
-            if (!json.path(TYPE).isMissingNode()) {
-                tpb.setType(Policy.Type.valueOf(type));
+            if (dstPort != 0) {
+                tpb.setDstPort(dstPort);
+            }
+            if (srcPort != 0) {
+                tpb.setSrcPort(srcPort);
             }
-            tpb.setSelector(tsb.build());
-
-            return tpb.build();
-        } else {
-            // TODO: handle more policy types
-            return null;
         }
 
-
+        return tpb.build();
     }
 
 }

@@ -20,9 +20,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode;
 
 import org.onosproject.rest.AbstractWebResource;
 import org.onosproject.segmentrouting.Policy;
-import org.onosproject.segmentrouting.SegmentRoutingManager;
 import org.onosproject.segmentrouting.SegmentRoutingService;
-import org.onosproject.segmentrouting.TunnelPolicy;
 
 import javax.ws.rs.Consumes;
 import javax.ws.rs.DELETE;
@@ -59,10 +57,9 @@ public class PolicyWebResource extends AbstractWebResource {
         ObjectNode policyJson = (ObjectNode) mapper.readTree(input);
         SegmentRoutingService srService = get(SegmentRoutingService.class);
         Policy policyInfo = POLICY_CODEC.decode(policyJson, this);
-        if (policyInfo.type() == Policy.Type.TUNNEL_FLOW) {
-            TunnelPolicy policy = new TunnelPolicy((SegmentRoutingManager) srService, (TunnelPolicy) policyInfo);
-            srService.createPolicy(policy);
 
+        if (policyInfo.type() == Policy.Type.TUNNEL_FLOW) {
+            srService.createPolicy(policyInfo);
             return Response.ok().build();
         } else {
             return Response.serverError().build();
@@ -78,6 +75,7 @@ public class PolicyWebResource extends AbstractWebResource {
         Policy policyInfo = POLICY_CODEC.decode(policyJson, this);
         // TODO: Check the result
         srService.removePolicy(policyInfo);
+
         return Response.ok().build();
 
     }

@@ -19,8 +19,6 @@ import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 
 import org.onosproject.rest.AbstractWebResource;
-import org.onosproject.segmentrouting.DefaultTunnel;
-import org.onosproject.segmentrouting.SegmentRoutingManager;
 import org.onosproject.segmentrouting.SegmentRoutingService;
 import org.onosproject.segmentrouting.Tunnel;
 
@@ -59,9 +57,7 @@ public class TunnelWebResource extends AbstractWebResource {
         ObjectNode tunnelJson = (ObjectNode) mapper.readTree(input);
         SegmentRoutingService srService = get(SegmentRoutingService.class);
         Tunnel tunnelInfo = TUNNEL_CODEC.decode(tunnelJson, this);
-        Tunnel tunnel = new DefaultTunnel((SegmentRoutingManager) srService,
-                tunnelInfo.id(), tunnelInfo.labelIds());
-        srService.createTunnel(tunnel);
+        srService.createTunnel(tunnelInfo);
 
         return Response.ok().build();
     }