Miscellaneous fixes#2175
Conversation
WalkthroughThis update introduces several enhancements and refactorings across network and wireless management scripts, Docker initialization, and related PHP modules. Notable changes include the addition of helper functions for network device existence and carrier status checks, improved logic for selecting network interfaces, and more robust handling of wireless security types, including roaming profiles. Docker configuration logic is consolidated into a new function, with refined storage driver selection and log rotation options. Wireless SSID handling is improved with a dedicated quote-escaping function, and bitrate normalization is added for wireless statistics. UI and backend checks are updated to align with these backend improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant rc.wireless
participant WPA_Supplicant
participant NetworkInterface
User->>rc.wireless: Start WiFi
rc.wireless->>NetworkInterface: carrier(interface, retries)
alt Interface exists and carrier up
rc.wireless->>rc.wireless: Determine SECURITY types (incl. roaming)
rc.wireless->>rc.wireless: For each SECURITY, call wpa_configuration()
rc.wireless->>WPA_Supplicant: Launch with generated config
WPA_Supplicant-->>rc.wireless: Connection result
else Interface missing or no carrier
rc.wireless-->>User: Fail to start WiFi
end
sequenceDiagram
participant rc.docker
participant docker_read_options
participant NetworkInterface
rc.docker->>docker_read_options: Initialize Docker options
docker_read_options->>NetworkInterface: carrier(interface, timeout)
docker_read_options->>docker_read_options: Select active port, set options
docker_read_options-->>rc.docker: Docker options ready
rc.docker->>rc.docker: Start or restart Docker with new options
sequenceDiagram
participant update.wireguard.php
participant isPort
participant carrier
update.wireguard.php->>isPort: Check device existence
alt Exists
update.wireguard.php->>carrier: Check carrier status (with loop)
alt Carrier up
update.wireguard.php-->>update.wireguard.php: Use device for routing
else No carrier
update.wireguard.php->>isPort: Check wlan0
update.wireguard.php->>carrier: Check wlan0 carrier
alt wlan0 carrier up
update.wireguard.php-->>update.wireguard.php: Switch to wlan0
else
update.wireguard.php-->>update.wireguard.php: No suitable device
end
end
else
update.wireguard.php-->>update.wireguard.php: No device found
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
emhttp/plugins/dynamix/include/update.wireguard.php(1 hunks)etc/rc.d/rc.docker(2 hunks)etc/rc.d/rc.wireless(7 hunks)
🔇 Additional comments (11)
etc/rc.d/rc.wireless (6)
62-69: Function renamed and enhanced with existence checkThe
carrier_upfunction has been renamed tocarrierand improved to check for device existence before attempting to read carrier status. This change harmonizes with similar carrier detection functions in other network-related scripts.
93-94: Updated function call to use new carrier functionThe code appropriately uses the renamed
carrierfunction with its boolean return value.
107-107: Updated variable assignment to use carrier functionThe code has been updated to use the new
carrierfunction for status assignment.
188-188: Enhanced check in wifi_running functionThe
wifi_runningfunction has been updated to include an existence check for the interface directory before reading carrier status, making it more robust.
215-220: Updated carrier checks in wifi_startThe code has been appropriately updated to use the new
carrierfunction with its boolean return value.
286-290: Improved security protocol detection logicThe security protocol detection now uses regex pattern matching (
=~operator) instead of exact string equality checks, allowing for more flexible detection. This change helps address a regression error in security detection.emhttp/plugins/dynamix/include/update.wireguard.php (2)
35-37: Added helper function to check port existenceNew
port()helper function to check for the existence of a network device.
53-54: Updated thisNet() function to use new helpersThe
thisNet()function now uses the new helper functions for better device selection logic.etc/rc.d/rc.docker (3)
39-46: Enhanced carrier function with polling mechanismThe
carrierfunction has been improved to check for device existence and to actively poll for carrier status with retries, returning a boolean result.
51-51: Updated active port selection logicThe port selection logic now uses the boolean return value from the carrier function instead of string comparison, making the code more robust.
263-263: Updated conditional for carrier detectionModified to use the boolean return from the carrier function rather than string comparison.
bergware
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/include/NetworkInfo.php (1)
58-62: Improved bitrate parsing and normalization.The updated code properly normalizes network bitrates by extracting numeric values and ensuring consistent formatting with appropriate units. This enhances reliability when parsing output from the
iw wlan0 linkcommand.Consider extracting this parsing logic into a reusable function to avoid code duplication between rxrate and txrate processing:
57 $rxrate = explode(': ', $speed[3])[1]; - $tmprate = explode(' ', $rxrate); - $rxrate = intval($tmprate[0] ?? 0).' '.($tmprate[1] ?? 'Bit/s'); 60 $txrate = explode(': ', $speed[4])[1]; - $tmprate = explode(' ', $txrate); - $txrate = intval($tmprate[0] ?? 0).' '.($tmprate[1] ?? 'Bit/s'); + + // Helper function to normalize bitrate values + function normalizeBitrate($rate) { + $parts = explode(' ', $rate); + return intval($parts[0] ?? 0).' '.($parts[1] ?? 'Bit/s'); + } + + $rxrate = normalizeBitrate($rxrate); + $txrate = normalizeBitrate($txrate);
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
etc/rc.d/rc.wireless (1)
38-38: Fix the typo in "decprecated".The word "deprecated" is misspelled as "decprecated" in the translation for WEP security type.
- "WEP") echo "WEP (decprecated)" ;; + "WEP") echo "WEP (deprecated)" ;;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
emhttp/plugins/dynamix/include/update.wireguard.php(3 hunks)etc/rc.d/rc.docker(4 hunks)etc/rc.d/rc.wireless(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- emhttp/plugins/dynamix/include/update.wireguard.php
🔇 Additional comments (14)
etc/rc.d/rc.wireless (7)
45-59: Good addition of security priority ranking.The new
priorityfunction establishes a clear hierarchy for security protocols, which improves the automatic selection process by prioritizing more secure protocols (WPA3 Enterprise) over less secure ones.
80-87: Well-implemented carrier detection with retries.The renamed and improved
carrierfunction now properly checks for interface existence before attempting to read carrier status and implements a configurable retry mechanism. This makes network interface detection more robust.
175-175: Improved logging for wpa_configuration.Adding the translated security type to the logs is a great improvement for troubleshooting.
181-182: Effective use of regex for protocol variants.Using regex pattern matching (
=~) instead of exact matching (==) is an excellent improvement that handles all protocol variants including roaming profiles with a single condition.Also applies to: 196-198
190-195: Good support for Fast Transition (FT) protocols.The addition of explicit support for Fast Transition protocols (FT/PSK, FT/SAE, FT/IEEE-802.1X) enhances roaming capabilities between access points in the same network.
201-202: Proper ieee80211w settings for different protocols.The script correctly sets Management Frame Protection (MFP) levels based on security protocol:
- Level 1 (optional) for WPA2 Enterprise
- Level 2 (required) for WPA3 protocols that mandate stronger protection
295-305: Greatly improved security type handling.The new approach for handling security types when "auto" is specified is more maintainable and flexible:
- It normalizes security type names (replacing spaces with hyphens)
- It tries security types in priority order (from most secure to least secure)
- It supports all protocol variants including roaming profiles
etc/rc.d/rc.docker (7)
39-46: Well-implemented carrier detection with retries.The improved
carrierfunction now checks for interface existence and implements a configurable retry mechanism with a default 10-second timeout. This makes network interface detection more robust.
48-53: Improved network interface selection logic.The new
docker_read_optionsfunction encapsulates network port detection in a more structured way, with the conditional statement using boolean returns from thecarrierfunction instead of string comparisons. This is more reliable and consistent with the rest of the system.
75-87: Enhanced storage driver selection with XFS support.The storage driver selection logic now explicitly handles XFS filesystems by setting the storage driver to overlay2, which is the recommended driver for this filesystem type.
89-97: Added configurable log rotation settings.The addition of global Docker log rotation options helps prevent log files from consuming excessive disk space. Default values are sensibly set but can be overridden through configuration.
99-114: Proper MTU and IPv6 handling.The code now:
- Adjusts Docker MTU to match the network interface when non-default
- Configures IPv6 for Docker containers when a default IPv6 route exists
- Sets up proper IPv6 NAT rules for container connectivity
- Disables IPv6 on docker0 interface when IPv6 is not available
This ensures containers have the correct network settings to match the host environment.
647-647: Properly refresh Docker options before service start.The addition of explicit
docker_read_optionscalls before starting or restarting Docker ensures that all configuration options and network settings are refreshed, leading to a more consistent and predictable behavior.Also applies to: 667-667
43-43:⚠️ Potential issueFix the brace expansion syntax in for loop.
The brace expansion syntax
{1..$e}won't work as expected with variables. Bash doesn't perform variable substitution within brace expansion.- for n in {1..$e}; do + for ((n=1; n<=$e; n++)); doLikely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
etc/rc.d/rc.docker (1)
53-53: Consider ordering wireless check by prioritizing more reliable interfacesThe wireless check happens after bridge/bond/ethernet interfaces, which is appropriate for most deployments. However, the line combines multiple operations (carrier check and assignment), which might be a bit hard to follow.
For clarity, consider splitting this into a separate conditional block:
- [[ ! $(carrier $PORT) && $(carrier wlan0 1) ]] && PORT=wlan0 + # Only use wireless if primary interface has no carrier + if [[ ! $(carrier $PORT) && $(carrier wlan0 1) ]]; then + PORT=wlan0 + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
emhttp/plugins/dynamix/include/update.wireguard.php(3 hunks)etc/rc.d/rc.docker(4 hunks)etc/rc.d/rc.wireless(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- etc/rc.d/rc.wireless
- emhttp/plugins/dynamix/include/update.wireguard.php
🔇 Additional comments (5)
etc/rc.d/rc.docker (5)
38-47: Well-designed carrier function with proper error handlingThe revised
carrierfunction now includes several important improvements:
- Checks if the interface exists before attempting to read its carrier status
- Implements a retry loop with configurable timeout (defaulting to 10 seconds)
- Returns boolean status (0 for success, 1 for failure) for proper error handling in calling code
- Prevents unnecessary sleep when checking with
e=1(single attempt)This change aligns with similar improvements in other network-related scripts, creating consistency across the system.
49-117: Good centralization of Docker configuration logicThe new
docker_read_optionsfunction effectively centralizes configuration logic that was previously scattered throughout the code. This improves maintainability and ensures consistent behavior.Key improvements:
- Unified network port detection logic
- Centralized Docker configuration reading/updating
- Improved storage driver selection with explicit handling for xfs filesystems
- Well-structured log rotation configuration
- Proper IPv6 setup with NAT rules when applicable
The function properly handles various edge cases and provides clear, logical progression through each configuration step.
83-85: Appropriate addition of overlay2 support for xfs filesystemThe explicit handling of xfs filesystems using overlay2 storage driver is a good improvement that fills a previous gap in the storage driver selection logic.
266-266: Correct usage of the updated carrier functionThis line properly leverages the new boolean-returning
carrierfunction with a 1-second timeout parameter, replacing the previous string comparison method. The condition now checks if the interface has carrier before considering it for network configuration.
648-648: Good placement of docker_read_options function callThe addition of
docker_read_optionsfunction calls at the beginning of both start and restart command cases ensures that Docker options and network port detection are refreshed before starting Docker services. This prevents potential issues with stale configuration or incorrect network interface detection.Also applies to: 668-669
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style