Skip to content

miniupnpd: use fw4 to indicate fw4 instead of nft#29964

Open
rohit-btuk wants to merge 1 commit into
openwrt:masterfrom
rohit-btuk:feature_miniupnpd_use_fw4_to_indicate_fw4
Open

miniupnpd: use fw4 to indicate fw4 instead of nft#29964
rohit-btuk wants to merge 1 commit into
openwrt:masterfrom
rohit-btuk:feature_miniupnpd_use_fw4_to_indicate_fw4

Conversation

@rohit-btuk

@rohit-btuk rohit-btuk commented Jul 10, 2026

Copy link
Copy Markdown

Change miniupnpd init script to use "fw4" to indicate use of fw4, instead of "nft".
This allows miniupnpd to work in a situation where the platform supports nftables, but not fw4.

📦 Package Details

Maintainer: ardeleanalex@gmail.com
(You can find this by checking the history of the package Makefile.)

Description:


🧪 Run Testing Details

  • OpenWrt Version:
  • OpenWrt Target/Subtarget:
  • OpenWrt Device:

✅ Formalities

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

If your PR contains a patch:

  • It can be applied using git am
  • It has been refreshed to avoid offsets, fuzzes, etc., using
    make package/<your-package>/refresh V=s
  • It is structured in a way that it is potentially upstreamable
    (e.g., subject line, commit description, etc.)
    We must try to upstream patches to reduce maintenance burden.

@rohit-btuk rohit-btuk closed this Jul 10, 2026
@rohit-btuk rohit-btuk reopened this Jul 10, 2026
@rohit-btuk
rohit-btuk force-pushed the feature_miniupnpd_use_fw4_to_indicate_fw4 branch from c3c4c5e to 1594e4b Compare July 10, 2026 12:44
@rohit-btuk
rohit-btuk marked this pull request as ready for review July 10, 2026 12:49

@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. The command -v nftcommand -v fw4 change is correct: $FW is later invoked as an executable ($FW -q zone, fw4 reload), so gating on the actual fw4 binary rather than nft is the right condition. One inline comment on the missing PKG_RELEASE bump. Not a backport; commit message matches the change.


Generated by Claude Code

USE_PROCD=1
PROG=/usr/sbin/miniupnpd
[ -x "$(command -v nft)" ] && FW="fw4" || FW="fw3"
[ -x "$(command -v fw4)" ] && FW="fw4" || FW="fw3"

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.

This is a functional change to how the firewall backend is selected (it alters runtime behavior on nftables-without-fw4 systems), not a cosmetic edit, so it should bump PKG_RELEASE so the updated init script is actually packaged and delivered to users. PKG_RELEASE is currently 3 and unchanged: net/miniupnpd/Makefile#L12. Please bump it to 4. (The openwrt bot's release audit flagged the same missing bump.)


Generated by Claude Code

@rohit-btuk rohit-btuk Jul 10, 2026

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.

@openwrt-ai made the change.

Change miniupnpd init script to use "fw4" to indicate use of fw4,
instead of "nft".
This allows miniupnpd to work in a situation where the platform
supports nftables, but not fw4.

Signed-off-by: Rohit Pandey <rohit.pandey@bt.com>
@rohit-btuk
rohit-btuk force-pushed the feature_miniupnpd_use_fw4_to_indicate_fw4 branch from 1594e4b to 381a7c5 Compare July 10, 2026 16:09

@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 updated commit; no new issues found. The previously flagged missing PKG_RELEASE bump has been addressed (34), so the updated init script will be repackaged. The only substantive change since the last review is that release bump; the command -v nftcommand -v fw4 init change was already confirmed correct.


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.

2 participants