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.

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

DHCP6: IPv6 Ready Logo test fixes #397

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jvfranklin
Copy link
Contributor

Implement USE_MULTICAST
Handle server-directed SOL_MAX_RT and INF_MAX_RT values Perform per RFC 8415 for NotOnLink and NoBinding error conditions Don't attempt to renew expired leases
Continue sending Info-Request messages until response is received
MAX_RC values aligned with RFC definitions

Implement USE_MULTICAST
Handle server-directed SOL_MAX_RT and INF_MAX_RT values
Perform per RFC 8415 for NotOnLink and NoBinding error conditions
Don't attempt to renew expired leases
Continue sending Info-Request messages until response is received
MAX_RC values aligned with RFC definitions
*/
if ((state->state == DH6S_RENEW) &&
(ap->prefix_vltime <= state->renew))
continue;
Copy link
Member

Choose a reason for hiding this comment

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

The comment does not match the code. vltime and renew values don't actually change - when we add an address we adjust the vltime at that point based on the acquisition time vs now time.
This change makes the assumption that RENEW only happens when t1 expires, but that isn't always the case.
So the address could still be valid and therefore should be renewed.

When an address does expire it is removed from the array, unless it's been requested via config.

So what is this code really trying to achieve?

eloop_timeout_add_sec(ifp->ctx->eloop,
INF_MAX_RD, dhcp6_failinform, ifp);
else
dhcp6_sendinform(ifp);
Copy link
Member

Choose a reason for hiding this comment

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

OK, I see what you're trying to fix here. We want to drop through to failinform so we can use the lastlease code, but only on the first failure.

So I think what we need to do is after we have logged the message we want to guard the state timing changes like so

if (state->state == DH6S_INFORM) {
      dhcp6_sendinform(ifp);
      return;
}

Then we change failinform to do dhcp6_fail(ifp, false) - ie we don't force a drop.
Adjust the code there to handle lastlease for INFORM and we should be good yes?

state->lerror == D6_STATUS_NOTONLINK) {
eloop_timeout_delete(ifp->ctx->eloop, NULL, ifp);
dhcp6_startdiscover(ifp);
}
Copy link
Member

Choose a reason for hiding this comment

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

We are no longer handling the state where we got a reply without an error but it didn't contain any addresses with error codes either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants