Merge remote-tracking branch 'origin/master'

+package org.onlab.onos.cli.net;
+
+import static com.google.common.collect.Lists.newArrayList;
+
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.karaf.shell.commands.Command;
+import org.onlab.onos.cli.AbstractShellCommand;
+import org.onlab.onos.net.Device;
+import org.onlab.onos.net.device.DeviceService;
+import org.onlab.onos.net.flow.FlowRule;
+import org.onlab.onos.net.flow.FlowRuleService;
+
+import com.google.common.collect.Maps;
+
+/**
+ * Lists all currently-known hosts.
+ */
+@Command(scope = "onos", name = "flows",
+description = "Lists all currently-known flows.")
+public class FlowsListCommand extends AbstractShellCommand {
+
+    private static final String FMT =
+            "   id=%s, selector=%s, treatment=%s, state=%s";
+
+    protected static final Comparator<FlowRule> ID_COMPARATOR = new Comparator<FlowRule>() {
+        @Override
+        public int compare(FlowRule f1, FlowRule f2) {
+            return Long.valueOf(f1.id().value()).compareTo(f2.id().value());
+        }
+    };
+
+    @Override
+    protected Object doExecute() throws Exception {
+        DeviceService deviceService = getService(DeviceService.class);
+        FlowRuleService service = getService(FlowRuleService.class);
+        Map<Device, List<FlowRule>> flows = getSortedFlows(deviceService, service);
+        for (Device d : deviceService.getDevices()) {
+            printFlows(d, flows.get(d));
+        }
+        return null;
+    }
+
+
+    /**
+     * Returns the list of devices sorted using the device ID URIs.
+     *
+     * @param service device service
+     * @return sorted device list
+     */
+    protected Map<Device, List<FlowRule>> getSortedFlows(DeviceService deviceService, FlowRuleService service) {
+        Map<Device, List<FlowRule>> flows = Maps.newHashMap();
+        List<FlowRule> rules;
+        for (Device d : deviceService.getDevices()) {
+            rules = newArrayList(service.getFlowEntries(d.id()));
+            Collections.sort(rules, ID_COMPARATOR);
+            flows.put(d, rules);
+        }
+        return flows;
+    }
+
+    /**
+     * Prints flows.
+     * @param d the device
+     * @param flows the set of flows for that device.
+     */
+    protected void printFlows(Device d, List<FlowRule> flows) {
+        print("Device: " + d.id());
+        for (FlowRule f : flows) {
+            print(FMT, f.id().value(), f.selector(), f.treatment(), f.state());
+        }
+
+    }
+
+}
\ No newline at end of file

@@ -2,6 +2,9 @@
 
     <command-bundle xmlns="http://karaf.apache.org/xmlns/shell/v1.1.0">
         <command>
+            <action class="org.onlab.onos.cli.net.FlowsListCommand"/>
+        </command>
+        <command>
             <action class="org.onlab.onos.cli.net.DevicesListCommand"/>
         </command>
         <command>

+package org.onlab.onos.cluster;
+
+import org.onlab.onos.event.AbstractEvent;
+import org.onlab.onos.net.DeviceId;
+
+/**
+ * Describes infrastructure device event.
+ */
+public class MastershipEvent extends AbstractEvent<MastershipEvent.Type, DeviceId> {
+
+    InstanceId master;
+
+    /**
+     * Type of mastership events.
+     */
+    public enum Type {
+        /**
+         * Signifies that the master for a device has changed.
+         */
+        MASTER_CHANGED
+    }
+
+    /**
+     * Creates an event of a given type and for the specified device, master,
+     * and the current time.
+     *
+     * @param type   device event type
+     * @param device event device subject
+     * @param master master ID subject
+     */
+    protected MastershipEvent(Type type, DeviceId device, InstanceId master) {
+        super(type, device);
+        this.master = master;
+    }
+
+    /**
+     * Creates an event of a given type and for the specified device, master,
+     * and time.
+     *
+     * @param type   mastership event type
+     * @param device event device subject
+     * @param master master ID subject
+     * @param time   occurrence time
+     */
+    protected MastershipEvent(Type type, DeviceId device, InstanceId master, long time) {
+        super(type, device, time);
+        this.master = master;
+    }
+
+    /**
+     * Returns the current master's ID as a subject.
+     *
+     * @return master ID subject
+     */
+    public InstanceId master() {
+        return master;
+    }
+}

+package org.onlab.onos.cluster;
+
+import org.onlab.onos.event.EventListener;
+
+/**
+ * Entity capable of receiving device mastership-related events.
+ */
+public interface MastershipListener extends EventListener<MastershipEvent> {
+
+}

 package org.onlab.onos.cluster;
 
+import java.util.Set;
+
+import org.onlab.onos.net.DeviceId;
+import org.onlab.onos.net.MastershipRole;
+
 /**
  * Service responsible for determining the controller instance mastership of
  * a device in a clustered environment. This is the central authority for
@@ -8,12 +13,42 @@ package org.onlab.onos.cluster;
  */
 public interface MastershipService {
 
-    // InstanceId getMasterFor(DeviceId deviceId)
-    // Set<DeviceId> getDevicesOf(InstanceId instanceId);
+    /**
+     * Returns the current master for a given device.
+     *
+     * @param deviceId the identifier of the device
+     * @return the ID of the master controller for the device
+     */
+    InstanceId getMasterFor(DeviceId deviceId);
+
+    /**
+     * Returns the devices for which a controller is master.
+     *
+     * @param instanceId the ID of the controller
+     * @return a set of device IDs
+     */
+    Set<DeviceId> getDevicesOf(InstanceId instanceId);
+
+    /**
+     * Returns the mastership status of this controller for a given device.
+     *
+     * @param deviceId the the identifier of the device
+     * @return the role of this controller instance
+     */
+    MastershipRole requestRoleFor(DeviceId deviceId);
 
-    // MastershipRole requestRoleFor(DeviceId deviceId);
+    /**
+     * Adds the specified mastership listener.
+     *
+     * @param listener the mastership listener
+     */
+    void addListener(MastershipListener listener);
 
-    // addListener/removeLister(MastershipListener listener);
-        // types of events would be MASTER_CHANGED (subject ==> deviceId; master ==> instanceId)
+    /**
+     * Removes the specified device listener.
+     *
+     * @param listener the mastership listener
+     */
+    void removeListemer(MastershipListener listener);
 
 }

 package org.onlab.onos.net.flow;
 
 import static com.google.common.base.MoreObjects.toStringHelper;
