Skip to content

ddns-scripts: add getlease6, an ip_source helper for DHCPv6 leases - #30242

Open
LongQT-sea wants to merge 1 commit into
openwrt:masterfrom
LongQT-sea:ddns-scripts-dhcp6lease
Open

ddns-scripts: add getlease6, an ip_source helper for DHCPv6 leases#30242
LongQT-sea wants to merge 1 commit into
openwrt:masterfrom
LongQT-sea:ddns-scripts-dhcp6lease

Conversation

@LongQT-sea

@LongQT-sea LongQT-sea commented Aug 10, 2026

Copy link
Copy Markdown

📦 Package Details

Maintainer: ddns-scripts declares no PKG_MAINTAINER; the most recent
Makefile activity is from @systemcrash and @feckert.

Description:

New subpackage ddns-scripts-getlease6, installed as /usr/sbin/getlease6.
It resolves a LAN host's current global IPv6 address from its DHCPv6 lease, so
an AAAA record can follow a host behind the router as the delegated prefix
rotates — today that needs a second updater running on every host that must be
reachable.

option ip_source 'script'
option ip_script '/usr/sbin/getlease6 myhost'
option use_ipv6  '1'
  • Selects by hostname or DUID; -a/--iaid narrows to one IAID where a host
    holds a lease per NIC (same DUID and hostname, several addresses).
  • Matches against the prefixes the interface actually holds, each with its own
    length, so /60 and /62 assignments work and a ULA is never published.
  • -w picks an upstream where the router holds several delegations; several
    matches are announced rather than chosen silently.
  • With no host, or a selector matching nothing, lists the leases odhcpd holds
    with hostname, DUID and IAID.
  • Exactly one address on stdout, every diagnostic on stderr.

Only additive to the shared Makefile, apart from the PKG_RELEASE bump.
The new dependencies are effectively free on a stock image: ucode and
ucode-mod-ubus are already pulled in by firewall4 and netifd, and
ucode-mod-log by luci-base.


🧪 Run Testing Details

  • OpenWrt Version: 25.12.5 (r33051-f5dae5ece4)
  • OpenWrt Target/Subtarget: ramips/mt7621
  • OpenWrt Device: Xiaomi Mi Router 3G (xiaomi,mi-router-3g)

Built with openwrt/sdk:ramips-mt7621-25.12.5 (make package/ddns-scripts/check V=s passes), installed with apk add --allow-untrusted, and exercised on the
device above: lookups by hostname, by DUID, and by IAID for a two-NIC host; the
lease listing; and through ddns-scripts' own ip_script path, confirming one
address on stdout and nothing on stderr.

Also verified with two upstreams delegating a prefix each (wan_6
2001:db8:1::/64, wanb_62001:db8:2::/64) on a LAN that additionally
carries a ULA: -w returns the address formed from the named upstream's
delegation, an unqualified lookup announces both matches and picks the same one
on every run, -w naming an interface with no delegation exits 1, and the ULA
is never printed.


✅ Formalities

  • I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

If your PR contains a patch:

Not applicable — this PR adds no patches.

🤖 Generated with Claude Code

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 new commit.

Nothing here blocks a merge — the packaging side checks out: PKG_RELEASE bumped for the added subpackage, the Package/…/install block uses tabs, the metadata block two spaces, ucode-mod-ubus and ucode-mod-log both exist as packages, #!/usr/bin/ucode matches how in-tree ucode executables are written, and the existing net/ddns-scripts/test-version.sh already matches ddns-scripts*, so the generic CI version probe stays covered for the new subpackage. The ubus field names used (ipv6-prefix-assignment.address/.mask, ipv6-prefix.address/.mask) match what netifd exports, and the address printed to stdout is one that iptoarr already parsed, so nothing unvalidated reaches ddns-scripts' eval.

The one real (if small) correctness item is the sel_label precedence; the rest are optional. I did not flag the missing odhcpd dependency: odhcpd and odhcpd-ipv6only are build variants with no shared PROVIDES, so DEPENDS:=+odhcpd would drag in the DHCPv4 variant and conflict with the stock image — documenting it in the description, as done here, looks like the right call.

No CI checks have reported on f34fd56 yet.


Generated by Claude Code

