net/upnp: Complete service improvements 2/2 by Self-Hosting-Group · Pull Request #5256 · opnsense/plugins

Self-Hosting-Group · 2026-02-25T09:12:00Z

More specific allow third-party mapping UI option
Remove unnecessary binat anchor registration
Update missed changelog

Follow-up to #5126

fichtner · 2026-02-25T09:35:13Z

            }

-            if (!empty($upnp_config['allow_third_party_mapping'])) {
+            if (($upnp_config['allow_third_party_mapping'] ?? '') == '1' || ($upnp_config['allow_third_party_mapping'] ?? '') == 'upnp-igd') {


feels like this overcomplicates things and imposes "upnp-igd" on the meaning when in reality we're flipping pcp with this and the secure mode simply is handled as a separate toggle?

It may look overcomplicated to you, but that's only because the daemon initially had the meaningless option name secure_mode for UPnP IGD, PCP support was added later. secure_mode only applies to UPnP IGD, whereas pcp_allow_thirdparty (reversed) only applies to PCP. The same option is also available in OpenWrt, and I cannot imagine why anyone would make two options out of it.

Fair, still don't like the handling of this with four times ?? '' and two times checking the same flag. something that an in_array() can do much more concisely.

still pending

- More specific allow third-party mapping UI option - Remove unnecessary `binat` anchor registration - Fix debug logging not treating `-v -v` (follow-up to 7658677) - Update missed changelog Follow-up to opnsense#5126

fichtner · 2026-03-02T08:33:51Z

-        default:
            break;
+        case 'debug':
+            $cmd_frmt[] = '-vv';


need evidence -v -v works differently than -vv

The commit message explains why that is needed for the special behaving daemon: Fix debug logging not treating -v -v (follow-up to 7658677)

The miniupnpd code suggests it doesn't care if it's -vv or -v -v which is why I raised the question after you said it's wrong. I just want a dependable answer.

https://github.com/miniupnp/miniupnp/blob/f33ae48402835b5b93029b1861a32630f9f17d1b/miniupnpd/miniupnpd.c#L1609-L1615

https://github.com/miniupnp/miniupnp/blob/f33ae48402835b5b93029b1861a32630f9f17d1b/miniupnpd/miniupnpd.c#L2233

both -v -v and -vv trigger verbosity_level++; twice if I read this correctly. I just want to know what mistake I made so I can avoid it in the future.

I made a mistake. When I first tested it, it didn't seem to work, but I just tested it again and now it works. Sorry!

- More specific allow third-party mapping UI option - Remove unnecessary `binat` anchor registration - Fix debug logging not treating `-v -v` (follow-up to 7658677) - Update missed changelog Follow-up to opnsense#5126

Self-Hosting-Group · 2026-03-03T14:31:01Z

I don't know if you want a changelog or to what extent. The list of new/removed UI options can be dropped, or the changelog reduced to just one line, as you prefer.

fichtner · 2026-03-03T14:48:07Z

I don't know if you want a changelog or to what extent. The list of new/removed UI options can be dropped, or the changelog reduced to just one line, as you prefer.

Changelog seems good, thanks!

Self-Hosting-Group · 2026-03-04T08:00:40Z

@fichtner Both PRs ready.

fichtner · 2026-03-04T08:34:57Z

Merged, thanks!

Self-Hosting-Group · 2026-03-04T08:46:32Z

Thank you. I am happy that you have removed the length limit of the new entries in the changelog; it looks more compact.

Self-Hosting-Group · 2026-03-04T08:58:06Z

It's not all contributed by…, also you improved my limited PHP experience. Thank you!

fichtner · 2026-03-04T08:59:06Z

It's not all contributed by…, also you improved my limited PHP experience. Thank you!

we don't assert credits for helping with contributions since it's day to day work anyway

fichtner · 2026-03-04T09:00:35Z

FWIW, I'm pushing the plugin changes into today's 26.1.3. Need to look and discuss the ports PR at a later point in time.

Self-Hosting-Group · 2026-03-04T10:59:29Z

FWIW: In order to keep the maintenance effort for the added tested patches to a minimum in the future, I have suggested them to the daemon project and just updated the FreeBSD ports PR. PS: There is already a note about this in the changelog.

Self-Hosting-Group · 2026-03-04T10:59:47Z

https://redirect.github.com/freebsd/freebsd-ports/pull/402

fichtner · 2026-03-04T11:06:45Z

If you want my professional opinion work directly upstream and forget FreeBSD. Your PR nears the 1 year mark. Nobody is willing to invest time to help out contributors. One of the reasons why we do things differently in OPNsense.

Self-Hosting-Group · 2026-03-04T11:33:00Z

Exactly, that was my plan! ;-) I never wanted to add the same patches multiple times downstream (for multiple router OSs). However, little is happening upstream. Therefore, I attempted to resolve the daemon logging issues for OPNsense customers (I'm not a user), in this manner, as I feel like I have no other choice. See daemon activity: https://github.com/miniupnp

fichtner · 2026-03-04T12:01:19Z

Well, we can add some legitimacy for your patches from here, but ultimately a small fork to point to for patches would be preferable to keep the repo noise down in the ports tree. I did something similar with ports-mgmt/pkg, see opnsense/ports@bbd09e7 so we have https://github.com/opnsense/pkg to hold modifications (which are also easier visible and reviewable)

Self-Hosting-Group · 2026-03-12T15:24:18Z

Good idea! I'll come back to it soon.

fichtner reviewed Feb 25, 2026

View reviewed changes

Comment thread net/upnp/src/etc/inc/plugins.inc.d/miniupnpd.inc Outdated

fichtner reviewed Feb 25, 2026

View reviewed changes

Self-Hosting-Group force-pushed the not-applied branch 4 times, most recently from 7216aa4 to b0852db Compare February 27, 2026 15:49

Self-Hosting-Group changed the title ~~net/upnp: Not applied in #5126~~ net/upnp: Complete service improvements 2/2 Feb 27, 2026

net/upnp: Complete service improvements 2/2 opnsense#5256

bcca2b7

- More specific allow third-party mapping UI option - Remove unnecessary `binat` anchor registration - Fix debug logging not treating `-v -v` (follow-up to 7658677) - Update missed changelog Follow-up to opnsense#5126

fichtner reviewed Mar 2, 2026

View reviewed changes

Comment thread net/upnp/src/etc/inc/plugins.inc.d/miniupnpd.inc Outdated

fichtner reviewed Mar 2, 2026

View reviewed changes

Comment thread net/upnp/pkg-descr Outdated

Self-Hosting-Group force-pushed the not-applied branch from b0852db to fe81fd2 Compare March 3, 2026 14:30

fichtner reviewed Mar 3, 2026

View reviewed changes

Comment thread net/upnp/src/etc/inc/plugins.inc.d/miniupnpd.inc

Self-Hosting-Group force-pushed the not-applied branch from fe81fd2 to bcca2b7 Compare March 4, 2026 07:55

Self-Hosting-Group marked this pull request as ready for review March 4, 2026 08:00

fichtner self-assigned this Mar 4, 2026

fichtner reviewed Mar 4, 2026

View reviewed changes

Comment thread net/upnp/pkg-descr Outdated

Apply suggestion from @fichtner

1de9ce3

fichtner reviewed Mar 4, 2026

View reviewed changes

Comment thread net/upnp/src/etc/inc/plugins.inc.d/miniupnpd.inc Outdated

Apply suggestion from @fichtner

8f152fe

fichtner approved these changes Mar 4, 2026

View reviewed changes

fichtner merged commit 6c81f6d into opnsense:master Mar 4, 2026

leandroscardua pushed a commit to leandroscardua/plugins that referenced this pull request Apr 4, 2026

net/upnp: Complete service improvements 2/2 (opnsense#5256)

e600c8d

Conversation

Self-Hosting-Group commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fichtner Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Self-Hosting-Group commented Mar 3, 2026

Uh oh!

Uh oh!

fichtner commented Mar 3, 2026

Uh oh!

Self-Hosting-Group commented Mar 4, 2026

Uh oh!

Uh oh!

Uh oh!

fichtner commented Mar 4, 2026

Uh oh!

Self-Hosting-Group commented Mar 4, 2026

Uh oh!

Self-Hosting-Group commented Mar 4, 2026

Uh oh!

fichtner commented Mar 4, 2026

Uh oh!

fichtner commented Mar 4, 2026

Uh oh!

Self-Hosting-Group commented Mar 4, 2026

Uh oh!

Self-Hosting-Group commented Mar 4, 2026

Uh oh!

fichtner commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Self-Hosting-Group commented Mar 4, 2026

Uh oh!

fichtner commented Mar 4, 2026

Uh oh!

Self-Hosting-Group commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Self-Hosting-Group commented Feb 25, 2026 •

edited

Loading

fichtner Mar 4, 2026 •

edited

Loading

fichtner commented Mar 4, 2026 •

edited

Loading