flowrule states functional

Change-Id: Id310f146d4ef2a59993f31d60062464a24df4560

 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 packets;
     private final long bytes;
     private final FlowRuleState state;
 
-
-    public 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.packets = 0;
-        this.bytes = 0;
-        this.id = FlowId.valueOf(this.hashCode());
-        this.created = System.currentTimeMillis();
-    }
+    private final FlowId id;
 
     public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector,
             TrafficTreatment treatment, int priority, FlowRuleState state,
@@ -59,7 +48,37 @@ public class DefaultFlowRule implements FlowRule {
 
     public DefaultFlowRule(FlowRule rule, FlowRuleState state) {
         this(rule.deviceId(), rule.selector(), rule.treatment(),
-                rule.priority(), state);
+                rule.priority(), state, rule.id());
+    }
+
+    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.packets = 0;
+        this.bytes = 0;
+        this.id = FlowId.valueOf(this.hashCode());
+        this.created = System.currentTimeMillis();
+    }
+
+    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.state = state;
+        this.life = 0;
+        this.packets = 0;
+        this.bytes = 0;
+        this.id = flowId;
+        this.created = System.currentTimeMillis();
     }
 
 
@@ -128,13 +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) {
-            FlowRule that = (FlowRule) obj;
-            return Objects.equals(deviceId, that.deviceId()) &&
-                    Objects.equals(id, that.id());
+            DefaultFlowRule that = (DefaultFlowRule) obj;
+            return Objects.equals(deviceId, that.deviceId) &&
+                    Objects.equals(id, that.id);
         }
         return false;
     }

@@ -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
     }
 
     /**

@@ -24,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
@@ -38,4 +45,6 @@ public interface FlowRuleProviderService extends ProviderService<FlowRuleProvide
      */
     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,6 @@ 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;
 
@@ -73,18 +72,14 @@ implements FlowRuleService, FlowRuleProviderRegistry {
     }
 
     @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 = 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
@@ -93,7 +88,7 @@ implements FlowRuleService, FlowRuleProviderRegistry {
             FlowRule 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);
         }
 
@@ -139,22 +134,30 @@ implements FlowRuleService, FlowRuleProviderRegistry {
         public void flowMissing(FlowRule flowRule) {
             checkNotNull(flowRule, FLOW_RULE_NULL);
             checkValidity();
-            // TODO Auto-generated method stub
+            log.info("Flow {} has not been installed.");
 
         }
 
         @Override
+        public void extraneousFlow(FlowRule flowRule) {
+            checkNotNull(flowRule, FLOW_RULE_NULL);
+            checkValidity();
+            log.info("Flow {} is on switch but not in store.");
+        }
+
+        @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.
@@ -167,31 +170,23 @@ implements FlowRuleService, FlowRuleProviderRegistry {
         @Override
         public void pushFlowMetrics(DeviceId deviceId, Iterable<FlowRule> flowEntries) {
             List<FlowRule> storedRules = Lists.newLinkedList(store.getFlowEntries(deviceId));
-            List<FlowRule> switchRules = Lists.newLinkedList(flowEntries);
-            Iterator<FlowRule> switchRulesIterator = switchRules.iterator();
-            List<FlowRule> extraRules = Lists.newLinkedList();
+            //List<FlowRule> switchRules = Lists.newLinkedList(flowEntries);
+            Iterator<FlowRule> switchRulesIterator = flowEntries.iterator(); //switchRules.iterator();
 
             while (switchRulesIterator.hasNext()) {
                 FlowRule rule = switchRulesIterator.next();
                 if (storedRules.remove(rule)) {
-                    // we both have the rule let's update some info then.
-                    log.info("rule {} is added. {}", rule.id(), rule.state());
+                    // we both have the rule, let's update some info then.
                     flowAdded(rule);
                 } else {
-                    // the device a rule the store does not have
-                    extraRules.add(rule);
+                    // 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);
             }
-            if (extraRules.size() > 0) {
-                log.warn("Device {} has extra flow rules: {}", deviceId, extraRules);
-                // TODO do something with this.
-            }
-
-
         }
     }
 

@@ -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;
@@ -37,10 +38,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.
@@ -131,7 +132,7 @@ public class FlowRuleManagerTest {
 
         addFlowRule(1);
         assertEquals("should still be 2 rules", 2, flowCount());
-        validateEvents();
+        validateEvents(RULE_UPDATED);
     }
 
     @Test
@@ -149,9 +150,9 @@ public class FlowRuleManagerTest {
 
         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("3 entries should result", fel.containsAll(Lists.newArrayList(r1, r2, r3)));
     }
 
     @Test
@@ -167,10 +168,10 @@ public class FlowRuleManagerTest {
         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());
 
         mgr.removeFlowRules(rem1);
-        assertEquals("1 rule should still exist", 1, flowCount());
+        assertEquals("3 rule should still exist", 3, flowCount());
     }
 
     @Test

 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,26 @@ 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 +74,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 +100,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();
     }
 
+
+
 }

@@ -53,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) {
@@ -72,14 +72,14 @@ public class FlowRuleBuilder {
                     buildSelector(), buildTreatment(), stat.getPriority(),
                     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(),
                     FlowRuleState.REMOVED, removed.getDurationNsec() / 1000000,
                     removed.getPacketCount().getValue(), removed.getByteCount().getValue(),
-                    (int) (removed.getCookie().getValue() & 0xFFFFFFFF));
+                    removed.getCookie().getValue());
         }
     }
 

@@ -95,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());
     }
 
 
@@ -109,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);
         }