-
Notifications
You must be signed in to change notification settings - Fork 2.1k
gnrc/rpl: switching between DODAGs #21759
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.
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
base: master
Are you sure you want to change the base?
Conversation
`gnrc_rpl_instance_remove` can't fail, so it was already always returning `true`.
/* Not used yet */ | ||
gnrc_rpl_dodag_t *which_dodag(gnrc_rpl_dodag_t *d1, gnrc_rpl_dodag_t *d2) | ||
int which_dodag(gnrc_rpl_dodag_t *d1, gnrc_rpl_dio_t *dio) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wanted to compare two gnrc_rpl_dodag_t
s, we'd first need to allocate the whole dodag structure based on the DIO, and even create a new RPL instance too because we only support one DODAG per instance.
I don't think this makes sense, given that in practice we only ever compare the info from DIO
s with the current DODAG.
if (GNRC_RPL_COUNTER_GREATER_THAN(dio->version_number, dodag->version)) { | ||
if (dodag->node_status == GNRC_RPL_ROOT_NODE) { | ||
dodag->version = GNRC_RPL_COUNTER_INCREMENT(dio->version_number); | ||
trickle_reset_timer(&dodag->trickle); | ||
} | ||
else { | ||
dodag->version = dio->version_number; | ||
gnrc_rpl_local_repair(dodag); | ||
} | ||
} | ||
else if (GNRC_RPL_COUNTER_GREATER_THAN(dodag->version, dio->version_number)) { | ||
trickle_reset_timer(&dodag->trickle); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, different DODAG versions should be treated as different DODAGs because they affect also our parent set etc. (e.g., per RFC all parents in the parent set must be the same DODAG version). So the old logic of just increasing our version and leaving the rest as it is wasn't really RFC conformant.
Is my understanding correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version number is incremented when something happens that requires that the DODAG be rebuilt. It's not a different DODAG. One should attach to the highest version number (with lollipop numbering, I think)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version number is incremented when something happens that requires that the DODAG be rebuilt. It's not a different DODAG. One should attach to the highest version number (with lollipop numbering, I think)
Yes, that was also my understanding.
But implementation wise, I was thinking that we could still treat a new DODAG version the same as we treat joining a completely different DODAG: clear the parent set, reset trickle, reset other parameters...
The old implementation effectively also just triggered a local repair (=> implemented in RIOT as poising of all ranks and cleanup of the current dodag).
But thinking about it again now, we probably don't want to reset all parameters, like dtsn or DAO sequence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note: I don't think that increasing a DODAG version as a root is currently even implemented in RIOT.
sys/include/net/gnrc/rpl.h
Outdated
#define GNRC_RPL_GROUNDED_SHIFT (7) | ||
#define GNRC_RPL_MOP_SHIFT (3) | ||
#define GNRC_RPL_OPT_TRANSIT_E_FLAG_SHIFT (7) | ||
#define GNRC_RPL_OPT_TRANSIT_E_FLAG (1 << GNRC_RPL_OPT_TRANSIT_E_FLAG_SHIFT) | ||
#define GNRC_RPL_SHIFTED_MOP_MASK (0x7) | ||
#define GNRC_RPL_PRF_MASK (0x7) | ||
#define GNRC_RPL_PREFIX_AUTO_ADDRESS_BIT (1 << 6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These defines are missing the documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to appropriate groups and documented with ff5ad54.
Edit: Hmm, I have to document every item separately... Ok, will fix that.
The DIO has a LifeTime, so only remove it when that expires. |
Contribution description
Extend RPL to allow nodes to switch between DODAGs that belong to the same RPL instance, based on the preference in the objective function.
This is needed, e.g., for joining a new DODAG version of the same DODAG, or to switch to another DODAG that belongs to the same RPL instance but has a different ID (and thus different root) and better properties.
Based on the RFC sections 6550#3.2, 6550#8.2 and 6552#4.2.
cc @benpicco
Testing procedure
Can be tested with a network of min. 4 nodes using the
gnrc_networking
example:N{1-3}
and connect them in a line.N1
root for DODAG ID2001:db8:1::1
=>
N2
andN3
will join the DODAG with rank 256 and 512 respectively.N4
also as root for the same RPL instance, but with with a different DODAG ID (e.g.,2001:db8:1::2
).N4
to other nodes.=>
N1
andN2
will stay in previous DODAG2001:db8:1::1
,N3
will switch to new DODAG2001:db8:1::2
because it will result in a better rank (256 instead of 512).Prior to this PR,
N{1-3}
would have just blindly rejected the DIOs fromN4
instead of comparing its parameters against their current DODAG.Open Question
Should we remove a public IP address from an interface when we leave the DODAG that we got the prefix from?
So far, we've keep the IP (also when leaving a DODAG because, e.g., the parent timed out). But if we then join another DODAG with different prefix, the default
CONFIG_GNRC_NETIF_IPV6_ADDRS_NUMOF = 2
doesn't allow us to add another address with the new prefix.Issues/PRs references
Part of #21574 -> needed for switching between floating and grounded DODAGs.