Comment thread net/ddns-scripts/files/getlease6.uc Outdated
Comment thread net/ddns-scripts/files/usr/sbin/getlease6.uc
Comment on lines +465 to +466
notify(`${sel_label} matches ${length(candidates)} global addresses ` +
`on '${iface}', using ${chosen} (${hint})`);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

notify() writes to syslog on every invocation, and ddns-scripts re-runs ip_script once per check interval — data=$(eval "$ip_script" ...) in dynamic_dns_functions.sh:893, default 10 minutes. A setup that is permanently ambiguous (host with a lease per NIC, or a LAN carrying two delegations) therefore logs this line every 10 minutes indefinitely while the address it prints never changes. The notify() at line 390 has the same shape but is bounded by host_warned. Would fail() (stderr only) be enough here, leaving syslog for the cases that actually stop the lookup?


Generated by Claude Code

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Right that it repeats, but fail() would silence it rather than move it: the script branch redirects stderr to ERRFILE and never reads it back, and it is deleted at dynamic_dns_functions.sh:1151, so in daemon mode syslog is the only place this is ever seen. The process also exits per invocation, so the host_warned guard can't carry across runs — suppressing the repeat would need state on disk.

Kept it as notify() on that basis: the line names the address it chose, so it stays actionable, and a permanently ambiguous setup is one -w or -a should be pinning down. Happy to drop it to stderr if you'd rather have the log quiet.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

A data point on this: with two upstreams each delegating a prefix and no -w, three back-to-back runs produced three daemon.warn lines — so the cadence you describe is real, and the setup is ordinary rather than exotic, since a dual-WAN box with the default config hits it.

I still lean toward keeping notify(): the line names -w as the fix and goes quiet as soon as it is passed, whereas choosing silently between two upstreams seems worse than a repeated log line. If you would rather it not repeat at all, the only way I can see is state under $ddns_rundir, which feels like more machinery than the warning is worth — happy to be told otherwise.

Comment thread net/ddns-scripts/Makefile Outdated
@LongQT-sea
LongQT-sea force-pushed the ddns-scripts-dhcp6lease branch from f34fd56 to a7cd2ea Compare August 11, 2026 06:21

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed the changes since f34fd56 (help text, IAID hex form, sel_label precedence).

Both items from the previous round are addressed: sel_label now follows the documented -d precedence, so a failed lookup no longer names a hostname that was never compared, and -a accepts a 0x-prefixed hex value. The new parsing holds up — hex() guards with isxdigit and then strtoll(v, &e, 16), which accepts the 0x/0X prefix, so hex("0x1f") is 31 rather than NaN, and the regexes match the whole string before conversion so int() cannot truncate at a stray character. Nothing here blocks a merge; the one inline note is about the help text, not the code.

On the repeated notify() for a permanently ambiguous setup: your reasoning stands — the script branch redirects stderr to ERRFILE and never reads it back, so fail() would drop the line entirely rather than relocate it, and a per-invocation process cannot carry a suppression flag. Leaving that as is.


Generated by Claude Code

Comment thread net/ddns-scripts/files/getlease6.uc Outdated
@feckert

feckert commented Aug 12, 2026

Copy link
Copy Markdown
Member

@LongQT-sea If I’ve understood you correctly, you’d like to enable the globally accessible downstream IPv6 addresses behind your router (LAN) to have a DNS entry registered with a DDNS provider.
I’d like to see the full UCI configuration for, say, 4 IPv6 addresses.

For this to work, we’d need to incorporate it into LuCI, and that’s where I see problems. Since the options for scripts aren’t available as separate UCI options? How should this be resolved?

@LongQT-sea

LongQT-sea commented Aug 12, 2026

Copy link
Copy Markdown
Author

@feckert Yes, that's it. The record points at one specific LAN host, so there is one config service section per host — the same way several IPv4 records already work. The sections differ only in the record they update and the argument to getlease6.

The first two are from a running setup (credentials and domain redacted); the last two are there to show the other selectors:

