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.

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

[RFC] Cleanup network configuration #4047

Open
mherwege opened this issue Jan 17, 2024 · 2 comments
Open

[RFC] Cleanup network configuration #4047

mherwege opened this issue Jan 17, 2024 · 2 comments
Labels
enhancement An enhancement or new feature of the Core

Comments

@mherwege
Copy link
Contributor

mherwege commented Jan 17, 2024

Several add-onsand services use different ways to limit the part of the network being scanned for discovery. This concerns binding discovery, but also add-on discovery and more general services like mdns and upnp.

Ideally, we should have one set of parameters that controls this OH wide, and not allow each add-on or service to make their own decisions on it. I want to avoid a user having to configure this in every add-on or service in a slightly different way. By default, we should be scanning the main network of OH (however it is defined) to avoid scanning too broadly. Advanced configurations would allow scanning broader.

We have had a set of settings for the network in OH already for a while. Specific parts of OH can use these settings to limit their network interactions to specific parts of the network.
The parameters available are network.xml:

  • primaryAddress: A subnet (e.g. 192.168.1.0/24).
  • broadcastAddress: A broadcast address (e.g. 192.168.1.255).
  • useOnlyOneAddress: Use only one IP address per interface and family.
  • useIPv6: Use IPv6 Addresses if available.

These settings are used in various parts of OH. I could identify following usage:

With #3981, another configuration option that could be used in bindings or services was introduced, the network-interface context. openhab/openhab-addons#16145 adds this to the network binding.
The aim of this enhancement is to again be able to limit network address iterations, multicasts or broadcasts. I believe there is some overlap with the broadcastAddress parameter in the core network settings. Additionally, having to configure this for each service may confuse users, as it is not clear what parameter takes precedence and has effect where.

My suggestion is to move this interface selection to the network settings, and any binding or service that wants to make use of it should use the system wide setting, instead of the context. As a second step, we should revisit all network scans/broadcasts in the code to then make use of that system wide parameter.

In detail:

  • make the primaryAddress parameter the main identifier to find out what interface and/or broadcast address to use. It probably makes sense to have the automatically detected one confirmed/changed in the setup wizard.
  • remove the broadcastAddress parameter.
  • add a networkInterface parameter (that can take multiple values). Default to the network interface of the primaryAddress.
  • NetworkAddressService.getConfiguredBroadcast can return the broadcast address of the interface with the primary address to avoid having to change binding code.
  • replace usage of the network-service context and get the values from the network parameter.

I am not clear on the onlyUseOneAddress yet, although the purpose is similar, limiting the range used.

I would like to avoid focussing on the interface names for user facing configuration. This is a more advanced concept than IP addresses. Users would know what IP range they want to work in, not necessarily the name of the network interface. In the UI, this can be presented combined to make that clear.

I am OK to work on this, but would like to get your opinions on this. I have the feeling we are layering too many configuration options that do more or less the same thing. I would like to keep them central and use them everywhere.

@wborn @J-N-K What do you think?
It may be obvious from this RFC that I am not a big fan of #3981 in its current form. And I am sorry to be late with my feedback here. It would force a user to go and change configurations everywhere (right now it is only used in one place). While it gives a lot of configuration flexibility, I am not convinced it is the right thing to do. It increases configuration complexity. As we have system wide network settings, normal user expectation would be to control it all there.

EDIT: Added some more text on the reason to propose changing this.

@mherwege mherwege added the enhancement An enhancement or new feature of the Core label Jan 17, 2024
@mherwege mherwege changed the title Request for Feedback: Cleanup network configuration [RFC] Cleanup network configuration Jan 18, 2024
@wborn
Copy link
Member

wborn commented Feb 2, 2024

Yes the current network configuration is a bit of a mess and not consistently used probably also because of its limitations.
It is probably best to be able to select the default network interfaces and have the possibility to override the default values when necessary at add-on or Thing level.

I am not clear on the onlyUseOneAddress yet, although the purpose is similar, limiting the range used.

The reason for this option is explained in eclipse-archived/smarthome#6005

@splatch
Copy link
Contributor

splatch commented Mar 6, 2024

You can not run away from making "network" an API citizen, especially if you ever consider making better support for containers which multiply network interfaces even in most basic setups.

I've published some network api concepts 9 months ago; which actually stayed in my drawer for couple months. You can see org.connectorio.addons.network, org.connectorio.addons.network.ip and org.connectorio.addons.network.jvm.internal, primarily to see if it can be used to centralized management of network access from bindings. Usage of network api can be observed in AmsAdsInterfaceDiscoveryService.

Diving more (actually speculating), thanks to osgi ServiceFactory concept you can provide a multiplexed NetworkRegistry instance which will filter its entries based on bundles which make use of it. With this engineering trick your binding might still rely on enumerated networks, while control over access to them them is shifted somewhere else. This is essential to provide a unified way to make network access management possible, otherwise it will spread across many edges of bridges/things, probably beyond user oversight. Maybe OH is not a network router, but it does act as a hub, and depending on setup, framework role might be to interconnect incompatible networks.
I've purposely not made IpNetwork a concrete type because it can be supplied through JVM (read only). Under Linux there are ways to manage network level settings from userspace over dbus (using systemd-network or NetworkManager namespaces).
I also made one step further by not assuming IP to be only one kind of network, because i.e. RS485 or CAN interface can be used to launch network as well (hence there is NetworkInterfaceType). If you think of it even more, ZWave and many other radio standards, rely on network node concepts which are flattened to thing instances. In case of matter you might even have an overlay network which span from zigbee radio to ip layer.

I know all above is over-engineered, especially for my own needs, but its not about me, but platform view. ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core
Projects
None yet
Development

No branches or pull requests

3 participants