[CONTROLLER-336] SimpleForwardingImpl hits NPE when .frm member is null and new host is discovered Created: 17/Apr/14  Updated: 05/May/15  Resolved: 05/May/15

Status: Resolved
Project: controller
Component/s: adsal
Affects Version/s: Helium
Fix Version/s: None

Type: Bug
Reporter: Flavio Fernandes Assignee: Flavio Fernandes
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified
Environment:

Operating System: All
Platform: All
URL: https://gist.github.com/anonymous/eba571a29abeaad5b914


Attachments: Text File file_773.txt    
External issue ID: 773
Priority: Low

 Description   

<flaviof> hi folks. I'm trying latest integration build, and hit an exception: https://gist.github.com/eba571a29abeaad5b914
<flaviof> looking at the trace: 2014-04-17 18:29:50.840 EDT [Thread-22] ERROR o.o.c.h.internal.HostTracker - Exception on new host notification
<flaviof> java.lang.NullPointerException: null
<flaviof> at org.opendaylight.controller.samples.simpleforwarding.internal.SimpleForwardingImpl.installPerHostRules(SimpleForwardingImpl.java:653) ~[na:na]
<flaviof> at org.opendaylight.controller.samples.simpleforwarding.internal.SimpleForwardingImpl.notifyHTClient(SimpleForwardingImpl.java:840) ~[na:na]
<flaviof> at org.opendaylight.controller.hosttracker.internal.HostTracker.notifyHostLearnedOrRemoved(HostTracker.java:568) [bundlefile:na]
<flaviof> at org.opendaylight.controller.hosttracker.internal.HostTracker.access$600(HostTracker.java:103) [bundlefile:na]
<flaviof> at org.opendaylight.controller.hosttracker.internal.HostTracker$NotifyHostThread.run(HostTracker.java:536) [bundlefile:na]
<flaviof> is it reasonable to assume that the 'frm' attribute in the SimpleForwardingImpl object can be null?
<flaviof> Madhu: this looks like code you wrote back in Mar/13... any ideas?
<aboch> flaviof: since the dependency on frm is not strict (you can see it in simple forwarding Activator), yes, there can be a time window when SF is brought up until frm comes up, during which frm reference is null
<flaviof> interesting. I see: https://www.dropbox.com/s/288q5amswsjhgm8/Screenshot%202014-04-17%2018.51.41.png
<flaviof> thanks Alessandro
<aboch> I am not sure though why SF dependencies on frm and ht were not made strict
<aboch> I think they should be
<aboch> np
<flaviof> is this something worth pursuing at all?
<flaviof> I'm not familiar on the things are worth documenting/fixing. specially on the simpleFwd code
<aboch> SF is in samples, sample apps. It installs (*, /32) entries for reaching the newly discovered hosts.
<aboch> It allows a mininet pingall to work out of the box
<flaviof> gotcha.
<aboch> feel free to propose a patch
<flaviof> all right!
<flaviof> Seems simple enough. I will give it a try.
<flaviof> aboch: should start by opening a bug in bugzilla and assign it to me?
<aboch> yeah
<flaviof> aboch: will do
<aboch> and thanks
<flaviof> aboch: np!



 Comments   
Comment by Flavio Fernandes [ 17/Apr/14 ]

Attachment file_773.txt has been added with description: https://gist.github.com/anonymous/eba571a29abeaad5b914

Comment by Flavio Fernandes [ 18/Apr/14 ]

This is actually rather simple to reproduce.
Using virtualization, distrib.

1) start mininet as follows:

sudo mn --topo single,3 --mac --switch ovsk --controller remote,ip=172.16.150.1

2) from mininet prompt, start ping from h1 to h2:

mininet> h1 ping h2

3) After h1 ping h2 command started, launch ODL on remote host:

[dhcp-89-147:opendaylight]$ pwd
/Users/ffernand/ODL/projects/integration.git/distributions/virtualization/target/distributions-virtualization-0.1.2-SNAPSHOT-osgipackage/opendaylight
[dhcp-89-147:opendaylight]$ ./run.sh -virt ovsdb

Comment by Flavio Fernandes [ 18/Apr/14 ]

I tried the suggested change from Alessandro (below). It did not help
with this issue. So, I will go back to my original plan of not letting the
.rulesDB member be out of sync with .frm

==

<flaviof> aboch: Is the ruesDB tightly coupled with the .frm?
<flaviof> s/ruesDB/rulesDB
<flaviof> in other words, should we avoid adding/removing entries to rulesDB if the frm == null?
<aboch> I think the purpose of that map is to store the flows succesfully installed

  • Madhu is now known as Madhu_offline
    <aboch> I think it is fair to make simple forw strictly dependent on frm. If you can consistently reporduce the issue, have you tried whether
    <aboch> c.add(createContainerServiceDependency(containerName).setService(
    <aboch> IForwardingRulesManager.class).setCallbacks(
    <aboch> "setForwardingRulesManager", "unsetForwardingRulesManager")
    <aboch> - .setRequired(false));
    <aboch> + .setRequired(true));
    <aboch> fixes the issue /
    <flaviof> oh... that is good
    <flaviof> I will try reproducing it
    <flaviof> i thin the way I did it was to have ping going in mini net before calling 'run.sh'
    <flaviof> I'll try your proposed change once I can consistently reproduce it.
Comment by Flavio Fernandes [ 18/Apr/14 ]

(In reply to ffernandes from comment #2)
> I tried the suggested change from Alessandro (below). It did not help
> with this issue. So, I will go back to my original plan of not letting the
> .rulesDB member be out of sync with .frm
>
> ==
>
> <flaviof> aboch: Is the ruesDB tightly coupled with the .frm?
> <flaviof> s/ruesDB/rulesDB
> <flaviof> in other words, should we avoid adding/removing entries to
> rulesDB if the frm == null?
> <aboch> I think the purpose of that map is to store the flows succesfully
> installed
> * Madhu is now known as Madhu_offline
> <aboch> I think it is fair to make simple forw strictly dependent on frm.
> If you can consistently reporduce the issue, have you tried whether
> <aboch>
> c.add(createContainerServiceDependency(containerName).setService(
> <aboch> IForwardingRulesManager.class).setCallbacks(
> <aboch> "setForwardingRulesManager",
> "unsetForwardingRulesManager")
> <aboch> - .setRequired(false));
> <aboch> + .setRequired(true));
> <aboch> fixes the issue /
> <flaviof> oh... that is good
> <flaviof> I will try reproducing it
> <flaviof> i thin the way I did it was to have ping going in mini net before
> calling 'run.sh'
> <flaviof> I'll try your proposed change once I can consistently reproduce
> it.

Oops... my bad. Alessandro's proposed fix DID address the problem. I made a mistake and incorrectly concluded it did not.

Comment by Carol Sanders [ 05/May/15 ]

This bug is part of the project to Move all ADSAL associated component bugs to ADSAL.

Generated at Wed Feb 07 19:52:46 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.