config service 'docker_host_ipv6'
    option service_name 'cloudflare.com-v4'
    option lookup_host  'q12-ddns.example.net'
    option domain       'q12-ddns@example.net'
    option username     'Bearer'
    option password     '<api-token>'
    option use_ipv6     '1'
    option interface    'wan_6'
    option ip_source    'script'
    option ip_script    '/usr/sbin/getlease6 docker-host'
    option cacert       '/etc/ssl/certs'
    option use_https    '1'
    option enabled      '1'

config service 'OpenWrt_500_ipv6'
    option service_name 'cloudflare.com-v4'
    option lookup_host  'q12-500.example.net'
    option domain       'q12-500@example.net'
    option username     'Bearer'
    option password     '<api-token>'
    option use_ipv6     '1'
    option interface    'wan_6'
    option ip_source    'script'
    option ip_script    '/usr/sbin/getlease6 openwrt-500'
    option cacert       '/etc/ssl/certs'
    option use_https    '1'
    option enabled      '1'

config service 'iMac_ipv6'
    # ... same as above, except:
    option lookup_host  'desktop.example.net'
    option domain       'desktop@example.net'
    option ip_script    '/usr/sbin/getlease6 -d 000100012e753273888888888788'

config service 'laptop_ipv6'
    # ... same as above, except:
    option lookup_host  'laptop.example.net'
    option domain       'laptop@example.net'
    option ip_script    '/usr/sbin/getlease6 -d 000100013202954d7c3b2d920fd1 -a 0'

iMac_ipv6 selects by DUID because that host sends no hostname over DHCPv6; laptop_ipv6 adds -a because that host has two NICs and therefore holds one lease per NIC. option interface 'wan_6' runs the updater when the delegation changes, which is when these addresses change.

LuCI already covers it — ip_source includes o.value("script", _("Script")) (https://github.com/openwrt/luci/blob/1c70b2b3d0185c394c3ac23de3086c8476e41b87/applications/luci-app-ddns/htdocs/luci-static/resources/view/ddns/overview.js#L811), ip_script (https://github.com/openwrt/luci/blob/1c70b2b3d0185c394c3ac23de3086c8476e41b87/applications/luci-app-ddns/htdocs/luci-static/resources/view/ddns/overview.js#L867-L871) is the "Script" field depending on it, and dynamic_dns_lucihelper.sh:83 takes the same route. Advanced → IP address source → Script:

image

What is missing is only a lease picker instead of a free-text path. Glad to do that as a follow-up, but as a new ip_source value it is mostly a ddns-scripts change — a branch in get_current_ip, UCI options for the selectors, validation — plus core referencing a binary from an optional subpackage, so I would keep it out of this PR. Do you want it, and should the source be called something like dhcpv6-lease?

@LongQT-sea
LongQT-sea force-pushed the ddns-scripts-dhcp6lease branch from a7cd2ea to f561f6f Compare August 12, 2026 07:56

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 new commit; no new issues found.

The only change since a7cd2ea is the two-line -h wording for -a, which is exactly the correction from the last round: it now says plainly that the 0x prefix has to be added when pasting a hex IAID out of LuCI, so the "copied 10, got decimal 10" trap is spelled out rather than implied. Commit message is unchanged and still matches the diff, and the three FormalityCheck runs on f561f6f are green.

The two open threads (notify() repeating in a permanently ambiguous setup, and storing the script as files/usr/sbin/getlease6) are unchanged and stay as they are — neither blocks a merge.


Generated by Claude Code

@feckert

feckert commented Aug 13, 2026

Copy link
Copy Markdown
Member

We could store the option required by a script as a UCI options, and a wrapper script would extract this from uci and run the script with the necessary arguments.
For this to work, we need to extend the JSON schema to include a field called for example backend (The name may also be different).

For script backends:

{
	"name": "dnspod.cn",
	"backend": "script"
	"ipv4": {
		"url": "update_dnspod_cn.sh"
	},
	"ipv6": {
		"url": "update_dnspod_cn.sh"
	}
}

For generic backends:

{
	"name": "afraid.org-basicauth",
	"backend": "generic"
	"ipv4": {
		"url": "https://[USERNAME]:[PASSWORD]@freedns.afraid.org/nic/update?hostname=[DOMAIN]&myip=[IP]"
	},
	"ipv6": {
		"url": "https://[USERNAME]:[PASSWORD]@freedns.afraid.org/nic/update?hostname=[DOMAIN]&myip=[IP]"
	}
}

Those using the generic DDNS implementation without their own script should use the value generic; the script backends should use the value script. We can then test this in various ways and include the option to add further UCI parameters for validation. We can then test this in various ways and include the option to add further UCI parameters for validation.

We’re already having problems at the moment with the generic stuff because we’re using some parameters incorrectly, as there isn’t a suitable field in the uci and so also in the LuCI. We could also fix this.
Just to give you something to think about -:)