+import static org.slf4j.LoggerFactory.getLogger;
 
 import java.util.Objects;
 
 import org.onlab.onos.net.DeviceId;
+import org.slf4j.Logger;
 
 public class DefaultFlowRule implements FlowRule {
 
+    private final Logger log = getLogger(getClass());
+
     private final DeviceId deviceId;
     private final int priority;
     private final TrafficSelector selector;
     private final TrafficTreatment treatment;
-    private final FlowId id;
     private final long created;
     private final long life;
-    private final long idle;
     private final long packets;
     private final long bytes;
+    private final FlowRuleState state;
+
+    private final FlowId id;
+
+    public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector,
+            TrafficTreatment treatment, int priority, FlowRuleState state,
+            long life, long packets, long bytes, long flowId) {
+        this.deviceId = deviceId;
+        this.priority = priority;
+        this.selector = selector;
+        this.treatment = treatment;
+        this.state = state;
+
+        this.id = FlowId.valueOf(flowId);
 
+        this.life = life;
+        this.packets = packets;
+        this.bytes = bytes;
+        this.created = System.currentTimeMillis();
+    }
+
+    public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector,
+            TrafficTreatment treatement, int priority) {
+        this(deviceId, selector, treatement, priority, FlowRuleState.CREATED);
+    }
+
+    public DefaultFlowRule(FlowRule rule, FlowRuleState state) {
+        this(rule.deviceId(), rule.selector(), rule.treatment(),
+                rule.priority(), state, rule.id());
+    }
 
-    public DefaultFlowRule(DeviceId deviceId,
-            TrafficSelector selector, TrafficTreatment treatment, int priority) {
+    private DefaultFlowRule(DeviceId deviceId,
+            TrafficSelector selector, TrafficTreatment treatment,
+            int priority, FlowRuleState state) {
         this.deviceId = deviceId;
         this.priority = priority;
         this.selector = selector;
         this.treatment = treatment;
+        this.state = state;
         this.life = 0;
-        this.idle = 0;
         this.packets = 0;
         this.bytes = 0;
         this.id = FlowId.valueOf(this.hashCode());
         this.created = System.currentTimeMillis();
     }
 
-    public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector,
-            TrafficTreatment treatment, int priority,
-            long life, long idle, long packets, long bytes, Integer flowId) {
+    private DefaultFlowRule(DeviceId deviceId,
+            TrafficSelector selector, TrafficTreatment treatment,
+            int priority, FlowRuleState state, FlowId flowId) {
         this.deviceId = deviceId;
         this.priority = priority;
         this.selector = selector;
         this.treatment = treatment;
-
-        this.id = FlowId.valueOf(flowId);
-
-        this.life = life;
-        this.idle = idle;
-        this.packets = packets;
-        this.bytes = bytes;
+        this.state = state;
+        this.life = 0;
+        this.packets = 0;
+        this.bytes = 0;
+        this.id = flowId;
         this.created = System.currentTimeMillis();
     }
 
@@ -83,11 +113,6 @@ public class DefaultFlowRule implements FlowRule {
     }
 
     @Override
-    public long idleMillis() {
-        return idle;
-    }
-
-    @Override
     public long packets() {
         return packets;
     }
@@ -98,6 +123,12 @@ public class DefaultFlowRule implements FlowRule {
     }
 
     @Override
+    public FlowRuleState state() {
+        return this.state;
+    }
+
+
+    @Override
     /*
      * The priority and statistics can change on a given treatment and selector
      *
@@ -116,18 +147,14 @@ public class DefaultFlowRule implements FlowRule {
      * @see java.lang.Object#equals(java.lang.Object)
      */
     public boolean equals(Object obj) {
+
+        if (this == obj) {
+            return true;
+        }
         if (obj instanceof FlowRule) {
             DefaultFlowRule that = (DefaultFlowRule) obj;
-            if (!this.deviceId().equals(that.deviceId())) {
-                return false;
-            }
-            if (!this.treatment().equals(that.treatment())) {
-                return false;
-            }
-            if (!this.selector().equals(that.selector())) {
-                return false;
-            }
-            return true;
+            return Objects.equals(deviceId, that.deviceId) &&
+                    Objects.equals(id, that.id);
         }
         return false;
     }
@@ -141,8 +168,8 @@ public class DefaultFlowRule implements FlowRule {
                 .add("selector", selector)
                 .add("treatment", treatment)
                 .add("created", created)
+                .add("state", state)
                 .toString();
     }
 
-
 }

 package org.onlab.onos.net.flow;
 
