- Adds error handling for Segment routing CLI

- Removes SegmentRoutingManager reference from TunnelHandler and PolicyHandler Change-Id: Iab6acdc489d41a63ebf6b37b65d0e82448a8b25a

- Adds error handling for Segment routing CLI
- Removes SegmentRoutingManager reference from TunnelHandler and PolicyHandler Change-Id: Iab6acdc489d41a63ebf6b37b65d0e82448a8b25a
sangho · Gerrit Code Review
Commit 71abe1ba2560e18950e7267f3cb4f554516957f2 71abe1ba 1 parent 38c7eda6
Showing 8 changed files with 195 additions and 102 deletions
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/PolicyHandler.java
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/PolicyAddCommand.java
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/PolicyRemoveCommand.java
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/TunnelAddCommand.java
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/TunnelRemoveCommand.java
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/PolicyHandler.java
View file @71abe1b
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/PolicyHandler.java
View file @71abe1b
@@ -19,10 +19,12 @@ package org.onosproject.segmentrouting;
 import org.onlab.packet.Ethernet;
 import org.onlab.packet.IpPrefix;
 import org.onosproject.cli.net.IpProtocol;
+import org.onosproject.core.ApplicationId;
 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.FlowObjectiveService;
 import org.onosproject.net.flowobjective.ForwardingObjective;
 import org.onosproject.store.service.EventuallyConsistentMap;
 import org.slf4j.Logger;
