From bb910e52ec60508ad70f72382db389d6b348f5af Mon Sep 17 00:00:00 2001
From: Ryan Izard <ryan.izard@bigswitch.com>
Date: Wed, 13 Jul 2016 17:11:03 -0400
Subject: [PATCH] Create wrapper class that takes an OFMessage and allows for
direct use in other containers where we don't care about the XID of the
message. For example, if we want to create a unit test with predictable
results but the XIDs are different depending on unit test execution, we can
use the wrapper class to guarantee consistency. In the main code, we can more
easily compare OFMessages without care for the XID and create cool data
structures like caches.
---
.../util/OFMessageUtils.java | 73 ++++++++++++++-----
.../forwarding/ForwardingTest.java | 24 +++---
.../net/floodlightcontroller/hub/HubTest.java | 2 +-
.../learningswitch/LearningSwitchTest.java | 8 +-
.../loadbalancer/LoadBalancerTest.java | 2 +-
.../staticentry/StaticFlowTests.java | 2 +-
6 files changed, 74 insertions(+), 37 deletions(-)
diff --git a/src/main/java/net/floodlightcontroller/util/OFMessageUtils.java b/src/main/java/net/floodlightcontroller/util/OFMessageUtils.java
index ffca61530..2259e6f0a 100644
--- a/src/main/java/net/floodlightcontroller/util/OFMessageUtils.java
+++ b/src/main/java/net/floodlightcontroller/util/OFMessageUtils.java
@@ -35,6 +35,61 @@ public class OFMessageUtils {
*/
private OFMessageUtils() {};
+ /**
+ * Simple class to streamline the use of OFMessage's
+ * equalsIgnoreXid() and hashCodeIgnoreXid() functions.
+ * Use this class to wrap OFMessages prior to inserting
+ * them in containers where lookup or equality checks
+ * should not include the XID.
+ *
+ * See {@link net.floodlightcontroller.util.OFMessageDamper}
+ * as an example where it's used to help cache OFMessages.
+ * @author rizard
+ */
+ public static class OFMessageIgnoreXid {
+ private OFMessage m;
+
+ private OFMessageIgnoreXid() {}
+ private OFMessageIgnoreXid(OFMessage m) {
+ this.m = m;
+ }
+
+ /**
+ * Wrap an OFMessage to ignore the XID
+ * when checking for equality or computing
+ * the OFMessage's hash.
+ * @param m
+ * @return
+ */
+ public static OFMessageIgnoreXid of(OFMessage m) {
+ return new OFMessageIgnoreXid(m);
+ }
+
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + ((m == null) ? 0 : m.hashCodeIgnoreXid());
+ return result;
+ }
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (obj == null)
+ return false;
+ if (getClass() != obj.getClass())
+ return false;
+ OFMessageIgnoreXid other = (OFMessageIgnoreXid) obj;
+ if (m == null) {
+ if (other.m != null)
+ return false;
+ } else if (!m.equalsIgnoreXid(other.m))
+ return false;
+ return true;
+ }
+ }
+
/**
* Get the ingress port of a packet-in message. The manner in which
* this is done depends on the OpenFlow version. OF1.0 and 1.1 have
@@ -81,24 +136,6 @@ public class OFMessageUtils {
return pi.getMatch().get(MatchField.VLAN_VID) == null ? OFVlanVidMatch.UNTAGGED : pi.getMatch().get(MatchField.VLAN_VID);
}
- /**
- * Returns true if each object is deeply-equal in the same manner that
- * Object's equals() does with the exception of the XID field, which is
- * ignored; otherwise, returns false.
- *
- * NOTE: This function is VERY INEFFICIENT and creates a new OFMessage
- * object in order to the the comparison minus the XID. It is advised
- * that you use it sparingly and ideally only within unit tests.
- *
- * @param a; object A to compare
- * @param b; object B to compare
- * @return true if A and B are deeply-equal; false otherwise
- */
- public static boolean equalsIgnoreXid(OFMessage a, OFMessage b) {
- OFMessage.Builder mb = b.createBuilder().setXid(a.getXid());
- return a.equals(mb.build());
- }
-
/**
* Writes an OFPacketOut message to a switch.
*
diff --git a/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java b/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
index 9d68ba9f9..a3e5dab0b 100644
--- a/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
+++ b/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
@@ -488,15 +488,15 @@ public class ForwardingTest extends FloodlightTestCase {
for (OFMessage m: msglist) {
if (m instanceof OFFlowMod)
- assertTrue(OFMessageUtils.equalsIgnoreXid(fm1, m));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(fm1), OFMessageUtils.OFMessageIgnoreXid.of(m));
else if (m instanceof OFPacketOut) {
- assertTrue(OFMessageUtils.equalsIgnoreXid(packetOut, m));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(packetOut), OFMessageUtils.OFMessageIgnoreXid.of(m));
}
}
OFMessage m = wc2.getValue();
assert (m instanceof OFFlowMod);
- assertTrue(OFMessageUtils.equalsIgnoreXid(m, fm2));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(m), OFMessageUtils.OFMessageIgnoreXid.of(fm2));
removeDeviceFromContext();
}
@@ -558,15 +558,15 @@ public class ForwardingTest extends FloodlightTestCase {
for (OFMessage m: msglist) {
if (m instanceof OFFlowMod)
- assertTrue(OFMessageUtils.equalsIgnoreXid(fm1, m));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(fm1), OFMessageUtils.OFMessageIgnoreXid.of(m));
else if (m instanceof OFPacketOut) {
- assertTrue(OFMessageUtils.equalsIgnoreXid(packetOutIPv6, m));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(packetOutIPv6), OFMessageUtils.OFMessageIgnoreXid.of(m));
}
}
OFMessage m = wc2.getValue();
assert (m instanceof OFFlowMod);
- assertTrue(OFMessageUtils.equalsIgnoreXid(m, fm2));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(m), OFMessageUtils.OFMessageIgnoreXid.of(fm2));
removeDeviceFromContext();
}
@@ -619,8 +619,8 @@ public class ForwardingTest extends FloodlightTestCase {
assertTrue(wc1.hasCaptured());
assertTrue(wc2.hasCaptured());
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc1.getValue(), fm1));
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc2.getValue(), packetOut));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc1.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(fm1));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc2.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(packetOut));
removeDeviceFromContext();
}
@@ -673,8 +673,8 @@ public class ForwardingTest extends FloodlightTestCase {
assertTrue(wc1.hasCaptured());
assertTrue(wc2.hasCaptured());
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc1.getValue(), fm1));
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc2.getValue(), packetOutIPv6));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc1.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(fm1));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc2.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(packetOutIPv6));
removeDeviceFromContext();
}
@@ -763,7 +763,7 @@ public class ForwardingTest extends FloodlightTestCase {
verify(sw1, sw2, routingEngine);
assertTrue(wc1.hasCaptured());
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc1.getValue(), packetOutFlooded));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc1.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(packetOutFlooded));
removeDeviceFromContext();
}
@@ -796,7 +796,7 @@ public class ForwardingTest extends FloodlightTestCase {
verify(sw1, sw2, routingEngine);
assertTrue(wc1.hasCaptured());
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc1.getValue(), packetOutFloodedIPv6));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc1.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(packetOutFloodedIPv6));
removeDeviceFromContext();
}
diff --git a/src/test/java/net/floodlightcontroller/hub/HubTest.java b/src/test/java/net/floodlightcontroller/hub/HubTest.java
index 6fcd37e81..61ddd24ba 100644
--- a/src/test/java/net/floodlightcontroller/hub/HubTest.java
+++ b/src/test/java/net/floodlightcontroller/hub/HubTest.java
@@ -138,7 +138,7 @@ public class HubTest extends FloodlightTestCase {
assertTrue(wc1.hasCaptured());
OFMessage m = wc1.getValue();
- assertTrue(OFMessageUtils.equalsIgnoreXid(m, po));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(m), OFMessageUtils.OFMessageIgnoreXid.of(po));
}
@Test
diff --git a/src/test/java/net/floodlightcontroller/learningswitch/LearningSwitchTest.java b/src/test/java/net/floodlightcontroller/learningswitch/LearningSwitchTest.java
index eb884f440..8796e3f62 100644
--- a/src/test/java/net/floodlightcontroller/learningswitch/LearningSwitchTest.java
+++ b/src/test/java/net/floodlightcontroller/learningswitch/LearningSwitchTest.java
@@ -204,7 +204,7 @@ public class LearningSwitchTest extends FloodlightTestCase {
verify(mockSwitch);
assertTrue(wc1.hasCaptured());
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc1.getValue(), po));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc1.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(po));
// Verify the MAC table inside the switch
assertEquals(OFPort.of(1), result);
@@ -291,9 +291,9 @@ public class LearningSwitchTest extends FloodlightTestCase {
assertTrue(wc1.hasCaptured());
assertTrue(wc2.hasCaptured());
assertTrue(wc3.hasCaptured());
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc1.getValue(), packetOut));
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc2.getValue(), fm1));
- assertTrue(OFMessageUtils.equalsIgnoreXid(wc3.getValue(), fm2));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc1.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(packetOut));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc2.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(fm1));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(wc3.getValue()), OFMessageUtils.OFMessageIgnoreXid.of(fm2));
// Verify the MAC table inside the switch
assertEquals(OFPort.of(1), result);
diff --git a/src/test/java/net/floodlightcontroller/loadbalancer/LoadBalancerTest.java b/src/test/java/net/floodlightcontroller/loadbalancer/LoadBalancerTest.java
index 09bfec33a..9bd3c98fe 100644
--- a/src/test/java/net/floodlightcontroller/loadbalancer/LoadBalancerTest.java
+++ b/src/test/java/net/floodlightcontroller/loadbalancer/LoadBalancerTest.java
@@ -541,7 +541,7 @@ public class LoadBalancerTest extends FloodlightTestCase {
for (OFMessage m: msglist1) {
if (m instanceof OFPacketOut)
- assertTrue(OFMessageUtils.equalsIgnoreXid(arpReplyPacketOut1, m));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(arpReplyPacketOut1), OFMessageUtils.OFMessageIgnoreXid.of(m));
else
assertTrue(false); // unexpected message
}
diff --git a/src/test/java/net/floodlightcontroller/staticentry/StaticFlowTests.java b/src/test/java/net/floodlightcontroller/staticentry/StaticFlowTests.java
index a2a6eb964..e2b52156f 100644
--- a/src/test/java/net/floodlightcontroller/staticentry/StaticFlowTests.java
+++ b/src/test/java/net/floodlightcontroller/staticentry/StaticFlowTests.java
@@ -175,7 +175,7 @@ public class StaticFlowTests extends FloodlightTestCase {
// dont' bother testing the cookie; just copy it over
goodFlowMod = goodFlowMod.createBuilder().setCookie(testFlowMod.getCookie()).build();
// .. so we can continue to use .equals()
- assertTrue(OFMessageUtils.equalsIgnoreXid(goodFlowMod, testFlowMod));
+ assertEquals(OFMessageUtils.OFMessageIgnoreXid.of(goodFlowMod), OFMessageUtils.OFMessageIgnoreXid.of(testFlowMod));
}
--
GitLab