added hashcode and equals to treatments
UnmodifiableCollection can be bad in a hashCode Change-Id: I55700541dc7ab46b21e5e9e9cc19c70f0c7f7494
Showing
8 changed files
with
176 additions
and
25 deletions
| ... | @@ -143,7 +143,7 @@ public class DefaultFlowRule implements FlowRule { | ... | @@ -143,7 +143,7 @@ public class DefaultFlowRule implements FlowRule { |
| 143 | @Override | 143 | @Override |
| 144 | public String toString() { | 144 | public String toString() { |
| 145 | return toStringHelper(this) | 145 | return toStringHelper(this) |
| 146 | - .add("id", id) | 146 | + .add("id", Long.toHexString(id.value())) |
| 147 | .add("deviceId", deviceId) | 147 | .add("deviceId", deviceId) |
| 148 | .add("priority", priority) | 148 | .add("priority", priority) |
| 149 | .add("selector", selector.criteria()) | 149 | .add("selector", selector.criteria()) | ... | ... |
| 1 | package org.onlab.onos.net.flow; | 1 | package org.onlab.onos.net.flow; |
| 2 | 2 | ||
| 3 | +import static org.slf4j.LoggerFactory.getLogger; | ||
| 4 | + | ||
| 5 | +import java.util.Collections; | ||
| 6 | +import java.util.LinkedList; | ||
| 7 | +import java.util.List; | ||
| 8 | +import java.util.Objects; | ||
| 9 | + | ||
| 3 | import org.onlab.onos.net.PortNumber; | 10 | import org.onlab.onos.net.PortNumber; |
| 4 | import org.onlab.onos.net.flow.instructions.Instruction; | 11 | import org.onlab.onos.net.flow.instructions.Instruction; |
| 5 | import org.onlab.onos.net.flow.instructions.Instructions; | 12 | import org.onlab.onos.net.flow.instructions.Instructions; |
| ... | @@ -8,12 +15,6 @@ import org.onlab.packet.MacAddress; | ... | @@ -8,12 +15,6 @@ import org.onlab.packet.MacAddress; |
| 8 | import org.onlab.packet.VlanId; | 15 | import org.onlab.packet.VlanId; |
| 9 | import org.slf4j.Logger; | 16 | import org.slf4j.Logger; |
| 10 | 17 | ||
| 11 | -import java.util.Collections; | ||
| 12 | -import java.util.LinkedList; | ||
| 13 | -import java.util.List; | ||
| 14 | - | ||
| 15 | -import static org.slf4j.LoggerFactory.getLogger; | ||
| 16 | - | ||
| 17 | /** | 18 | /** |
| 18 | * Default traffic treatment implementation. | 19 | * Default traffic treatment implementation. |
| 19 | */ | 20 | */ |
| ... | @@ -44,6 +45,25 @@ public final class DefaultTrafficTreatment implements TrafficTreatment { | ... | @@ -44,6 +45,25 @@ public final class DefaultTrafficTreatment implements TrafficTreatment { |
| 44 | return new Builder(); | 45 | return new Builder(); |
| 45 | } | 46 | } |
| 46 | 47 | ||
| 48 | + //FIXME: Order of instructions may affect hashcode | ||
| 49 | + @Override | ||
| 50 | + public int hashCode() { | ||
| 51 | + return Objects.hash(instructions); | ||
| 52 | + } | ||
| 53 | + | ||
| 54 | + @Override | ||
| 55 | + public boolean equals(Object obj) { | ||
| 56 | + if (this == obj) { | ||
| 57 | + return true; | ||
| 58 | + } | ||
| 59 | + if (obj instanceof DefaultTrafficTreatment) { | ||
| 60 | + DefaultTrafficTreatment that = (DefaultTrafficTreatment) obj; | ||
| 61 | + return Objects.equals(instructions, that.instructions); | ||
| 62 | + | ||
| 63 | + } | ||
| 64 | + return false; | ||
| 65 | + } | ||
| 66 | + | ||
| 47 | /** | 67 | /** |
| 48 | * Builds a list of treatments following the following order. | 68 | * Builds a list of treatments following the following order. |
| 49 | * Modifications -> Group -> Output (including drop) | 69 | * Modifications -> Group -> Output (including drop) |
| ... | @@ -66,6 +86,7 @@ public final class DefaultTrafficTreatment implements TrafficTreatment { | ... | @@ -66,6 +86,7 @@ public final class DefaultTrafficTreatment implements TrafficTreatment { |
| 66 | private Builder() { | 86 | private Builder() { |
| 67 | } | 87 | } |
| 68 | 88 | ||
| 89 | + @Override | ||
| 69 | public Builder add(Instruction instruction) { | 90 | public Builder add(Instruction instruction) { |
| 70 | if (drop) { | 91 | if (drop) { |
| 71 | return this; | 92 | return this; | ... | ... |
| ... | @@ -3,6 +3,8 @@ package org.onlab.onos.net.flow.instructions; | ... | @@ -3,6 +3,8 @@ package org.onlab.onos.net.flow.instructions; |
| 3 | import static com.google.common.base.MoreObjects.toStringHelper; | 3 | import static com.google.common.base.MoreObjects.toStringHelper; |
| 4 | import static com.google.common.base.Preconditions.checkNotNull; | 4 | import static com.google.common.base.Preconditions.checkNotNull; |
| 5 | 5 | ||
| 6 | +import java.util.Objects; | ||
| 7 | + | ||
| 6 | import org.onlab.onos.net.PortNumber; | 8 | import org.onlab.onos.net.PortNumber; |
| 7 | import org.onlab.onos.net.flow.instructions.L2ModificationInstruction.L2SubType; | 9 | import org.onlab.onos.net.flow.instructions.L2ModificationInstruction.L2SubType; |
| 8 | import org.onlab.onos.net.flow.instructions.L2ModificationInstruction.ModEtherInstruction; | 10 | import org.onlab.onos.net.flow.instructions.L2ModificationInstruction.ModEtherInstruction; |
| ... | @@ -117,6 +119,24 @@ public final class Instructions { | ... | @@ -117,6 +119,24 @@ public final class Instructions { |
| 117 | return toStringHelper(type()).toString(); | 119 | return toStringHelper(type()).toString(); |
| 118 | 120 | ||
| 119 | } | 121 | } |
| 122 | + | ||
| 123 | + @Override | ||
| 124 | + public int hashCode() { | ||
| 125 | + return Objects.hash(type()); | ||
| 126 | + } | ||
| 127 | + | ||
| 128 | + @Override | ||
| 129 | + public boolean equals(Object obj) { | ||
| 130 | + if (this == obj) { | ||
| 131 | + return true; | ||
| 132 | + } | ||
| 133 | + if (obj instanceof DropInstruction) { | ||
| 134 | + DropInstruction that = (DropInstruction) obj; | ||
| 135 | + return Objects.equals(type(), that.type()); | ||
| 136 | + | ||
| 137 | + } | ||
| 138 | + return false; | ||
| 139 | + } | ||
| 120 | } | 140 | } |
| 121 | 141 | ||
| 122 | 142 | ||
| ... | @@ -140,6 +160,26 @@ public final class Instructions { | ... | @@ -140,6 +160,26 @@ public final class Instructions { |
| 140 | return toStringHelper(type().toString()) | 160 | return toStringHelper(type().toString()) |
| 141 | .add("port", port).toString(); | 161 | .add("port", port).toString(); |
| 142 | } | 162 | } |
| 163 | + | ||
| 164 | + @Override | ||
| 165 | + public int hashCode() { | ||
| 166 | + return Objects.hash(port, type()); | ||
| 167 | + } | ||
| 168 | + | ||
| 169 | + @Override | ||
| 170 | + public boolean equals(Object obj) { | ||
| 171 | + if (this == obj) { | ||
| 172 | + return true; | ||
| 173 | + } | ||
| 174 | + if (obj instanceof OutputInstruction) { | ||
| 175 | + OutputInstruction that = (OutputInstruction) obj; | ||
| 176 | + Objects.equals(port, that.port); | ||
| 177 | + | ||
| 178 | + } | ||
| 179 | + return false; | ||
| 180 | + } | ||
| 143 | } | 181 | } |
| 144 | 182 | ||
| 145 | } | 183 | } |
| 184 | + | ||
| 185 | + | ... | ... |
| ... | @@ -2,6 +2,8 @@ package org.onlab.onos.net.flow.instructions; | ... | @@ -2,6 +2,8 @@ package org.onlab.onos.net.flow.instructions; |
| 2 | 2 | ||
| 3 | import static com.google.common.base.MoreObjects.toStringHelper; | 3 | import static com.google.common.base.MoreObjects.toStringHelper; |
| 4 | 4 | ||
| 5 | +import java.util.Objects; | ||
| 6 | + | ||
| 5 | import org.onlab.packet.MacAddress; | 7 | import org.onlab.packet.MacAddress; |
| 6 | import org.onlab.packet.VlanId; | 8 | import org.onlab.packet.VlanId; |
| 7 | 9 | ||
| ... | @@ -74,6 +76,25 @@ public abstract class L2ModificationInstruction implements Instruction { | ... | @@ -74,6 +76,25 @@ public abstract class L2ModificationInstruction implements Instruction { |
| 74 | .add("mac", mac).toString(); | 76 | .add("mac", mac).toString(); |
| 75 | } | 77 | } |
| 76 | 78 | ||
| 79 | + @Override | ||
| 80 | + public int hashCode() { | ||
| 81 | + return Objects.hash(mac, subtype); | ||
| 82 | + } | ||
| 83 | + | ||
| 84 | + @Override | ||
| 85 | + public boolean equals(Object obj) { | ||
| 86 | + if (this == obj) { | ||
| 87 | + return true; | ||
| 88 | + } | ||
| 89 | + if (obj instanceof ModEtherInstruction) { | ||
| 90 | + ModEtherInstruction that = (ModEtherInstruction) obj; | ||
| 91 | + return Objects.equals(mac, that.mac) && | ||
| 92 | + Objects.equals(subtype, that.subtype); | ||
| 93 | + | ||
| 94 | + } | ||
| 95 | + return false; | ||
| 96 | + } | ||
| 97 | + | ||
| 77 | 98 | ||
| 78 | } | 99 | } |
| 79 | 100 | ||
| ... | @@ -103,6 +124,25 @@ public abstract class L2ModificationInstruction implements Instruction { | ... | @@ -103,6 +124,25 @@ public abstract class L2ModificationInstruction implements Instruction { |
| 103 | .add("id", vlanId).toString(); | 124 | .add("id", vlanId).toString(); |
| 104 | } | 125 | } |
| 105 | 126 | ||
| 127 | + @Override | ||
| 128 | + public int hashCode() { | ||
| 129 | + return Objects.hash(vlanId, subtype()); | ||
| 130 | + } | ||
| 131 | + | ||
| 132 | + @Override | ||
| 133 | + public boolean equals(Object obj) { | ||
| 134 | + if (this == obj) { | ||
| 135 | + return true; | ||
| 136 | + } | ||
| 137 | + if (obj instanceof ModVlanIdInstruction) { | ||
| 138 | + ModVlanIdInstruction that = (ModVlanIdInstruction) obj; | ||
| 139 | + return Objects.equals(vlanId, that.vlanId); | ||
| 140 | + | ||
| 141 | + } | ||
| 142 | + return false; | ||
| 143 | + } | ||
| 144 | + | ||
| 145 | + | ||
| 106 | } | 146 | } |
| 107 | 147 | ||
| 108 | /** | 148 | /** |
| ... | @@ -131,6 +171,24 @@ public abstract class L2ModificationInstruction implements Instruction { | ... | @@ -131,6 +171,24 @@ public abstract class L2ModificationInstruction implements Instruction { |
| 131 | .add("pcp", Long.toHexString(vlanPcp)).toString(); | 171 | .add("pcp", Long.toHexString(vlanPcp)).toString(); |
| 132 | } | 172 | } |
| 133 | 173 | ||
| 174 | + @Override | ||
| 175 | + public int hashCode() { | ||
| 176 | + return Objects.hash(vlanPcp, subtype()); | ||
| 177 | + } | ||
| 178 | + | ||
| 179 | + @Override | ||
| 180 | + public boolean equals(Object obj) { | ||
| 181 | + if (this == obj) { | ||
| 182 | + return true; | ||
| 183 | + } | ||
| 184 | + if (obj instanceof ModVlanPcpInstruction) { | ||
| 185 | + ModVlanPcpInstruction that = (ModVlanPcpInstruction) obj; | ||
| 186 | + return Objects.equals(vlanPcp, that.vlanPcp); | ||
| 187 | + | ||
| 188 | + } | ||
| 189 | + return false; | ||
| 190 | + } | ||
| 191 | + | ||
| 134 | } | 192 | } |
| 135 | 193 | ||
| 136 | 194 | ... | ... |
| ... | @@ -2,6 +2,8 @@ package org.onlab.onos.net.flow.instructions; | ... | @@ -2,6 +2,8 @@ package org.onlab.onos.net.flow.instructions; |
| 2 | 2 | ||
| 3 | import static com.google.common.base.MoreObjects.toStringHelper; | 3 | import static com.google.common.base.MoreObjects.toStringHelper; |
| 4 | 4 | ||
| 5 | +import java.util.Objects; | ||
| 6 | + | ||
| 5 | import org.onlab.packet.IpPrefix; | 7 | import org.onlab.packet.IpPrefix; |
| 6 | 8 | ||
| 7 | /** | 9 | /** |
| ... | @@ -66,5 +68,23 @@ public abstract class L3ModificationInstruction implements Instruction { | ... | @@ -66,5 +68,23 @@ public abstract class L3ModificationInstruction implements Instruction { |
| 66 | .add("ip", ip).toString(); | 68 | .add("ip", ip).toString(); |
| 67 | } | 69 | } |
| 68 | 70 | ||
| 71 | + @Override | ||
| 72 | + public int hashCode() { | ||
| 73 | + return Objects.hash(ip, subtype()); | ||
| 74 | + } | ||
| 75 | + | ||
| 76 | + @Override | ||
| 77 | + public boolean equals(Object obj) { | ||
| 78 | + if (this == obj) { | ||
| 79 | + return true; | ||
| 80 | + } | ||
| 81 | + if (obj instanceof ModIPInstruction) { | ||
| 82 | + ModIPInstruction that = (ModIPInstruction) obj; | ||
| 83 | + return Objects.equals(ip, that.ip); | ||
| 84 | + | ||
| 85 | + } | ||
| 86 | + return false; | ||
| 87 | + } | ||
| 88 | + | ||
| 69 | } | 89 | } |
| 70 | } | 90 | } | ... | ... |
| 1 | package org.onlab.onos.net.intent.impl; | 1 | package org.onlab.onos.net.intent.impl; |
| 2 | 2 | ||
| 3 | -import com.google.common.collect.ImmutableMap; | 3 | +import static com.google.common.base.Preconditions.checkNotNull; |
| 4 | +import static java.util.concurrent.Executors.newSingleThreadExecutor; | ||
| 5 | +import static org.onlab.onos.net.intent.IntentState.COMPILING; | ||
| 6 | +import static org.onlab.onos.net.intent.IntentState.FAILED; | ||
| 7 | +import static org.onlab.onos.net.intent.IntentState.INSTALLED; | ||
| 8 | +import static org.onlab.onos.net.intent.IntentState.INSTALLING; | ||
| 9 | +import static org.onlab.onos.net.intent.IntentState.RECOMPILING; | ||
| 10 | +import static org.onlab.onos.net.intent.IntentState.WITHDRAWING; | ||
| 11 | +import static org.onlab.onos.net.intent.IntentState.WITHDRAWN; | ||
| 12 | +import static org.onlab.util.Tools.namedThreads; | ||
| 13 | +import static org.slf4j.LoggerFactory.getLogger; | ||
| 14 | + | ||
| 15 | +import java.util.ArrayList; | ||
| 16 | +import java.util.List; | ||
| 17 | +import java.util.Map; | ||
| 18 | +import java.util.Objects; | ||
| 19 | +import java.util.concurrent.ConcurrentHashMap; | ||
| 20 | +import java.util.concurrent.ConcurrentMap; | ||
| 21 | +import java.util.concurrent.ExecutorService; | ||
| 22 | + | ||
| 4 | import org.apache.felix.scr.annotations.Activate; | 23 | import org.apache.felix.scr.annotations.Activate; |
| 5 | import org.apache.felix.scr.annotations.Component; | 24 | import org.apache.felix.scr.annotations.Component; |
| 6 | import org.apache.felix.scr.annotations.Deactivate; | 25 | import org.apache.felix.scr.annotations.Deactivate; |
| ... | @@ -25,20 +44,7 @@ import org.onlab.onos.net.intent.IntentStore; | ... | @@ -25,20 +44,7 @@ import org.onlab.onos.net.intent.IntentStore; |
| 25 | import org.onlab.onos.net.intent.IntentStoreDelegate; | 44 | import org.onlab.onos.net.intent.IntentStoreDelegate; |
| 26 | import org.slf4j.Logger; | 45 | import org.slf4j.Logger; |
| 27 | 46 | ||
| 28 | -import java.util.ArrayList; | 47 | +import com.google.common.collect.ImmutableMap; |
| 29 | -import java.util.List; | ||
| 30 | -import java.util.Map; | ||
| 31 | -import java.util.Objects; | ||
| 32 | -import java.util.concurrent.ConcurrentHashMap; | ||
| 33 | -import java.util.concurrent.ConcurrentMap; | ||
| 34 | -import java.util.concurrent.ExecutorService; | ||
| 35 | - | ||
| 36 | -import static com.google.common.base.Preconditions.checkNotNull; | ||
| 37 | -import static java.util.concurrent.Executors.newSingleThreadExecutor; | ||
| 38 | -import static org.onlab.onos.net.intent.IntentState.*; | ||
| 39 | -import static org.onlab.util.Tools.delay; | ||
| 40 | -import static org.onlab.util.Tools.namedThreads; | ||
| 41 | -import static org.slf4j.LoggerFactory.getLogger; | ||
| 42 | 48 | ||
| 43 | /** | 49 | /** |
| 44 | * An implementation of Intent Manager. | 50 | * An implementation of Intent Manager. |
| ... | @@ -61,7 +67,7 @@ public class IntentManager | ... | @@ -61,7 +67,7 @@ public class IntentManager |
| 61 | private final AbstractListenerRegistry<IntentEvent, IntentListener> | 67 | private final AbstractListenerRegistry<IntentEvent, IntentListener> |
| 62 | listenerRegistry = new AbstractListenerRegistry<>(); | 68 | listenerRegistry = new AbstractListenerRegistry<>(); |
| 63 | 69 | ||
| 64 | - private ExecutorService executor = newSingleThreadExecutor(namedThreads("onos-intents")); | 70 | + private final ExecutorService executor = newSingleThreadExecutor(namedThreads("onos-intents")); |
| 65 | 71 | ||
| 66 | private final IntentStoreDelegate delegate = new InternalStoreDelegate(); | 72 | private final IntentStoreDelegate delegate = new InternalStoreDelegate(); |
| 67 | private final TopologyChangeDelegate topoDelegate = new InternalTopoChangeDelegate(); | 73 | private final TopologyChangeDelegate topoDelegate = new InternalTopoChangeDelegate(); |
| ... | @@ -417,7 +423,7 @@ public class IntentManager | ... | @@ -417,7 +423,7 @@ public class IntentManager |
| 417 | for (IntentId intentId : intentIds) { | 423 | for (IntentId intentId : intentIds) { |
| 418 | Intent intent = getIntent(intentId); | 424 | Intent intent = getIntent(intentId); |
| 419 | uninstallIntent(intent); | 425 | uninstallIntent(intent); |
| 420 | - delay(1000); | 426 | + |
| 421 | executeRecompilingPhase(intent); | 427 | executeRecompilingPhase(intent); |
| 422 | } | 428 | } |
| 423 | 429 | ... | ... |
| 1 | package org.onlab.onos.net.intent.impl; | 1 | package org.onlab.onos.net.intent.impl; |
| 2 | 2 | ||
| 3 | import static org.onlab.onos.net.flow.DefaultTrafficTreatment.builder; | 3 | import static org.onlab.onos.net.flow.DefaultTrafficTreatment.builder; |
| 4 | +import static org.slf4j.LoggerFactory.getLogger; | ||
| 4 | 5 | ||
| 5 | import java.util.Iterator; | 6 | import java.util.Iterator; |
| 6 | 7 | ||
| ... | @@ -21,6 +22,7 @@ import org.onlab.onos.net.flow.TrafficTreatment; | ... | @@ -21,6 +22,7 @@ import org.onlab.onos.net.flow.TrafficTreatment; |
| 21 | import org.onlab.onos.net.intent.IntentExtensionService; | 22 | import org.onlab.onos.net.intent.IntentExtensionService; |
| 22 | import org.onlab.onos.net.intent.IntentInstaller; | 23 | import org.onlab.onos.net.intent.IntentInstaller; |
| 23 | import org.onlab.onos.net.intent.PathIntent; | 24 | import org.onlab.onos.net.intent.PathIntent; |
| 25 | +import org.slf4j.Logger; | ||
| 24 | 26 | ||
| 25 | /** | 27 | /** |
| 26 | * Installer for {@link PathIntent path connectivity intents}. | 28 | * Installer for {@link PathIntent path connectivity intents}. |
| ... | @@ -28,6 +30,8 @@ import org.onlab.onos.net.intent.PathIntent; | ... | @@ -28,6 +30,8 @@ import org.onlab.onos.net.intent.PathIntent; |
| 28 | @Component(immediate = true) | 30 | @Component(immediate = true) |
| 29 | public class PathIntentInstaller implements IntentInstaller<PathIntent> { | 31 | public class PathIntentInstaller implements IntentInstaller<PathIntent> { |
| 30 | 32 | ||
| 33 | + private final Logger log = getLogger(getClass()); | ||
| 34 | + | ||
| 31 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) | 35 | @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY) |
| 32 | protected IntentExtensionService intentManager; | 36 | protected IntentExtensionService intentManager; |
| 33 | 37 | ||
| ... | @@ -82,6 +86,7 @@ public class PathIntentInstaller implements IntentInstaller<PathIntent> { | ... | @@ -82,6 +86,7 @@ public class PathIntentInstaller implements IntentInstaller<PathIntent> { |
| 82 | FlowRule rule = new DefaultFlowRule(link.src().deviceId(), | 86 | FlowRule rule = new DefaultFlowRule(link.src().deviceId(), |
| 83 | builder.build(), treatment, | 87 | builder.build(), treatment, |
| 84 | 123, appId, 600); | 88 | 123, appId, 600); |
| 89 | + | ||
| 85 | flowRuleService.removeFlowRules(rule); | 90 | flowRuleService.removeFlowRules(rule); |
| 86 | prev = link.dst(); | 91 | prev = link.dst(); |
| 87 | } | 92 | } | ... | ... |
| ... | @@ -103,6 +103,7 @@ public class SimpleFlowRuleStore | ... | @@ -103,6 +103,7 @@ public class SimpleFlowRuleStore |
| 103 | public synchronized void deleteFlowRule(FlowRule rule) { | 103 | public synchronized void deleteFlowRule(FlowRule rule) { |
| 104 | FlowEntry entry = getFlowEntry(rule); | 104 | FlowEntry entry = getFlowEntry(rule); |
| 105 | if (entry == null) { | 105 | if (entry == null) { |
| 106 | + //log.warn("Cannot find rule {}", rule); | ||
| 106 | return; | 107 | return; |
| 107 | } | 108 | } |
| 108 | entry.setState(FlowEntryState.PENDING_REMOVE); | 109 | entry.setState(FlowEntryState.PENDING_REMOVE); |
| ... | @@ -125,7 +126,7 @@ public class SimpleFlowRuleStore | ... | @@ -125,7 +126,7 @@ public class SimpleFlowRuleStore |
| 125 | return new FlowRuleEvent(Type.RULE_UPDATED, rule); | 126 | return new FlowRuleEvent(Type.RULE_UPDATED, rule); |
| 126 | } | 127 | } |
| 127 | 128 | ||
| 128 | - flowEntries.put(did, rule); | 129 | + //flowEntries.put(did, rule); |
| 129 | return null; | 130 | return null; |
| 130 | } | 131 | } |
| 131 | 132 | ... | ... |
-
Please register or login to post a comment