Route CLI improvements and bug fixes

Change-Id: I4b4547f578cc053dc150066dadb68b6b2cbb82ee

+/*
+ * Copyright 2016 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.cli.net;
+
+import org.apache.karaf.shell.commands.Command;
+import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.incubator.net.routing.NextHop;
+import org.onosproject.incubator.net.routing.Route;
+import org.onosproject.incubator.net.routing.RouteService;
+
+import java.util.Collection;
+import java.util.Set;
+
+/**
+ * Command to show information about routing next hops.
+ */
+@Command(scope = "onos", name = "next-hops",
+        description = "Lists all next hops in the route store")
+public class NextHopsListCommand extends AbstractShellCommand {
+
+    private static final String FORMAT_HEADER =
+        "   Network            Next Hop";
+    private static final String FORMAT_ROUTE =
+        "   %-18s %-15s";
+
+    private static final String FORMAT_TABLE = "Table: %s";
+    private static final String FORMAT_TOTAL = "   Total: %d";
+
+    private static final String FORMAT = "ip=%s, mac=%s, numRoutes=%s";
+
+    @Override
+    protected void execute() {
+        RouteService service = AbstractShellCommand.get(RouteService.class);
+
+        Set<NextHop> nextHops = service.getNextHops();
+
+        nextHops.forEach(nextHop -> {
+            Collection<Route> routes = service.getRoutesForNextHop(nextHop.ip());
+            print(FORMAT, nextHop.ip(), nextHop.mac(), routes.size());
+        });
+
+    }
+
+}

@@ -18,6 +18,7 @@ package org.onosproject.cli.net;
 
 
 import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
 import org.apache.karaf.shell.commands.Command;
+import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.IpPrefix;
 import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.cli.AbstractShellCommand;
 import org.onosproject.incubator.net.routing.Route;
 import org.onosproject.incubator.net.routing.Route;
@@ -36,13 +37,18 @@ public class RouteRemoveCommand extends AbstractShellCommand {
             required = true)
             required = true)
     String prefixString = null;
     String prefixString = null;
 
 
+    @Argument(index = 1, name = "prefix", description = "Next hop IP address",
+            required = true)
+    String nextHopString = null;
+
     @Override
     @Override
     protected void execute() {
     protected void execute() {
         RouteAdminService service = AbstractShellCommand.get(RouteAdminService.class);
         RouteAdminService service = AbstractShellCommand.get(RouteAdminService.class);
 
 
         IpPrefix prefix = IpPrefix.valueOf(prefixString);
         IpPrefix prefix = IpPrefix.valueOf(prefixString);
+        IpAddress nextHop = IpAddress.valueOf(nextHopString);
 
 
-        service.withdraw(Collections.singleton(new Route(Route.Source.STATIC, prefix, null)));
+        service.withdraw(Collections.singleton(new Route(Route.Source.STATIC, prefix, nextHop)));
     }
     }
 
 
 }
 }

@@ -505,6 +505,9 @@
         <command>
         <command>
             <action class="org.onosproject.cli.net.RouteRemoveCommand"/>
             <action class="org.onosproject.cli.net.RouteRemoveCommand"/>
         </command>
         </command>
+        <command>
+            <action class="org.onosproject.cli.net.NextHopsListCommand"/>
+        </command>
 
 
         <command>
         <command>
             <action class="org.onosproject.cli.net.GlobalLabelCommand"/>
             <action class="org.onosproject.cli.net.GlobalLabelCommand"/>

