Skip to content
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.

Sign up for GitHub

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

silabs-multiprotocol: Make the OTBR infrastructure network interface configurable #3416

Closed
wants to merge 3 commits into from
Closed

silabs-multiprotocol: Make the OTBR infrastructure network interface configurable #3416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e3e69a9
silabs-multiprotocol: Make the OTBR infra netif configurable
PaulSD Jan 17, 2024
ebad8b3
silabs-multiprotocol: Minor quoting, formatting, and wording cleanups
PaulSD Jan 17, 2024
c635f4b
silabs-multiprotocol: Bump version and changelog
PaulSD Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions silabs-multiprotocol/DOCS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,15 @@ Add-on configuration:
| Configuration | Description |
|--------------------|--------------------------------------------------------|
| device (mandatory) | Serial service where the Silicon Labs radio is attached |
| baudrate | Serial port baudrate (depends on firmware) |
| baudrate | Serial port baudrate (depends on firmware) |
| flow_control | If hardware flow control should be enabled (depends on firmware) |
| autoflash_firmware | Automatically install/update firmware (Home Assistant SkyConnect/Yellow) |
| network_device | Host and port where CPC daemon can find the Silicon Labs radio (takes precedence over device) |
| autoflash_firmware | Automatically install/update firmware (Home Assistant SkyConnect/Yellow) |
| cpcd_trace | Co-Processor Communication tracing (trace in log) |
| 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 |
| otbr_enable | Enable OpenThread Border Router |
| otbr_log_level | Set the log level of the OpenThread Border Router Agent |
| otbr_infra_if | HA host network interface name to bind the "infrastructure" side of the OpenThread Border Router to |
| otbr_firewall | Configure firewall to block unnecessary traffic to/from the OpenThread Border Router |

## Architecture

Expand Down
23 changes: 13 additions & 10 deletions silabs-multiprotocol/config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,27 @@ privileged:
- NET_ADMIN
image: homeassistant/{arch}-addon-silabs-multiprotocol
init: false
schema:
device: device(subsystem=tty)?
baudrate: list(57600|115200|230400|460800|921600)
flow_control: bool?
network_device: str?
autoflash_firmware: bool
cpcd_trace: bool
otbr_enable: bool
otbr_log_level: list(debug|info|notice|warning|error|critical|alert|emergency)
otbr_infra_if: str?
otbr_firewall: bool
options:
device: null
baudrate: "460800"
flow_control: true
network_device: null
autoflash_firmware: true
cpcd_trace: false
otbr_enable: true
otbr_log_level: notice
otbr_infra_if: null
otbr_firewall: true
ports:
9999/tcp: null
Expand All @@ -40,15 +53,5 @@ ports_description:
9999/tcp: EmberZNet EZSP/ASH port
8080/tcp: OpenThread Web port
8081/tcp: OpenThread REST API port
schema:
device: device(subsystem=tty)?
baudrate: list(57600|115200|230400|460800|921600)
flow_control: bool?
network_device: str?
autoflash_firmware: bool
cpcd_trace: bool
otbr_enable: bool
otbr_log_level: list(debug|info|notice|warning|error|critical|alert|emergency)
otbr_firewall: bool
stage: experimental
startup: services
56 changes: 31 additions & 25 deletions silabs-multiprotocol/rootfs/etc/s6-overlay/s6-rc.d/otbr-agent/run
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,73 +5,79 @@

. /etc/s6-overlay/scripts/otbr-agent-common

declare backbone_if
declare device
declare baudrate
declare flow_control
declare otbr_log_level
declare otbr_log_level_int
declare otbr_infra_if
declare otbr_rest_listen
declare otbr_rest_listen_port

backbone_if="$(bashio::api.supervisor 'GET' '/network/info' '' 'first(.interfaces[] | select (.primary == true)) .interface')"

otbr_log_level=$(bashio::string.lower "$(bashio::config otbr_log_level)")
case "${otbr_log_level}" in
debug)
otbr_log_level_int="7"
otbr_log_level_int='7'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a particular reason to use this style?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Single quote does not interpolate; Double quote interpolates.
So, the idea is to avoid interpolation when it isn't needed.

Obviously this is a minor nit, so if you don't like this change then I can drop it.

;;
info)
otbr_log_level_int="6"
otbr_log_level_int='6'
;;
notice)
otbr_log_level_int="5"
otbr_log_level_int='5'
;;
warning)
otbr_log_level_int="4"
otbr_log_level_int='4'
;;
error)
otbr_log_level_int="3"
otbr_log_level_int='3'
;;
critical)
otbr_log_level_int="2"
otbr_log_level_int='2'
;;
alert)
otbr_log_level_int="1"
otbr_log_level_int='1'
;;
emergency)
otbr_log_level_int="0"
otbr_log_level_int='0'
;;
*)
bashio::exit.nok "Unknown otbr_log_level: ${otbr_log_level}"
;;
esac

if [ -z ${backbone_if} ]; then
bashio::log.warning "No primary network interface found! Using static eth0."
backbone_if="eth0"
if bashio::config.has_value 'otbr_infra_if'; then
otbr_infra_if="$(bashio::config 'otbr_infra_if')"
bashio::log.info "Using configured network interface ${otbr_infra_if} for otbr_infra_if."
else
otbr_infra_if="$(bashio::api.supervisor 'GET' '/network/info' '' 'first(.interfaces[] | select (.primary == true)) .interface')"
if [ -n "${otbr_infra_if}" ]; then
bashio::log.info "Using primary network interface ${otbr_infra_if} for otbr_infra_if."
else
bashio::log.warning 'No primary network interface found! Using static eth0 for otbr_infra_if.'
otbr_infra_if='eth0'
Comment on lines +56 to +57
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a bit legacy. eth0 is almost never the right interface. It would be better if we exit here with an error, e.g.

