ONOS-6516 - Fix for NullPointer Exceptions found during OFDPA testing
Change-Id: I2fa20194ab6e47334c4a882ddf929597259946f8
diff --git a/core/common/src/main/java/org/onosproject/codec/impl/NextObjectiveCodec.java b/core/common/src/main/java/org/onosproject/codec/impl/NextObjectiveCodec.java
index b221b3c..3cb5178 100644
--- a/core/common/src/main/java/org/onosproject/codec/impl/NextObjectiveCodec.java
+++ b/core/common/src/main/java/org/onosproject/codec/impl/NextObjectiveCodec.java
@@ -139,7 +139,7 @@
break;
default:
throw new IllegalArgumentException("The requested type " + typeStr +
- " is not defined for FilteringObjective.");
+ " is not defined for NextObjective.");
}
// decode treatments
@@ -172,7 +172,7 @@
break;
default:
throw new IllegalArgumentException("The requested operation " + opStr +
- " is not defined for FilteringObjective.");
+ " is not defined for NextObjective.");
}
return nextObjective;
diff --git a/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java b/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
index 5eaaebb..3e242d1 100644
--- a/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
+++ b/core/net/src/main/java/org/onosproject/net/flowobjective/impl/FlowObjectiveManager.java
@@ -291,7 +291,7 @@
}
}
if (queued) {
- log.debug("Queued forwarding objective {} for nextId {} meant for device {}",
+ log.info("Queued forwarding objective {} for nextId {} meant for device {}",
fwd.id(), fwd.nextId(), deviceId);
}
return queued;
@@ -319,7 +319,7 @@
}
}
if (queued) {
- log.debug("Queued next objective {} with operation {} meant for device {}",
+ log.info("Queued next objective {} with operation {} meant for device {}",
next.id(), next.op(), deviceId);
}
return queued;
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2Pipeline.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2Pipeline.java
index 5aef043..a76aa46 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2Pipeline.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2Pipeline.java
@@ -223,7 +223,7 @@
// automatically denied. The DENY filter is used to deny packets
// that are otherwise permitted by the PERMIT filter.
// Use ACL table flow rules here for DENY filtering objectives
- log.debug("filter objective other than PERMIT currently not supported");
+ log.warn("filter objective other than PERMIT currently not supported");
fail(filteringObjective, ObjectiveError.UNSUPPORTED);
}
}
@@ -348,20 +348,20 @@
boolean install, ApplicationId applicationId) {
// This driver only processes filtering criteria defined with switch
// ports as the key
- PortCriterion portCriterion;
+ PortCriterion portCriterion = null;
EthCriterion ethCriterion = null;
VlanIdCriterion vidCriterion = null;
if (!filt.key().equals(Criteria.dummy()) &&
filt.key().type() == Criterion.Type.IN_PORT) {
portCriterion = (PortCriterion) filt.key();
- } else {
- log.warn("No key defined in filtering objective from app: {}. Not"
- + "processing filtering objective", applicationId);
- fail(filt, ObjectiveError.BADPARAMS);
- return;
}
- log.debug("Received filtering objective for dev/port: {}/{}", deviceId,
- portCriterion.port());
+ if (portCriterion == null) {
+ log.debug("No IN_PORT defined in filtering objective from app: {}",
+ applicationId);
+ } else {
+ log.debug("Received filtering objective for dev/port: {}/{}", deviceId,
+ portCriterion.port());
+ }
// convert filtering conditions for switch-intfs into flowrules
FlowRuleOperations.Builder ops = FlowRuleOperations.builder();
for (Criterion criterion : filt.conditions()) {
@@ -415,7 +415,7 @@
if (vidCriterion == null) {
// NOTE: it is possible that a filtering objective only has ethCriterion
- log.debug("filtering objective missing VLAN, cannot program VLAN Table");
+ log.info("filtering objective missing VLAN, cannot program VLAN Table");
} else {
/*
* NOTE: Separate vlan filtering rules and assignment rules
@@ -543,14 +543,20 @@
// ofdpa cannot match on ALL portnumber, so we need to use separate
// rules for each port.
List<PortNumber> portnums = new ArrayList<>();
- if (portCriterion != null && PortNumber.ALL.equals(portCriterion.port())) {
- for (Port port : deviceService.getPorts(deviceId)) {
- if (port.number().toLong() > 0 && port.number().toLong() < OFPP_MAX) {
- portnums.add(port.number());
+ if (portCriterion != null) {
+ if (PortNumber.ALL.equals(portCriterion.port())) {
+ for (Port port : deviceService.getPorts(deviceId)) {
+ if (port.number().toLong() > 0 && port.number().toLong() < OFPP_MAX) {
+ portnums.add(port.number());
+ }
}
+ } else {
+ portnums.add(portCriterion.port());
}
} else {
- portnums.add(portCriterion.port());
+ log.warn("Filtering Objective missing Port Criterion . " +
+ "VLAN Table cannot be programmed for {}",
+ deviceId);
}
for (PortNumber pnum : portnums) {
@@ -611,100 +617,199 @@
}
//handling untagged packets via assigned VLAN
- if (vidCriterion.vlanId() == VlanId.NONE) {
+ if (vidCriterion != null && vidCriterion.vlanId() == VlanId.NONE) {
vidCriterion = (VlanIdCriterion) Criteria.matchVlanId(assignedVlan);
}
+ List<FlowRule> rules = new ArrayList<>();
+ OfdpaMatchVlanVid ofdpaMatchVlanVid = null;
+ if (vidCriterion != null && requireVlanExtensions()) {
+ ofdpaMatchVlanVid = new OfdpaMatchVlanVid(vidCriterion.vlanId());
+ }
// ofdpa cannot match on ALL portnumber, so we need to use separate
// rules for each port.
List<PortNumber> portnums = new ArrayList<>();
- if (portCriterion != null && PortNumber.ALL.equals(portCriterion.port())) {
- for (Port port : deviceService.getPorts(deviceId)) {
- if (port.number().toLong() > 0 && port.number().toLong() < OFPP_MAX) {
- portnums.add(port.number());
+ if (portCriterion != null) {
+ if (PortNumber.ALL.equals(portCriterion.port())) {
+ for (Port port : deviceService.getPorts(deviceId)) {
+ if (port.number().toLong() > 0 && port.number().toLong() < OFPP_MAX) {
+ portnums.add(port.number());
+ }
}
+ } else {
+ portnums.add(portCriterion.port());
+ }
+ for (PortNumber pnum : portnums) {
+ rules.add(buildTmacRuleForIpv4(ethCriterion,
+ vidCriterion,
+ ofdpaMatchVlanVid,
+ applicationId,
+ pnum));
+ rules.add(buildTmacRuleForMpls(ethCriterion,
+ vidCriterion,
+ ofdpaMatchVlanVid,
+ applicationId,
+ pnum));
+ rules.add(buildTmacRuleForIpv6(ethCriterion,
+ vidCriterion,
+ ofdpaMatchVlanVid,
+ applicationId,
+ pnum));
}
} else {
- portnums.add(portCriterion.port());
- }
-
- List<FlowRule> rules = new ArrayList<>();
- for (PortNumber pnum : portnums) {
- OfdpaMatchVlanVid ofdpaMatchVlanVid = new OfdpaMatchVlanVid(vidCriterion.vlanId());
- // for unicast IP packets
- TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
- TrafficTreatment.Builder treatment = DefaultTrafficTreatment.builder();
- if (matchInPortTmacTable()) {
- selector.matchInPort(pnum);
- }
- if (requireVlanExtensions()) {
- selector.extension(ofdpaMatchVlanVid, deviceId);
- } else {
- selector.matchVlanId(vidCriterion.vlanId());
- }
- selector.matchEthType(Ethernet.TYPE_IPV4);
- selector.matchEthDst(ethCriterion.mac());
- treatment.transition(UNICAST_ROUTING_TABLE);
- FlowRule rule = DefaultFlowRule.builder()
- .forDevice(deviceId)
- .withSelector(selector.build())
- .withTreatment(treatment.build())
- .withPriority(DEFAULT_PRIORITY)
- .fromApp(applicationId)
- .makePermanent()
- .forTable(TMAC_TABLE).build();
- rules.add(rule);
- //for MPLS packets
- selector = DefaultTrafficSelector.builder();
- treatment = DefaultTrafficTreatment.builder();
- if (matchInPortTmacTable()) {
- selector.matchInPort(pnum);
- }
- if (requireVlanExtensions()) {
- selector.extension(ofdpaMatchVlanVid, deviceId);
- } else {
- selector.matchVlanId(vidCriterion.vlanId());
- }
- selector.matchEthType(Ethernet.MPLS_UNICAST);
- selector.matchEthDst(ethCriterion.mac());
- treatment.transition(MPLS_TABLE_0);
- rule = DefaultFlowRule.builder()
- .forDevice(deviceId)
- .withSelector(selector.build())
- .withTreatment(treatment.build())
- .withPriority(DEFAULT_PRIORITY)
- .fromApp(applicationId)
- .makePermanent()
- .forTable(TMAC_TABLE).build();
- rules.add(rule);
- /*
- * TMAC rules for IPv6 packets
- */
- selector = DefaultTrafficSelector.builder();
- treatment = DefaultTrafficTreatment.builder();
- if (matchInPortTmacTable()) {
- selector.matchInPort(pnum);
- }
- if (requireVlanExtensions()) {
- selector.extension(ofdpaMatchVlanVid, deviceId);
- } else {
- selector.matchVlanId(vidCriterion.vlanId());
- }
- selector.matchEthType(Ethernet.TYPE_IPV6);
- selector.matchEthDst(ethCriterion.mac());
- treatment.transition(UNICAST_ROUTING_TABLE);
- rule = DefaultFlowRule.builder()
- .forDevice(deviceId)
- .withSelector(selector.build())
- .withTreatment(treatment.build())
- .withPriority(DEFAULT_PRIORITY)
- .fromApp(applicationId)
- .makePermanent()
- .forTable(TMAC_TABLE).build();
- rules.add(rule);
+ rules.add(buildTmacRuleForIpv4(ethCriterion,
+ vidCriterion,
+ ofdpaMatchVlanVid,
+ applicationId,
+ null));
+ rules.add(buildTmacRuleForMpls(ethCriterion,
+ vidCriterion,
+ ofdpaMatchVlanVid,
+ applicationId,
+ null));
+ rules.add(buildTmacRuleForIpv6(ethCriterion,
+ vidCriterion,
+ ofdpaMatchVlanVid,
+ applicationId,
+ null));
}
return rules;
}
+ /**
+ * Builds TMAC rules for IPv4 packets.
+ *
+ * @param ethCriterion
+ * @param vidCriterion
+ * @param ofdpaMatchVlanVid
+ * @param applicationId
+ * @param pnum
+ * @return TMAC rule for IPV4 packets
+ */
+ private FlowRule buildTmacRuleForIpv4(EthCriterion ethCriterion,
+ VlanIdCriterion vidCriterion,
+ OfdpaMatchVlanVid ofdpaMatchVlanVid,
+ ApplicationId applicationId,
+ PortNumber pnum) {
+ TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
+ TrafficTreatment.Builder treatment = DefaultTrafficTreatment.builder();
+ if (pnum != null) {
+ if (matchInPortTmacTable()) {
+ selector.matchInPort(pnum);
+ } else {
+ log.info("Pipeline does not support IN_PORT matching in TMAC table, " +
+ "ignoring the IN_PORT criteria");
+ }
+ }
+ if (vidCriterion != null) {
+ if (requireVlanExtensions()) {
+ selector.extension(ofdpaMatchVlanVid, deviceId);
+ } else {
+ selector.matchVlanId(vidCriterion.vlanId());
+ }
+ }
+ selector.matchEthType(Ethernet.TYPE_IPV4);
+ selector.matchEthDst(ethCriterion.mac());
+ treatment.transition(UNICAST_ROUTING_TABLE);
+ return DefaultFlowRule.builder()
+ .forDevice(deviceId)
+ .withSelector(selector.build())
+ .withTreatment(treatment.build())
+ .withPriority(DEFAULT_PRIORITY)
+ .fromApp(applicationId)
+ .makePermanent()
+ .forTable(TMAC_TABLE).build();
+ }
+
+ /**
+ * Builds TMAC rule for MPLS packets.
+ *
+ * @param ethCriterion
+ * @param vidCriterion
+ * @param ofdpaMatchVlanVid
+ * @param applicationId
+ * @param pnum
+ * @return TMAC rule for MPLS packets
+ */
+ private FlowRule buildTmacRuleForMpls(EthCriterion ethCriterion,
+ VlanIdCriterion vidCriterion,
+ OfdpaMatchVlanVid ofdpaMatchVlanVid,
+ ApplicationId applicationId,
+ PortNumber pnum) {
+ TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
+ TrafficTreatment.Builder treatment = DefaultTrafficTreatment.builder();
+ if (pnum != null) {
+ if (matchInPortTmacTable()) {
+ selector.matchInPort(pnum);
+ } else {
+ log.info("Pipeline does not support IN_PORT matching in TMAC table, " +
+ "ignoring the IN_PORT criteria");
+ }
+ }
+ if (vidCriterion != null) {
+ if (requireVlanExtensions()) {
+ selector.extension(ofdpaMatchVlanVid, deviceId);
+ } else {
+ selector.matchVlanId(vidCriterion.vlanId());
+ }
+ }
+ selector.matchEthType(Ethernet.MPLS_UNICAST);
+ selector.matchEthDst(ethCriterion.mac());
+ treatment.transition(MPLS_TABLE_0);
+ return DefaultFlowRule.builder()
+ .forDevice(deviceId)
+ .withSelector(selector.build())
+ .withTreatment(treatment.build())
+ .withPriority(DEFAULT_PRIORITY)
+ .fromApp(applicationId)
+ .makePermanent()
+ .forTable(TMAC_TABLE).build();
+ }
+
+ /**
+ * Builds TMAC rules for IPv6 packets.
+ *
+ * @param ethCriterion
+ * @param vidCriterion
+ * @param ofdpaMatchVlanVid
+ * @param applicationId
+ * @param pnum
+ * @return TMAC rule for IPV6 packets
+ */
+ private FlowRule buildTmacRuleForIpv6(EthCriterion ethCriterion,
+ VlanIdCriterion vidCriterion,
+ OfdpaMatchVlanVid ofdpaMatchVlanVid,
+ ApplicationId applicationId,
+ PortNumber pnum) {
+ TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
+ TrafficTreatment.Builder treatment = DefaultTrafficTreatment.builder();
+ if (pnum != null) {
+ if (matchInPortTmacTable()) {
+ selector.matchInPort(pnum);
+ } else {
+ log.info("Pipeline does not support IN_PORT matching in TMAC table, " +
+ "ignoring the IN_PORT criteria");
+ }
+ }
+ if (vidCriterion != null) {
+ if (requireVlanExtensions()) {
+ selector.extension(ofdpaMatchVlanVid, deviceId);
+ } else {
+ selector.matchVlanId(vidCriterion.vlanId());
+ }
+ }
+ selector.matchEthType(Ethernet.TYPE_IPV6);
+ selector.matchEthDst(ethCriterion.mac());
+ treatment.transition(UNICAST_ROUTING_TABLE);
+ return DefaultFlowRule.builder()
+ .forDevice(deviceId)
+ .withSelector(selector.build())
+ .withTreatment(treatment.build())
+ .withPriority(DEFAULT_PRIORITY)
+ .fromApp(applicationId)
+ .makePermanent()
+ .forTable(TMAC_TABLE).build();
+ }
+
protected List<FlowRule> processEthDstOnlyFilter(EthCriterion ethCriterion,
ApplicationId applicationId) {
ImmutableList.Builder<FlowRule> builder = ImmutableList.builder();
@@ -833,7 +938,7 @@
for (Instruction ins : fwd.treatment().allInstructions()) {
if (ins instanceof OutputInstruction) {
OutputInstruction o = (OutputInstruction) ins;
- if (o.port() == PortNumber.CONTROLLER) {
+ if (o != null && PortNumber.CONTROLLER.equals(o.port())) {
ttBuilder.add(o);
} else {
log.warn("Only allowed treatments in versatile forwarding "
@@ -850,6 +955,10 @@
if (fwd.nextId() != null) {
// overide case
NextGroup next = getGroupForNextObjective(fwd.nextId());
+ if (next == null) {
+ fail(fwd, ObjectiveError.BADPARAMS);
+ return Collections.emptySet();
+ }
List<Deque<GroupKey>> gkeys = appKryo.deserialize(next.data());
// we only need the top level group's key to point the flow to it
Group group = groupService.getGroup(deviceId, gkeys.get(0).peekFirst());
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3Pipeline.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3Pipeline.java
index 4546b31..edf8c38 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3Pipeline.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3Pipeline.java
@@ -155,7 +155,7 @@
}
// 0x0000XXXX is UNI interface.
if (portCriterion.port().toLong() > MPLS_UNI_PORT_MAX) {
- log.error("Filering Objective invalid logical port {}",
+ log.error("Filtering Objective invalid logical port {}",
portCriterion.port().toLong());
fail(filteringObjective, ObjectiveError.BADPARAMS);
return;