Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rpl: remove rpl_get_my_dodag() dependency in rpl_delete_all_parents() #2633

Merged
merged 1 commit into from
Apr 28, 2015
Merged

rpl: remove rpl_get_my_dodag() dependency in rpl_delete_all_parents() #2633

merged 1 commit into from
Apr 28, 2015

Conversation

cgundogan
Copy link
Member

This PR removes the call to rpl_get_my_dodag() in rpl_delete_all_parents().
Furthermore, there are minor changes to the logic whether to delete a parent or not.

@cgundogan cgundogan added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking labels Mar 17, 2015
@BytesGalore BytesGalore mentioned this pull request Mar 17, 2015
Closed
21 tasks
@OlegHahm
Copy link
Member

Rational?

@cgundogan
Copy link
Member Author

In the current implementation rpl_delete_all_parents() deletes all parents from the default dodag. It is imperative to remove the dependencies to the default dodag gradually, to make it possible to handle multiple dodags.

BytesGalore
BytesGalore reviewed Mar 31, 2015
View reviewed changes
sys/net/routing/rpl/rpl_dodag.c
@@ -207,13 +207,9 @@ rpl_parent_t *rpl_find_parent(ipv6_addr_t *address)

void rpl_delete_parent(rpl_parent_t *parent)
{
rpl_dodag_t *my_dodag = rpl_get_my_dodag();

if ((my_dodag != NULL) && rpl_equal_id(&my_dodag->my_preferred_parent->addr,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cgundogan I assume that rpl_equal_id(...) is obsolete/wrong in the RPL implementation since we now provide multiple DODAGs?

@OlegHahm OlegHahm force-pushed the master branch 2 times, most recently from 9f184dd to 45554bf Compare March 31, 2015 13:01
@OlegHahm OlegHahm assigned BytesGalore and unassigned OlegHahm Apr 9, 2015
@cgundogan cgundogan force-pushed the rpl_remove_rpl_get_my_dodag_from_parent_delete branch from 33a77e6 to aa09faa Compare April 13, 2015 16:35
@cgundogan
Copy link
Member Author

rebased to current master

@BytesGalore
Copy link
Member

ACK from my side, do we need a Double-ACK here?

@OlegHahm
Copy link
Member

Did you test it?

@BytesGalore
Copy link
Member

@OlegHahm only a function test for rpl_delete_all_parents() which works as intended, but now I see that leaving a DODAG makes trouble.
I investigate it currently.

BytesGalore
BytesGalore reviewed Apr 14, 2015
View reviewed changes
sys/net/routing/rpl/rpl_dodag.c
for (int i = 0; i < RPL_MAX_PARENTS; i++) {
memset(&parents[i], 0, sizeof(parents[i]));
if (dodag->instance->id == parents[i].dodag->instance->id &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OlegHahm @cgundogan found it,
@cgundogan could you add a check if the specific parent has a DODAG set, for instance:

if (parents[i].dodag && dodag->instance->id == parents[i].dodag->instance->id &&

Now iterating over the parents without a set DODAG will crash on access ->instance...

@BytesGalore
Copy link
Member

I filled up the parent array manually with parent structs containing a valid DODAG. Sorry for that, the test was frivolous.
Next time I will pay more attention.

@cgundogan cgundogan force-pushed the rpl_remove_rpl_get_my_dodag_from_parent_delete branch from aa09faa to 19c2ab8 Compare April 14, 2015 08:09
@cgundogan
Copy link
Member Author

addressed @BytesGalore's comment and included a check if a parent's Dodag is set

@BytesGalore
Copy link
Member

nice thx, 👍 if travis gets happy I will revive my ACK

@BytesGalore
Copy link
Member

kicked travis

@OlegHahm
Copy link
Member

kicked travis

and again

@BytesGalore
Copy link
Member

Nice, my ACK is back

@BytesGalore
Copy link
Member

huh? forgot to merge -> GO

BytesGalore pushed a commit that referenced this pull request Apr 28, 2015
Merge pull request #2633 from cgundogan/rpl_remove_rpl_get_my_dodag_f…
2ffe2e6 
…rom_parent_delete

rpl: remove rpl_get_my_dodag() dependency in rpl_delete_all_parents()
@BytesGalore BytesGalore merged commit 2ffe2e6 into RIOT-OS:master Apr 28, 2015
@cgundogan cgundogan deleted the rpl_remove_rpl_get_my_dodag_from_parent_delete branch July 31, 2015 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@BytesGalore BytesGalore

Labels
Area: network Area: Networking Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cgundogan @OlegHahm @BytesGalore