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

Linux impl of mDNS is not setting interfaceId on browse/resolve callback #8232

Closed
samadDotDev opened this issue Jul 8, 2021 · 3 comments · Fixed by #9484
Closed

Linux impl of mDNS is not setting interfaceId on browse/resolve callback #8232

samadDotDev opened this issue Jul 8, 2021 · 3 comments · Fixed by #9484

Comments

@samadDotDev
Copy link
Contributor

Problem

Linux platform implementation of chip::Mdns is not setting interfaceId for browse/resolve callbacks once retrieved from avahi service browse/resolve.

Proposed Solution

MdnsAvahi::HandleBrowse and MdnsAvahi::HandleResolve need to set interfaceId in MdnsService structure before returning the services to browse callback.

diff --git a/src/platform/Linux/MdnsImpl.cpp b/src/platform/Linux/MdnsImpl.cpp
index 205d840c83..be6b9df526 100644
--- a/src/platform/Linux/MdnsImpl.cpp
+++ b/src/platform/Linux/MdnsImpl.cpp
@@ -591,6 +591,11 @@ void MdnsAvahi::HandleBrowse(AvahiServiceBrowser * browser, AvahiIfIndex interfa
             CopyTypeWithoutProtocol(service.mType, type);
             service.mProtocol               = GetProtocolInType(type);
             service.mAddressType            = ToAddressType(protocol);
+            service.mInterface              = INET_NULL_INTERFACEID;
+            if (interface != AVAHI_IF_UNSPEC)
+            {
+                service.mInterface = static_cast<chip::Inet::InterfaceId>(interface);
+            }
             service.mType[kMdnsTypeMaxSize] = 0;
             context->mServices.push_back(service);
         }
@@ -670,6 +675,11 @@ void MdnsAvahi::HandleResolve(AvahiServiceResolver * resolver, AvahiIfIndex inte
         result.mProtocol    = GetProtocolInType(type);
         result.mPort        = port;
         result.mAddressType = ToAddressType(protocol);
+        result.mInterface   = INET_NULL_INTERFACEID;
+        if (interface != AVAHI_IF_UNSPEC)
+        {
+            result.mInterface = static_cast<chip::Inet::InterfaceId>(interface);
+        }
         Platform::CopyString(result.mHostName, host_name);
         // Returned value is full QName, want only host part.
         char * dot = strchr(result.mHostName, '.');

@bzbarsky-apple
Copy link
Contributor

Does this end up breaking use of devices that have an ipv6 link local address with a Linux controller?

@cecille @andy31415

@samadDotDev
Copy link
Contributor Author

Yes it does, the interfaceId is required for addressing link-local ipv6 addresses and callbacks don't return it for such addresses.

@cecille
Copy link
Contributor

cecille commented Aug 25, 2021

The issue is correct and this can break IPv6 in practice. Depending on the network stack, if you use a LL address and there's only one interface (the case for most of the devices implemented here if I'm not mistaken), it will probably work even without the scope set. But yeah, the interface should be set. If you throw that patch up as a PR, I'm sure it'll get approved.

bzbarsky-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 4, 2021
Without this resolving something to a link-local IP won't work right.

Fixes project-chip#8232
bzbarsky-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 4, 2021
Without this resolving something to a link-local IP won't work right.

Fixes project-chip#8232
bzbarsky-apple pushed a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 4, 2021
Without this resolving something to a link-local IP won't work right.

Fixes project-chip#8232
woody-apple pushed a commit that referenced this issue Sep 7, 2021
Without this resolving something to a link-local IP won't work right.

Fixes #8232

Co-authored-by: Abdul Samad <[email protected]>
kpschoedel pushed a commit to kpschoedel/connectedhomeip that referenced this issue Sep 9, 2021
…t-chip#9484)

Without this resolving something to a link-local IP won't work right.

Fixes project-chip#8232

Co-authored-by: Abdul Samad <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants