Skip to content

Fix network interface configuration option enumeration - #5006

Merged
holgerfriedrich merged 2 commits into
openhab:mainfrom
Nadahar:fix-nic-config-options
Sep 7, 2025
Merged

Fix network interface configuration option enumeration#5006
holgerfriedrich merged 2 commits into
openhab:mainfrom
Nadahar:fix-nic-config-options

Conversation

@Nadahar

@Nadahar Nadahar commented Sep 4, 2025

Copy link
Copy Markdown
Contributor

After a lot of troubleshooting of the network binding, I've found that one of the reasons for its extreme slowness is right here in core, and stems from #3981.

This relates to openhab/openhab-addons#17956 (comment)

In short, when network-interface context was added, it was implemented by enumerating all the network interfaces on the host. As a part of trying to find a suitable label for a network interface label, InetAddress.getHostName() was used. This method tries to reverse lookup a hostname from an IP address, which can mean all kind of network activity. This is done for each IP address that is attached to a network interface, which makes it quite slow. On my computer, doing this enumeration once takes 4.27 seconds, but the actual time it takes will depend on many things, like the number of network interfaces and the local network, what name resolution service is configured etc.

The thing is that in the vast majority of cases, a hostname isn't resolved - and it isn't necessary either in my view, since these IPs all belong to the host running OH.

I've removed the use of InetAddress.getHostName(), and instead use the JVM provided "interface name", convert _ into space and then converts it to Title Case, to try to make it more similar to the label convention.

In addition, there's a quirk between the JVM and Windows. Windows keeps all network interfaces, physical or virtual, that has ever been on the computer in the list of network interfaces, but those that are no longer in use are "hidden". The JVM however, doesn't respect/understand this, so it will return all of them. This means that on Windows computers, you will usually have a huge list of network interfaces returned by the JVM. To try to mitigate this a bit, I've added a check that requires the network interface to be "up" to be included in the configuration options.

@Nadahar
Nadahar requested a review from a team as a code owner September 4, 2025 04:11
@Nadahar

Nadahar commented Sep 4, 2025

Copy link
Copy Markdown
Contributor Author

If accepted, this should probably be backported included to 4.x, as the impact is quite severe.

@Nadahar

Nadahar commented Sep 4, 2025

Copy link
Copy Markdown
Contributor Author

I just concluded that the IP address of the interface should probably be added to the name in parentheses to make it easier to understand which is which, so I'm converting this PR to a draft until I've done that.

@Nadahar
Nadahar marked this pull request as draft September 4, 2025 04:17
@Nadahar

Nadahar commented Sep 4, 2025

Copy link
Copy Markdown
Contributor Author

I've now added the IP addresses in parentheses, network interface selection now looks like this:

bilde

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
@Nadahar
Nadahar force-pushed the fix-nic-config-options branch from a165520 to 76eeed3 Compare September 4, 2025 13:57
@Nadahar
Nadahar marked this pull request as ready for review September 4, 2025 13:58
@Nadahar

Nadahar commented Sep 4, 2025

Copy link
Copy Markdown
Contributor Author

@openhab-bot

Copy link
Copy Markdown
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/network-binding-things-are-slow-to-start-and-slow-to-load-in-the-settings-ui/157001/27

@openhab-bot

Copy link
Copy Markdown
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/network-binding-slows-down-openhab-loading-very-much-on-boot/165937/25

@openhab-bot

Copy link
Copy Markdown
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/critical-slow-15minutes-thing-init-after-updating-from-4-1-0-to-4-1-1/152906/6

@holgerfriedrich holgerfriedrich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Avoiding getHostName is likely a good approach, as it might do a reverse lookup.
https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/net/InetSocketAddress.html#getHostName()
There have been posts from back long ago that this may take some time to complete.

Skipping the inactive interfaces (specifically targeting the behaviour of Windows) seems useful as well.

I am a bit concerned that this has some side effects on platforms we did not test yet:

  1. docker setups (we always had a few users reporting problems during startup, maybe due to the add-on finders, maybe due to the large number of interfaces....)
  2. getName might behave differently on the Mac platform (though, I would expect Unix style interface names)
  3. Possible dependence on JDK

AI claims that 2) and 3) should not be an issue.

WDYT about point 1? Do you have experience with Docker?

@holgerfriedrich holgerfriedrich added the enhancement An enhancement or new feature of the Core label Sep 7, 2025
@lsiepel

lsiepel commented Sep 7, 2025

Copy link
Copy Markdown
Contributor
  • ocker setups (we always had a few users reporting problems during startup, maybe due to the add-on finders, maybe due to the large number of interfaces....)

I’m not sure what doubts you have about Docker, but most of the reported problems actually come from Docker-based setups. That’s precisely why this PR—together with the network binding PR in addons—is so important: it directly addresses those issues.

@holgerfriedrich

Copy link
Copy Markdown
Member

Ok, then let's go for it. We are in an early phase of 5.1 anyway....
Just waiting for the minor code change requested above.

@lsiepel

lsiepel commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

Once it works as expected without side efffects in 5.1.x, i opt to backport it to 5.0.x and also 4.3.x as this is a long standing bug that especially for weaker systems (users stuck in 4.3.x) has effect.

@Nadahar

Nadahar commented Sep 7, 2025

Copy link
Copy Markdown
Contributor Author

I am a bit concerned that this has some side effects on platforms we did not test yet:

1. docker setups (we always had a few users reporting problems during startup, maybe due to the add-on finders, maybe due to the large number of interfaces....)

2. [getName](https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/net/NetworkInterface.html#getName()) might behave differently on the Mac platform (though, I would expect Unix style interface names)

3. Possible dependence on JDK

AI claims that 2) and 3) should not be an issue.

WDYT about point 1? Do you have experience with Docker?

  1. I have never run OH inside Docker, but I would expect this problem to "bite" Docker installations quite hard because of the "intricate network setup" (extra/virtual network interfaces and "interim" IP addresses). I don't know how involved it is to build the OH docker image, if it's not too much hassle, I could test it on a VM (given some directions for where to find the docker build procedure).

  2. I have used getName() on all platforms in the past, I can't remember any issues. That said, getName() is already in use today, that's not new for this PR. I think the interface names are "Unix style" across the board - it even assigns "Unix style" names on Windows, something that isn't found in the OS anywhere, so I'm pretty sure the JVM generated these itself (on Windows). What would be much more descriptive/useful for the users on Windows at least, is getDisplayName(). This matches what the network interfaces are called on Windows, but these names are generally very long and won't fit into OHs "label restrictions". I can build and test this PR on macOS and Linux VMs if you'd like some reassurance that it will play nicely.

  3. I don't think so. As far as I know, this behavior has been unchanged at least since Java 6, probably longer.

@holgerfriedrich

Copy link
Copy Markdown
Member

It's fine, we can merge as it is. We can test the docker snapshots built by the CI after that.

Can you just apply the code suggestion before merge?

…/core/config/core/internal/net/NetworkConfigOptionProvider.java

Co-authored-by: Holger Friedrich <mail@holger-friedrich.de>
Signed-off-by: Nadahar <Nadahar@users.noreply.github.com>
@Nadahar

Nadahar commented Sep 7, 2025

Copy link
Copy Markdown
Contributor Author

It's fine, we can merge as it is. We can test the docker snapshots built by the CI after that.

Can you just apply the code suggestion before merge?

I did, but my signature is "wrong" on this commit because my GitHub setup doesn't match my "OH setup".

@holgerfriedrich holgerfriedrich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@holgerfriedrich
holgerfriedrich merged commit adfa286 into openhab:main Sep 7, 2025
4 checks passed
@holgerfriedrich holgerfriedrich added this to the 5.1 milestone Sep 7, 2025
@lsiepel

lsiepel commented Oct 17, 2025

Copy link
Copy Markdown
Contributor

@holgerfriedrich can you consider this for a backport to 5.0.3 ?
I forgot to ask you this yesterday, so this is not in 5.0.2 :-/

@yndtrud

yndtrud commented Dec 18, 2025

Copy link
Copy Markdown

@holgerfriedrich can you consider this for a backport to 5.0.3 ? I forgot to ask you this yesterday, so this is not in 5.0.2 :-/

Could you tell me? Have any changes been made in 5.0.3? It's too early to upgrade to 5.1.0. I have 5.0.3 installed and nothing seems to have changed.

@lsiepel

lsiepel commented Dec 18, 2025

Copy link
Copy Markdown
Contributor

This was not backported. 5.1.0 will be released very shortly, so i don;t think it will be backported anymore.

@yndtrud

yndtrud commented Dec 18, 2025

Copy link
Copy Markdown

This was not backported. 5.1.0 will be released very shortly, so i don;t think it will be backported anymore.

I'll wait! Thank you!

@Nadahar

Nadahar commented Dec 18, 2025

Copy link
Copy Markdown
Contributor Author

This was not backported. 5.1.0 will be released very shortly, so i don;t think it will be backported anymore.

I still think it should be backported to 4.x, as far as I know, there has been no reports of any bad consequences of this PR.

@lsiepel

lsiepel commented Dec 18, 2025

Copy link
Copy Markdown
Contributor

That is for @openhab/core-maintainers to decide

@yndtrud

yndtrud commented Dec 19, 2025

Copy link
Copy Markdown

This was not backported. 5.1.0 will be released very shortly, so i don;t think it will be backported anymore.

I still think it should be backported to 4.x, as far as I know, there has been no reports of any bad consequences of this PR.

Yes, I upgraded to 5 because everything that was preventing me from upgrading from 4 to 5 was fixed within four months. 5.0.3 is more resource-intensive than 4, but everything seems fine. I'm waiting for 5.1.1 to fix the 5.1.0 release (which we're also waiting for) so the network issues can be resolved. Thanks anyway!

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

Successfully merging this pull request may close these issues.

5 participants