DHCP6: lastlease behavior after Confim non-response by jvfranklin · Pull Request #387 · NetworkConfiguration/dhcpcd

jvfranklin · 2024-10-17T15:26:03Z

If lastlease is enabled, and dhcpcd is unable to confirm its prior lease, after timeout, don't continue to solicit. Confine lastlease behavior to only the CONFIRM state.

This cleans up some minor issues with the initial commit for DHCPv6 lastlease.

rsmarples · 2024-10-18T08:04:05Z

src/dhcp6.c

-	} else if (state->recv && ifp->options->options & DHCPCD_LASTLEASE) {
+	} else if (state->state == DH6S_CONFIRM && ifp->options->options & DHCPCD_LASTLEASE) {
 		dhcp6_bind(ifp, NULL, NULL);
+		return;


Why return?

We couldn't find any DHCPv6 server, so we do use the last lease, but we should retry and DISCOVER yes?

Soliciting for a lease while using the current one could lead to problems if the DHCPv6 server has ping checking enabled, in the case where you come back up on the same network. We've seen it fail the ping check (ie, gets a response) and issue a new IP address when that happens, thinking there's an address conflict. I think making this behavior optional with the lastlease option allows everybody to choose which way they want to go, since it is kind of a couple of competing requirements we're trying to untangle here. What do you think?

Does it matter it gets a new IP address? I mean, we have just failed to CONFIRM it?
This behavior is long standing in the DHCPv4 side of things and am just trying to get the same behavior both sides.

For the product I work on, it would for sure. We've had customer complaints due to address changes like this in the past. It wouldn't fly for us at all.

Well, DHCP address can and do change. Shouldn't count on them being static. Even if you get a new address, the old one will still persist here.

Anyway, how would you feel instead with moving to the REBIND state immediately then?

That sounds like a very good idea. Let me try it out against the Ready Logo tests to make sure it doesn't cause any unexpected behaviors, though I don't really expect it to, and I'll update this PR.

Is this the right way to do it?
dhcp6_fail()
} else if (state->state == DH6S_CONFIRM && ifp->options->options & DHCPCD_LASTLEASE) {
dhcp6_bind(ifp, NULL, NULL);
state->state = DH6S_REBIND;
dhcp6_startrebind(ifp);
return;
}

Looks ok here.

rsmarples · 2024-10-18T08:07:30Z

src/dhcp6.c

 		dhcp6_addrequestedaddrs(ifp);
 		eloop_timeout_delete(ifp->ctx->eloop, NULL, ifp);
-	} else if (state->recv && ifp->options->options & DHCPCD_LASTLEASE) {
+	} else if (state->state == DH6S_CONFIRM && ifp->options->options & DHCPCD_LASTLEASE) {


Can we also allow the state DH6S_REBIND please?
IF you ask for a Prefix Delegation you REBIND, otherwise CONFIRM and ideally we want the behavior in both states.
And we should still check state->recv to ensure we ARE using the last lease in this state.

Sure, I'm not seeing any issues with those changes in my own testing.

I still feel we should check if we have recv still. Why did you remove it?

I've got it in my local branch, just haven't pushed it yet. It'll be correct in my next push along with the rebind change.

Testing this some more, I'm seeing situations where state->recv is null when it's in the CONFIRM state. It looks like in dhcp6_bind, it gets set to null after the Solicit/Adv/Request/Reply transaction. So if the link bounces, and dhcpcd sends a Confirm the check fails. I see the same behavior if I restart dhcpcd while it has a valid lease. Is that expected?

OK, leave it then and I'll have a look at it later as it won't work with Prefix Delegation as that will be in the REBIND state but with CONFIRM timings - it's silly, but that's the RFC.

Thanks! I've updated the commit now.

If lastlease is enabled, and dhcpcd is unable to confirm its prior lease, after timeout, bind the lease and move to the REBIND state. Confine lastlease behavior to the CONFIRM and REBIND states.

rsmarples reviewed Oct 18, 2024

View reviewed changes

jvfranklin force-pushed the lastlease3 branch from e7370f8 to a28b875 Compare October 23, 2024 15:43

DHCP6: lastlease behavior after Confirm non-response

eee603b

If lastlease is enabled, and dhcpcd is unable to confirm its prior lease, after timeout, bind the lease and move to the REBIND state. Confine lastlease behavior to the CONFIRM and REBIND states.

jvfranklin force-pushed the lastlease3 branch from a28b875 to eee603b Compare October 25, 2024 14:14

rsmarples merged commit 1b573da into NetworkConfiguration:master Oct 29, 2024

Conversation

jvfranklin commented Oct 17, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants