UPnP leases issue #3298
Merged
UPnP leases issue #3298
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Preparatory change for better logging in portmapping
There is no need for the call to check_mapping_loop because there is an outer check that will work every time as long as it tickets fast enough.
…check period We currently check every health check period for renewals but every 5 minutes for devices. It is too complicated to have 2 different setting and there is no need. It is harder to code, document and to understand and there is not much benefit from it. So, use health check period for all checks. Nice and simple.
The logging before didn't make it clear if it was remaining lease or total lease time.
Some upnp logs are throttle controlled so that users are not swamped by upnp messages, if upnp is not working. However, when upnp logging is explicitly enabled then it makes no sense to throttle them.
clemahieu
approved these changes
May 25, 2021
clemahieu
pushed a commit
that referenced
this pull request
May 28, 2021
* Inverted logic for refreshing upnp port mapping * Refresh logic for UPnP lease to consider more than one protocol * mapping_protocol pretty print function to_string() Preparatory change for better logging in portmapping * Remove extraneous and needless call to check_mapping_loop There is no need for the call to check_mapping_loop because there is an outer check that will work every time as long as it tickets fast enough. * Remove extraneous and needless logging * Pretty print function for port_mapping and upnp_state classes * Check for missing mappings, renewals or gateway changes every health check period We currently check every health check period for renewals but every 5 minutes for devices. It is too complicated to have 2 different setting and there is no need. It is harder to code, document and to understand and there is not much benefit from it. So, use health check period for all checks. Nice and simple. * Improve logging regarding remaining upnp lease The logging before didn't make it clear if it was remaining lease or total lease time. * Always print all upnp logs if unpn logging is enabled Some upnp logs are throttle controlled so that users are not swamped by upnp messages, if upnp is not working. However, when upnp logging is explicitly enabled then it makes no sense to throttle them. Co-authored-by: Dimitrios Siganos <dimitris@siganos.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is intended to address the proposed changes on #3295 proposed by https://github.com/sudokai0