+/*
+ * Copyright 2016 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.incubator.net.routing;
+
+import org.onlab.packet.IpAddress;
+import org.onlab.packet.MacAddress;
+
+import java.util.Objects;
+
+import static com.google.common.base.MoreObjects.toStringHelper;
+
+/**
+ * Describes a routing next hop.
+ */
+public class NextHop {
+
+    private final IpAddress ip;
+    private final MacAddress mac;
+
+    /**
+     * Creates a new next hop.
+     *
+     * @param ip IP address
+     * @param mac MAC address
+     */
+    public NextHop(IpAddress ip, MacAddress mac) {
+        this.ip = ip;
+        this.mac = mac;
+    }
+
+    /**
+     * Returns the IP address of the next hop.
+     *
+     * @return IP address
+     */
+    public IpAddress ip() {
+        return ip;
+    }
+
+    /**
+     * Returns the MAC address of the next hop.
+     *
+     * @return MAC address
+     */
+    public MacAddress mac() {
+        return mac;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(ip, mac);
+    }
+
+    @Override
+    public boolean equals(Object other) {
+        if (this == other) {
+            return true;
+        }
+
+        if (!(other instanceof NextHop)) {
+            return false;
+        }
+
+        NextHop that = (NextHop) other;
+
+        return Objects.equals(this.ip, that.mac) &&
+                Objects.equals(this.ip, that.mac);
+    }
+
+    @Override
+    public String toString() {
+        return toStringHelper(this)
+                .add("ip", ip)
+                .add("mac", mac)
+                .toString();
+    }
+}

@@ -20,6 +20,8 @@ import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MacAddress;
 
 
+import static com.google.common.base.MoreObjects.toStringHelper;
+
 /**
 /**
  * Represents a route with the next hop MAC address resolved.
  * Represents a route with the next hop MAC address resolved.
  */
  */
@@ -80,4 +82,13 @@ public class ResolvedRoute {
     public MacAddress nextHopMac() {
     public MacAddress nextHopMac() {
         return nextHopMac;
         return nextHopMac;
     }
     }
+
+    @Override
+    public String toString() {
+        return toStringHelper(this)
+                .add("prefix", prefix)
+                .add("nextHop", nextHop)
+                .add("nextHopMac", nextHopMac)
+                .toString();
+    }
 }
 }

@@ -21,6 +21,7 @@ import org.onlab.packet.IpPrefix;
 
 
 import java.util.Objects;
 import java.util.Objects;
 
 
+import static com.google.common.base.MoreObjects.toStringHelper;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkNotNull;
 
 
@@ -70,7 +71,8 @@ public class Route {
      */
      */
     public Route(Source source, IpPrefix prefix, IpAddress nextHop) {
     public Route(Source source, IpPrefix prefix, IpAddress nextHop) {
         checkNotNull(prefix);
         checkNotNull(prefix);
-        checkArgument(nextHop == null || prefix.version().equals(nextHop.version()), VERSION_MISMATCH);
+        checkNotNull(nextHop);
+        checkArgument(prefix.version().equals(nextHop.version()), VERSION_MISMATCH);
 
 
         this.source = checkNotNull(source);
         this.source = checkNotNull(source);
         this.prefix = prefix;
         this.prefix = prefix;
@@ -124,4 +126,12 @@ public class Route {
         return Objects.equals(this.prefix, that.prefix) &&
         return Objects.equals(this.prefix, that.prefix) &&
                 Objects.equals(this.nextHop, that.nextHop);
                 Objects.equals(this.nextHop, that.nextHop);
     }
     }
+
+    @Override
+    public String toString() {
+        return toStringHelper(this)
+                .add("prefix", prefix)
+                .add("nextHop", nextHop)
+                .toString();
+    }
 }
 }

@@ -21,6 +21,7 @@ import org.onosproject.event.ListenerService;
 
 
 import java.util.Collection;
 import java.util.Collection;
 import java.util.Map;
 import java.util.Map;
+import java.util.Set;
 
 
 /**
 /**
  * Unicast IP route service.
  * Unicast IP route service.
@@ -44,4 +45,19 @@ public interface RouteService extends ListenerService<RouteEvent, RouteListener>
      */
      */
     Route longestPrefixMatch(IpAddress ip);
     Route longestPrefixMatch(IpAddress ip);
 
 
+    /**
+     * Returns the routes for the given next hop.
+     *
+     * @param nextHop next hop IP address
+     * @return routes for this next hop
+     */
+    Collection<Route> getRoutesForNextHop(IpAddress nextHop);
+
+    /**
+     * Returns all next hops in the route store.
+     *
+     * @return set of next hops
+     */
+    Set<NextHop> getNextHops();
+
 }
 }

