Skip to content

Commit

Permalink
Match: consistently hide Fields w/ broken prereqs LOXI-75
Browse files Browse the repository at this point in the history
Previously, isExact(), isPartiallyMasked() and isFullyWildcarded()
did not check whether the required prerequisites for a field
had been set. get() and getMasked() did, leading to inconsistent
results / NPEs.

This makes all of the methods honor the prerequisite checks.
It also adds a unit test checking the behavior.
  • Loading branch information
andi-bigswitch committed Nov 20, 2015
1 parent 0ba42d4 commit 3850039
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package org.projectfloodlight.protocol.match;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertThat;
import static org.projectfloodlight.openflow.protocol.match.MatchField.ETH_TYPE;
import static org.projectfloodlight.openflow.protocol.match.MatchField.IPV4_SRC;

import java.util.Arrays;

import org.hamcrest.Matchers;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
import org.projectfloodlight.openflow.protocol.OFFactories;
import org.projectfloodlight.openflow.protocol.OFFactory;
import org.projectfloodlight.openflow.protocol.OFVersion;
import org.projectfloodlight.openflow.protocol.match.Match;
import org.projectfloodlight.openflow.protocol.match.MatchField;
import org.projectfloodlight.openflow.types.EthType;
import org.projectfloodlight.openflow.types.IPv4Address;

@RunWith(Parameterized.class)
public class OFMatchPrerequisitesTest {
private final OFFactory factory;

@Parameters(name="{index}.ChannelHandlerVersion={0}")
public static Iterable<Object[]> data() {
return Arrays.asList(new Object[][] {
{OFVersion.OF_10},
{OFVersion.OF_13},
{OFVersion.OF_14}
});
}

public OFMatchPrerequisitesTest(OFVersion version) {
factory = OFFactories.getFactory(version);
}

@Test
public void testPreRequisitesNotMet() {
Match match = factory.buildMatch()
.setExact(ETH_TYPE, EthType.IPv6)
.setExact(IPV4_SRC, IPv4Address.of("1.2.3.4"))
.build();

assertThat(match.get(ETH_TYPE), equalTo(EthType.IPv6));
assertThat(match.isExact(ETH_TYPE), equalTo(true));
assertThat(match.isPartiallyMasked(ETH_TYPE), equalTo(false));
assertThat(match.isFullyWildcarded(ETH_TYPE), equalTo(false));

assertThat(match.get(IPV4_SRC), nullValue());
assertThat(match.isExact(IPV4_SRC), equalTo(false));
assertThat(match.isPartiallyMasked(IPV4_SRC), equalTo(false));
assertThat(match.isFullyWildcarded(IPV4_SRC), equalTo(true));

Iterable<MatchField<?>> matchFields = match.getMatchFields();
assertThat(matchFields, Matchers.<MatchField<?>>iterableWithSize(1));
assertThat(matchFields, Matchers.<MatchField<?>>contains(MatchField.ETH_TYPE));
}

@Test
public void testPreRequisitesMet() {
Match match = factory.buildMatch()
.setExact(ETH_TYPE, EthType.IPv4)
.setExact(IPV4_SRC, IPv4Address.of("1.2.3.4"))
.build();

assertThat(match.get(ETH_TYPE), equalTo(EthType.IPv4));
assertThat(match.isExact(ETH_TYPE), equalTo(true));
assertThat(match.isPartiallyMasked(ETH_TYPE), equalTo(false));
assertThat(match.isFullyWildcarded(ETH_TYPE), equalTo(false));

assertThat(match.get(IPV4_SRC), equalTo(IPv4Address.of("1.2.3.4")));
assertThat(match.isExact(IPV4_SRC), equalTo(true));
assertThat(match.isPartiallyMasked(IPV4_SRC), equalTo(false));
assertThat(match.isFullyWildcarded(IPV4_SRC), equalTo(false));

Iterable<MatchField<?>> matchFields = match.getMatchFields();
assertThat(matchFields, Matchers.<MatchField<?>>iterableWithSize(2));
assertThat(matchFields, Matchers.<MatchField<?>>contains(MatchField.ETH_TYPE, MatchField.IPV4_SRC));
}


}
7 changes: 7 additions & 0 deletions java_gen/templates/custom/OFMatchV3.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ public boolean isExact(MatchField<?> field) {
if (!supports(field))
throw new UnsupportedOperationException("${msg.name} does not support matching on field " + field.getName());

if(!field.arePrerequisitesOK(this))
return false;

OFOxm<?> oxm = this.oxmList.get(field);

return oxm != null && !oxm.isMasked();
Expand All @@ -67,6 +70,8 @@ public boolean isExact(MatchField<?> field) {
public boolean isFullyWildcarded(MatchField<?> field) {
if (!supports(field))
throw new UnsupportedOperationException("${msg.name} does not support matching on field " + field.getName());
if(!field.arePrerequisitesOK(this))
return true;

OFOxm<?> oxm = this.oxmList.get(field);

Expand All @@ -77,6 +82,8 @@ public boolean isFullyWildcarded(MatchField<?> field) {
public boolean isPartiallyMasked(MatchField<?> field) {
if (!supports(field))
throw new UnsupportedOperationException("${msg.name} does not support matching on field " + field.getName());
if(!field.arePrerequisitesOK(this))
return false;

OFOxm<?> oxm = this.oxmList.get(field);

Expand Down

0 comments on commit 3850039

Please sign in to comment.