-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
silabs-multiprotocol: add otbr NAT64 support #3991
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
Conversation
📝 WalkthroughWalkthroughA new configuration option, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Add-on Startup
participant otbr-agent-configure (s6 oneshot)
participant otbr-agent (s6 service)
participant ot-ctl
participant iptables
User->>Add-on Startup: Enables otbr_nat64 in config
Add-on Startup->>otbr-agent-configure: Start oneshot service
otbr-agent-configure->>ot-ctl: Enable NAT64 (if otbr_nat64)
otbr-agent-configure->>ot-ctl: Enable DNS upstream (if otbr_nat64)
otbr-agent-configure->>ot-ctl: Set txpower 6
Add-on Startup->>otbr-agent: Start service
otbr-agent->>iptables: Add IPv4 NAT64 rules (if otbr_nat64)
otbr-agent->>iptables: Add IPv6 firewall rules
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 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 (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
silabs-multiprotocol/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent-configure/up (1)
1-1
: Add shebang for clarity and executable context
Including a shebang (e.g.,#!/usr/bin/env sh
) at the top of theup
script improves readability and ensures the correct interpreter is used.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
silabs-multiprotocol/DOCS.md
(1 hunks)silabs-multiprotocol/Dockerfile
(2 hunks)silabs-multiprotocol/config.yaml
(2 hunks)silabs-multiprotocol/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent-configure/type
(1 hunks)silabs-multiprotocol/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent-configure/up
(1 hunks)silabs-multiprotocol/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent/run
(1 hunks)silabs-multiprotocol/rootfs/etc/s6-overlay/scripts/otbr-agent-configure.sh
(1 hunks)silabs-multiprotocol/translations/en.yaml
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
`*/**(html|markdown|md)`: - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure t...
*/**(html|markdown|md)
: - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
silabs-multiprotocol/DOCS.md
silabs-multiprotocol/config.yaml
silabs-multiprotocol/Dockerfile
`*/**(html|markdown|md)`: - Use bold to mark UI strings. - If "" are used to mark UI strings, replace them by bold.
*/**(html|markdown|md)
: - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
silabs-multiprotocol/DOCS.md
silabs-multiprotocol/config.yaml
silabs-multiprotocol/Dockerfile
`*/**(html|markdown|md)`: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
*/**(html|markdown|md)
: - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
silabs-multiprotocol/DOCS.md
silabs-multiprotocol/config.yaml
silabs-multiprotocol/Dockerfile
`*/**(html|markdown|md)`: - Use sentence-style capitalization also in headings.
*/**(html|markdown|md)
: - Use sentence-style capitalization also in headings.
silabs-multiprotocol/DOCS.md
silabs-multiprotocol/config.yaml
silabs-multiprotocol/Dockerfile
`*/**(html|markdown|md)`: do not comment on HTML used for icons
*/**(html|markdown|md)
: do not comment on HTML used for icons
silabs-multiprotocol/DOCS.md
silabs-multiprotocol/config.yaml
silabs-multiprotocol/Dockerfile
`*/**(html|markdown|md)`: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
*/**(html|markdown|md)
: Avoid flagging inline HTML for embedding videos in future reviews for this repository.
silabs-multiprotocol/DOCS.md
silabs-multiprotocol/config.yaml
silabs-multiprotocol/Dockerfile
🔇 Additional comments (8)
silabs-multiprotocol/config.yaml (2)
35-35
: Expose new NAT64 option with default disabled
Theotbr_nat64: false
setting adds NAT64 support control, defaulting to off to avoid unintended IPv4 translation.
54-54
: Validate NAT64 option as boolean in schema
Includingotbr_nat64: bool
ensures configuration validation will catch non-boolean values for this new option.silabs-multiprotocol/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent-configure/type (1)
1-1
: Configure service as one-shot
Theoneshot
type correctly instructs s6-rc to run this service only once at startup.silabs-multiprotocol/translations/en.yaml (1)
37-41
: Approve translation entry for NAT64 option
The newotbr_nat64
translation key with name and description accurately conveys the feature to end users.silabs-multiprotocol/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent/run (1)
101-106
: NAT64 configuration looks good!The code correctly applies the necessary iptables rules to enable NAT64 forwarding when the configuration option is enabled. This implementation properly marks packets from the Thread interface and allows bidirectional communication through the backbone interface.
silabs-multiprotocol/rootfs/etc/s6-overlay/scripts/otbr-agent-configure.sh (2)
1-12
: NAT64 configuration implementation is complete.The script correctly enables NAT64 and DNS upstream features using
ot-ctl
commands when the configuration option is enabled. The logging provides clear indication when this feature is activated.
13-15
: TX power setting explanation is appropriate.Setting the TX power to 6 dBm is well-explained in the comments, ensuring consistent link quality among Thread routers.
silabs-multiprotocol/Dockerfile (1)
238-240
:✅ Verification successful
Build configuration for NAT64 is properly implemented.
The added CMake flags correctly enable NAT64 functionality, set the appropriate CIDR range, and enable DNS upstream query support during the build process.
🏁 Script executed:
#!/bin/bash # Verify that the NAT64 CIDR doesn't conflict with common home network ranges echo "Checking if the NAT64 CIDR (192.168.255.0/24) might conflict with common home networks:" echo "- 192.168.0.0/24 (very common)" echo "- 192.168.1.0/24 (very common)" echo "- 10.0.0.0/8 (common for larger networks)" echo "- 172.16.0.0/12 (less common but still used)" echo "" echo "The chosen CIDR (192.168.255.0/24) is in the private address space but uses a high third octet (255) which minimizes the likelihood of conflicts with typical home networks."Length of output: 884
NAT64 Build Flags Confirmed
File: silabs‑multiprotocol/Dockerfile (Lines 238–240)
- The CMake flag
-DOTBR_NAT64=ON
correctly enables NAT64 functionality.- The CIDR
-DOT_POSIX_NAT64_CIDR="192.168.255.0/24"
resides in private address space with a high third octet, minimizing conflicts with typical home networks.- The flag
-DOTBR_DNS_UPSTREAM_QUERY=ON
properly enables DNS upstream query support.All checks pass—no changes required.
silabs-multiprotocol/DOCS.md
Outdated
@@ -83,6 +83,7 @@ Add-on configuration: | |||
| otbr_enable | Enable OpenThread BorderRouter | | |||
| otbr_log_level | Set the log level of the OpenThread BorderRouter Agent | | |||
| otbr_firewall | Enable OpenThread Border Router firewall to block unnecessary traffic | | |||
| orbr_nat64 | Enable OpenThread Border Router NAT64 to allow Thread devices accessing IPv4 addresses | |
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.
Fix typo in configuration key
The table lists orbr_nat64
but the actual option is otbr_nat64
. Correct the key and refine description grammar.
Apply:
-| orbr_nat64 | Enable OpenThread Border Router NAT64 to allow Thread devices accessing IPv4 addresses |
+| otbr_nat64 | Enable OpenThread Border Router NAT64 to allow Thread devices to access IPv4 addresses |
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| orbr_nat64 | Enable OpenThread Border Router NAT64 to allow Thread devices accessing IPv4 addresses | | |
| otbr_nat64 | Enable OpenThread Border Router NAT64 to allow Thread devices to access IPv4 addresses | |
Thanks, will there be an alternative to this addon? |
No, there will be no alternative. We do not recommend on using the multiprotocol approach, as it had led to many issues. ../Frenck |
This adds the otbr nat64 support for the multiprotocl addon.
All changes were copied from the otbr addon and tested locally.
Summary by CodeRabbit