flowrules no longer install a timeout but are monitored by onos in order to be expired

Change-Id: Ibd1a5952349d7ccb27c92b4982d04574f31424c0

@@ -38,6 +38,8 @@ import org.slf4j.Logger;
 @Component(immediate = true)
 public class ReactiveForwarding {
 
+    private static final int TIMEOUT = 10;
+
     private final Logger log = getLogger(getClass());
 
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
@@ -192,7 +194,7 @@ public class ReactiveForwarding {
             treat.setOutput(portNumber);
 
             FlowRule f = new DefaultFlowRule(context.inPacket().receivedFrom().deviceId(),
-                    builder.build(), treat.build(), 0, appId);
+                    builder.build(), treat.build(), 0, appId, TIMEOUT);
 
             flowRuleService.applyFlowRules(f);
         }

@@ -27,11 +27,12 @@ public class DefaultFlowRule implements FlowRule {
 
     private final ApplicationId appId;
 
-    private boolean expired;
+    private final int timeout;
 
     public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector,
             TrafficTreatment treatment, int priority, FlowRuleState state,
-            long life, long packets, long bytes, long flowId, boolean expired) {
+            long life, long packets, long bytes, long flowId, boolean expired,
+            int timeout) {
         this.deviceId = deviceId;
         this.priority = priority;
         this.selector = selector;
@@ -39,26 +40,30 @@ public class DefaultFlowRule implements FlowRule {
         this.state = state;
         this.appId = ApplicationId.valueOf((int) (flowId >> 32));
         this.id = FlowId.valueOf(flowId);
-        this.expired = expired;
         this.life = life;
         this.packets = packets;
         this.bytes = bytes;
         this.created = System.currentTimeMillis();
+        this.timeout = timeout;
     }
 
     public DefaultFlowRule(DeviceId deviceId, TrafficSelector selector,
-            TrafficTreatment treatement, int priority, ApplicationId appId) {
-        this(deviceId, selector, treatement, priority, FlowRuleState.CREATED, appId);
+            TrafficTreatment treatement, int priority, ApplicationId appId,
+            int timeout) {
+        this(deviceId, selector, treatement, priority,
+                FlowRuleState.CREATED, appId, timeout);
     }
 
     public DefaultFlowRule(FlowRule rule, FlowRuleState state) {
         this(rule.deviceId(), rule.selector(), rule.treatment(),
-                rule.priority(), state, rule.id(), rule.appId());
+                rule.priority(), state, rule.id(), rule.appId(),
+                rule.timeout());
     }
 
     private DefaultFlowRule(DeviceId deviceId,
             TrafficSelector selector, TrafficTreatment treatment,
-            int priority, FlowRuleState state, ApplicationId appId) {
+            int priority, FlowRuleState state, ApplicationId appId,
+            int timeout) {
         this.deviceId = deviceId;
         this.priority = priority;
         this.selector = selector;
@@ -69,13 +74,16 @@ public class DefaultFlowRule implements FlowRule {
         this.bytes = 0;
         this.appId = appId;
 
+        this.timeout = timeout;
+
         this.id = FlowId.valueOf((((long) appId().id()) << 32) | (this.hash() & 0xffffffffL));
         this.created = System.currentTimeMillis();
     }
 
     private DefaultFlowRule(DeviceId deviceId,
             TrafficSelector selector, TrafficTreatment treatment,
-            int priority, FlowRuleState state, FlowId flowId, ApplicationId appId) {
+            int priority, FlowRuleState state, FlowId flowId, ApplicationId appId,
+            int timeout) {
         this.deviceId = deviceId;
         this.priority = priority;
         this.selector = selector;
@@ -86,6 +94,7 @@ public class DefaultFlowRule implements FlowRule {
         this.bytes = 0;
         this.appId = appId;
         this.id = flowId;
+        this.timeout = timeout;
         this.created = System.currentTimeMillis();
     }
 
@@ -149,7 +158,7 @@ public class DefaultFlowRule implements FlowRule {
      * @see java.lang.Object#equals(java.lang.Object)
      */
     public int hashCode() {
-        return Objects.hash(deviceId, id);
+        return Objects.hash(deviceId, selector, priority);
     }
 
     public int hash() {
@@ -170,7 +179,10 @@ public class DefaultFlowRule implements FlowRule {
         if (obj instanceof DefaultFlowRule) {
             DefaultFlowRule that = (DefaultFlowRule) obj;
             return Objects.equals(deviceId, that.deviceId) &&
-                    Objects.equals(id, that.id);
+                    //Objects.equals(id, that.id) &&
+                    Objects.equals(priority, that.priority) &&
+                    Objects.equals(selector, that.selector);
+
         }
         return false;
     }
@@ -181,16 +193,16 @@ public class DefaultFlowRule implements FlowRule {
                 .add("id", id)
                 .add("deviceId", deviceId)
                 .add("priority", priority)
-                .add("selector", selector)
-                .add("treatment", treatment)
+                .add("selector", selector.criteria())
+                .add("treatment", treatment == null ? "N/A" : treatment.instructions())
                 .add("created", created)
                 .add("state", state)
                 .toString();
     }
 
     @Override
-    public boolean expired() {
-        return expired;
+    public int timeout() {
+        return timeout > MAX_TIMEOUT ? MAX_TIMEOUT : this.timeout;
     }
 
 }

@@ -3,8 +3,9 @@ package org.onlab.onos.net.flow;
 import static org.slf4j.LoggerFactory.getLogger;
 
 import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Objects;
+import java.util.Set;
 
 import org.onlab.onos.net.PortNumber;
 import org.onlab.onos.net.flow.criteria.Criteria;
@@ -16,22 +17,42 @@ import org.slf4j.Logger;
 
 public final class DefaultTrafficSelector implements TrafficSelector {
 
-    private final List<Criterion> selector;
+    private final Set<Criterion> selector;
 
-    private DefaultTrafficSelector(List<Criterion> selector) {
-        this.selector = Collections.unmodifiableList(selector);
+    private DefaultTrafficSelector(Set<Criterion> selector) {
+        this.selector = Collections.unmodifiableSet(selector);
     }
 
     @Override
-    public List<Criterion> criteria() {
+    public Set<Criterion> criteria() {
         return selector;
     }
 
+    @Override
+    public int hashCode() {
+        return Objects.hash(selector);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj instanceof DefaultTrafficSelector) {
+            DefaultTrafficSelector that = (DefaultTrafficSelector) obj;
+            return Objects.equals(selector, that.selector);
+
+        }
+        return false;
+    }
+
+
+
     public static class Builder implements TrafficSelector.Builder {
 
         private final Logger log = getLogger(getClass());
 
-        private final List<Criterion> selector = new LinkedList<>();
+        private final Set<Criterion> selector = new HashSet<>();
 
         @Override
         public Builder add(Criterion criterion) {
@@ -39,38 +60,47 @@ public final class DefaultTrafficSelector implements TrafficSelector {
             return this;
         }
 
+        @Override
         public Builder matchInport(PortNumber port) {
             return add(Criteria.matchInPort(port));
         }
 
+        @Override
         public Builder matchEthSrc(MacAddress addr) {
             return add(Criteria.matchEthSrc(addr));
         }
 
+        @Override
         public Builder matchEthDst(MacAddress addr) {
             return add(Criteria.matchEthDst(addr));
         }
 
+        @Override
         public Builder matchEthType(short ethType) {
             return add(Criteria.matchEthType(ethType));
         }
 
+        @Override
         public Builder matchVlanId(VlanId vlanId) {
             return add(Criteria.matchVlanId(vlanId));
         }
 
+        @Override
         public Builder matchVlanPcp(Byte vlanPcp) {
             return add(Criteria.matchVlanPcp(vlanPcp));
         }
 
+        @Override
         public Builder matchIPProtocol(Byte proto) {
             return add(Criteria.matchIPProtocol(proto));
         }
 
+        @Override
         public Builder matchIPSrc(IpPrefix ip) {
             return add(Criteria.matchIPSrc(ip));
         }
 
+        @Override
         public Builder matchIPDst(IpPrefix ip) {
             return add(Criteria.matchIPDst(ip));
         }

@@ -9,6 +9,7 @@ import org.onlab.onos.net.DeviceId;
  */
 public interface FlowRule {
 
+    static final int MAX_TIMEOUT = 60;
 
     public enum FlowRuleState {
         /**
@@ -112,10 +113,9 @@ public interface FlowRule {
     long bytes();
 
     /**
-     * Indicates that this flow has expired at the device.
-     *
-     * @return true if it has expired, false otherwise
+     * Returns the timeout for this flow requested by an application.
+     * @return integer value of the timeout
      */
-    boolean expired();
+    int timeout();
 
 }

@@ -8,6 +8,8 @@ import org.onlab.onos.net.provider.Provider;
  */
 public interface FlowRuleProvider extends Provider {
 
+    static final int POLL_INTERVAL = 5;
+
     /**
      * Instructs the provider to apply the specified flow rules to their
      * respective devices.

@@ -17,27 +17,6 @@ public interface FlowRuleProviderService extends ProviderService<FlowRuleProvide
     void flowRemoved(FlowRule flowRule);
 
     /**
-     * Signals that a flow rule is missing for some network traffic.
-     *
-     * @param flowRule information about traffic in need of flow rule(s)
-     */
-    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
-     */
-    void flowAdded(FlowRule flowRule);
-
-    /**
      * Pushes the collection of flow entries currently applied on the given
      * device.
      *

 package org.onlab.onos.net.flow;
 
-import java.util.List;
+import java.util.Set;
 
 import org.onlab.onos.net.PortNumber;
 import org.onlab.onos.net.flow.criteria.Criterion;
@@ -18,7 +18,7 @@ public interface TrafficSelector {
      *
      * @return list of criteria
      */
-    List<Criterion> criteria();
+    Set<Criterion> criteria();
 
     /**
      * Builder of traffic selector entities.

@@ -2,6 +2,8 @@ package org.onlab.onos.net.flow.criteria;
 
 import static com.google.common.base.MoreObjects.toStringHelper;
 
+import java.util.Objects;
+
 import org.onlab.onos.net.PortNumber;
 import org.onlab.onos.net.flow.criteria.Criterion.Type;
 import org.onlab.packet.IpPrefix;
@@ -137,6 +139,25 @@ public final class Criteria {
             return toStringHelper(type().toString())
                     .add("port", port).toString();
         }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(port);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj instanceof PortCriterion) {
+                PortCriterion that = (PortCriterion) obj;
+                return Objects.equals(port, that.port);
+
+            }
+            return false;
+        }
+
     }
 
 
@@ -164,6 +185,27 @@ public final class Criteria {
                     .add("mac", mac).toString();
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(mac, type);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj instanceof EthCriterion) {
+                EthCriterion that = (EthCriterion) obj;
+                return Objects.equals(mac, that.mac) &&
+                        Objects.equals(type, that.type);
+
+
+            }
+            return false;
+        }
+
+
     }
 
     public static final class EthTypeCriterion implements Criterion {
@@ -189,6 +231,25 @@ public final class Criteria {
                     .add("ethType", Long.toHexString(ethType)).toString();
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(ethType);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj instanceof EthTypeCriterion) {
+                EthTypeCriterion that = (EthTypeCriterion) obj;
+                return Objects.equals(ethType, that.ethType);
+
+
+            }
+            return false;
+        }
+
     }
 
 
@@ -217,6 +278,26 @@ public final class Criteria {
                     .add("ip", ip).toString();
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(ip, type);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj instanceof IPCriterion) {
+                IPCriterion that = (IPCriterion) obj;
+                return Objects.equals(ip, that.ip) &&
+                        Objects.equals(type, that.type);
+
+
+            }
+            return false;
+        }
+
     }
 
 
@@ -243,6 +324,25 @@ public final class Criteria {
                     .add("protocol", Long.toHexString(proto)).toString();
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(proto);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj instanceof IPProtocolCriterion) {
+                IPProtocolCriterion that = (IPProtocolCriterion) obj;
+                return Objects.equals(proto, that.proto);
+
+
+            }
+            return false;
+        }
+
     }
 
 
@@ -269,6 +369,25 @@ public final class Criteria {
                     .add("pcp", Long.toHexString(vlanPcp)).toString();
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(vlanPcp);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj instanceof VlanPcpCriterion) {
+                VlanPcpCriterion that = (VlanPcpCriterion) obj;
+                return Objects.equals(vlanPcp, that.vlanPcp);
+
+
+            }
+            return false;
+        }
+
     }
 
 
@@ -296,6 +415,25 @@ public final class Criteria {
                     .add("id", vlanId).toString();
         }
 
+        @Override
+        public int hashCode() {
+            return Objects.hash(vlanId);
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            if (obj instanceof VlanIdCriterion) {
+                VlanIdCriterion that = (VlanIdCriterion) obj;
+                return Objects.equals(vlanId, that.vlanId);
+
+
+            }
+            return false;
+        }
+
     }
 
 

@@ -5,6 +5,9 @@ import static org.slf4j.LoggerFactory.getLogger;
 
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
@@ -59,6 +62,8 @@ implements FlowRuleService, FlowRuleProviderRegistry {
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected DeviceService deviceService;
 
+    private final Map<FlowRule, AtomicInteger> deadRounds = new ConcurrentHashMap<>();
+
     @Activate
     public void activate() {
         store.setDelegate(delegate);
@@ -84,6 +89,7 @@ implements FlowRuleService, FlowRuleProviderRegistry {
             FlowRule f = flowRules[i];
             final Device device = deviceService.getDevice(f.deviceId());
             final FlowRuleProvider frp = getProvider(device.providerId());
+            deadRounds.put(f, new AtomicInteger(0));
             store.storeFlowRule(f);
             frp.applyFlowRule(f);
         }
@@ -98,6 +104,7 @@ implements FlowRuleService, FlowRuleProviderRegistry {
             f = flowRules[i];
             device = deviceService.getDevice(f.deviceId());
             frp = getProvider(device.providerId());
+            deadRounds.remove(f);
             store.deleteFlowRule(f);
             frp.removeFlowRule(f);
         }
@@ -161,11 +168,7 @@ implements FlowRuleService, FlowRuleProviderRegistry {
             switch (stored.state()) {
             case ADDED:
             case PENDING_ADD:
-                if (flowRule.expired()) {
-                    event = store.removeFlowRule(flowRule);
-                } else {
                     frp.applyFlowRule(stored);
-                }
                 break;
             case PENDING_REMOVE:
             case REMOVED:
@@ -181,8 +184,8 @@ implements FlowRuleService, FlowRuleProviderRegistry {
             }
         }
 
-        @Override
-        public void flowMissing(FlowRule flowRule) {
+
+        private void flowMissing(FlowRule flowRule) {
             checkNotNull(flowRule, FLOW_RULE_NULL);
             checkValidity();
             Device device = deviceService.getDevice(flowRule.deviceId());
@@ -209,19 +212,22 @@ implements FlowRuleService, FlowRuleProviderRegistry {
 
         }
 
-        @Override
-        public void extraneousFlow(FlowRule flowRule) {
+
+        private void extraneousFlow(FlowRule flowRule) {
             checkNotNull(flowRule, FLOW_RULE_NULL);
             checkValidity();
             removeFlowRules(flowRule);
             log.debug("Flow {} is on switch but not in store.", flowRule);
         }
 
-        @Override
-        public void flowAdded(FlowRule flowRule) {
+
+        private void flowAdded(FlowRule flowRule) {
             checkNotNull(flowRule, FLOW_RULE_NULL);
             checkValidity();
 
+            if (deadRounds.containsKey(flowRule) &&
+                    checkRuleLiveness(flowRule, store.getFlowRule(flowRule))) {
+
                 FlowRuleEvent event = store.addOrUpdateFlowRule(flowRule);
                 if (event == null) {
                     log.debug("No flow store event generated.");
@@ -229,6 +235,21 @@ implements FlowRuleService, FlowRuleProviderRegistry {
                     log.debug("Flow {} {}", flowRule, event.type());
                     post(event);
                 }
+            } else {
+                removeFlowRules(flowRule);
+            }
+
+        }
+
+        private boolean checkRuleLiveness(FlowRule swRule, FlowRule storedRule) {
+            int timeout = storedRule.timeout();
+            if (storedRule.packets() != swRule.packets()) {
+                deadRounds.get(swRule).set(0);
+                return true;
+            }
+
+            return (deadRounds.get(swRule).getAndIncrement() *
+                    FlowRuleProvider.POLL_INTERVAL) <= timeout;
 
         }
 

@@ -9,7 +9,9 @@ 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.Collections;
 import java.util.List;
+import java.util.Set;
 
 import org.junit.After;
 import org.junit.Before;
@@ -42,6 +44,7 @@ import org.onlab.onos.net.provider.AbstractProvider;
 import org.onlab.onos.net.provider.ProviderId;
 import org.onlab.onos.store.trivial.impl.SimpleFlowRuleStore;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 
@@ -52,6 +55,7 @@ public class FlowRuleManagerTest {
 
     private static final ProviderId PID = new ProviderId("of", "foo");
     private static final DeviceId DID = DeviceId.deviceId("of:001");
+    private static final int TIMEOUT = 10;
     private static final Device DEV = new DefaultDevice(
             PID, DID, Type.SWITCH, "", "", "", "");
 
@@ -96,7 +100,7 @@ public class FlowRuleManagerTest {
     private FlowRule flowRule(int tsval, int trval) {
         TestSelector ts = new TestSelector(tsval);
         TestTreatment tr = new TestTreatment(trval);
-        return new DefaultFlowRule(DID, ts, tr, 0, appId);
+        return new DefaultFlowRule(DID, ts, tr, 0, appId, TIMEOUT);
     }
 
     private FlowRule flowRule(FlowRule rule, FlowRuleState state) {
@@ -105,7 +109,8 @@ public class FlowRuleManagerTest {
 
     private FlowRule addFlowRule(int hval) {
         FlowRule rule = flowRule(hval, hval);
-        providerService.flowAdded(rule);
+        service.applyFlowRules(rule);
+
         assertNotNull("rule should be found", service.getFlowEntries(DID));
         return rule;
     }
@@ -135,13 +140,18 @@ public class FlowRuleManagerTest {
     public void getFlowEntries() {
         assertTrue("store should be empty",
                 Sets.newHashSet(service.getFlowEntries(DID)).isEmpty());
-        addFlowRule(1);
-        addFlowRule(2);
+        FlowRule f1 = addFlowRule(1);
+        FlowRule f2 = addFlowRule(2);
+
         assertEquals("2 rules should exist", 2, flowCount());
+
+        providerService.pushFlowMetrics(DID, ImmutableList.of(f1, f2));
         validateEvents(RULE_ADDED, RULE_ADDED);
 
         addFlowRule(1);
         assertEquals("should still be 2 rules", 2, flowCount());
+
+        providerService.pushFlowMetrics(DID, ImmutableList.of(f1));
         validateEvents(RULE_UPDATED);
     }
 
@@ -179,8 +189,10 @@ public class FlowRuleManagerTest {
     public void removeFlowRules() {
         FlowRule f1 = addFlowRule(1);
         FlowRule f2 = addFlowRule(2);
-        addFlowRule(3);
+        FlowRule f3 = addFlowRule(3);
         assertEquals("3 rules should exist", 3, flowCount());
+
+        providerService.pushFlowMetrics(DID, ImmutableList.of(f1, f2, f3));
         validateEvents(RULE_ADDED, RULE_ADDED, RULE_ADDED);
 
         FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
@@ -200,8 +212,9 @@ public class FlowRuleManagerTest {
     @Test
     public void flowRemoved() {
         FlowRule f1 = addFlowRule(1);
+        FlowRule f2 = addFlowRule(2);
+        providerService.pushFlowMetrics(f1.deviceId(), ImmutableList.of(f1, f2));
         service.removeFlowRules(f1);
-        addFlowRule(2);
         FlowRule rem1 = flowRule(f1, FlowRuleState.REMOVED);
         providerService.flowRemoved(rem1);
         validateEvents(RULE_ADDED, RULE_ADDED, RULE_REMOVED);
@@ -209,9 +222,11 @@ public class FlowRuleManagerTest {
         providerService.flowRemoved(rem1);
         validateEvents();
 
-        FlowRule f3 = flowRule(flowRule(3, 3), FlowRuleState.ADDED);
-        providerService.flowAdded(f3);
+        FlowRule f3 = flowRule(3, 3);
+        service.applyFlowRules(f3);
+        providerService.pushFlowMetrics(f3.deviceId(), Collections.singletonList(f3));
         validateEvents(RULE_ADDED);
+
         providerService.flowRemoved(f3);
         validateEvents();
     }
@@ -223,9 +238,10 @@ public class FlowRuleManagerTest {
         FlowRule f3 = flowRule(3, 3);
 
 
+
+        mgr.applyFlowRules(f1, f2, f3);
         FlowRule updatedF1 = flowRule(f1, FlowRuleState.ADDED);
         FlowRule updatedF2 = flowRule(f2, FlowRuleState.ADDED);
-        mgr.applyFlowRules(f1, f2, f3);
 
         providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2));
 
@@ -233,7 +249,7 @@ public class FlowRuleManagerTest {
                 validateState(FlowRuleState.PENDING_ADD, FlowRuleState.ADDED,
                         FlowRuleState.ADDED));
 
-        validateEvents(RULE_UPDATED, RULE_UPDATED);
+        validateEvents(RULE_ADDED, RULE_ADDED);
     }
 
     @Test
@@ -241,15 +257,15 @@ public class FlowRuleManagerTest {
         FlowRule f1 = flowRule(1, 1);
         FlowRule f2 = flowRule(2, 2);
         FlowRule f3 = flowRule(3, 3);
+        mgr.applyFlowRules(f1, f2);
 
         FlowRule updatedF1 = flowRule(f1, FlowRuleState.ADDED);
         FlowRule updatedF2 = flowRule(f2, FlowRuleState.ADDED);
         FlowRule updatedF3 = flowRule(f3, FlowRuleState.ADDED);
-        mgr.applyFlowRules(f1, f2);
 
         providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2, updatedF3));
 
-        validateEvents(RULE_UPDATED, RULE_UPDATED);
+        validateEvents(RULE_ADDED, RULE_ADDED);
 
     }
 
@@ -271,7 +287,7 @@ public class FlowRuleManagerTest {
 
         providerService.pushFlowMetrics(DID, Lists.newArrayList(updatedF1, updatedF2));
 
-        validateEvents(RULE_UPDATED, RULE_UPDATED, RULE_REMOVED);
+        validateEvents(RULE_ADDED, RULE_ADDED, RULE_REMOVED);
 
     }
 
@@ -386,7 +402,7 @@ public class FlowRuleManagerTest {
         }
 
         @Override
-        public List<Criterion> criteria() {
+        public Set<Criterion> criteria() {
             return null;
         }
 

 package org.onlab.onos.store.trivial.impl;
 
-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;
 
@@ -116,18 +115,21 @@ public class SimpleFlowRuleStore
         DeviceId did = rule.deviceId();
 
         // check if this new rule is an update to an existing entry
-        if (flowEntries.containsEntry(did, rule)) {
-            //synchronized (flowEntries) {
+        FlowRule stored = getFlowRule(rule);
+        if (stored != null) {
             // 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);
-            //}
+
+            if (stored.state() == FlowRuleState.PENDING_ADD) {
+                return new FlowRuleEvent(Type.RULE_ADDED, rule);
+            }
             return new FlowRuleEvent(Type.RULE_UPDATED, rule);
         }
 
         flowEntries.put(did, rule);
-        return new FlowRuleEvent(RULE_ADDED, rule);
+        return null;
     }
 
     @Override
@@ -140,11 +142,4 @@ public class SimpleFlowRuleStore
         }
         //}
     }
-
-
-
-
-
-
-
 }

@@ -77,7 +77,6 @@ public class FlowModBuilder {
                 .setCookie(U64.of(cookie.value()))
                 .setBufferId(OFBufferId.NO_BUFFER)
                 .setActions(actions)
-                .setIdleTimeout(10)
                 .setMatch(match)
                 .setFlags(Collections.singleton(OFFlowModFlags.SEND_FLOW_REM))
                 .setPriority(priority)

@@ -71,7 +71,7 @@ public class FlowRuleBuilder {
                     buildSelector(), buildTreatment(), stat.getPriority(),
                     FlowRuleState.ADDED, stat.getDurationNsec() / 1000000,
                     stat.getPacketCount().getValue(), stat.getByteCount().getValue(),
-                    stat.getCookie().getValue(), false);
+                    stat.getCookie().getValue(), false, stat.getIdleTimeout());
         } else {
             // TODO: revisit potentially.
             return new DefaultFlowRule(DeviceId.deviceId(Dpid.uri(dpid)),
@@ -79,7 +79,8 @@ public class FlowRuleBuilder {
                     FlowRuleState.REMOVED, removed.getDurationNsec() / 1000000,
                     removed.getPacketCount().getValue(), removed.getByteCount().getValue(),
                     removed.getCookie().getValue(),
-                    removed.getReason() == OFFlowRemovedReason.IDLE_TIMEOUT.ordinal());
+                    removed.getReason() == OFFlowRemovedReason.IDLE_TIMEOUT.ordinal(),
+                    stat.getIdleTimeout());
         }
     }
 

@@ -127,7 +127,7 @@ public class OpenFlowRuleProvider extends AbstractProvider implements FlowRulePr
 
         @Override
         public void switchAdded(Dpid dpid) {
-            FlowStatsCollector fsc = new FlowStatsCollector(controller.getSwitch(dpid), 5);
+            FlowStatsCollector fsc = new FlowStatsCollector(controller.getSwitch(dpid), POLL_INTERVAL);
             fsc.start();
             collectors.put(dpid, fsc);
         }

@@ -176,7 +176,7 @@
         </module>
 
         <module name="ParameterNumber">
-            <property name="max" value="10"/>
+            <property name="max" value="15"/>
             <property name="tokens" value="CTOR_DEF"/>
         </module>
         <!-- Checks for whitespace                               -->