miniupnpd: restore nftables mappings on reload

[Clackz]

The nftables variant installs its forwarding/NAT rules into the upnp_forward, upnp_prerouting and upnp_postrouting chains. Those chains are declared inside the main inet fw4 table by the usr/share/nftables.d/ snippets this package ships, so every firewall start/reload rebuilds them from the templates and leaves them present but empty. The daemon keeps running and has no way of noticing that the rules it installed are gone, so all existing port mappings silently stop working until a client happens to re-request them or the daemon is restarted.

Nothing else recovers from this either:

• /etc/hotplug.d/iface/20-firewall issues fw4 -q reload on ifup/ifupdate, while /etc/hotplug.d/iface/50-miniupnpd exits early both for any action other than ifup and when the external device name is unchanged.
• the daemon itself never re-installs rules it already reported as active.

The iptables variant is not affected, because its rules live in a MINIUPNPD chain of its own which firewall3 does not flush — that is why files/firewall3.include only has to re-add the jump rules and never touches the daemon. For nftables the equivalent hook was added by f1c69d0 ("miniupnpd: rework firewall4 integration") and removed again by 4c934ae / #21931 ("miniupnpd: remove uci-defaults and fw4-include files for nftables variant"), which left the nftables variant with no way to recover at all. Note that files/miniupnpd.defaults.nftables is currently gone from the tree while files/miniupnpd.defaults.iptables is still there and still installed by Package/miniupnpd-iptables/install.

Re-introduce the hook for the nftables variant. The daemon is restarted rather than left alone because a restart is what makes it re-read its lease file.

Why the restart is detached and delayed

The removal in #21931 was justified with "produces a firewall error on service miniupnpd restart". That is this: fw4 executes script includes while it still holds /var/run/fw4.lock (/sbin/fw4 runs ACTION=includes inside { flock -x 1000; ... } 1000>$LOCK), and start_service() contains

nft -s -t -n list chain inet fw4 upnp_forward >/dev/null 2>&1 || fw4 reload

so a synchronous restart can make the daemon block on the lock that fw4 still holds. Running the restart detached, one second later, avoids that. fw4 sources the include (. '$path') after closing fd 1000, so the child inherits no lock fd; it is only the daemon's own fw4 reload fallback that would take the lock.

Only IPv4 mappings are stored in the lease file, so PCP mappings and IPv6 pinholes cannot be restored by a restart. This is a pre-existing limitation and it is documented in the script.

fw4 include options

Only type and path are valid for a script include. family and reload are accepted by fw3 but marked UNSUPPORTED by fw4's parse_include(), which warns and ignores them, so the uci-defaults does not set them even though miniupnpd.defaults.iptables does.

Run tested

bcm53xx / ImmortalWrt 25.12.2, fw4 + miniupnpd-nftables built from this tree:

• before: a plain fw4 reload emptied both chains, left the daemon PID unchanged, and killed the five active mappings; a 120 s watch showed no recovery at all. Simulated ACTION=ifup / ACTION=ifupdate on the WAN interface left the mappings at zero as well.
• with this patch: one fw4 reload restarts the daemon once (confirmed in the daemon's own log: single shutting down / starting pair) and re-installs all five mappings within about two seconds, with no fw4 lock error.
• a real WAN reconnect (ifup wan, which triggers both an ifup and an ifupdate reload) restored all mappings the same way.
• the include is sourced, not executed, so the file does not need the executable bit and INSTALL_DATA is correct.

Relation to other open work

#28765 also touches this package but does not address the wiped-chains case: its 50-miniupnpd keeps the same early-exit logic. It does touch the Package/miniupnpd-nftables/install block, so one of the two will need a small rebase. Its uci-defaults is installed as 99-miniupnpd-upnpd-migration, which does not clash with the 99-miniupnpd used here. It also renames the upnp_lease_file UCI option to lease_file in a different section; the script here reads the current name first and falls back to /var/run/miniupnpd.leases, which stays the default value there.

Signed-off-by is in the commit.

[BKPepe]

Ping @stangri

[openwrt-ai]

The PR body states fw4 sources script includes (. '$path'), while line 18 here says it executes them. If it is sourced, every exit 0 in this file — this one and lines 26, 27, 28, 31, 34, 40 — terminates fw4 itself, so any script include registered after miniupnpd's is skipped. Should these be return 0?

---

Generated by Claude Code

[openwrt-ai]

nit: miniupnpd.init:156 only emits lease_file= when upnp_lease_file is set, so with the option unset the daemon keeps no lease file at all and this hardcoded fallback can only ever match a stale leftover — restarting on it kills the running daemon without restoring anything.

Suggested change

	leases="$(uci -q get upnpd.config.upnp_lease_file)"
	[ -n "$leases" ] || leases=/var/run/miniupnpd.leases
	[ -s "$leases" ] || exit 0
	leases="$(uci -q get upnpd.config.upnp_lease_file)"
	[ -n "$leases" ] && [ -s "$leases" ] || exit 0

---

Generated by Claude Code