From: Alessandro Boch Date: Sat, 24 Aug 2013 02:43:32 +0000 (-0700) Subject: Fix Match intersection logic X-Git-Tag: releasepom-0.1.0~178^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=f69f22a30c6e21cd61b7221de4ab5fd4a69a655f;hp=-c Fix Match intersection logic - Change in Match.java: Empty match represents all packets (universal set) while empty set is the null match. So, in case of no intersection, return a null Match. intersect() and getIntersection() optimized in case of universal and empty sets - Changes in MatchTest.java to junit the above changes - Change in FlowEntry.java: Match.mergewithFilter() is now returning a new Match instance Change-Id: Idd3ff80d2fbcc11c3f83757268806596b7b2fc50 Signed-off-by: Alessandro Boch --- f69f22a30c6e21cd61b7221de4ab5fd4a69a655f diff --git a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowEntry.java b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowEntry.java index e8c5f648fa..83106a391c 100644 --- a/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowEntry.java +++ b/opendaylight/forwardingrulesmanager/api/src/main/java/org/opendaylight/controller/forwardingrulesmanager/FlowEntry.java @@ -168,14 +168,13 @@ public class FlowEntry implements Cloneable, Serializable { public FlowEntry mergeWith(ContainerFlow containerFlow) { Match myMatch = flow.getMatch(); - // Based on this flow direction, rearrange the match - Match match = containerFlow.getMatch(); + Match filter = containerFlow.getMatch(); // Merge - myMatch.mergeWithFilter(match); + Match merge = myMatch.mergeWithFilter(filter); // Replace this Flow's match with merged version - flow.setMatch(myMatch); + flow.setMatch(merge); return this; } diff --git a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java index 7a94d05bf1..30c25af57f 100644 --- a/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java +++ b/opendaylight/sal/api/src/main/java/org/opendaylight/controller/sal/match/Match.java @@ -287,12 +287,13 @@ public class Match implements Cloneable, Serializable { * The result is the match object representing the intersection of the set * of packets described by this match with the set of packets described by * the filter match. If the intersection of the two sets is empty, the - * return match will be empty as well. + * return match will be null. * * @param filter * the match with which attempting the merge * @return a new Match object describing the set of packets represented by - * the intersection of this and the filter matches. + * the intersection of this and the filter matches. null if the + * intersection is empty. */ public Match mergeWithFilter(Match filter) { return this.getIntersection(filter); @@ -301,55 +302,70 @@ public class Match implements Cloneable, Serializable { /** * Return the match representing the intersection of the set of packets * described by this match with the set of packets described by the other - * match. Such as m.intersect(m) == m and m.intersect(o) == o where o is an - * empty match. + * match. Such as m.getIntersection(m) == m, m.getIntersection(u) == m and + * m.getIntersection(o) == o where u is an empty match (universal set, all + * packets) and o is the null match (empty set). * * @param other * the match with which computing the intersection * @return a new Match object representing the intersection of the set of * packets described by this match with the set of packets described - * by the other match. + * by the other match. null when the intersection is the empty set. */ public Match getIntersection(Match other) { + // If no intersection, return the empty set + if (!this.intersetcs(other)) { + return null; + } + // Check if any of the two is the universal match + if (this.getMatches() == 0) { + return other.clone(); + } + if (other.getMatches() == 0) { + return this.clone(); + } + // Derive the intersection Match intersection = new Match(); - if (this.intersetcs(other)) { - for (MatchType type : MatchType.values()) { - if (this.isAny(type) && other.isAny(type)) { - continue; - } - if (this.isAny(type)) { - intersection.setField(other.getField(type).clone()); - continue; - } else if (other.isAny(type)) { - intersection.setField(this.getField(type).clone()); - continue; - } - // Either they are equal or it is about IP address - switch (type) { - // When it is about IP address, take the wider prefix address between the twos - case NW_SRC: - case NW_DST: - MatchField thisField = this.getField(type); - MatchField otherField = other.getField(type); - InetAddress thisAddress = (InetAddress) thisField.getValue(); - InetAddress otherAddress = (InetAddress) otherField.getValue(); - InetAddress thisMask = (InetAddress) thisField.getMask(); - InetAddress otherMask = (InetAddress) otherField.getMask(); - - int thisMaskLen = (thisMask == null) ? ((thisAddress instanceof Inet4Address) ? 32 : 128) - : NetUtils.getSubnetMaskLength(thisMask); - int otherMaskLen = (otherMask == null) ? ((otherAddress instanceof Inet4Address) ? 32 : 128) - : NetUtils.getSubnetMaskLength(otherMask); - if (otherMaskLen < thisMaskLen) { - intersection.setField(new MatchField(type, NetUtils.getSubnetPrefix(otherAddress, otherMaskLen), otherMask)); - } else { - intersection.setField(new MatchField(type, NetUtils.getSubnetPrefix(thisAddress, thisMaskLen), thisMask)); - } - break; - default: - // this and other match field are equal for this type, pick this match field - intersection.setField(this.getField(type).clone()); + for (MatchType type : MatchType.values()) { + if (this.isAny(type) && other.isAny(type)) { + continue; + } + if (this.isAny(type)) { + intersection.setField(other.getField(type).clone()); + continue; + } else if (other.isAny(type)) { + intersection.setField(this.getField(type).clone()); + continue; + } + // Either they are equal or it is about IP address + switch (type) { + // When it is about IP address, take the wider prefix address + // between the twos + case NW_SRC: + case NW_DST: + MatchField thisField = this.getField(type); + MatchField otherField = other.getField(type); + InetAddress thisAddress = (InetAddress) thisField.getValue(); + InetAddress otherAddress = (InetAddress) otherField.getValue(); + InetAddress thisMask = (InetAddress) thisField.getMask(); + InetAddress otherMask = (InetAddress) otherField.getMask(); + + int thisMaskLen = (thisMask == null) ? ((thisAddress instanceof Inet4Address) ? 32 : 128) : NetUtils + .getSubnetMaskLength(thisMask); + int otherMaskLen = (otherMask == null) ? ((otherAddress instanceof Inet4Address) ? 32 : 128) : NetUtils + .getSubnetMaskLength(otherMask); + if (otherMaskLen < thisMaskLen) { + intersection.setField(new MatchField(type, NetUtils.getSubnetPrefix(otherAddress, otherMaskLen), + otherMask)); + } else { + intersection.setField(new MatchField(type, NetUtils.getSubnetPrefix(thisAddress, thisMaskLen), + thisMask)); } + break; + default: + // this and other match field are equal for this type, pick this + // match field + intersection.setField(this.getField(type).clone()); } } return intersection; @@ -372,10 +388,14 @@ public class Match implements Cloneable, Serializable { * is non empty */ public boolean intersetcs(Match other) { - if (this.getMatches() == 0 || other.getMatches() == 0) { - // No intersection with the empty set + // No intersection with the empty set + if (other == null) { return false; } + // Always intersection with the universal set + if (this.getMatches() == 0 || other.getMatches() == 0) { + return true; + } // Iterate through the MatchType defined in the filter for (MatchType type : MatchType.values()) { if (this.isAny(type) || other.isAny(type)) { diff --git a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java index 5dcb898dfb..f3c7a95b0e 100644 --- a/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java +++ b/opendaylight/sal/api/src/test/java/org/opendaylight/controller/sal/match/MatchTest.java @@ -565,8 +565,9 @@ public class MatchTest { Assert.assertTrue(((InetAddress)i.getField(MatchType.NW_SRC).getValue()).equals(ip2)); Assert.assertTrue(((InetAddress)i.getField(MatchType.NW_SRC).getMask()).equals(ipm2)); + // Empty set i = m2.getIntersection(m3); - Assert.assertTrue(i.getMatches() == 0); + Assert.assertNull(i); Match m4 = new Match(); m4.setField(MatchType.DL_TYPE, ethType); @@ -608,9 +609,12 @@ public class MatchTest { Assert.assertTrue(m6.intersetcs(m6)); Assert.assertTrue(m6.getIntersection(m6).equals(m6)); - // Empty match, represents the empty set - Match o = new Match(); - Assert.assertTrue(m6.getIntersection(o).equals(o)); - Assert.assertTrue(o.getIntersection(m6).equals(o)); + // Empty match, represents the universal set (all packets) + Match u = new Match(); + Assert.assertTrue(m6.getIntersection(u).equals(m6)); + Assert.assertTrue(u.getIntersection(m6).equals(m6)); + + // No intersection with null match, empty set + Assert.assertNull(m6.getIntersection(null)); } }