eclipse-archived / smarthome

Eclipse SmartHome™ project
https://www.eclipse.org/smarthome/
Eclipse Public License 2.0
865 stars 782 forks source link

mDNS binds to IPv6 only #577

Closed kaikreuzer closed 8 years ago

kaikreuzer commented 8 years ago

migrated from Bugzilla #423513 status NEW severity enhancement in component Core for --- Reported in version unspecified on platform PC Assigned to: Project Inbox

On 2013-12-07 16:11:55 -0500, Kai Kreuzer wrote:

Migrated from https://code.google.com/p/openhab/issues/detail?id=288

On 2015-07-02 13:10:50 -0400, Jayaraj Esvar wrote:

Hi, In module org.eclipse.smarthome.io.transport.mdns under class MDNSClientImpl activate() Please JmDNS.create(InetAddress addr, String name) instead of JmDNS.create().

Thanks & regards, Jayaraj Esvar

On 2015-07-06 05:48:37 -0400, Kai Kreuzer wrote:

Thanks Jayaraj,

What do you suggest to fill in as addr and name parameters?

On 2015-07-07 01:26:12 -0400, Jayaraj Esvar wrote:

Hi, Small logic needs to be added to find on which Ip version the controller is attached to and add that instance of Inetaddress.

InetAddress addr = getLocalIpv4/6Address(); <-- decide based on the current attachment, ignore if it is pointing to localhost. String hostname = InetAddress.getByName(addr.getHostName()).toString();

On 2015-07-08 04:40:32 -0400, Kai Kreuzer wrote:

Would you like to suggest a PR for these changes?

On 2015-07-08 15:37:50 -0400, Kai Kreuzer wrote:

* Bug 471727 has been marked as a duplicate of this bug. *

maggu2810 commented 8 years ago

Just to add the note that I see this one on some of my systems, too.

maggu2810 commented 8 years ago

(@lolodomo) Have you ever tried to use "-Djava.net.preferIPv4Stack=true" as a work around until a PR is created?

maggu2810 commented 8 years ago

@kaikreuzer We could create one MDNSClient for every IP of ever interface (so all IPv4 and IPv6 addresses) except loopback.

    private static Set<InetAddress> getAllInetAddresses(final boolean skipLoopback) {
        final Set<InetAddress> addresses = new HashSet<>();
        Enumeration<NetworkInterface> itIfaces;
        try {
            itIfaces = NetworkInterface.getNetworkInterfaces();
        } catch (final SocketException ex) {
            return addresses;
        }
        while (itIfaces.hasMoreElements()) {
            final NetworkInterface iface = itIfaces.nextElement();
            try {
                if (skipLoopback && iface.isLoopback()) {
                    continue;
                }
            } catch (final SocketException ex) {
                continue;
            }
            final Enumeration<InetAddress> itAddresses = iface.getInetAddresses();
            while (itAddresses.hasMoreElements()) {
                final InetAddress address = itAddresses.nextElement();
                if (skipLoopback && address.isLoopbackAddress()) {
                    continue;
                }
                addresses.add(address);
            }
        }
        return addresses;
    }

Then we have to change the MDNSDiscoveryService implementation to handle 1..n MDNSClient references instead of 1..1

But also this could lead to problems if interfaces are attached later, network interfaces changes etc.

Perhaps the one that calls getAllInetAddresses and manages the MDNSService instances could run in a background thread that checks the "all intet addresses" periodically and add / remove the serivces.

I am not aware of any Java interface that informs about changed local IP addresses.

lolodomo commented 8 years ago

@maggu2810 : I tried adding "-Djava.net.preferIPv4Stack=true" (hopping I did it at the right place) and it does not help, neither in Eclipse IDE on my Windows 10 PC nor on my RPI 2.

lolodomo commented 8 years ago

Any progress ?

maggu2810 commented 8 years ago

I think we need a volunteer that implements a solution and assume that no one is currently working on that.

lolodomo commented 8 years ago

Ok, I studied the source code and that is clear for me what needs to be changed. I would suggest a slightly different approach for the fix that would require no change of cardinality in OSGI service declaration. My proposal is to change only the interface MDNSClient replacing

public JmDNS getClient();

by

public Set<JmDNS> getClientNetInterfaces();

In class MDNSClientImpl, I will create a set of JmDNS instances using create(InetAddress addr, String name), as many as identified by your function getAllInetAddresses. Finally, I will have to change MDNSDiscoveryService and MDNSServiceImpl.to use getClientNetInterfaces instead of getClient.

@maggu2810 :Is it ok for you ?

Your solution looks good too but to be honest with you, I have no idea what to delcare in OSGI-INF/mdnsclient.xml to have the service automatically started several times, for each network interface.

maggu2810 commented 8 years ago

If I understand what you want to do, I think it does not solve the problem at all.

If you start the runtime with no assigned IP address to your network interfaces no registration will be done. If your interfaces reveices an IP address later, no registration is done. Also if the IP address of one network interface change, no new registration is done.

I have not such a deepen look at the code, so I could be wrong with my assumption. But if it is correct, then I don't know if the situation is really better ...

lolodomo commented 8 years ago

Another option would be to have two mdnsclient services, one for IP v4 and one for IP v6. So in this case, we will have to handle a cardinality 1..n. MDNSClientImpl would be replaced by MDNSIPV4ClientImpl and MDNSIPV6ClientImpl mdnsclient.xml would be replaced by mdnsipv4client.xml and mdnsipv6client.xml. If there is no IP v6 interface for example, MDNSIPV6ClientImpl.getClient will return null. @maggu2810 : is it a better implementation ?

