PDINFO needs to be populated differently to prevent false detections of address changes

[rssor]

Because dhcp6c splits up all IAs into separate renewals, any given renewal will contain prefix info for only one IA. For a client with a delegated prefix and a stateful address, one renewal contains the prefix info and the other will not, tripping the new address logic every time. PDINFO seems guaranteed to be different, as the likely cases from renewal to renewal are:

• NA -> PD: PDINFO will go from empty to non-empty
• PD -> PD: PDINFO probably corresponds to a separate delegated prefix with its own IA_PD, so not equal
• PD -> NA: PDINFO will go from non-empty to empty

Rather than sourcing the prefix information from the response, it's probably necessarily to make client6_script take a dhcp6_if * parameter so it can enumerate all IAs and populate all prefix information for the interface. It looks like update_ia will have already made any necessary changes to the information stored within the interface.

The code here: https://github.com/opnsense/dhcp6c/blob/555bdcdeff3e0f450d50a713e46aa39394544fa5/dhcp6c_script.c#L425

Should probably be adapted to enumerate ove rthe PDs inside this: https://github.com/opnsense/dhcp6c/blob/555bdcdeff3e0f450d50a713e46aa39394544fa5/config.h#L91

I think this is main trigger for what I ran into here, but happens with ANY prefix delegations set up in combination with stateful addresses as near as I can tell: https://forum.opnsense.org/index.php?topic=34241.0

This approach would have the added benefit of fixing the issue with not only a single IA_PD as is possible in the UI, but also my slightly more exotic setup that needs several IA_PDs set up via config file overrides.

[fichtner]

It’s no wonder nobody has tried to implement this in decades. ;) I think a core ticket is more appropriate. Moving it there…

[fichtner]

Perhaps this helps https://github.com/opnsense/core/commit/03519568aa7ed00743ec74eb6914cfe831a388f6

To install:

# opnsense-patch 03519568aa7

Cheers, Franco

[rssor]

Made it through the first renewal with that patch and dpinger didn't get restarted so it looks like that does it for now.

Thanks!

[fichtner]

@rssor happy to hear. We might have to do something about dpinger instances spawning too rapidly. It seems like a luxury issue with the ISP handing out an excessive number of PDs but it's good to progress here.

FWIW, I added a small follow up as well in e454ed03bf0 to simplify the resulting code.

Cheers, Franco

[rssor]

The PDs were simply an accelerator, it would happen even in totally standard configs with 1 stateful address and 1 delegated prefix. The other factor is that my ISP's equipment hands out short lease times which is probably why it had gone undetected until now. In a standard configuration there's just a slight chance of an extra dpinger spawning every lease renewal (prior to your patch).

Should I revert the first patch before applying that one?

[fichtner]

No, the new patch is on top of the other one. :)

Fair point, but we need to be realistic about probable case problems. We could pad all the service handling in the code down to the operating system (the rc system and service utility have the same issue in FreeBSD, but mostly nobody notices because it's not being automated as much). It's a lot of work given that FreeBSD itself and lots of third party software "does not care" so I'd say it's an uphill battle in any case. I'd rather like to work on improving IPv6 in general.

To some degree I think it's the dpinger PID file handling glitching out here but again I have no interest in fixing third party code for things that can mostly be avoided.

Cheers, Franco

[rssor]

Totally understandable.

The only remaining case where I see extra dpingers spawn is I think totally unrelated to this issue, more info back in the other thread, it looks like the ipv6 related ones have been stamped out. Or at least, by updating the renew handling to ignore PDINFO the underlying root cause can't be triggered by the dhcp6c script.

My assumption is that since wannewipv6 was being started with FORCE=force for all renews, it was attempting to restart dpinger (as the source address may have changed), but with anywhere from 2 (normal config supported by the GUI) to 6 (my weirder setup) renew responses coming in simultaneously, my guess is that some number of those attempted to kill the old dpinger and then replace it simultaneously. Possibly the same with updaterrd.sh, which also crept up over time.

So I had a chance to get up to 5 new dpingers every 30 minutes, but more normal setups probably only have a chance to spawn one duplicate every 3.5+ days.

That at least lines up with some of the log messages about failing to kill non-existing pids during renews, I didn't dig very far into core stuff at all once I realized there where multiples of that script running simultaneously.

[fichtner]

Shipped in 23.1.9 -- initial feedback looking good 😎