Patch request - ignore advertise messages with none of requested data and missed status codes about dhcp6c HOT 11 CLOSED

Wow, incredible response time! Thank you.

I will check out the commits but given my C coding days are more than 25 years ago, I likely will have little to add ;) (also the reason why I didn't create a PR myself). Happy to test though.

Again, thanks for being so responsive.

from dhcp6c.

I omitted the non-functional changes. I presume the person doing the patch neither cared about -Werror or 80 character line limits. 😉 The tabs were also off in some functional lines so the end result looks a bit different.

Also just for readability it would be better to do non-functional updates in separate commits.

from dhcp6c.

Hi @Greelan,

Thanks for the detailed context. Is 437b308 + cff2641 what you are looking for? :)

If you need a test package I can provide one tomorrow.

Cheers,
Franco

from dhcp6c.

The commits look consistent with the Debian patch contents (with the d_printf function adjustment to match the OPNsense context). Here's hoping it works!

from dhcp6c.

Actually, I do note that these changes in the Debian patch aren't in your commit (they relate to lines 1266 and 1269 of OPNsense's dhcp6c):

@@ -1484,10 +1484,10 @@ client6_recv()
 
 	switch(dh6->dh6_msgtype) {
 	case DH6_ADVERTISE:
-		(void)client6_recvadvert(ifp, dh6, len, &optinfo);
+		client6_recvadvert(ifp, dh6, len, &optinfo);
 		break;
 	case DH6_REPLY:
-		(void)client6_recvreply(ifp, dh6, len, &optinfo);
+		client6_recvreply(ifp, dh6, len, &optinfo);

Presume there is a reason not to add these?

from dhcp6c.

To test:

# pkg add -f https://pkg.opnsense.org/FreeBSD:12:amd64/20.7/misc/dhcp6c-20200512_1.txz

To revert:

# opnsense-revert dhcp6c

from dhcp6c.

Thank you. Initial testing looks good. I see in the logs dhcp6c receiving an UnspecFail status code, and as a consequence reporting advertise contains no address/prefix. So it resends the solicit. Sometimes the process is repeated, ie it receives another UnspecFail status code. Eventually it receives a correct advertise and then successfully requests the address and prefix.

I will continue to monitor, but it is looking positive. :)

from dhcp6c.

Heads up that I am still seeing some odd behaviour with dhcp6c.

The patch does address the issue of dhcp6c wrongly responding to an UnspecFail as if it was an address/prefix advertise. It sends another solicit instead, until an address/prefix is actually received.

However, I have noticed on at least one occasion dhcp6c sending another release immediately after receiving an address/prefix. It then sends a request, but that request is ineffective because of the release. The end result is that no address/prefix is received despite repeated requests being made.

The odd behaviour is the sending of the release - it is not clear why that happens at that point in the process.

I will post some logs when I get a chance later, to show the sequence. I will also do some more testing to see whether this is consistent behaviour or happens only intermittently.

from dhcp6c.

Attached are the promised logs: dhcplogs.txt

This shows a reload event on the WAN (Interfaces->Overview->WAN->Reload), from when dhcp6c is SIGHUPed. A release is sent, a reply to the release is received, and a solicit is then sent. The first response is an UnspecFail. So dhcp6c sends another solicit. This time the response is to advertise a GUA for the WAN and a prefix.

At this point it would be expected that dhcp6c would request the address and prefix. But first another release is sent, and replied to, before the request is made. Presumably because of the release, the request is not responded to, so it is repeated.

The logs then show another reload event. On this occasion, the sequence follows as expected - release, response, solicit, advertise, request, response. There is no UnspecFail on this occasion because the dhcpv6 server and proxy don't have a lease assigned to the client.

In the first reload event, there is a release both after the GUA/prefix is received, and also immediately after the UnspecFail is received (so: UnspecFail response, release (no response), solicit, response, release, response, request...).

I don't profess to know a lot about dhcp6c, but I wonder whether the two releases are related? Eg, is the second based off a timer after the first? Should the first be happening at all?

While not scientific, I did do some more testing to see if I could replicate the behaviour, by hitting reload on the WAN again. I couldn't replicate the behaviour. On the first occasion, the UnspecFail response was followed by a release being sent, but then the solicit, response and request sequence happened as expected. On the second occasion, the sequence was: UnspecFail, solicit, UnspecFail, solicit, response, request - which I would have thought is the expected behaviour with the patch.

My initial testing straight after installing the patch also did not exhibit the behaviour I just experienced today. So it is a bit of a mystery.

Sorry I can't shed more light.

from dhcp6c.

BTW, an aside on the logs. I had to run OPNsense's logs through tail -r to get them into reverse order, which I find more natural to read. Not sure if I am missing something, but is there no way of having the GUI display OPNsense's logs in reverse order (oldest to newest)? I thought I saw on some screenshots online of an earlier version a checkbox for exactly that, but it seems to have disappeared.

from dhcp6c.

@Greelan use clog -f for tail -f or clog for cat behaviour. I haven't looked closer yet. We may be looking at an implementation limitation in dhcp6c as the HUP was added by us so the only way to reload prior was stopping and starting so the same lease will very likely not be acquired anyway.

from dhcp6c.