+import com.google.common.base.Objects;
+
 /**
  * Representation of a Flow ID.
  */
 public final class FlowId {
 
-    private final int flowid;
+    private final long flowid;
 
-    private FlowId(int id) {
+    private FlowId(long id) {
         this.flowid = id;
     }
 
-    public static FlowId valueOf(int id) {
+    public static FlowId valueOf(long id) {
         return new FlowId(id);
     }
 
-    public int value() {
+    public long value() {
         return flowid;
     }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj.getClass()  == this.getClass()) {
+            FlowId that = (FlowId) obj;
+            return Objects.equal(this.flowid, that.flowid);
+        }
+        return false;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hashCode(this.flowid);
+    }
 }

@@ -8,6 +8,42 @@ import org.onlab.onos.net.DeviceId;
  */
 public interface FlowRule {
 
+
+    public enum FlowRuleState {
+        /**
+         * Indicates that this rule has been created.
+         */
+        CREATED,
+
+        /**
+         * Indicates that this rule has been submitted for addition.
+         * Not necessarily in  the flow table.
+         */
+        PENDING_ADD,
+
+        /**
+         * Rule has been added which means it is in the flow table.
+         */
+        ADDED,
+
+        /**
+         * Flow has been marked for removal, might still be in flow table.
+         */
+        PENDING_REMOVE,
+
+        /**
+         * Flow has been removed from flow table and can be purged.
+         */
+        REMOVED
+    }
+
+    /**
+     * Returns the flow rule state.
+     *
+     * @return flow rule state
+     */
+    FlowRuleState state();
+
     //TODO: build cookie value
     /**
      * Returns the ID of this flow.
@@ -54,13 +90,6 @@ public interface FlowRule {
     long lifeMillis();
 
     /**
-     * Returns the number of milliseconds this flow rule has been idle.
-     *
-     * @return number of millis
-     */
-    long idleMillis();
-
-    /**
      * Returns the number of packets this flow rule has matched.
      *
      * @return number of packets

@@ -19,7 +19,12 @@ public class FlowRuleEvent extends AbstractEvent<FlowRuleEvent.Type, FlowRule> {
         /**
          * Signifies that a flow rule has been removed.
          */
-        RULE_REMOVED
+        RULE_REMOVED,
+
+        /**
+         * Signifies that a rule has been updated.
+         */
+        RULE_UPDATED
     }
 
     /**

 package org.onlab.onos.net.flow;
 
+import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.provider.ProviderService;
 
 /**
@@ -23,6 +24,13 @@ public interface FlowRuleProviderService extends ProviderService<FlowRuleProvide
     void flowMissing(FlowRule flowRule);
 
     /**
+     * Signals that a flow rule is on the switch but not in the store.
+     *
+     * @param flowRule the extra flow rule
+     */
+    void extraneousFlow(FlowRule flowRule);
+
+    /**
      * Signals that a flow rule was indeed added.
      *
      * @param flowRule the added flow rule
@@ -35,6 +43,8 @@ public interface FlowRuleProviderService extends ProviderService<FlowRuleProvide
      *
      * @param flowRules collection of flow rules
      */
-    void pushFlowMetrics(Iterable<FlowRule> flowRules);
+    void pushFlowMetrics(DeviceId deviceId, Iterable<FlowRule> flowRules);
+
+
 
 }

 package org.onlab.onos.net.flow;
 
-import java.util.List;
-
 import org.onlab.onos.net.DeviceId;
 
 /**
@@ -31,10 +29,8 @@ public interface FlowRuleService {
      * device reconnects to the controller.
      *
      * @param flowRules one or more flow rules
-     * throws SomeKindOfException that indicates which ones were applied and
-     *                  which ones failed
      */
-    List<FlowRule> applyFlowRules(FlowRule... flowRules);
+    void applyFlowRules(FlowRule... flowRules);
 
     /**
      * Removes the specified flow rules from their respective devices. If the

@@ -16,12 +16,18 @@ public interface FlowRuleStore {
     Iterable<FlowRule> getFlowEntries(DeviceId deviceId);
 
     /**
-     * Stores a new flow rule, and generates a FlowRule for it.
+     * Stores a new flow rule without generating events.
      *
      * @param rule the flow rule to add
-     * @return a flow entry
      */
-    FlowRule storeFlowRule(FlowRule rule);
+    void storeFlowRule(FlowRule rule);
+
+    /**
+     * Deletes a flow rule without generating events.
+     *
+     * @param rule the flow rule to delete
+     */
+    void deleteFlowRule(FlowRule rule);
 
     /**
      * Stores a new flow rule, or updates an existing entry.

@@ -3,7 +3,7 @@ package org.onlab.onos.net.flow.impl;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static org.slf4j.LoggerFactory.getLogger;
 
-import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 
 import org.apache.felix.scr.annotations.Activate;
@@ -17,7 +17,9 @@ import org.onlab.onos.event.EventDeliveryService;
 import org.onlab.onos.net.Device;
 import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.device.DeviceService;
+import org.onlab.onos.net.flow.DefaultFlowRule;
 import org.onlab.onos.net.flow.FlowRule;
+import org.onlab.onos.net.flow.FlowRule.FlowRuleState;
 import org.onlab.onos.net.flow.FlowRuleEvent;
 import org.onlab.onos.net.flow.FlowRuleListener;
 import org.onlab.onos.net.flow.FlowRuleProvider;
@@ -29,17 +31,19 @@ import org.onlab.onos.net.provider.AbstractProviderRegistry;
 import org.onlab.onos.net.provider.AbstractProviderService;
 import org.slf4j.Logger;
 
+import com.google.common.collect.Lists;
+
 @Component(immediate = true)
 @Service
 public class FlowRuleManager
-        extends AbstractProviderRegistry<FlowRuleProvider, FlowRuleProviderService>
-        implements FlowRuleService, FlowRuleProviderRegistry {
+extends AbstractProviderRegistry<FlowRuleProvider, FlowRuleProviderService>
+implements FlowRuleService, FlowRuleProviderRegistry {
 
     public static final String FLOW_RULE_NULL = "FlowRule cannot be null";
     private final Logger log = getLogger(getClass());
 
     private final AbstractListenerRegistry<FlowRuleEvent, FlowRuleListener>
-            listenerRegistry = new AbstractListenerRegistry<>();
+    listenerRegistry = new AbstractListenerRegistry<>();
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected FlowRuleStore store;
@@ -68,27 +72,24 @@ public class FlowRuleManager
     }
 
     @Override
-    public List<FlowRule> applyFlowRules(FlowRule... flowRules) {
-        List<FlowRule> entries = new ArrayList<FlowRule>();
-
+    public void applyFlowRules(FlowRule... flowRules) {
         for (int i = 0; i < flowRules.length; i++) {
-            FlowRule f = flowRules[i];
+            FlowRule f = new DefaultFlowRule(flowRules[i], FlowRuleState.PENDING_ADD);
             final Device device = deviceService.getDevice(f.deviceId());
             final FlowRuleProvider frp = getProvider(device.providerId());
-            entries.add(store.storeFlowRule(f));
+            store.storeFlowRule(f);
             frp.applyFlowRule(f);
         }
-
-        return entries;
     }
 
     @Override
     public void removeFlowRules(FlowRule... flowRules) {
+        FlowRule f;
         for (int i = 0; i < flowRules.length; i++) {
-            FlowRule f = flowRules[i];
+            f = new DefaultFlowRule(flowRules[i], FlowRuleState.PENDING_REMOVE);
             final Device device = deviceService.getDevice(f.deviceId());
             final FlowRuleProvider frp = getProvider(device.providerId());
-            store.removeFlowRule(f);
+            store.deleteFlowRule(f);
             frp.removeFlowRule(f);
         }
 
@@ -111,8 +112,8 @@ public class FlowRuleManager
     }
 
     private class InternalFlowRuleProviderService
-            extends AbstractProviderService<FlowRuleProvider>
-            implements FlowRuleProviderService {
+    extends AbstractProviderService<FlowRuleProvider>
+    implements FlowRuleProviderService {
 
         protected InternalFlowRuleProviderService(FlowRuleProvider provider) {
             super(provider);
@@ -134,22 +135,30 @@ public class FlowRuleManager
         public void flowMissing(FlowRule flowRule) {
             checkNotNull(flowRule, FLOW_RULE_NULL);
             checkValidity();
-            // TODO Auto-generated method stub
+            log.info("Flow {} has not been installed.", flowRule);
 
         }
 
         @Override
+        public void extraneousFlow(FlowRule flowRule) {
+            checkNotNull(flowRule, FLOW_RULE_NULL);
+            checkValidity();
+            log.info("Flow {} is on switch but not in store.", flowRule);
+        }
+
+        @Override
         public void flowAdded(FlowRule flowRule) {
             checkNotNull(flowRule, FLOW_RULE_NULL);
             checkValidity();
 
             FlowRuleEvent event = store.addOrUpdateFlowRule(flowRule);
             if (event == null) {
-                log.debug("Flow {} updated", flowRule);
+                log.debug("No flow store event generated.");
             } else {
-                log.debug("Flow {} added", flowRule);
+                log.debug("Flow {} {}", flowRule, event.type());
                 post(event);
             }
+
         }
 
         // Posts the specified event to the local event dispatcher.
@@ -160,9 +169,25 @@ public class FlowRuleManager
         }
 
         @Override
-        public void pushFlowMetrics(Iterable<FlowRule> flowEntries) {
-            // TODO Auto-generated method stub
-
+        public void pushFlowMetrics(DeviceId deviceId, Iterable<FlowRule> flowEntries) {
+            List<FlowRule> storedRules = Lists.newLinkedList(store.getFlowEntries(deviceId));
+
+            Iterator<FlowRule> switchRulesIterator = flowEntries.iterator();
+
+            while (switchRulesIterator.hasNext()) {
+                FlowRule rule = switchRulesIterator.next();
+                if (storedRules.remove(rule)) {
+                    // we both have the rule, let's update some info then.
+                    flowAdded(rule);
+                } else {
+                    // the device has a rule the store does not have
+                    extraneousFlow(rule);
+                }
+            }
+            for (FlowRule rule : storedRules) {
+                // there are rules in the store that aren't on the switch
+                flowMissing(rule);
+            }
         }
     }
 

@@ -6,6 +6,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_ADDED;
 import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_REMOVED;
+import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_UPDATED;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -25,6 +26,7 @@ import org.onlab.onos.net.device.DeviceListener;
 import org.onlab.onos.net.device.DeviceService;
 import org.onlab.onos.net.flow.DefaultFlowRule;
 import org.onlab.onos.net.flow.FlowRule;
+import org.onlab.onos.net.flow.FlowRule.FlowRuleState;
 import org.onlab.onos.net.flow.FlowRuleEvent;
 import org.onlab.onos.net.flow.FlowRuleListener;
 import org.onlab.onos.net.flow.FlowRuleProvider;
@@ -37,10 +39,10 @@ import org.onlab.onos.net.flow.criteria.Criterion;
 import org.onlab.onos.net.flow.instructions.Instruction;
 import org.onlab.onos.net.provider.AbstractProvider;
 import org.onlab.onos.net.provider.ProviderId;
+import org.onlab.onos.net.trivial.impl.SimpleFlowRuleStore;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
-import org.onlab.onos.net.trivial.impl.SimpleFlowRuleStore;
 
 /**
  * Test codifying the flow rule service & flow rule provider service contracts.
@@ -56,7 +58,7 @@ public class FlowRuleManagerTest {
 
     protected FlowRuleService service;
     protected FlowRuleProviderRegistry registry;
-    protected FlowRuleProviderService providerSerivce;
+    protected FlowRuleProviderService providerService;
     protected TestProvider provider;
     protected TestListener listener = new TestListener();
 
@@ -72,7 +74,7 @@ public class FlowRuleManagerTest {
         mgr.activate();
         mgr.addListener(listener);
         provider = new TestProvider(PID);
-        providerSerivce = registry.register(provider);
+        providerService = registry.register(provider);
         assertTrue("provider should be registered",
                 registry.getProviders().contains(provider.id()));
     }
@@ -94,10 +96,15 @@ public class FlowRuleManagerTest {
         return new DefaultFlowRule(DID, ts, tr, 0);
     }
 
-    private void addFlowRule(int hval) {
+    private FlowRule flowRule(FlowRule rule, FlowRuleState state) {
+        return new DefaultFlowRule(rule, state);
+    }
+
+    private FlowRule addFlowRule(int hval) {
         FlowRule rule = flowRule(hval, hval);
-        providerSerivce.flowAdded(rule);
+        providerService.flowAdded(rule);
         assertNotNull("rule should be found", service.getFlowEntries(DID));
+        return rule;
     }
 
     private void validateEvents(FlowRuleEvent.Type ... events) {
@@ -131,60 +138,91 @@ public class FlowRuleManagerTest {
 
         addFlowRule(1);
         assertEquals("should still be 2 rules", 2, flowCount());
-        validateEvents();
+        validateEvents(RULE_UPDATED);
+    }
+
+
+    //backing store is sensitive to the order of additions/removals
+    private boolean validateState(FlowRuleState... state) {
+        Iterable<FlowRule> rules = service.getFlowEntries(DID);
+        int i = 0;
+        for (FlowRule f : rules) {
+            if (f.state() != state[i]) {
+                return false;
+            }
+            i++;
+        }
+        return true;
     }
 
     @Test
     public void applyFlowRules() {
-        TestSelector ts = new TestSelector(1);
+
         FlowRule r1 = flowRule(1, 1);
         FlowRule r2 = flowRule(1, 2);
         FlowRule r3 = flowRule(1, 3);
 
-        //current FlowRules always return 0. FlowEntries inherit the value
-        FlowRule e1 = new DefaultFlowRule(DID, ts, r1.treatment(), 0);
-        FlowRule e2 = new DefaultFlowRule(DID, ts, r2.treatment(), 0);
-        FlowRule e3 = new DefaultFlowRule(DID, ts, r3.treatment(), 0);
-        List<FlowRule> fel = Lists.newArrayList(e1, e2, e3);
-
         assertTrue("store should be empty",
                 Sets.newHashSet(service.getFlowEntries(DID)).isEmpty());
-        List<FlowRule> ret = mgr.applyFlowRules(r1, r2, r3);
+        mgr.applyFlowRules(r1, r2, r3);
         assertEquals("3 rules should exist", 3, flowCount());
-        assertTrue("3 entries should result", fel.containsAll(ret));
+        assertTrue("Entries should be pending add.",
+                validateState(FlowRuleState.PENDING_ADD, FlowRuleState.PENDING_ADD,
+                        FlowRuleState.PENDING_ADD));
     }
 
     @Test
     public void removeFlowRules() {
-        addFlowRule(1);
-        addFlowRule(2);
+        FlowRule f1 = addFlowRule(1);
+        FlowRule f2 = addFlowRule(2);
         addFlowRule(3);
         assertEquals("3 rules should exist", 3, flowCount());
         validateEvents(RULE_ADDED, RULE_ADDED, RULE_ADDED);
 
-        FlowRule rem1 = flowRule(1, 1);
-        FlowRule rem2 = flowRule(2, 2);
+        FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
+        FlowRule rem2 = flowRule(f2, FlowRuleState.REMOVED);
         mgr.removeFlowRules(rem1, rem2);
         //removing from north, so no events generated
         validateEvents();
-        assertEquals("1 rule should exist", 1, flowCount());
+        assertEquals("3 rule should exist", 3, flowCount());
+        assertTrue("Entries should be pending remove.",
+                validateState(FlowRuleState.CREATED, FlowRuleState.PENDING_REMOVE,
+                        FlowRuleState.PENDING_REMOVE));
 
         mgr.removeFlowRules(rem1);
-        assertEquals("1 rule should still exist", 1, flowCount());
+        assertEquals("3 rule should still exist", 3, flowCount());
     }
 
     @Test
     public void flowRemoved() {
-        addFlowRule(1);
+        FlowRule f1 = addFlowRule(1);
         addFlowRule(2);
-        FlowRule rem1 = flowRule(1, 1);
-        providerSerivce.flowRemoved(rem1);
+        FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
+        providerService.flowRemoved(rem1);
         validateEvents(RULE_ADDED, RULE_ADDED, RULE_REMOVED);
 
-        providerSerivce.flowRemoved(rem1);
+        providerService.flowRemoved(rem1);
         validateEvents();
     }
 
+    @Test
+    public void flowMetrics() {
+        FlowRule f1 = flowRule(1, 1);
+        FlowRule f2 = flowRule(2, 2);
+        FlowRule f3 = flowRule(3, 3);
+
+        FlowRule updatedF1 = flowRule(f1, FlowRuleState.ADDED);
+        FlowRule updatedF2 = flowRule(f2, FlowRuleState.ADDED);
+        mgr.applyFlowRules(f1, f2, f3);
+
+        providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2));
+
+        assertTrue("Entries should be added.",
+                validateState(FlowRuleState.PENDING_ADD, FlowRuleState.ADDED,
+                        FlowRuleState.ADDED));
+        //TODO: add tests for flowmissing and extraneous flows
+    }
+
     private static class TestListener implements FlowRuleListener {
         final List<FlowRuleEvent> events = new ArrayList<>();
 

 package org.onlab.onos.net.trivial.impl;
 
-import com.google.common.collect.HashMultimap;
-import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Multimap;
+import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_ADDED;
+import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_REMOVED;
+import static org.slf4j.LoggerFactory.getLogger;
+
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Service;
 import org.onlab.onos.net.DeviceId;
-import org.onlab.onos.net.flow.DefaultFlowRule;
 import org.onlab.onos.net.flow.FlowRule;
 import org.onlab.onos.net.flow.FlowRuleEvent;
+import org.onlab.onos.net.flow.FlowRuleEvent.Type;
 import org.onlab.onos.net.flow.FlowRuleStore;
 import org.slf4j.Logger;
 
-import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_ADDED;
-import static org.onlab.onos.net.flow.FlowRuleEvent.Type.RULE_REMOVED;
-import static org.slf4j.LoggerFactory.getLogger;
+import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Multimap;
 
 /**
  * Manages inventory of flow rules using trivial in-memory implementation.
@@ -28,7 +29,7 @@ public class SimpleFlowRuleStore implements FlowRuleStore {
     private final Logger log = getLogger(getClass());
 
     // store entries as a pile of rules, no info about device tables
-    private final Multimap<DeviceId, FlowRule> flowEntries = HashMultimap.create();
+    private final Multimap<DeviceId, FlowRule> flowEntries = ArrayListMultimap.create();
 
     @Activate
     public void activate() {
@@ -46,12 +47,27 @@ public class SimpleFlowRuleStore implements FlowRuleStore {
     }
 
     @Override
-    public FlowRule storeFlowRule(FlowRule rule) {
+    public void storeFlowRule(FlowRule rule) {
+        DeviceId did = rule.deviceId();
+        flowEntries.put(did, rule);
+    }
+
+    @Override
+    public void deleteFlowRule(FlowRule rule) {
         DeviceId did = rule.deviceId();
-        FlowRule entry = new DefaultFlowRule(did,
-                                             rule.selector(), rule.treatment(), rule.priority());
-        flowEntries.put(did, entry);
-        return entry;
+
+        /*
+         *  find the rule and mark it for deletion.
+         *  Ultimately a flow removed will come remove it.
+         */
+
+        if (flowEntries.containsEntry(did, rule)) {
+            synchronized (flowEntries) {
+
+                flowEntries.remove(did, rule);
+                flowEntries.put(did, rule);
+            }
+        }
     }
 
     @Override