Suggested change
bashio::log.warning 'No primary network interface found! Using static eth0 for otbr_infra_if.'
otbr_infra_if='eth0'
bashio::exit.nok 'No primary network interface found!'

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I thought this seemed like an odd choice for a default, but I didn't want to break anyone if this is actually used somewhere. I will change it.

fi
fi

mkdir -p /data/thread && ln -sft /var/lib /data/thread || bashio::exit.nok "Could not create directory /var/lib/thread to store Thread data."
mkdir -p /data/thread && ln -sft /var/lib /data/thread || bashio::exit.nok 'Could not create directory /var/lib/thread to store Thread data.'

if bashio::config.true 'otbr_firewall'; then
bashio::log.info "Setup OTBR firewall..."
bashio::log.info 'Setup OTBR firewall...'
ipset create -exist otbr-ingress-deny-src hash:net family inet6
ipset create -exist otbr-ingress-deny-src-swap hash:net family inet6
ipset create -exist otbr-ingress-allow-dst hash:net family inet6
ipset create -exist otbr-ingress-allow-dst-swap hash:net family inet6

ip6tables -N $otbr_forward_ingress_chain
ip6tables -I FORWARD 1 -o $thread_if -j $otbr_forward_ingress_chain
ip6tables -I FORWARD 1 -o "${thread_if}" -j $otbr_forward_ingress_chain

ip6tables -A $otbr_forward_ingress_chain -m pkttype --pkt-type unicast -i ${thread_if} -j DROP
ip6tables -A $otbr_forward_ingress_chain -m pkttype --pkt-type unicast -i "${thread_if}" -j DROP
ip6tables -A $otbr_forward_ingress_chain -m set --match-set otbr-ingress-deny-src src -j DROP
ip6tables -A $otbr_forward_ingress_chain -m set --match-set otbr-ingress-allow-dst dst -j ACCEPT
ip6tables -A $otbr_forward_ingress_chain -m pkttype --pkt-type unicast -j DROP
ip6tables -A $otbr_forward_ingress_chain -j ACCEPT

ip6tables -N $otbr_forward_egress_chain
ip6tables -I FORWARD 2 -i $thread_if -j $otbr_forward_egress_chain
ip6tables -I FORWARD 2 -i "${thread_if}" -j $otbr_forward_egress_chain
ip6tables -A $otbr_forward_egress_chain -j ACCEPT
else
# Make sure ip6tables (as used by Docker) allow IP forwarding
Expand All @@ -80,25 +86,25 @@ else
ip6tables-legacy -P FORWARD ACCEPT
fi

otbr_rest_listen="::"
otbr_rest_listen='::'
otbr_rest_listen_port="$(bashio::addon.port 8081)"

# If user port is not set, listen on local interface only
if ! bashio::var.has_value "${otbr_rest_listen_port}"; then
otbr_rest_listen="$(bashio::addon.ip_address)"
otbr_rest_listen_port="8081"
otbr_rest_listen_port='8081'
elif [ "${otbr_rest_listen_port}" != "8081" ]; then
bashio::log.warning "Custom OpenThread REST API port is not supported. Using 8081."
otbr_rest_listen_port="8081"
bashio::log.warning 'Custom OpenThread REST API port is not supported. Using 8081.'
otbr_rest_listen_port='8081'
fi

# Store REST API listen information for check script
echo "${otbr_rest_listen}" > /tmp/otbr-agent-rest-api
echo "${otbr_rest_listen_port}" >> /tmp/otbr-agent-rest-api

bashio::log.info "Starting otbr-agent..."
bashio::log.info 'Starting otbr-agent...'
exec s6-notifyoncheck -d -s 300 -w 300 -n 0 \
"/usr/sbin/otbr-agent" -I ${thread_if} -B "${backbone_if}" \
"/usr/sbin/otbr-agent" -I "${thread_if}" -B "${otbr_infra_if}" \
--rest-listen-address "${otbr_rest_listen}" \
-d${otbr_log_level_int} -v \
"spinel+cpc://cpcd_0?iid=2&iid-list=0"
11 changes: 9 additions & 2 deletions silabs-multiprotocol/translations/en.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,22 @@ configuration:
description: Enable tracing for the Co-Processor Communication daemon.
otbr_enable:
name: Enable OpenThread Border Router
description: Enable OpenThread Border Router agent.
description: Enable the OpenThread Border Router.
otbr_log_level:
name: OpenThread Border Router agent log level
description: >-
Set logging level of the OpenThread Border Router agent (otbr-agent).
otbr_infra_if:
name: OpenThread Border Router infrastructure interface
description: >-
HA host network interface name to bind the "infrastructure" side (vs the
"Thread" side) of the OpenThread Border Router to. By default, the
first interface with an associated default route is used.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use what is considered primary by NetworkManager. Is the first interface with an associated default what NetworkManager considers primary? 🤔

Copy link
Author

@PaulSD PaulSD Jan 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I said "first" here because we used this code: bashio::api.supervisor 'GET' '/network/info' '' 'first(.interfaces[] | select (.primary == true)) .interface '

However, I think (but am not entirely sure) that NetworkManager will only ever have one primary interface, and I think the primary interface is the most recently activated interface that has an associated default route (either IPv4 or IPv6 default route).

otbr_firewall:
name: OTBR firewall
description: >-
Use OpenThread Border Router firewall to block unnecessary traffic.
Configure firewall to block unnecessary traffic to/from the OpenThread
Border Router.
network:
9999/tcp: EmberZNet EZSP/ASH port
8080/tcp: OpenThread Web port
Expand Down