[OPNFLWPLUG-490] [Lithium redesign] flow not sent to switch if priority is not explicitly set. Created: 05/Jun/15  Updated: 27/Sep/21  Resolved: 11/Jun/15

Status: Resolved
Project: OpenFlowPlugin
Component/s: General
Affects Version/s: None
Fix Version/s: None

Type: Bug
Reporter: Jamo Luhrsen Assignee: Unassigned
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Operating System: All
Platform: All

External issue ID: 3603

 Description   

If a priority is not explicitly set, the default (0x8000 == 32768) should
be used and given in the flow_mod to the switch. This used to be the case,
but now we are rejecting and showing an NPE ERROR:

2015-06-05 15:44:46,121 | ERROR | lt-dispatcher-21 | DataTreeChangeListenerActor | 179 - org.opendaylight.controller.sal-distributed-datastore - 1.2.0.SNAPSHOT | Error notifying listener org.opendaylight.controller.md.sal.binding.impl.BindingDOMDataTreeChangeListenerAdapter@2b426549
java.lang.NullPointerException: flow priority must not be null
at com.google.common.base.Preconditions.checkNotNull(Preconditions.java:226)[61:com.google.guava:18.0.0]
at org.opendaylight.openflowplugin.impl.registry.flow.FlowRegistryKeyFactory$FlowRegistryKeyDto.<init>(FlowRegistryKeyFactory.java:41)
at org.opendaylight.openflowplugin.impl.registry.flow.FlowRegistryKeyFactory.create(FlowRegistryKeyFactory.java:29)
at org.opendaylight.openflowplugin.impl.services.SalFlowServiceImpl.addFlow(SalFlowServiceImpl.java:68)
at org.opendaylight.yangtools.yang.binding.util.RpcMethodInvokerWithInput.invokeOn(RpcMethodInvokerWithInput.java:30)[69:org.opendaylight.yangtools.yang-binding:0.7.0.SNAPSHOT]
at org.opendaylight.yangtools.yang.binding.util.AbstractMappedRpcInvoker.invokeRpc(AbstractMappedRpcInvoker.java:52)[69:org.opendaylight.yangtools.yang-binding:0.7.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.binding.impl.BindingDOMRpcImplementationAdapter.invoke(BindingDOMRpcImplementationAdapter.java:85)[157:org.opendaylight.controller.sal-binding-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.binding.impl.BindingDOMRpcImplementationAdapter.invokeRpc(BindingDOMRpcImplementationAdapter.java:72)[157:org.opendaylight.controller.sal-binding-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.dom.broker.impl.RoutedDOMRpcRoutingTableEntry.invokeRpc(RoutedDOMRpcRoutingTableEntry.java:57)[154:org.opendaylight.controller.sal-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.dom.broker.impl.DOMRpcRoutingTable.invokeRpc(DOMRpcRoutingTable.java:186)[154:org.opendaylight.controller.sal-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.dom.broker.impl.DOMRpcRouter.invokeRpc(DOMRpcRouter.java:124)[154:org.opendaylight.controller.sal-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.binding.impl.RpcServiceAdapter.invoke0(RpcServiceAdapter.java:64)[157:org.opendaylight.controller.sal-binding-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.binding.impl.RpcServiceAdapter.access$000(RpcServiceAdapter.java:42)[157:org.opendaylight.controller.sal-binding-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.binding.impl.RpcServiceAdapter$RpcInvocationStrategy.invoke(RpcServiceAdapter.java:156)[157:org.opendaylight.controller.sal-binding-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.binding.impl.RpcServiceAdapter.invoke(RpcServiceAdapter.java:95)[157:org.opendaylight.controller.sal-binding-broker-impl:1.2.0.SNAPSHOT]
at com.sun.proxy.$Proxy82.addFlow(Unknown Source)[184:org.opendaylight.openflowplugin.model.flow-service:0.1.0.SNAPSHOT]
at org.opendaylight.openflowplugin.applications.frm.impl.FlowForwarder.add(FlowForwarder.java:149)[188:org.opendaylight.openflowplugin.applications.forwardingrules-manager:0.1.0.SNAPSHOT]
at org.opendaylight.openflowplugin.applications.frm.impl.FlowForwarder.add(FlowForwarder.java:47)[188:org.opendaylight.openflowplugin.applications.forwardingrules-manager:0.1.0.SNAPSHOT]
at org.opendaylight.openflowplugin.applications.frm.impl.AbstractListeningCommiter.onDataTreeChanged(AbstractListeningCommiter.java:59)[188:org.opendaylight.openflowplugin.applications.forwardingrules-manager:0.1.0.SNAPSHOT]
at org.opendaylight.controller.md.sal.binding.impl.BindingDOMDataTreeChangeListenerAdapter.onDataTreeChanged(BindingDOMDataTreeChangeListenerAdapter.java:41)[157:org.opendaylight.controller.sal-binding-broker-impl:1.2.0.SNAPSHOT]
at org.opendaylight.controller.cluster.datastore.DataTreeChangeListenerActor.dataChanged(DataTreeChangeListenerActor.java:53)[179:org.opendaylight.controller.sal-distributed-datastore:1.2.0.SNAPSHOT]
at org.opendaylight.controller.cluster.datastore.DataTreeChangeListenerActor.handleReceive(DataTreeChangeListenerActor.java:37)[179:org.opendaylight.controller.sal-distributed-datastore:1.2.0.SNAPSHOT]
at org.opendaylight.controller.cluster.common.actor.AbstractUntypedActor.onReceive(AbstractUntypedActor.java:34)[171:org.opendaylight.controller.sal-clustering-commons:1.2.0.SNAPSHOT]
at akka.actor.UntypedActor$$anonfun$receive$1.applyOrElse(UntypedActor.scala:167)[164:com.typesafe.akka.actor:2.3.10]
at akka.actor.Actor$class.aroundReceive(Actor.scala:467)[164:com.typesafe.akka.actor:2.3.10]
at akka.actor.UntypedActor.aroundReceive(UntypedActor.scala:97)[164:com.typesafe.akka.actor:2.3.10]
at akka.actor.ActorCell.receiveMessage(ActorCell.scala:516)[164:com.typesafe.akka.actor:2.3.10]
at akka.actor.ActorCell.invoke(ActorCell.scala:487)[164:com.typesafe.akka.actor:2.3.10]
at akka.dispatch.Mailbox.processMailbox(Mailbox.scala:254)[164:com.typesafe.akka.actor:2.3.10]
at akka.dispatch.Mailbox.run(Mailbox.scala:221)[164:com.typesafe.akka.actor:2.3.10]
at akka.dispatch.Mailbox.exec(Mailbox.scala:231)[164:com.typesafe.akka.actor:2.3.10]
at scala.concurrent.forkjoin.ForkJoinTask.doExec(ForkJoinTask.java:260)[161:org.scala-lang.scala-library:2.10.4.v20140209-180020-VFINAL-b66a39653b]
at scala.concurrent.forkjoin.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1339)[161:org.scala-lang.scala-library:2.10.4.v20140209-180020-VFINAL-b66a39653b]
at scala.concurrent.forkjoin.ForkJoinPool.runWorker(ForkJoinPool.java:1979)[161:org.scala-lang.scala-library:2.10.4.v20140209-180020-VFINAL-b66a39653b]
at scala.concurrent.forkjoin.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:107)[161:org.scala-lang.scala-library:2.10.4.v20140209-180020-VFINAL-b66a39653b]

example flow xml:
<flow xmlns="urn:opendaylight:flow:inventory">
<instructions><instruction>
<order>0</order>
<apply-actions>
<action>
<order>0</order>
<output-action>
<output-node-connector>FLOOD</output-node-connector>
</output-action>
</action>
</apply-actions>
</instruction>
</instructions>
<match>
<ethernet-match>
<ethernet-type>
<type>0x800</type>
</ethernet-type>
</ethernet-match>
<ipv4-source>11.3.0.0/16</ipv4-source>
<ipv4-destination>99.0.0.0/8</ipv4-destination>
</match>
<table_id>0</table_id>
<id>1</id>
</flow>

 Comments   
Comment by Michal Rehak [ 08/Jun/15 ]

Hi,
priority is mandatory according openflow spec. Unfortunately yang models describing flow have priority leaf as optional (default). In my opinion ofp should not be responsible for dealing with incomplete inputs. Priority could be covered by adding mandatory attribute to corresponding leaf in model or setting default value in model (I vote for mandatory attribute).

But models can be touched in Be. Current state is that ofp throws an exception at the point where priority is used. Does this break any tests or applications? Isn't it rational to fix rather those? Because this will be the path ofp will walk in Be?

Just asking. If there are strong arguments on doing all validations and default value related stuff in ofp than I am all ear.

Comment by Jozef Gloncak [ 08/Jun/15 ]

If the solution with default priority is used then there is patch
https://git.opendaylight.org/gerrit/22087.

Comment by Jamo Luhrsen [ 08/Jun/15 ]

I raised this bug because this is something was working one way (using 0x8000) as default priority, then changed so that it wasn't using and throwing the NPE.

This is for the Li-redesign plugin.

Currently, the He-plugin is taking no priority and using default 0x8000

We should keep the same external behavior for these things if possible.

From a previous email exchange, I got the idea that using a default
priority was an explicit thing we wanted to handle:
https://lists.opendaylight.org/pipermail/openflowplugin-dev/2015-May/003182.html

Let me know the decision, because I'd like to get a test in CI specific
to this bug since its something that has changed on us recently and we
should know when it's changing on us in the future.

I do agree that we should be more restrictive to what we accept as a
flow and even reject these things at the REST layer if they are wrong.
Obviously, this is a Be enhancement.

Comment by Jozef Gloncak [ 09/Jun/15 ]

Returning bug back to default assignee

Comment by Michal Rehak [ 10/Jun/15 ]

Ok, let's keep this behavior for Li and leave mandatorization for Be.

Comment by Jamo Luhrsen [ 10/Jun/15 ]

(In reply to michal rehak from comment #5)
> Ok, let's keep this behavior for Li and leave mandatorization for Be.

Michal,

please confirm that we are going to have the he-codebase doing this one way (allowing no priority) and the li-codebase (the redesign) doing it the other way (not allowing priority).

Thanks,
JamO

Comment by Michal Rehak [ 11/Jun/15 ]

Hi Jamo,
currently Li and He codebases behave the same way regarding missing flow priority. Both allow it and replace empty value with 0x8000.

Any changes to this behavior will be (eventually) done in Be.

Generated at Wed Feb 07 20:32:37 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.