Suggestions from Randall#41
Merged
Merged
Conversation
Update _qs_now_in_window() to save and restore the caller's original IFS instead of resetting it to a single space after parsing comma-separated day-of-week values. The previous implementation temporarily set IFS=',' while iterating over the configured DOW list, but restored it with IFS=' '. This could drop the default tab and newline delimiters and unintentionally affect later word-splitting behavior elsewhere in the script. This change makes the schedule check safer by: - saving the original IFS before parsing the DOW list - restoring the exact original IFS after parsing completes - keeping the existing minute-of-day window behavior unchanged - continuing to support 7 as an alias for Sunday - handling wrapped DOW ranges such as 5-1 for Friday through Monday The update is intended to be behavior-preserving for normal schedules while preventing subtle shell parsing side effects after scheduler evaluation.
Remove existing FlexQoS POSTROUTING jump rules before adding them back in `iptables_static_rules()`. This makes the static rule setup idempotent and prevents duplicate IPv4 and IPv6 mangle-table jumps from accumulating across restarts or repeated QoS rule applications.
Replace the `tcp>udp` protocol sentinel in `parse_iptablerule()` with an explicit `"both"` state and expand it directly before emitting rules. This prevents custom iptables rules from depending on the caller's IFS value while preserving the existing tcp, udp, and all protocol behavioir.
Use command-scoped IFS assignments when reading custom iptables and appdb rule fields instead of temporarily changing the shell-wide IFS value. This preserves existing rule parsing behavior while reducing the chance of future IFS leakage or caller/callee coupling.
Return a failure status from `download_file()` when curl cannot retrieve the remote script, remove any partial temp file, and stop `update()` before re-executing the installer. This prevents the update path from continuing after a failed download.
Set the QoS apply flag when `qos_type` is changed to Adaptive QoS, not only when `qos_enable` is changed. This ensures service `start_qos`, flowcache policy application, restart prompting, and conntrack flushing run when QoS is already enabled but the active QoS type is not Adaptive QoS.
Replace the raw script path/argument startup log with a clearer structured message that includes the action, mode, process ID, parent process ID, and original arguments.
Cache `qdisc`, HTB overhead, MTU-derived packet minimums, and `fq_codel` parameters during TC rule generation. This avoids repeated `am_settings_get`, `nvram`, `awk`, and arithmetic work while generating custom HTB class and `fq_codel qdisc` rules. Reuse cached values in `burst`, `cburst`, `quantum`, `overhead`, and `qdisc` helper paths.
Replace repeated echo|sed and echo|cut transformations in `parse_iptablerule()` with POSIX parameter expansion and small helper formatters. This keeps rule output behavior unchanged while reducing external process spawning when generating custom iptables rules.
Cache the `bwdpi_app_rulelist` lookup before parsing QoS category ordering instead of running nvram and grep each time category 4 is encountered. This avoids repeated external commands while preserving the existing `Streaming`/`Learn-From-Home` priority behavior.
Shift scheduled QoS stop jobs to the following day when a schedule window crosses midnight. Also align immediate schedule state by matching post-midnight windows against the previous schedule day, so restricted DOW schedules such as `Fri 22:00` -> `Sat 06:00` behave as described in the scheduler notes.
Allow wraparound day-of-week ranges in QoS scheduler validation, then expand them before writing cron jobs so behavior does not depend on cron-specific range handling. Tighten DOW validation so malformed lists with empty fields, such as trailing commas, are rejected instead of being silently accepted. Also clean up `format_ipset_arg` spacing and avoid overwriting positional parameters in `parse_iptablerule` by using an explicit protocol list.
Move flowcache policy and conntrack refresh out of the early startup path and run them only after the scheduler has aligned the final Adaptive QoS state. This avoids unnecessary flowcache/conntrack churn during scheduled-off windows, where startup could briefly apply the QoS-on policy before the scheduler immediately disables QoS again. Keep the start/check command path simple so it only logs invocation details and calls startup directly.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
To be reviewed when possible.