@@ -21,6 +21,7 @@ import org.onlab.packet.MacAddress;
 import org.onosproject.store.Store;
 import org.onosproject.store.Store;
 
 
 import java.util.Collection;
 import java.util.Collection;
+import java.util.Map;
 import java.util.Set;
 import java.util.Set;
 
 
 /**
 /**
@@ -66,6 +67,14 @@ public interface RouteStore extends Store<RouteEvent, RouteStoreDelegate> {
     Route longestPrefixMatch(IpAddress ip);
     Route longestPrefixMatch(IpAddress ip);
 
 
     /**
     /**
+     * Returns the routes that point to the given next hop IP address.
+     *
+     * @param ip IP address of the next hop
+     * @return routes for the given next hop
+     */
+    Collection<Route> getRoutesForNextHop(IpAddress ip);
+
+    /**
      * Updates a next hop IP and MAC in the store.
      * Updates a next hop IP and MAC in the store.
      *
      *
      * @param ip IP address
      * @param ip IP address
@@ -88,4 +97,11 @@ public interface RouteStore extends Store<RouteEvent, RouteStoreDelegate> {
      * @return MAC address
      * @return MAC address
      */
      */
     MacAddress getNextHop(IpAddress ip);
     MacAddress getNextHop(IpAddress ip);
+
+    /**
+     * Returns all next hops in the route store.
+     *
+     * @return next hops
+     */
+    Map<IpAddress, MacAddress> getNextHops();
 }
 }

@@ -25,6 +25,7 @@ import org.apache.felix.scr.annotations.Service;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.MacAddress;
 import org.onosproject.event.ListenerService;
 import org.onosproject.event.ListenerService;
+import org.onosproject.incubator.net.routing.NextHop;
 import org.onosproject.incubator.net.routing.ResolvedRoute;
 import org.onosproject.incubator.net.routing.ResolvedRoute;
 import org.onosproject.incubator.net.routing.Route;
 import org.onosproject.incubator.net.routing.Route;
 import org.onosproject.incubator.net.routing.RouteAdminService;
 import org.onosproject.incubator.net.routing.RouteAdminService;
@@ -146,6 +147,7 @@ public class RouteManager implements ListenerService<RouteEvent, RouteListener>,
      * @param event event
      * @param event event
      */
      */
     private void post(RouteEvent event) {
     private void post(RouteEvent event) {
+        log.debug("Sending event {}", event);
         synchronized (this) {
         synchronized (this) {
             listeners.values().forEach(l -> l.post(event));
             listeners.values().forEach(l -> l.post(event));
         }
         }
@@ -165,9 +167,22 @@ public class RouteManager implements ListenerService<RouteEvent, RouteListener>,
     }
     }
 
 
     @Override
     @Override