@@ -59,12 +75,17 @@ public class SimpleFlowRuleStore implements FlowRuleStore {
         DeviceId did = rule.deviceId();
 
         // check if this new rule is an update to an existing entry
-        for (FlowRule fe : flowEntries.get(did)) {
-            if (rule.equals(fe)) {
-                // TODO update the stats on this FlowRule?
-                return null;
+        if (flowEntries.containsEntry(did, rule)) {
+            synchronized (flowEntries) {
+                // Multimaps support duplicates so we have to remove our rule
+                // and replace it with the current version.
+
+                flowEntries.remove(did, rule);
+                flowEntries.put(did, rule);
             }
+            return new FlowRuleEvent(Type.RULE_UPDATED, rule);
         }
+
         flowEntries.put(did, rule);
         return new FlowRuleEvent(RULE_ADDED, rule);
     }
@@ -80,4 +101,6 @@ public class SimpleFlowRuleStore implements FlowRuleStore {
         }
     }
 
+
+
 }

@@ -88,6 +88,24 @@ public class FlowModBuilder {
 
     }
 
+    public OFFlowMod buildFlowDel() {
+        Match match = buildMatch();
+        List<OFAction> actions = buildActions();
+
+        OFFlowMod fm = factory.buildFlowDelete()
+                .setCookie(U64.of(cookie.value()))
+                .setBufferId(OFBufferId.NO_BUFFER)
+                .setActions(actions)
+                .setMatch(match)
+                .setFlags(Collections.singleton(OFFlowModFlags.SEND_FLOW_REM))
+                .setIdleTimeout(10)
+                .setHardTimeout(10)
+                .setPriority(priority)
+                .build();
+
+        return fm;
+    }
+
     private List<OFAction> buildActions() {
         List<OFAction> acts = new LinkedList<>();
         for (Instruction i : treatment.instructions()) {
@@ -246,4 +264,6 @@ public class FlowModBuilder {
         return mBuilder.build();
     }
 
+
+
 }

@@ -10,6 +10,7 @@ import org.onlab.onos.net.flow.DefaultFlowRule;
 import org.onlab.onos.net.flow.DefaultTrafficSelector;
 import org.onlab.onos.net.flow.DefaultTrafficTreatment;
 import org.onlab.onos.net.flow.FlowRule;
+import org.onlab.onos.net.flow.FlowRule.FlowRuleState;
 import org.onlab.onos.net.flow.TrafficSelector;
 import org.onlab.onos.net.flow.TrafficTreatment;
 import org.onlab.onos.net.flow.criteria.Criteria;
@@ -52,7 +53,7 @@ public class FlowRuleBuilder {
         this.match = entry.getMatch();
         this.actions = entry.getActions();
         this.dpid = dpid;
-        removed = null;
+        this.removed = null;
     }
 
     public FlowRuleBuilder(Dpid dpid, OFFlowRemoved removed) {
@@ -69,16 +70,16 @@ public class FlowRuleBuilder {
         if (stat != null) {
             return new DefaultFlowRule(DeviceId.deviceId(Dpid.uri(dpid)),
                     buildSelector(), buildTreatment(), stat.getPriority(),
-                    stat.getDurationNsec() / 1000000, stat.getIdleTimeout(),
+                    FlowRuleState.ADDED, stat.getDurationNsec() / 1000000,
                     stat.getPacketCount().getValue(), stat.getByteCount().getValue(),
-                    (int) (stat.getCookie().getValue() & 0xFFFFFFFF));
+                    stat.getCookie().getValue());
         } else {
             // TODO: revisit potentially.
             return new DefaultFlowRule(DeviceId.deviceId(Dpid.uri(dpid)),
                     buildSelector(), null, removed.getPriority(),
-                    removed.getDurationNsec() / 1000000, removed.getIdleTimeout(),
+                    FlowRuleState.REMOVED, removed.getDurationNsec() / 1000000,
                     removed.getPacketCount().getValue(), removed.getByteCount().getValue(),
-                    (int) (removed.getCookie().getValue() & 0xFFFFFFFF));
+                    removed.getCookie().getValue());
         }
     }
 

