-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(otbr): Add option to override backbone interface #4298
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?
Conversation
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Agent as otbr-agent
participant Config as Configuration
participant Supervisor as Supervisor API
participant System as System
Agent->>Config: Read backbone_interface
alt configured
Config-->>Agent: Return configured interface
Agent->>Agent: Use configured interface
else not configured
Agent->>Supervisor: Query primary interface
alt supervisor responds
Supervisor-->>Agent: Return primary interface
Agent->>Agent: Log and use primary interface
else supervisor unavailable
Agent->>System: Run `ip -br link show up`
alt enabled interfaces found
System-->>Agent: Return first enabled interface
Agent->>Agent: Log and use fallback interface
else none found
Agent->>Agent: Exit with error
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 2
🤖 Fix all issues with AI agents
In `@openthread_border_router/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent/run`:
- Around line 81-88: The fallback selection for backbone_if in the run script
incorrectly assumes loopback is always first (it uses awk 'NR==2'); update the
block that sets backbone_if to pick the first non-loopback interface instead
(e.g. pipe ip -br link show up through a filter to exclude "lo" and take the
first match) so backbone_if="$(ip -br link show up | awk '$1 != "lo" {print $1;
exit}')" (or equivalent grep -v/awk combo); change the code that assigns
backbone_if and keep the existing error and log handling (references:
backbone_if variable and the fallback block in the run script).
In `@openthread_border_router/translations/en.yaml`:
- Around line 13-17: The description for backbone_interface is awkwardly
phrased; update the sentence "If not specified it would fallback to the primary
interface." to present tense with a comma and correct spacing: change it to "If
not specified, it falls back to the primary interface." Ensure the
backbone_interface.description entry is edited accordingly (use "falls back" as
two words and insert the comma after "specified").
🧹 Nitpick comments (1)
openthread_border_router/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent/run (1)
26-79: Verify optionalbackbone_interfacehandling when unset.
Ifbashio::configreturnsnullfor missing keys, Line 26 setsbackbone_iftonull, so Line 76 won’t trigger fallback andotbr-agentwill be passed an invalid interface. Guard withbashio::config.has_valueand quote the-zcheck.🛠️ Suggested update
-backbone_if=$(bashio::config 'backbone_interface') +backbone_if="" +if bashio::config.has_value 'backbone_interface'; then + backbone_if="$(bashio::config 'backbone_interface')" +fi @@ -if [ -z ${backbone_if} ]; then +if [ -z "${backbone_if}" ]; then
3e11dea to
bbc924a
Compare
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
🤖 Fix all issues with AI agents
In `@openthread_border_router/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent/run`:
- Around line 76-79: Quote the variable test and only log the "using primary
interface" message when the supervisor API actually returned a value: change the
conditional to test [ -z "${backbone_if}" ] and call bashio::api.supervisor to
populate backbone_if before emitting a warning; if backbone_if is non-empty log
the existing warning via bashio::log.warning referencing the resolved
backbone_if, otherwise log a different error/warning that the supervisor
returned no interface. Ensure references to backbone_if, bashio::api.supervisor
and bashio::log.warning are updated accordingly.
openthread_border_router/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent/run
Outdated
Show resolved
Hide resolved
bbc924a to
6672f62
Compare
|
would love to have this ability. |
|
@agners Do you think a more sophisticated fallback for default network interface would be necessary? Previously it was just hardcoded "eth0" (that no longer in use with modern systemd/Linux kernels). Current behaviour is to pick first interface if primary interface is not available from Supervisor API. |
This PR adds option to override interface used for backbone network.
Currently it always uses primary interface, and that's really inconvenient with multi-VLAN setup.
For example i prefer to have my IoT devices to be isolated in a separate network with strict firewall policies enforced on router.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.