From 7bfbc1ce33b231e57d4f8d13596f4501e04569ce Mon Sep 17 00:00:00 2001
From: Hyunsun Moon <hyunsun.moon@gmail.com>
Date: Tue, 5 Apr 2016 16:40:43 -0700
Subject: [PATCH] ONOS-4154 generates consistent hash for flow ID across
 multiple instances

Change-Id: I8de850dcc5d6d40ed563f7d476c6e13a09060093
---
 .../onosproject/net/flow/DefaultFlowRule.java | 20 +++++++++++--
 .../net/flow/DefaultFlowRuleTest.java         | 30 +++++++++++++++++++
 .../net/flow/DefaultTrafficSelectorTest.java  | 23 ++++++++++++++
 .../net/flow/impl/FlowRuleManagerTest.java    |  2 +-
 4 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java b/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
index b8605f1878..313cbfc34c 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/DefaultFlowRule.java
@@ -16,6 +16,11 @@
 package org.onosproject.net.flow;
 
 import com.google.common.annotations.Beta;
+import com.google.common.base.Charsets;
+import com.google.common.hash.Funnel;
+import com.google.common.hash.HashCode;
+import com.google.common.hash.HashFunction;
+import com.google.common.hash.Hashing;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.DefaultGroupId;
 import org.onosproject.core.GroupId;
@@ -393,9 +398,20 @@ public class DefaultFlowRule implements FlowRule {
         }
 
         private int hash() {
-            return Objects.hash(deviceId, priority, selector, tableId);
-        }
+            Funnel<TrafficSelector> selectorFunnel = (from, into) -> from.criteria()
+                    .stream()
+                    .forEach(c -> into.putString(c.toString(), Charsets.UTF_8));
 
+            HashFunction hashFunction = Hashing.murmur3_32();
+            HashCode hashCode = hashFunction.newHasher()
+                    .putString(deviceId.toString(), Charsets.UTF_8)
+                    .putObject(selector, selectorFunnel)
+                    .putInt(priority)
+                    .putInt(tableId)
+                    .hash();
+
+            return hashCode.asInt();
+        }
     }
 
     @Override
diff --git a/core/api/src/test/java/org/onosproject/net/flow/DefaultFlowRuleTest.java b/core/api/src/test/java/org/onosproject/net/flow/DefaultFlowRuleTest.java
index e8230f7663..c0f11d6254 100644
--- a/core/api/src/test/java/org/onosproject/net/flow/DefaultFlowRuleTest.java
+++ b/core/api/src/test/java/org/onosproject/net/flow/DefaultFlowRuleTest.java
@@ -158,4 +158,34 @@ public class DefaultFlowRuleTest {
         assertThat(rule.treatment(), is(TREATMENT));
         assertThat(rule.timeout(), is(44));
     }
+
+    /**
+     * Tests flow ID is consistent.
+     */
+    @Test
+    public void testCreationWithConsistentFlowId() {
+        final FlowRule rule1 =
+                DefaultFlowRule.builder()
+                        .forDevice(did("1"))
+                        .withSelector(SELECTOR)
+                        .withTreatment(TREATMENT)
+                        .withPriority(22)
+                        .forTable(1)
+                        .fromApp(APP_ID)
+                        .makeTemporary(44)
+                        .build();
+
+        final FlowRule rule2 =
+                DefaultFlowRule.builder()
+                        .forDevice(did("1"))
+                        .withSelector(SELECTOR)
+                        .withTreatment(TREATMENT)
+                        .withPriority(22)
+                        .forTable(1)
+                        .fromApp(APP_ID)
+                        .makeTemporary(44)
+                        .build();
+
+        new EqualsTester().addEqualityGroup(rule1.id(), rule2.id()).testEquals();
+    }
 }
diff --git a/core/api/src/test/java/org/onosproject/net/flow/DefaultTrafficSelectorTest.java b/core/api/src/test/java/org/onosproject/net/flow/DefaultTrafficSelectorTest.java
index 923de26f92..aaa3db5be2 100644
--- a/core/api/src/test/java/org/onosproject/net/flow/DefaultTrafficSelectorTest.java
+++ b/core/api/src/test/java/org/onosproject/net/flow/DefaultTrafficSelectorTest.java
@@ -15,13 +15,16 @@
  */
 package org.onosproject.net.flow;
 
+import java.util.List;
 import java.util.Set;
 
+import com.google.common.collect.Lists;
 import org.hamcrest.Description;
 import org.hamcrest.Factory;
 import org.hamcrest.Matcher;
 import org.hamcrest.TypeSafeMatcher;
 import org.junit.Test;
+import org.onlab.packet.Ethernet;
 import org.onlab.packet.Ip6Address;
 import org.onlab.packet.IpPrefix;
 import org.onlab.packet.MacAddress;
@@ -77,6 +80,26 @@ public class DefaultTrafficSelectorTest {
                 .testEquals();
     }
 
+    /**
+     * Tests criteria order is consistent.
+     */
+    @Test
+    public void testCriteriaOrder() {
+        final TrafficSelector selector1 = DefaultTrafficSelector.builder()
+                .matchInPort(PortNumber.portNumber(11))
+                .matchEthType(Ethernet.TYPE_ARP)
+                .build();
+        final TrafficSelector selector2 = DefaultTrafficSelector.builder()
+                .matchEthType(Ethernet.TYPE_ARP)
+                .matchInPort(PortNumber.portNumber(11))
+                .build();
+
+        List<Criterion> criteria1 = Lists.newArrayList(selector1.criteria());
+        List<Criterion> criteria2 = Lists.newArrayList(selector2.criteria());
+
+        new EqualsTester().addEqualityGroup(criteria1, criteria2).testEquals();
+    }
+
     /**
      * Hamcrest matcher to check that a selector contains a
      * Criterion with the specified type.
diff --git a/core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java b/core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
index 1e621f5185..235699d74a 100644
--- a/core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
+++ b/core/net/src/test/java/org/onosproject/net/flow/impl/FlowRuleManagerTest.java
@@ -569,7 +569,7 @@ public class FlowRuleManagerTest {
 
         @Override
         public Set<Criterion> criteria() {
-            return null;
+            return Collections.emptySet();
         }
 
         @Override