lolodomo commented 8 years ago

@maggu2810 : what you are refering to is another case where OH could be started before network interfaces are setup. Can this really happen in real life ?! What I try to solve now is not this problem. Even when all network interfaces are setup on my server, openHAB creates just one unique instance of jmDns without specifying any network interface. As a result jmDns chooses automatically one default network interface. The fix I propose would allow to have one jmDns instance for each network interface (one for IP c4 and one for IP v6).

As you previously explained, to manage the dynamic attachment of network interfaces, something else has to be added like a specific thread to look for network interfaces. In my opinion, in practice, we don't need about that.

lolodomo commented 8 years ago

By the way, if we choose my first implementation, that is one unique service MDNSClient managing a set of jmDNS instances, what you request could be easily implemented by just adding in this class a cyclic thread running a check of network interfaces and updating accordingly the set of jmDNS instances. So I will go in that direction if that is OK for everybody. That is even not too much difficult to implement I think.

maggu2810 commented 8 years ago

@maggu2810 : what you are refering to is another case where OH could be started before network interfaces are setup. Can this really happen in real life ?!

Sure, if I power off my router and my embedded devices, the embedded devices running an ESH based solution are up before the router assignes an IP address. The IP address is assigned if the router is up and it can be after the ESH solution is started. Or think about a wireless connection that is little unstable. Put your machine to another network (you change the IP range from e.g. 192.168.1.x to 192.168.2.x). All that involves that the runtime has to be restarted.

lolodomo commented 8 years ago

Considering @maggu2810 's acceptable request to have something dynamic that evolves with network interfaces appearing or disappearing with the time, I think the clean solution is to have as many MDNSClient services as there are network interfaces ON at a certain time. That would require to have services with a dynamic reference to 0..n MDNSClient services and so the fix requires someone with a good knowledge in OSGi components, services, reference between services and probably service factory, I have clearly not the level of knowledge required to implement this change. So in this case, you will have to find someone else to implement this fix.

Now if you accept a more basic and intermediary change that will only consider all the network interfaces present when OH is started, I could implement one fix.

@kaikreuzer and @maggu2810 : let me know your decision.

maggu2810 commented 8 years ago

I googled a little bit and found the NetworkElement class by Cisco that provides e.g.:

Does someone know a Java library that provides such a network interface "inspector"?

kaikreuzer commented 8 years ago

All that involves that the runtime has to be restarted.

And it will imho stay this way. Afaik, Java has a big problem if you change the local ip address while it is running. Even if we find some libs that can handle that and notify you about it, I am pretty sure that Jetty, upnp, the ssh console, the remote debug port and everything else that uses local TCP sockets will have a problem.

So for this issue here, I would go with @lolodomo's suggestion to only regard the interfaces that are already available at startup (but all of them and not just the first).

maggu2810 commented 8 years ago

Even if we find some libs that can handle that and notify you about it, I am pretty sure that Jetty, upnp, the ssh console, the remote debug port and everything else that uses local TCP sockets will have a problem.

I don't see the parallel here. Jetty, ssh console and the remote debug port are listen (TCP) sockets. They listen on a specific IP range. If I am not wrong the userspace don't care about the interface at all, that is managed by the kernel. So if my Jetty listens on 0.0.0.0 interface can go up and down and change the addresses. It does not care.

Publishing / advertising is another point, this should be done for every IP address of every interface that is up.

But I could be wrong.

kaikreuzer commented 8 years ago

So if my Jetty listens on 0.0.0.0 interface can go up and down and change the addresses. It does not care.

Ok, if this is the case, forget my remark. Nonetheless, my expectation is that not much will work if I change my ip while keeping openHAB running. But that would need to be tested. Anyhow, this is not directly related to this issue, is it?

maggu2810 commented 8 years ago

Anyhow, this is not directly related to this issue, is it?

It has been your argument that we have the problem also for other stuff e.g. Jetty. Not mine :wink:

kaikreuzer commented 8 years ago

But I understood you were asking to solve this issue by dynamically reacting on interface changes. And I now only meant that I would be fine with solving it for the (static) interfaces, which are there at startup.

maggu2810 commented 8 years ago

I am happy if @lolodomo finds the time to solve that for the interfaces that are up on startup. But I don't think that it is the final solution and we should keep an issue open that is needs some time to be resolved (for dynamically changes), too. For openHAB it could be fine to state that it does not handle such changes, but for the ESH framework I assume it should be solved some time.

kaikreuzer commented 8 years ago

Yep, fine with that as a second step.

lolodomo commented 8 years ago

OK that is now (almost) working with my patched code. One question for inet v6 expert: I discovered that I have in fact 2 inet v6 IP for the same interface, one is tagged scope global and the other scope link. Any clear explanation about what it is ? Should I filter the scope link IP ? Or consider only scope global ? I discovered that in the freebox binding, I badly defined the service type. "_fbx-api._tcp" should be replaced by "_fbx-api._tcp.local.". I will provide a fix for that too. But I can confirm that only a JmDNS instance created with one IP v4 can discover this service.

itn3rd77 commented 8 years ago

Just looking through the old issues in hope to get the number down. Is this fixed with #1742 and we can close the issue?

kaikreuzer commented 8 years ago

Yes, I think so - thanks for noticing!