@@ -10,6 +10,7 @@ import org.apache.felix.scr.annotations.Component;
 import org.apache.felix.scr.annotations.Deactivate;
 import org.apache.felix.scr.annotations.Reference;
 import org.apache.felix.scr.annotations.ReferenceCardinality;
+import org.onlab.onos.net.DeviceId;
 import org.onlab.onos.net.flow.FlowRule;
 import org.onlab.onos.net.flow.FlowRuleProvider;
 import org.onlab.onos.net.flow.FlowRuleProviderRegistry;
@@ -94,8 +95,16 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
 
     @Override
     public void removeFlowRule(FlowRule... flowRules) {
-        // TODO Auto-generated method stub
+        for (int i = 0; i < flowRules.length; i++) {
+            removeRule(flowRules[i]);
+        }
+
+    }
+
 
+    private void removeRule(FlowRule flowRule) {
+        OpenFlowSwitch sw = controller.getSwitch(Dpid.dpid(flowRule.deviceId().uri()));
+        sw.sendMsg(new FlowModBuilder(flowRule, sw.factory()).buildFlowDel());
     }
 
 
@@ -108,7 +117,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
 
         @Override
         public void switchAdded(Dpid dpid) {
-            FlowStatsCollector fsc = new FlowStatsCollector(controller.getSwitch(dpid), 1);
+            FlowStatsCollector fsc = new FlowStatsCollector(controller.getSwitch(dpid), 5);
             fsc.start();
             collectors.put(dpid, fsc);
         }
