Skip to content

Services: ISC DHCPv6 - show "tracking" interfaces when enabled and offer an explicit disable#8576

Merged
AdSchellevis merged 3 commits intomasterfrom
FR8528_dhcpv6_track
Apr 24, 2025
Merged

Services: ISC DHCPv6 - show "tracking" interfaces when enabled and offer an explicit disable#8576
AdSchellevis merged 3 commits intomasterfrom
FR8528_dhcpv6_track

Conversation

@AdSchellevis
Copy link
Copy Markdown
Member

@AdSchellevis AdSchellevis commented Apr 23, 2025

Needed in case someone would like to use dnsmasq or kea instead.

To avoid large changes, we opt for a minimal set here. In services_dhcpv6.php, we add a separate form and handler in case tracking (without dhcpd6track6allowoverride) is set, which either flushes the unused isc-dhcpv6 server configuration when enabled (default) or writes a small section only including ['enabled' => -1]. For visibility, we show the calculated range as would be set by dhcpd_dhcp6_configure() when tracking is used.

The backend code then double checks the services which er explicitly disabled (-1) and skip processing for these (not enabled).

In order to make people aware of the fact that an isc-dhcpv6 server could be running, make sure the menu system also reflects reality.

Since router advertisements are stored within the same container and will need a toggle as well, keep the value of ramode so we have a way to intervene in a similar way as for dhcpv6.
One small side affect of this commit is that it will show "Services: Router Advertisements" for the tracking interface, which we need to implement later.

One of the building blocks for: #8528

@AdSchellevis AdSchellevis added the cleanup Low impact changes label Apr 23, 2025
@AdSchellevis AdSchellevis requested a review from fichtner April 23, 2025 14:18
@AdSchellevis AdSchellevis self-assigned this Apr 23, 2025
…er an explicit disable option for the service in question so someone could use dnsmasq or kea instead.

To avoid large changes, we opt for a  minimal set here.
In services_dhcpv6.php, we add a separate form and handler in case tracking (without dhcpd6track6allowoverride) is set, which either flushes the unused isc-dhcpv6 server configuration when enabled (default) or writes a small section only including ['enabled' => -1].
For visibility, we show the calculated range as would be set by dhcpd_dhcp6_configure() when tracking is used.

The backend code then double checks the services which er explicitly disabled (-1) and skip processing for these (not enabled).

In order to make people aware of the fact that an isc-dhcpv6 server could be running, make sure the menu system also reflects reality.

Since router advertisements are stored within the same container and will need a toggle as well, keep the value of ramode so we have a way to intervene in a similar way as for dhcpv6.
One small side affect of this commit is that it will show "Services: Router Advertisements" for the tracking interface, which we need to implement later.

One of the building blocks for: #8528
!empty($config['interfaces'][$_GET['if']]['track6-interface']))) {
$if = $_GET['if'];
} else {
/* if no interface is provided this invoke is invalid */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

wouldn't remove this at first glance, the page shows garbage when invoked this way?

**/
if ($if === null) {
/* if no interface is provided this invoke is invalid */
header(url_safe('Location: /index.php'));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ah ok you moved this here

Comment thread src/www/services_dhcpv6.php Outdated
$this_server['ramode'] = $dhcpdv6cfg[$if]['ramode'];
}
if (empty($_POST['enable'])) {
$this_server['enable'] = '-1';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the downside of this is when the interface is switched to static the value will be '' or '0' again and it re-enabled the auto-mode which potentially leads to more code and all the 'enable' use flags need to be checked/adjusted too empty() doesn't work anymore. Maybe you have done it, i just want to mention it here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't think the interfaces page mangles with dhcpv6 section, the only reference I could find was this one :

core/src/www/interfaces.php

Lines 624 to 626 in 39d5ff1

if (isset($config['dhcpdv6']) && isset($config['dhcpdv6'][$if]['enable']) && !preg_match('/^staticv6/', $pconfig['type6']) && !isset($pconfig['dhcpd6track6allowoverride'])) {
$input_errors[] = gettext("The DHCPv6 Server is active on this interface and it can be used only with a static IPv6 configuration. Please disable the DHCPv6 Server service on this interface first, then change the interface configuration.");
}

which is merely a validation (and might need a change, but haven't looked at that yet)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah stuff like that, just to keep in mind

Co-authored-by: Franco Fichtner <franco@opnsense.org>
…led an offer an explicit disable option for the service in question so someone could use dnsmasq instead.

More or less the same construction as added for dhcpv6, using the ramode field to switch between types (disabled or assisted).

While here, also bugfix fieldname in services_dhcpv6.php

also for #8528
Copy link
Copy Markdown
Member

@fichtner fichtner left a comment

Choose a reason for hiding this comment

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

ok, let's just see how this behaves on master

@AdSchellevis AdSchellevis merged commit 727967e into master Apr 24, 2025
@AdSchellevis AdSchellevis deleted the FR8528_dhcpv6_track branch April 24, 2025 15:22
fichtner added a commit that referenced this pull request Jun 10, 2025
…fer an explicit disable (#8576)

* Services: ISC DHCPv6 - show "tracking" interfaces when enabled an offer an explicit disable option for the service in question so someone could use dnsmasq or kea instead.

To avoid large changes, we opt for a  minimal set here.
In services_dhcpv6.php, we add a separate form and handler in case tracking (without dhcpd6track6allowoverride) is set, which either flushes the unused isc-dhcpv6 server configuration when enabled (default) or writes a small section only including ['enabled' => -1].
For visibility, we show the calculated range as would be set by dhcpd_dhcp6_configure() when tracking is used.

The backend code then double checks the services which er explicitly disabled (-1) and skip processing for these (not enabled).

In order to make people aware of the fact that an isc-dhcpv6 server could be running, make sure the menu system also reflects reality.

Since router advertisements are stored within the same container and will need a toggle as well, keep the value of ramode so we have a way to intervene in a similar way as for dhcpv6.
One small side affect of this commit is that it will show "Services: Router Advertisements" for the tracking interface, which we need to implement later.

One of the building blocks for: #8528

* Update src/www/services_dhcpv6.php

Co-authored-by: Franco Fichtner <franco@opnsense.org>

* Services: Router Advertisements: show "tracking" interfaces when enabled an offer an explicit disable option for the service in question so someone could use dnsmasq instead.

More or less the same construction as added for dhcpv6, using the ramode field to switch between types (disabled or assisted).

While here, also bugfix fieldname in services_dhcpv6.php

also for #8528

---------

Co-authored-by: Franco Fichtner <franco@opnsense.org>
(cherry picked from commit 727967e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cleanup Low impact changes

Development

Successfully merging this pull request may close these issues.

2 participants