silabs-multiprotocol: Make the OTBR infrastructure network interface configurable #3416
Conversation
The OpenThread Border Router agent currently only supports routing between a single "infrastructure" network interface and a single Thread network interface per agent process. (Multiple "infrastructure" interfaces can be specified, but this is for fail-over purposes and only one will be used at a time.) Thread device commissioning in HA currently requires an Android device which can communicate both with the Thread device via Bluetooth and with the OpenThread Border Router "infrastructure" interface via IPv6 with Router Advertisements and mDNS. Prior to this commit, the first HA host interface with an associated default route was always used as the OTBR "infrastructure" interface. This caused problems if the HA host has multiple interfaces and the Android device needed for commissioning is on a different network than the automatically-selected interface. To address that, this commit permits the user to configure the "infrastructure" interface.
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
PaulSD
changed the title
home-assistant
bot
added
cla-recheck
cla-signed
and removed
cla-recheck
cla-needed
labels
There was a problem hiding this comment.
In general, I am a bit hesitant about this PR. I see that this is useful for multi-homed Home Assistant installation, but if we go down this road, we'll have to add a similar option to more places (e.g. the OpenThread Border Router add-on, the Matter Server add-on...).
A feature I have in the back of my mind to add macvlan support for add-ons in general. Currently we are on the host network, which is not very nice anyways. A macvlan would have separate, but L2 access to a particular network interface.
Assuming we can configure a mavclan somewhere in HA, and then assign this to one or multiple add-ons would solve this much more elegant.
| 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' |
There was a problem hiding this comment.
Is there a particular reason to use this style?
There was a problem hiding this comment.
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.
| bashio::log.warning 'No primary network interface found! Using static eth0 for otbr_infra_if.' | ||
| otbr_infra_if='eth0' |
There was a problem hiding this comment.
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.
| 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!' |
There was a problem hiding this comment.
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.
| 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. |
There was a problem hiding this comment.
We use what is considered primary by NetworkManager. Is the first interface with an associated default what NetworkManager considers primary? 🤔
There was a problem hiding this comment.
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).
I agree that would be a cleaner solution, but it would require adding settings and UI to HA Core to facilitate creating macvlan networks and attaching them to host interfaces and add-ons ... And being new to HA and unfamiliar with all the use cases and internals, that is not something I'm comfortable tackling myself (at least not without some significant guidance/direction). Creating macvlan networks could go on the System -> Network page ... But where would the associations to add-ons be configured? (Maybe in each add-on's Info or Configuration tab?) ... And how would we determine/configure which add-ons should use this vs which add-ons should use the host network (I imagine some add-ons will need to be able to access multiple physical networks, so macvlan wouldn't work for those) - Should the add-on specify whether host network or macvlan should be used, or should that be configurable by the user for all add-ons, or ...?
Yeah, I see you just added similar code to the Matter Server add-on here, which should probably have similar configuration: https://github.com/home-assistant/addons/pull/3414/files |
|
Replying to @agners
I'm not sure that using macvlan is the correct solution here, since it's not that a multi-homed HA needs multiple MACs on each interface, but rather that add-ons should not make assumptions about what interfaces to use where it's necessary or desirable to pick one. Definitely some add-ons might benefit from their own L2 MAC and associated IP, but I don't think that's the main use case, and it potentially requires assigning many more external IPs to HA than necessary. I can see the benefits to assigning add-ons distinct internal IPs to permit inter-add-on communication, but not so much for external use. I think something inspired by swconfig or DSA to assign interfaces to add-ons makes more sense than just being able to give add-ons their own macvlan(s). (It would still be possible to create an external macvlan interface and assign it to a single add-on.) As an example use case, consider a dnsmasq add-on that serves DHCP clients on some interfaces, and uses a separate upstream interface for DNS. This could also solve the issue of Matter knowing which interface to used when passed a link-local IPv6 address; it could also be leveraged to reduce broadcast traffic to interfaces that do not need it (e.g.: mDNS requests for IoT devices being sent on a non-IoT VLAN). |
|
Sorry for the delay. I wanted to expand on this a bit more, so I made an architecture discussion at home-assistant/architecture#1034. Let's continue the more general discussion over there. As for the add-on specific/this PR at hand:
TBH, I am not quite happy with the interface selection we had to add to the Matter Python server. It makes assumption on which network the commissioning phone and the Matter server runs on. Ideally the Matter add-on should just listen on any interface where the user runs Matter devices on. But anyhow, that is a different story.
Since the OTBR indeed only supports a single infrastructure link we would even need such a selection when we support multiple Since this will be an optional option, I am not against adding it. On a bit a different note: For many people trying to actively use the Thread side of the Silicon Labs Multi-protocol add-on lead to issues quite quickly. Did you observe this too? Maybe you are aware, but we recently shifted our focus more on the pure Thread variant (see also the SkyConnect section of the Matter blog post). Or in different words, I rather prefer to see this added to the OpenThread Border Router add-on 😇 |
Ok, I can add this in both the Multiprotocol and OTBR add-ons. I'll work on that.
Depending on what you mean by "quite quickly" ... Yes, I have been observing some issues every few days:
I will try switching to the pure Thread variant and see if things improve for me. BTW, I've been keeping this comment updated with a list of issues I encountered (including the above) and how I dealt with them. Is there a better place that I should be posting this information? |
Probably depends on the amount of Thread traffic, but from what I've seen within a day.
Thanks for putting together the list! Technically, I guess each problem should have it's own issue. But before opening all separately, check if issues for the same problem already exist. I am quite sure that some of the Multiprotocol issues are already tracked in other issues. Some issues I think we have fixed/worked around (e.g. for SkyConnect we have a firmware which comes with a watchdog to workaround the firmware crash). The problem really is, there are a bit too many unreliable layers here, which makes it hard to reliably know what is happening and what's at fault, especially on the higher levels. Also we really shifted our focus on the pure Thread variant. So ideally retest your setup with the pure Thread variant, and then create separate issues for what is left. Issues which are related to the OTBR belong in this issue tracker, Matter issues we collect in the Core issue tracker. |
The OpenThread Border Router agent currently only supports routing between a single "infrastructure" network interface and a single Thread network interface per agent process. (Multiple "infrastructure" interfaces can be specified, but this is for fail-over purposes and only one will be used at a time.)
Thread device commissioning in HA currently requires an Android device which can communicate both with the Thread device via Bluetooth and with the OpenThread Border Router "infrastructure" interface via IPv6 with Router Advertisements and mDNS.
Prior to this commit, the first HA host interface with an associated default route was always used as the OTBR "infrastructure" interface. This caused problems if the HA host has multiple interfaces and the Android device needed for commissioning is on a different network than the automatically-selected interface.
To address that, this commit permits the user to configure the "infrastructure" interface.
(Side note: Historically, the "infrastructure" interface was sometimes conflated with the "backbone" interface, but those have now been unified under the term "infrastructure": openthread/openthread#9638 )