@@ -154,7 +163,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
                 entries.add(new FlowRuleBuilder(dpid, reply).build());
             }
             log.debug("sending flowstats to core {}", entries);
-            providerService.pushFlowMetrics(entries);
+            providerService.pushFlowMetrics(DeviceId.deviceId(Dpid.uri(dpid)), entries);
         }
 
     }

+package org.onlab.packet;
+
+import java.util.Arrays;
+
+/**
+ * A class representing an IPv4 address.
+ * <p/>
+ * TODO this class is a clone of IpPrefix and still needs to be modified to
+ * look more like an IpAddress.
+ */
+public final class IpAddress {
+
+    // TODO a comparator for netmasks? E.g. for sorting by prefix match order.
+
+    //IP Versions
+    public enum Version { INET, INET6 };
+
+    //lengths of address, in bytes
+    public static final int INET_LEN = 4;
+    public static final int INET6_LEN = 16;
+
+    //maximum CIDR value
+    public static final int MAX_INET_MASK = 32;
+    //no mask (no network), e.g. a simple address
+    public static final int DEFAULT_MASK = 0;
+
+    /**
+     * Default value indicating an unspecified address.
+     */
+    static final byte[] ANY = new byte [] {0, 0, 0, 0};
+
+    protected Version version;
+
+    protected byte[] octets;
+    protected int netmask;
+
+    private IpAddress(Version ver, byte[] octets, int netmask) {
+        this.version = ver;
+        this.octets = Arrays.copyOf(octets, INET_LEN);
+        this.netmask = netmask;
+    }
+
+    private IpAddress(Version ver, byte[] octets) {
+        this.version = ver;
+        this.octets = Arrays.copyOf(octets, INET_LEN);
+        this.netmask = DEFAULT_MASK;
+    }
+
+    /**
+     * Converts a byte array into an IP address.
+     *
+     * @param address a byte array
+     * @return an IP address
+     */
+    public static IpAddress valueOf(byte [] address) {
+        return new IpAddress(Version.INET, address);
+    }
+
+    /**
+     * Converts a byte array into an IP address.
+     *
+     * @param address a byte array
+     * @param netmask the CIDR value subnet mask
+     * @return an IP address
+     */
+    public static IpAddress valueOf(byte [] address, int netmask) {
+        return new IpAddress(Version.INET, address, netmask);
+    }
+
+    /**
+     * Helper to convert an integer into a byte array.
+     *
+     * @param address the integer to convert
+     * @return a byte array
+     */
+    private static byte [] bytes(int address) {
+        byte [] bytes = new byte [INET_LEN];
+        for (int i = 0; i < INET_LEN; i++) {
+            bytes[i] = (byte) ((address >> (INET_LEN - (i + 1)) * 8) & 0xff);
+        }
+
+        return bytes;
+    }
+
+    /**
+     * Converts an integer into an IPv4 address.
+     *
+     * @param address an integer representing an IP value
+     * @return an IP address
+     */
+    public static IpAddress valueOf(int address) {
+        return new IpAddress(Version.INET, bytes(address));
+    }
+
+    /**
+     * Converts an integer into an IPv4 address.
+     *
+     * @param address an integer representing an IP value
+     * @param netmask the CIDR value subnet mask
+     * @return an IP address
+     */
+    public static IpAddress valueOf(int address, int netmask) {
+        return new IpAddress(Version.INET, bytes(address), netmask);
+    }
+
+    /**
+     * Converts a dotted-decimal string (x.x.x.x) into an IPv4 address. The
+     * string can also be in CIDR (slash) notation. If the netmask is omitted,
+     * it will be set to DEFAULT_MASK (0).
+     *
+     * @param address a IP address in string form, e.g. "10.0.0.1", "10.0.0.1/24"
+     * @return an IP address
+     */
+    public static IpAddress valueOf(String address) {
+
+        final String [] parts = address.split("\\/");
+        if (parts.length > 2) {
+            throw new IllegalArgumentException("Malformed IP address string; "
+                    + "Address must take form \"x.x.x.x\" or \"x.x.x.x/y\"");
+        }
+
+        int mask = DEFAULT_MASK;
+        if (parts.length == 2) {
+            mask = Integer.valueOf(parts[1]);
+            if (mask > MAX_INET_MASK) {
+                throw new IllegalArgumentException(
+                        "Value of subnet mask cannot exceed "
+                                + MAX_INET_MASK);
+            }
+        }
+
+        final String [] net = parts[0].split("\\.");
+        if (net.length != INET_LEN) {
+            throw new IllegalArgumentException("Malformed IP address string; "
+                    + "Address must have four decimal values separated by dots (.)");
+        }
+        final byte [] bytes = new byte[INET_LEN];
+        for (int i = 0; i < INET_LEN; i++) {
+            bytes[i] = (byte) Short.parseShort(net[i], 10);
+        }
+        return new IpAddress(Version.INET, bytes, mask);
+    }
+
+    /**
+     * Returns the IP version of this address.
+     *
+     * @return the version
+     */
+    public Version version() {
+        return this.version;
+    }
+
+    /**
+     * Returns the IP address as a byte array.
+     *
+     * @return a byte array
+     */
+    public byte[] toOctets() {
+        return Arrays.copyOf(this.octets, INET_LEN);
+    }
+
+    /**
+     * Returns the IP address prefix length.
+     *
+     * @return prefix length
+     */
+    public int prefixLength() {
+        return netmask;
+    }
+
+    /**
+     * Returns the integral value of this IP address.
+     *
+     * @return the IP address's value as an integer
+     */
+    public int toInt() {
+        int address = 0;
+        for (int i = 0; i < INET_LEN; i++) {
+            address |= octets[i] << ((INET_LEN - (i + 1)) * 8);
+        }
+        return address;
+    }
+
+    /**
+     * Helper for computing the mask value from CIDR.
+     *
+     * @return an integer bitmask
+     */
+    private int mask() {
+        int shift = MAX_INET_MASK - this.netmask;
+        return ((Integer.MAX_VALUE >>> (shift - 1)) << shift);
+    }
+
+    /**
+     * Returns the subnet mask in IpAddress form. The netmask value for
+     * the returned IpAddress is 0, as the address itself is a mask.
+     *
+     * @return the subnet mask
+     */
+    public IpAddress netmask() {
+        return new IpAddress(Version.INET, bytes(mask()));
+    }
+
+    /**
+     * Returns the network portion of this address as an IpAddress.
+     * The netmask of the returned IpAddress is the current mask. If this
+     * address doesn't have a mask, this returns an all-0 IpAddress.
+     *
+     * @return the network address or null
+     */
+    public IpAddress network() {
+        if (netmask == DEFAULT_MASK) {
+            return new IpAddress(version, ANY, DEFAULT_MASK);
+        }
+
+        byte [] net = new byte [4];
+        byte [] mask = bytes(mask());
+        for (int i = 0; i < INET_LEN; i++) {
+            net[i] = (byte) (octets[i] & mask[i]);
+        }
+        return new IpAddress(version, net, netmask);
+    }
+
+    /**
+     * Returns the host portion of the IPAddress, as an IPAddress.
+     * The netmask of the returned IpAddress is the current mask. If this
+     * address doesn't have a mask, this returns a copy of the current
+     * address.
+     *
+     * @return the host address
+     */
+    public IpAddress host() {
+        if (netmask == DEFAULT_MASK) {
+            new IpAddress(version, octets, netmask);
+        }
+
+        byte [] host = new byte [INET_LEN];
+        byte [] mask = bytes(mask());
+        for (int i = 0; i < INET_LEN; i++) {
+            host[i] = (byte) (octets[i] & ~mask[i]);
+        }
+        return new IpAddress(version, host, netmask);
+    }
+
+    public boolean isMasked() {
+        return mask() != 0;
+    }
+
+    /**
+     * Determines whether a given address is contained within this IpAddress'
+     * network.
+     *
+     * @param other another IP address that could be contained in this network
+     * @return true if the other IP address is contained in this address'
+     * network, otherwise false
+     */
+    public boolean contains(IpAddress other) {
+        if (this.netmask <= other.netmask) {
+            // Special case where they're both /32 addresses
+            if (this.netmask == MAX_INET_MASK) {
+                return Arrays.equals(octets, other.octets);
+            }
+
+            // Mask the other address with our network mask
+            IpAddress otherMasked =
+                    IpAddress.valueOf(other.octets, netmask).network();
+
+            return network().equals(otherMasked);
+        }
+        return false;
+    }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + netmask;
+        result = prime * result + Arrays.hashCode(octets);
+        result = prime * result + ((version == null) ? 0 : version.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj == null) {
+            return false;
+        }
+        if (getClass() != obj.getClass()) {
+            return false;
+        }
+        IpAddress other = (IpAddress) obj;
+        if (netmask != other.netmask) {
+            return false;
+        }
+        if (!Arrays.equals(octets, other.octets)) {
+            return false;
+        }
+        if (version != other.version) {
+            return false;
+        }
+        return true;
+    }
+
+    @Override
+    /*
+     * (non-Javadoc)
+     * format is "x.x.x.x" for non-masked (netmask 0) addresses,
+     * and "x.x.x.x/y" for masked addresses.
+     *
+     * @see java.lang.Object#toString()
+     */
+    public String toString() {
+        final StringBuilder builder = new StringBuilder();
+        for (final byte b : this.octets) {
+            if (builder.length() > 0) {
+                builder.append(".");
+            }
+            builder.append(String.format("%d", b & 0xff));
+        }
+        if (netmask != DEFAULT_MASK) {
+            builder.append("/");
+            builder.append(String.format("%d", netmask));
+        }
+        return builder.toString();
+    }
+
+}

@@ -3,7 +3,9 @@ package org.onlab.packet;
 import java.util.Arrays;
 
 /**
- * A class representing an IPv4 address.
+ * A class representing an IPv4 prefix.
+ * <p/>
+ * A prefix consists of an IP address and a subnet mask.
  */
 public final class IpPrefix {