Committed by
Gerrit Code Review
ONOS-4154 generates consistent hash for flow ID across multiple instances
Change-Id: I8de850dcc5d6d40ed563f7d476c6e13a09060093
Showing
4 changed files
with
72 additions
and
3 deletions
| ... | @@ -16,6 +16,11 @@ | ... | @@ -16,6 +16,11 @@ |
| 16 | package org.onosproject.net.flow; | 16 | package org.onosproject.net.flow; |
| 17 | 17 | ||
| 18 | import com.google.common.annotations.Beta; | 18 | import com.google.common.annotations.Beta; |
| 19 | +import com.google.common.base.Charsets; | ||
| 20 | +import com.google.common.hash.Funnel; | ||
| 21 | +import com.google.common.hash.HashCode; | ||
| 22 | +import com.google.common.hash.HashFunction; | ||
| 23 | +import com.google.common.hash.Hashing; | ||
| 19 | import org.onosproject.core.ApplicationId; | 24 | import org.onosproject.core.ApplicationId; |
| 20 | import org.onosproject.core.DefaultGroupId; | 25 | import org.onosproject.core.DefaultGroupId; |
| 21 | import org.onosproject.core.GroupId; | 26 | import org.onosproject.core.GroupId; |
| ... | @@ -393,9 +398,20 @@ public class DefaultFlowRule implements FlowRule { | ... | @@ -393,9 +398,20 @@ public class DefaultFlowRule implements FlowRule { |
| 393 | } | 398 | } |
| 394 | 399 | ||
| 395 | private int hash() { | 400 | private int hash() { |
| 396 | - return Objects.hash(deviceId, priority, selector, tableId); | 401 | + Funnel<TrafficSelector> selectorFunnel = (from, into) -> from.criteria() |
| 402 | + .stream() | ||
| 403 | + .forEach(c -> into.putString(c.toString(), Charsets.UTF_8)); | ||
| 404 | + | ||
| 405 | + HashFunction hashFunction = Hashing.murmur3_32(); | ||
| 406 | + HashCode hashCode = hashFunction.newHasher() | ||
| 407 | + .putString(deviceId.toString(), Charsets.UTF_8) | ||
| 408 | + .putObject(selector, selectorFunnel) | ||
| 409 | + .putInt(priority) | ||
| 410 | + .putInt(tableId) | ||
| 411 | + .hash(); | ||
| 412 | + | ||
| 413 | + return hashCode.asInt(); | ||
| 397 | } | 414 | } |
| 398 | - | ||
| 399 | } | 415 | } |
| 400 | 416 | ||
| 401 | @Override | 417 | @Override | ... | ... |
| ... | @@ -158,4 +158,34 @@ public class DefaultFlowRuleTest { | ... | @@ -158,4 +158,34 @@ public class DefaultFlowRuleTest { |
| 158 | assertThat(rule.treatment(), is(TREATMENT)); | 158 | assertThat(rule.treatment(), is(TREATMENT)); |
| 159 | assertThat(rule.timeout(), is(44)); | 159 | assertThat(rule.timeout(), is(44)); |
| 160 | } | 160 | } |
| 161 | + | ||
| 162 | + /** | ||
| 163 | + * Tests flow ID is consistent. | ||
| 164 | + */ | ||
| 165 | + @Test | ||
| 166 | + public void testCreationWithConsistentFlowId() { | ||
| 167 | + final FlowRule rule1 = | ||
| 168 | + DefaultFlowRule.builder() | ||
| 169 | + .forDevice(did("1")) | ||
| 170 | + .withSelector(SELECTOR) | ||
| 171 | + .withTreatment(TREATMENT) | ||
| 172 | + .withPriority(22) | ||
| 173 | + .forTable(1) | ||
| 174 | + .fromApp(APP_ID) | ||
| 175 | + .makeTemporary(44) | ||
| 176 | + .build(); | ||
| 177 | + | ||
| 178 | + final FlowRule rule2 = | ||
| 179 | + DefaultFlowRule.builder() | ||
| 180 | + .forDevice(did("1")) | ||
| 181 | + .withSelector(SELECTOR) | ||
| 182 | + .withTreatment(TREATMENT) | ||
| 183 | + .withPriority(22) | ||
| 184 | + .forTable(1) | ||
| 185 | + .fromApp(APP_ID) | ||
| 186 | + .makeTemporary(44) | ||
| 187 | + .build(); | ||
| 188 | + | ||
| 189 | + new EqualsTester().addEqualityGroup(rule1.id(), rule2.id()).testEquals(); | ||
| 190 | + } | ||
| 161 | } | 191 | } | ... | ... |
| ... | @@ -15,13 +15,16 @@ | ... | @@ -15,13 +15,16 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.net.flow; | 16 | package org.onosproject.net.flow; |
| 17 | 17 | ||
| 18 | +import java.util.List; | ||
| 18 | import java.util.Set; | 19 | import java.util.Set; |
| 19 | 20 | ||
| 21 | +import com.google.common.collect.Lists; | ||
| 20 | import org.hamcrest.Description; | 22 | import org.hamcrest.Description; |
| 21 | import org.hamcrest.Factory; | 23 | import org.hamcrest.Factory; |
| 22 | import org.hamcrest.Matcher; | 24 | import org.hamcrest.Matcher; |
| 23 | import org.hamcrest.TypeSafeMatcher; | 25 | import org.hamcrest.TypeSafeMatcher; |
| 24 | import org.junit.Test; | 26 | import org.junit.Test; |
| 27 | +import org.onlab.packet.Ethernet; | ||
| 25 | import org.onlab.packet.Ip6Address; | 28 | import org.onlab.packet.Ip6Address; |
| 26 | import org.onlab.packet.IpPrefix; | 29 | import org.onlab.packet.IpPrefix; |
| 27 | import org.onlab.packet.MacAddress; | 30 | import org.onlab.packet.MacAddress; |
| ... | @@ -78,6 +81,26 @@ public class DefaultTrafficSelectorTest { | ... | @@ -78,6 +81,26 @@ public class DefaultTrafficSelectorTest { |
| 78 | } | 81 | } |
| 79 | 82 | ||
| 80 | /** | 83 | /** |
| 84 | + * Tests criteria order is consistent. | ||
| 85 | + */ | ||
| 86 | + @Test | ||
| 87 | + public void testCriteriaOrder() { | ||
| 88 | + final TrafficSelector selector1 = DefaultTrafficSelector.builder() | ||
| 89 | + .matchInPort(PortNumber.portNumber(11)) | ||
| 90 | + .matchEthType(Ethernet.TYPE_ARP) | ||
| 91 | + .build(); | ||
| 92 | + final TrafficSelector selector2 = DefaultTrafficSelector.builder() | ||
| 93 | + .matchEthType(Ethernet.TYPE_ARP) | ||
| 94 | + .matchInPort(PortNumber.portNumber(11)) | ||
| 95 | + .build(); | ||
| 96 | + | ||
| 97 | + List<Criterion> criteria1 = Lists.newArrayList(selector1.criteria()); | ||
| 98 | + List<Criterion> criteria2 = Lists.newArrayList(selector2.criteria()); | ||
| 99 | + | ||
| 100 | + new EqualsTester().addEqualityGroup(criteria1, criteria2).testEquals(); | ||
| 101 | + } | ||
| 102 | + | ||
| 103 | + /** | ||
| 81 | * Hamcrest matcher to check that a selector contains a | 104 | * Hamcrest matcher to check that a selector contains a |
| 82 | * Criterion with the specified type. | 105 | * Criterion with the specified type. |
| 83 | */ | 106 | */ | ... | ... |
| ... | @@ -569,7 +569,7 @@ public class FlowRuleManagerTest { | ... | @@ -569,7 +569,7 @@ public class FlowRuleManagerTest { |
| 569 | 569 | ||
| 570 | @Override | 570 | @Override |
| 571 | public Set<Criterion> criteria() { | 571 | public Set<Criterion> criteria() { |
| 572 | - return null; | 572 | + return Collections.emptySet(); |
| 573 | } | 573 | } |
| 574 | 574 | ||
| 575 | @Override | 575 | @Override | ... | ... |
-
Please register or login to post a comment