+    public Collection<Route> getRoutesForNextHop(IpAddress nextHop) {
+        return routeStore.getRoutesForNextHop(nextHop);
+    }
+
+    @Override
+    public Set<NextHop> getNextHops() {
+        return routeStore.getNextHops().entrySet().stream()
+                .map(entry -> new NextHop(entry.getKey(), entry.getValue()))
+                .collect(Collectors.toSet());
+    }
+
+    @Override
     public void update(Collection<Route> routes) {
     public void update(Collection<Route> routes) {
         synchronized (this) {
         synchronized (this) {
             routes.forEach(route -> {
             routes.forEach(route -> {
+                log.debug("Received update {}", route);
                 routeStore.updateRoute(route);
                 routeStore.updateRoute(route);
                 resolve(route);
                 resolve(route);
             });
             });
@@ -177,7 +192,10 @@ public class RouteManager implements ListenerService<RouteEvent, RouteListener>,
     @Override
     @Override
     public void withdraw(Collection<Route> routes) {
     public void withdraw(Collection<Route> routes) {
         synchronized (this) {
         synchronized (this) {
-            routes.forEach(route -> routeStore.removeRoute(route));
+            routes.forEach(route -> {
+                log.debug("Received withdraw {}", routes);
+                routeStore.removeRoute(route);
+            });
         }
         }
     }
     }
 
 

@@ -17,8 +17,10 @@
 package org.onosproject.incubator.store.routing.impl;
 package org.onosproject.incubator.store.routing.impl;
 
 
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.HashMultimap;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Multimap;
 import com.google.common.collect.Multimaps;
 import com.google.common.collect.Multimaps;
+import com.googlecode.concurrenttrees.common.KeyValuePair;
 import com.googlecode.concurrenttrees.radix.node.concrete.DefaultByteArrayNodeFactory;
 import com.googlecode.concurrenttrees.radix.node.concrete.DefaultByteArrayNodeFactory;
 import com.googlecode.concurrenttrees.radixinverted.ConcurrentInvertedRadixTree;
 import com.googlecode.concurrenttrees.radixinverted.ConcurrentInvertedRadixTree;
 import com.googlecode.concurrenttrees.radixinverted.InvertedRadixTree;
 import com.googlecode.concurrenttrees.radixinverted.InvertedRadixTree;
@@ -35,10 +37,14 @@ import org.onosproject.incubator.net.routing.RouteStore;
 import org.onosproject.incubator.net.routing.RouteStoreDelegate;
 import org.onosproject.incubator.net.routing.RouteStoreDelegate;
 import org.onosproject.incubator.net.routing.RouteTableId;
 import org.onosproject.incubator.net.routing.RouteTableId;
 import org.onosproject.store.AbstractStore;
 import org.onosproject.store.AbstractStore;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 
 import java.util.Collection;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
 import java.util.Map;
 import java.util.Map;
 import java.util.Set;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentHashMap;
@@ -51,6 +57,8 @@ import java.util.concurrent.ConcurrentHashMap;
 public class LocalRouteStore extends AbstractStore<RouteEvent, RouteStoreDelegate>
 public class LocalRouteStore extends AbstractStore<RouteEvent, RouteStoreDelegate>
         implements RouteStore {
         implements RouteStore {
 
 
+    private Logger log = LoggerFactory.getLogger(getClass());
+
     private Map<RouteTableId, RouteTable> routeTables;
     private Map<RouteTableId, RouteTable> routeTables;
     private static final RouteTableId IPV4 = new RouteTableId("ipv4");
     private static final RouteTableId IPV4 = new RouteTableId("ipv4");
     private static final RouteTableId IPV6 = new RouteTableId("ipv6");
     private static final RouteTableId IPV6 = new RouteTableId("ipv6");
@@ -74,7 +82,9 @@ public class LocalRouteStore extends AbstractStore<RouteEvent, RouteStoreDelegat
     public void removeRoute(Route route) {
     public void removeRoute(Route route) {
         RouteTable table = getDefaultRouteTable(route);
         RouteTable table = getDefaultRouteTable(route);
         table.remove(route);
         table.remove(route);
-        if (table.getRoutesForNextHop(route.nextHop()).isEmpty()) {
+        Collection<Route> routes = table.getRoutesForNextHop(route.nextHop());
+
+        if (routes.isEmpty()) {
             nextHops.remove(route.nextHop());
             nextHops.remove(route.nextHop());
         }
         }
     }
     }
@@ -99,8 +109,14 @@ public class LocalRouteStore extends AbstractStore<RouteEvent, RouteStoreDelegat
     }
     }
 
 
     @Override
     @Override
+    public Collection<Route> getRoutesForNextHop(IpAddress ip) {
+        return getDefaultRouteTable(ip).getRoutesForNextHop(ip);
+    }
+
+    @Override
     public void updateNextHop(IpAddress ip, MacAddress mac) {
     public void updateNextHop(IpAddress ip, MacAddress mac) {
         Collection<Route> routes = getDefaultRouteTable(ip).getRoutesForNextHop(ip);
         Collection<Route> routes = getDefaultRouteTable(ip).getRoutesForNextHop(ip);
+
         if (!routes.isEmpty() && !mac.equals(nextHops.get(ip))) {
         if (!routes.isEmpty() && !mac.equals(nextHops.get(ip))) {
             nextHops.put(ip, mac);
             nextHops.put(ip, mac);
 
 
@@ -125,6 +141,11 @@ public class LocalRouteStore extends AbstractStore<RouteEvent, RouteStoreDelegat
         return nextHops.get(ip);
         return nextHops.get(ip);
     }
     }
 
 
+    @Override
+    public Map<IpAddress, MacAddress> getNextHops() {
+        return ImmutableMap.copyOf(nextHops);
+    }
+
     private RouteTable getDefaultRouteTable(Route route) {
     private RouteTable getDefaultRouteTable(Route route) {
         return getDefaultRouteTable(route.prefix().address());
         return getDefaultRouteTable(route.prefix().address());
     }
     }
@@ -179,14 +200,27 @@ public class LocalRouteStore extends AbstractStore<RouteEvent, RouteStoreDelegat
                 routeTable.put(createBinaryString(route.prefix()), route);
                 routeTable.put(createBinaryString(route.prefix()), route);
 
 
                 // TODO manage routes from multiple providers
                 // TODO manage routes from multiple providers
-                reverseIndex.remove(route.nextHop(), oldRoute);
+
                 reverseIndex.put(route.nextHop(), route);
                 reverseIndex.put(route.nextHop(), route);
 
 
+                if (oldRoute != null) {
+                    reverseIndex.remove(oldRoute.nextHop(), oldRoute);
+
+                    if (reverseIndex.get(oldRoute.nextHop()).isEmpty()) {
+                        nextHops.remove(oldRoute.nextHop());
+                    }
+                }
+
                 if (oldRoute != null && !oldRoute.nextHop().equals(route.nextHop())) {
                 if (oldRoute != null && !oldRoute.nextHop().equals(route.nextHop())) {
                     // Remove old route because new one is different
                     // Remove old route because new one is different
                     // TODO ROUTE_UPDATED?
                     // TODO ROUTE_UPDATED?
                     notifyDelegate(new RouteEvent(RouteEvent.Type.ROUTE_REMOVED, new ResolvedRoute(oldRoute, null)));
                     notifyDelegate(new RouteEvent(RouteEvent.Type.ROUTE_REMOVED, new ResolvedRoute(oldRoute, null)));
                 }
                 }
+
+                MacAddress nextHopMac = nextHops.get(route.nextHop());
+                if (nextHopMac != null) {
+                    notifyDelegate(new RouteEvent(RouteEvent.Type.ROUTE_UPDATED, new ResolvedRoute(route, nextHopMac)));
+                }
             }
             }
         }
         }
 
 
@@ -199,9 +233,9 @@ public class LocalRouteStore extends AbstractStore<RouteEvent, RouteStoreDelegat
             synchronized (this) {
             synchronized (this) {
                 Route removed = routes.remove(route.prefix());
                 Route removed = routes.remove(route.prefix());
                 routeTable.remove(createBinaryString(route.prefix()));
                 routeTable.remove(createBinaryString(route.prefix()));
-                reverseIndex.remove(route.nextHop(), route);
 
 
                 if (removed != null) {
                 if (removed != null) {
+                    reverseIndex.remove(removed.nextHop(), removed);
                     notifyDelegate(new RouteEvent(RouteEvent.Type.ROUTE_REMOVED, new ResolvedRoute(route, null)));
                     notifyDelegate(new RouteEvent(RouteEvent.Type.ROUTE_REMOVED, new ResolvedRoute(route, null)));
                 }
                 }
             }
             }
@@ -223,7 +257,17 @@ public class LocalRouteStore extends AbstractStore<RouteEvent, RouteStoreDelegat
          * @return all routes
          * @return all routes
          */
          */
         public Collection<Route> getRoutes() {
         public Collection<Route> getRoutes() {
-            return routes.values();
+            Iterator<KeyValuePair<Route>> it =
+                    routeTable.getKeyValuePairsForKeysStartingWith("").iterator();
+
+            List<Route> routes = new LinkedList<>();
+
+            while (it.hasNext()) {
+                KeyValuePair<Route> entry = it.next();
+                routes.add(entry.getValue());
+            }
+
+            return routes;
         }
         }
 
 
         /**
         /**