-
Notifications
You must be signed in to change notification settings - Fork 764
net/upnp: Complete service improvements #5126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
net/upnp: Complete service improvements #5126
Conversation
4c98364 to
a42ac1d
Compare
|
See 26.1 coming soon. Can we continue here? |
|
If I just copy the syslog filter template, it will not be applied; a |
|
syslog is always restarted post-install for plugins:
Sure, the PR is still in draft mode... you need to signal when you're ready first. |
| /* Disable to workaround daemon bug with PCP/NAT-PMP epoch on BSD | ||
| if (!empty($upnp_config['sysuptime'])) { | ||
| $config_text .= "system_uptime=yes\n"; | ||
| } | ||
| } */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer not to comment out large blocks with /* */
| /* Disable to workaround daemon bug with PCP/NAT-PMP epoch on BSD | |
| if (!empty($upnp_config['sysuptime'])) { | |
| $config_text .= "system_uptime=yes\n"; | |
| } | |
| } */ | |
| if (!empty($upnp_config['sysuptime'])) { | |
| /* XXX disable to workaround daemon bug with PCP/NAT-PMP epoch on BSD */ | |
| //$config_text .= "system_uptime=yes\n"; | |
| } |
net/upnp/Makefile
Outdated
| PLUGIN_VERSION= 1.8 | ||
| PLUGIN_REVISION= 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| PLUGIN_VERSION= 1.8 | |
| PLUGIN_REVISION= 1 | |
| PLUGIN_VERSION= 1.9 |
Since you're adding syslog definition it would be clearer to make a new version
|
|
||
| 1.8 | ||
|
|
||
| ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes please, I did forget these
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about these? and can you bring it out of draft mode please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add that. Otherwise, there are no further changes here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the draft. Will create the ports PR.
a42ac1d to
74fc831
Compare
|
Sorry for the delay. I've just pushed the changes. Since the OPNsense 26.1 release is tomorrow and this PR is ready, it would be great if you could give me a quick comment on the following points. I'll then prepare the missing changelog and remove the draft flag from the PR to be merged.
I did this so that the PR wouldn't be merged unexpectedly too early. Thank you for the review.
|
Yep should be good as is.
Where are you logging to? System log? It may be better to add a separate log file and page for miniupnpd alone. The system log is capped to notice (so won't see info or debug) for historic reasons. |
74fc831 to
65199d4
Compare
|
PS: The last update changed the headings |
|
@fichtner @fraenki Critical issue detected in OPNsense 26.1 and registerAnchor function used in plugin.
|
|
Let us release this PR in a .2 version after the core has been corrected in a new version. |
|
@fichtner I also did a fresh installation of 26.1, including 26.1_4, to rule out errors during migration. But even then, the necessary anchors are not created. |
|
Thanks for the hint. I found it https://forum.opnsense.org/index.php?topic=50520.msg258554#msg258554 |
|
Thank you for fixing it so quickly! I've checked the patch and it's working. |
|
A 26.1_5 release? |
|
This should have been in today’s hotfix, but there likely won’t be another. We’re targeting a 26.1.1 next week for a lot of other reasons. Not sure how early or late in the week that will be. |

No description provided.