@@ -39,19 +41,38 @@ public class PolicyHandler {
     protected final Logger log = getLogger(getClass());
-    // FIXME: references to manager components should be avoided
+    private ApplicationId appId;
-    private final SegmentRoutingManager srManager;
+    private DeviceConfiguration deviceConfiguration;
+    private FlowObjectiveService flowObjectiveService;
+    private TunnelHandler tunnelHandler;
     private final EventuallyConsistentMap<String, Policy> policyStore;
+    public enum Result {
+        SUCCESS,
+        POLICY_EXISTS,
+        ID_EXISTS,
+        TUNNEL_NOT_FOUND,
+        POLICY_NOT_FOUND,
+        UNSUPPORTED_TYPE
+    }
+
     /**
      * Creates a reference.
      *
-     * @param srManager segment routing manager
+     * @param appId segment routing application ID
+     * @param deviceConfiguration DeviceConfiguration reference
+     * @param flowObjectiveService FlowObjectiveService reference
      * @param policyStore policy store
      */
-    public PolicyHandler(SegmentRoutingManager srManager,
+    public PolicyHandler(ApplicationId appId,
+                         DeviceConfiguration deviceConfiguration,
+                         FlowObjectiveService flowObjectiveService,
+                         TunnelHandler tunnelHandler,
                          EventuallyConsistentMap<String, Policy> policyStore) {
-        this.srManager = srManager;
+        this.appId = appId;
+        this.deviceConfiguration = deviceConfiguration;
+        this.flowObjectiveService = flowObjectiveService;
+        this.tunnelHandler = tunnelHandler;
         this.policyStore = policyStore;
     }
@@ -70,79 +91,89 @@ public class PolicyHandler {
     /**
      * Creates a policy using the policy information given.
-     *
      *  @param policy policy reference to create
+     *  @return ID_EXISTS if the same policy ID exists,
+     *  POLICY_EXISTS if the same policy exists, TUNNEL_NOT_FOUND if the tunnel
+     *  does not exists, UNSUPPORTED_TYPE if the policy type is not supported,
+     *  SUCCESS if the policy is created successfully
      */
-    public void createPolicy(Policy policy) {
+    public Result createPolicy(Policy policy) {
         if (policyStore.containsKey(policy.id())) {
             log.warn("The policy id {} exists already", policy.id());
-            return;
+            return Result.ID_EXISTS;
         }
         if (policyStore.containsValue(policy)) {
             log.warn("The same policy exists already");
-            return;
+            return Result.POLICY_EXISTS;
         }
         if (policy.type() == Policy.Type.TUNNEL_FLOW) {
             TunnelPolicy tunnelPolicy = (TunnelPolicy) policy;
-            Tunnel tunnel = srManager.getTunnel(tunnelPolicy.tunnelId());
+            Tunnel tunnel = tunnelHandler.getTunnel(tunnelPolicy.tunnelId());
             if (tunnel == null) {
-                log.error("Tunnel {} does not exists");
+                return Result.TUNNEL_NOT_FOUND;
-                return;
             }
             ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
                     .builder()
-                    .fromApp(srManager.appId)
+                    .fromApp(appId)
                     .makePermanent()
                     .nextStep(tunnel.groupId())
                     .withPriority(tunnelPolicy.priority())
                     .withSelector(buildSelector(policy))
                     .withFlag(ForwardingObjective.Flag.VERSATILE);
-            DeviceId source = srManager.deviceConfiguration.getDeviceId(tunnel.labelIds().get(0));
+            DeviceId source = deviceConfiguration.getDeviceId(tunnel.labelIds().get(0));
-            srManager.flowObjectiveService.forward(source, fwdBuilder.add());
+            flowObjectiveService.forward(source, fwdBuilder.add());
         } else {
             log.warn("Policy type {} is not supported yet.", policy.type());
+            return Result.UNSUPPORTED_TYPE;
         }
         policyStore.put(policy.id(), policy);
+
+        return Result.SUCCESS;
     }
     /**
      * Removes the policy given.
      *
      * @param policyInfo policy information to remove
+     * @return POLICY_NOT_FOUND if the policy to remove does not exists,
+     * SUCCESS if it is removed successfully
      */
-    public void removePolicy(Policy policyInfo) {
+    public Result removePolicy(Policy policyInfo) {
         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());
+                Tunnel tunnel = tunnelHandler.getTunnel(tunnelPolicy.tunnelId());
                 ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
                         .builder()
-                        .fromApp(srManager.appId)
+                        .fromApp(appId)
                         .makePermanent()
                         .withSelector(buildSelector(policy))
                         .withPriority(tunnelPolicy.priority())
                         .nextStep(tunnel.groupId())
                         .withFlag(ForwardingObjective.Flag.VERSATILE);
-                DeviceId source = srManager.deviceConfiguration.getDeviceId(tunnel.labelIds().get(0));
+                DeviceId source = deviceConfiguration.getDeviceId(tunnel.labelIds().get(0));
-                srManager.flowObjectiveService.forward(source, fwdBuilder.remove());
+                flowObjectiveService.forward(source, fwdBuilder.remove());
                 policyStore.remove(policyInfo.id());
             }
         } else {
             log.warn("Policy {} was not found", policyInfo.id());
+            return Result.POLICY_NOT_FOUND;
         }
+
+        return Result.SUCCESS;
     }
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
View file @71abe1b
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
View file @71abe1b
@@ -199,8 +199,10 @@ public class SegmentRoutingManager implements SegmentRoutingService {
         ipHandler = new IpHandler(this);
         routingRulePopulator = new RoutingRulePopulator(this);
         defaultRoutingHandler = new DefaultRoutingHandler(this);
-        tunnelHandler = new TunnelHandler(this, tunnelStore);
+        tunnelHandler = new TunnelHandler(linkService, deviceConfiguration,
-        policyHandler = new PolicyHandler(this, policyStore);
+                groupHandlerMap, tunnelStore);
+        policyHandler = new PolicyHandler(appId, deviceConfiguration,
+                flowObjectiveService, tunnelHandler, policyStore);
         packetService.addProcessor(processor, PacketProcessor.ADVISOR_MAX + 2);
         linkService.addListener(new InternalLinkListener());
@@ -240,33 +242,32 @@ public class SegmentRoutingManager implements SegmentRoutingService {
     }
     @Override
-    public void createTunnel(Tunnel tunnel) {
+    public TunnelHandler.Result createTunnel(Tunnel tunnel) {
-        tunnelHandler.createTunnel(tunnel);
+        return tunnelHandler.createTunnel(tunnel);
     }
     @Override
-    public void removeTunnel(Tunnel tunnel) {
+    public TunnelHandler.Result removeTunnel(Tunnel tunnel) {
         for (Policy policy: policyHandler.getPolicies()) {
             if (policy.type() == Policy.Type.TUNNEL_FLOW) {
                 TunnelPolicy tunnelPolicy = (TunnelPolicy) policy;
                 if (tunnelPolicy.tunnelId().equals(tunnel.id())) {
                     log.warn("Cannot remove the tunnel used by a policy");
-                    return;
+                    return TunnelHandler.Result.TUNNEL_IN_USE;
                 }
             }
         }
-        tunnelHandler.removeTunnel(tunnel);
+        return tunnelHandler.removeTunnel(tunnel);
     }
     @Override
-    public void removePolicy(Policy policy) {
+    public PolicyHandler.Result removePolicy(Policy policy) {
-        policyHandler.removePolicy(policy);
+        return policyHandler.removePolicy(policy);
-
     }
     @Override
-    public void createPolicy(Policy policy) {
+    public PolicyHandler.Result createPolicy(Policy policy) {
-        policyHandler.createPolicy(policy);
+        return policyHandler.createPolicy(policy);
     }
     @Override
@@ -319,35 +320,6 @@ 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
-     * @param objectiveId next objective ID to remove
-     * @return true, if succeeds, false otherwise
-     */
-    public boolean removeNextObjective(DeviceId deviceId, int objectiveId) {
-        return groupHandlerMap.get(deviceId).removeGroup(objectiveId);
-    }
-
     private class InternalPacketProcessor implements PacketProcessor {
         @Override
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
View file @71abe1b
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingService.java
View file @71abe1b
@@ -33,8 +33,12 @@ public interface SegmentRoutingService {
      * Creates a tunnel.
      *
      * @param tunnel tunnel reference to create
+     * @return WRONG_PATH if the tunnel path is wrong, ID_EXISTS if the tunnel ID
+     * exists already, TUNNEL_EXISTS if the same tunnel exists, INTERNAL_ERROR
+     * if the tunnel creation failed internally, SUCCESS if the tunnel is created
+     * successfully
      */
-    void createTunnel(Tunnel tunnel);
+    TunnelHandler.Result createTunnel(Tunnel tunnel);
     /**
      * Returns all policies.
@@ -47,20 +51,29 @@ public interface SegmentRoutingService {
      * Creates a policy.
      *
      * @param policy policy reference to create
+     * @return ID_EXISTS if the same policy ID exists,
+     *  POLICY_EXISTS if the same policy exists, TUNNEL_NOT_FOUND if the tunnel
+     *  does not exists, UNSUPPORTED_TYPE if the policy type is not supported,
+     *  SUCCESS if the policy is created successfully.
      */
-    void createPolicy(Policy policy);
+    PolicyHandler.Result createPolicy(Policy policy);
     /**
      * Removes a tunnel.
      *
      * @param tunnel tunnel reference to remove
+     * @return TUNNEL_NOT_FOUND if the tunnel to remove does not exists,
+     * INTERNAL_ERROR if the tunnel creation failed internally, SUCCESS
+     * if the tunnel is created successfully.
      */
-    void removeTunnel(Tunnel tunnel);
+    TunnelHandler.Result removeTunnel(Tunnel tunnel);
     /**
      * Removes a policy.
      *
      * @param policy policy reference to remove
+     * @return POLICY_NOT_FOUND if the policy to remove does not exists,
+     * SUCCESS if it is removed successfully
      */
-    void removePolicy(Policy policy);
+    PolicyHandler.Result removePolicy(Policy policy);
 }
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
View file @71abe1b
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
View file @71abe1b
@@ -17,6 +17,8 @@ package org.onosproject.segmentrouting;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Link;
+import org.onosproject.net.link.LinkService;
+import org.onosproject.segmentrouting.grouphandler.DefaultGroupHandler;
 import org.onosproject.segmentrouting.grouphandler.NeighborSet;
 import org.onosproject.store.service.EventuallyConsistentMap;
 import org.slf4j.Logger;
@@ -24,9 +26,9 @@ import org.slf4j.Logger;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
-import static com.google.common.base.Preconditions.checkNotNull;
 import static org.slf4j.LoggerFactory.getLogger;
 /**
@@ -35,14 +37,28 @@ import static org.slf4j.LoggerFactory.getLogger;
 public class TunnelHandler {
     protected final Logger log = getLogger(getClass());
-    private final SegmentRoutingManager srManager;
     private final DeviceConfiguration config;
     private final EventuallyConsistentMap<String, Tunnel> tunnelStore;
-
+    private Map<DeviceId, DefaultGroupHandler> groupHandlerMap;
-    public TunnelHandler(SegmentRoutingManager srm,
+    private LinkService linkService;
+
+    public enum Result {
+        SUCCESS,
+        WRONG_PATH,
+        TUNNEL_EXISTS,
+        ID_EXISTS,
+        TUNNEL_NOT_FOUND,
+        TUNNEL_IN_USE,
+        INTERNAL_ERROR
+    }
+
+    public TunnelHandler(LinkService linkService,
+                         DeviceConfiguration deviceConfiguration,
+                         Map<DeviceId, DefaultGroupHandler> groupHandlerMap,
                          EventuallyConsistentMap<String, Tunnel> tunnelStore) {
-        this.srManager = checkNotNull(srm);
+        this.linkService = linkService;
-        this.config = srm.deviceConfiguration;
+        this.config = deviceConfiguration;
+        this.groupHandlerMap = groupHandlerMap;
         this.tunnelStore = tunnelStore;
     }
@@ -50,60 +66,70 @@ public class TunnelHandler {
      * Creates a tunnel.
      *
      * @param tunnel tunnel reference to create a tunnel
-     * @return true if creation succeeded
+     * @return WRONG_PATH if the tunnel path is wrong, ID_EXISTS if the tunnel ID
+     * exists already, TUNNEL_EXISTS if the same tunnel exists, INTERNAL_ERROR
+     * if the tunnel creation failed internally, SUCCESS if the tunnel is created
+     * successfully
      */
-    public boolean createTunnel(Tunnel tunnel) {
+    public Result 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;
+            return Result.WRONG_PATH;
         }
         if (tunnelStore.containsKey(tunnel.id())) {
             log.warn("The same tunnel ID exists already");
-            return false;
+            return Result.ID_EXISTS;
         }
         if (tunnelStore.containsValue(tunnel)) {
             log.warn("The same tunnel exists already");
-            return false;
+            return Result.TUNNEL_EXISTS;
         }
         int groupId = createGroupsForTunnel(tunnel);
         if (groupId < 0) {
             log.error("Failed to create groups for the tunnel");
-            return false;
+            return Result.INTERNAL_ERROR;
         }
         tunnel.setGroupId(groupId);
         tunnelStore.put(tunnel.id(), tunnel);
-        return true;
+        return Result.SUCCESS;
     }
     /**
      * Removes the tunnel with the tunnel ID given.
      *
      * @param tunnelInfo tunnel information to delete tunnels
+     * @return TUNNEL_NOT_FOUND if the tunnel to remove does not exists,
+     * INTERNAL_ERROR if the tunnel creation failed internally, SUCCESS
+     * if the tunnel is created successfully.
      */
-    public void removeTunnel(Tunnel tunnelInfo) {
+    public Result removeTunnel(Tunnel tunnelInfo) {
         Tunnel tunnel = tunnelStore.get(tunnelInfo.id());
         if (tunnel != null) {
             DeviceId deviceId = config.getDeviceId(tunnel.labelIds().get(0));
             if (tunnel.isAllowedToRemoveGroup()) {
-                if (srManager.removeNextObjective(deviceId, tunnel.groupId())) {
+                if (groupHandlerMap.get(deviceId).removeGroup(tunnel.groupId())) {
                     tunnelStore.remove(tunnel.id());
                 } else {
                     log.error("Failed to remove the tunnel {}", tunnelInfo.id());
+                    return Result.INTERNAL_ERROR;
                 }
             } 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());
+            log.error("No tunnel found for tunnel ID {}", tunnelInfo.id());
+            return Result.TUNNEL_NOT_FOUND;
         }
+
+        return Result.SUCCESS;
     }
     /**
@@ -132,19 +158,21 @@ public class TunnelHandler {
     private int createGroupsForTunnel(Tunnel tunnel) {
         List<Integer> portNumbers;
-
+        final int groupError = -1;
-        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;
+            return groupError;
+        } else if (groupHandlerMap.get(deviceId) == null) {
+            log.warn("group handler not found for {}", deviceId);
+            return groupError;
         }
         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 (Link link: linkService.getDeviceEgressLinks(deviceId)) {
                 for (Integer port: portNumbers) {
                     if (link.src().port().toLong() == port) {
                         deviceIds.add(link.dst().deviceId());
@@ -159,14 +187,13 @@ public class TunnelHandler {
         // If the tunnel reuses any existing groups, then tunnel handler
         // should not remove the group.
-        if (srManager.hasNextObjectiveId(deviceId, ns)) {
+        if (groupHandlerMap.get(deviceId).hasNextObjectiveId(ns)) {
             tunnel.allowToRemoveGroup(false);
         } else {
             tunnel.allowToRemoveGroup(true);
         }
-        groupId = srManager.getNextObjectiveId(deviceId, ns);
-        return groupId;
+        return groupHandlerMap.get(deviceId).getNextObjectiveId(ns);
     }
 }
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/PolicyAddCommand.java
View file @71abe1b
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/PolicyAddCommand.java
View file @71abe1b
@@ -19,6 +19,7 @@ import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
 import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.segmentrouting.Policy;
+import org.onosproject.segmentrouting.PolicyHandler;
 import org.onosproject.segmentrouting.SegmentRoutingService;
 import org.onosproject.segmentrouting.TunnelPolicy;
@@ -31,7 +32,7 @@ public class PolicyAddCommand extends AbstractShellCommand {
     // TODO: Need to support skipping some parameters
-    @Argument(index = 0, name = "policy ID",
+    @Argument(index = 0, name = "ID",
             description = "policy ID",
             required = true, multiValued = false)
     String policyId;
@@ -41,37 +42,37 @@ public class PolicyAddCommand extends AbstractShellCommand {
             required = true, multiValued = false)
     int priority;
-    @Argument(index = 2, name = "src IP",
+    @Argument(index = 2, name = "src_IP",
             description = "src IP",
             required = false, multiValued = false)
     String srcIp;
-    @Argument(index = 3, name = "src port",
+    @Argument(index = 3, name = "src_port",
             description = "src port",
             required = false, multiValued = false)
     short srcPort;
-    @Argument(index = 4, name = "dst IP",
+    @Argument(index = 4, name = "dst_IP",
             description = "dst IP",
             required = false, multiValued = false)
     String dstIp;
-    @Argument(index = 5, name = "dst port",
+    @Argument(index = 5, name = "dst_port",
             description = "dst port",
             required = false, multiValued = false)
     short dstPort;
     @Argument(index = 6, name = "proto",
-            description = "proto",
+            description = "IP protocol",
             required = false, multiValued = false)
     String proto;
-    @Argument(index = 7, name = "policy type",
+    @Argument(index = 7, name = "policy_type",
             description = "policy type",
             required = true, multiValued = false)
     String policyType;
-    @Argument(index = 8, name = "tunnel ID",
+    @Argument(index = 8, name = "tunnel_ID",
             description = "tunnel ID",
             required = false, multiValued = false)
     String tunnelId;
@@ -103,11 +104,29 @@ public class PolicyAddCommand extends AbstractShellCommand {
         }
         if (Policy.Type.valueOf(policyType) == Policy.Type.TUNNEL_FLOW) {
             if (tunnelId == null) {
-                // TODO: handle errors
+                error("tunnel ID must be specified for TUNNEL_FLOW policy");
                 return;
             }
             tpb.setTunnelId(tunnelId);
         }
-        srService.createPolicy(tpb.build());
+        PolicyHandler.Result result = srService.createPolicy(tpb.build());
+
+        switch (result) {
+            case POLICY_EXISTS:
+                error("the same policy exists");
+                break;
+            case ID_EXISTS:
+                error("the same policy ID exists");
+                break;
+            case TUNNEL_NOT_FOUND:
+                error("the tunnel is not found");
+                break;
+            case UNSUPPORTED_TYPE:
+                error("the policy type specified is not supported");
+                break;
+            default:
+                break;
+        }
+
     }
 }
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/PolicyRemoveCommand.java
View file @71abe1b
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/PolicyRemoveCommand.java
View file @71abe1b
@@ -19,6 +19,7 @@ package org.onosproject.segmentrouting.cli;
 import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
 import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.segmentrouting.PolicyHandler;
 import org.onosproject.segmentrouting.SegmentRoutingService;
 import org.onosproject.segmentrouting.TunnelPolicy;
@@ -41,6 +42,9 @@ public class PolicyRemoveCommand extends AbstractShellCommand {
                 AbstractShellCommand.get(SegmentRoutingService.class);
         TunnelPolicy.Builder tpb = TunnelPolicy.builder().setPolicyId(policyId);
-        srService.removePolicy(tpb.build());
+        PolicyHandler.Result result = srService.removePolicy(tpb.build());
+        if (result == PolicyHandler.Result.POLICY_NOT_FOUND) {
+            print("ERROR: the policy is not found");
+        }
     }
 }
\ No newline at end of file
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/TunnelAddCommand.java
View file @71abe1b
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/TunnelAddCommand.java
View file @71abe1b
@@ -22,6 +22,7 @@ import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.segmentrouting.DefaultTunnel;
 import org.onosproject.segmentrouting.SegmentRoutingService;
 import org.onosproject.segmentrouting.Tunnel;
+import org.onosproject.segmentrouting.TunnelHandler;
 import java.util.ArrayList;
 import java.util.List;
@@ -58,6 +59,22 @@ public class TunnelAddCommand extends AbstractShellCommand {
         }
         Tunnel tunnel = new DefaultTunnel(tunnelId, labelIds);
-        srService.createTunnel(tunnel);
+        TunnelHandler.Result result = srService.createTunnel(tunnel);
+        switch (result) {
+            case ID_EXISTS:
+                print("ERROR: the same tunnel ID exists");
+                break;
+            case TUNNEL_EXISTS:
+                print("ERROR: the same tunnel exists");
+                break;
+            case INTERNAL_ERROR:
+                print("ERROR: internal tunnel creation error");
+                break;
+            case WRONG_PATH:
+                print("ERROR: the tunnel path is wrong");
+                break;
+            default:
+                break;
+        }
     }
 }
\ No newline at end of file
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/TunnelRemoveCommand.java
View file @71abe1b
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/cli/TunnelRemoveCommand.java
View file @71abe1b
@@ -23,6 +23,7 @@ import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.segmentrouting.DefaultTunnel;
 import org.onosproject.segmentrouting.SegmentRoutingService;
 import org.onosproject.segmentrouting.Tunnel;
+import org.onosproject.segmentrouting.TunnelHandler;
 /**
  * Command to remove a tunnel.
@@ -38,11 +39,20 @@ public class TunnelRemoveCommand extends AbstractShellCommand {
     @Override
     protected void execute() {
-
         SegmentRoutingService srService =
                 AbstractShellCommand.get(SegmentRoutingService.class);
         Tunnel tunnel = new DefaultTunnel(tunnelId, Lists.newArrayList());
-        srService.removeTunnel(tunnel);
+        TunnelHandler.Result result = srService.removeTunnel(tunnel);
+        switch (result) {
+            case TUNNEL_IN_USE:
+                print("ERROR: the tunnel is still in use");
+                break;
+            case TUNNEL_NOT_FOUND:
+                print("ERROR: the tunnel is not found");
+                break;
+            default:
+                break;
+        }
     }
 }
\ No newline at end of file