ddns-scripts can only discover the router's own address, so an AAAA
record cannot follow a host behind it as the delegated prefix rotates.
Tracking one today means running a second updater on every host that
must be reachable. getlease6 resolves that host's current global address
from its DHCPv6 lease, installed as /usr/sbin/getlease6:

  option ip_source 'script'
  option ip_script '/usr/sbin/getlease6 myhost'
  option use_ipv6  '1'

ip_script runs through eval, so the shebang picks the interpreter. ucode
reads the ubus dumps as native values, where shell would need jsonfilter
and its own IPv6 expansion.

The host is selected by hostname or by DUID, and -a narrows to one IAID
where a host holds a lease per NIC. Addresses are matched against the
prefixes the interface actually holds, each with its own length, so a
/60 or /62 assignment works and a ULA is never published; -w picks an
upstream where the router holds several delegations. Several matches are
announced rather than chosen silently. With no host, or a selector that
matches nothing, the leases odhcpd holds are listed with their hostname,
DUID and IAID.

Exactly one address goes to stdout and every diagnostic to stderr, since
ddns-scripts captures stdout and validates only that the first word of
ip_script is an executable path. The file header documents the static
lease and firewall rule that make the host reachable once the record
exists.

Signed-off-by: Tieu Long <long025733@gmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LongQT-sea
LongQT-sea force-pushed the ddns-scripts-dhcp6lease branch from f561f6f to c8aa313 Compare August 13, 2026 09:07
@LongQT-sea

Copy link
Copy Markdown
Author

Thanks for the suggestion. Worth separating one thing first though: getlease6 sits on the detection side (ip_source / ip_script), not the update side, so no provider JSON describes it — get_current_ip never reads service_name, update_url or update_script, and the two halves meet only at send_update "$CURRENT_IP" (dynamic_dns_updater.sh:337). So backend and getlease6 don't overlap — there's no provider here to label.

On script arguments: ip_script already takes them, and it reads as intended rather than incidental. getlocalip_sample.sh:9 documents:

# option ip_script    '/usr/lib/ddns/getlocalip_sample.sh -6' !!! parameters ALLOWED

slaac_sample.sh:9 passes two:

# option ip_script      '/usr/lib/ddns/slaac_sample.sh br-lan AA:BB:CC:DD:EE:FF'

and dynamic_dns_updater.sh:249 splits them so the executable check lands on $1:

set -- $ip_script     #handling script with parameters, we need a trick

getlease6 follows what those two already do.

For the LuCI side I'd rather add a proper ip_source for this than wrap ip_script — a new value with the selectors as their own UCI options, so nothing has to be typed as a path. I could do that as a follow-up once this lands.

@openwrt-ai openwrt-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed 1 new commit; no new issues found.

The only change since f561f6f is the relocation of the script from files/getlease6.uc to files/usr/sbin/getlease6.uc plus the matching INSTALL_BIN path, which is the layout nit from the first round — the file now sits at the path it installs to, like every other file this package ships. The script content is byte-identical to the version reviewed at f561f6f (git diff reports the move as a pure rename), and the commit was rebased onto current master with its message unchanged, so it still matches the diff.

Nothing else in the Makefile picks the new location up by accident: the one wildcard install is ./files/usr/share/ddns/default/* at line 491, so files/usr/sbin/ stays exclusive to ddns-scripts-getlease6. PKG_RELEASE is 3 against 2 on the rebase parent, still correct for an added subpackage with PKG_VERSION untouched. The three FormalityCheck runs on c8aa313 are green.

The remaining open thread — notify() repeating every check interval in a permanently ambiguous setup — is unchanged and stays as discussed; it does not block a merge.


Generated by Claude Code

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.

3 participants