[vlc-devel] [PATCH 6/7] mdns: Add basic ip address detection (Linux)

[Roland Bewick]

On 27/07/2019 2:04 PM, Roland Bewick wrote:
>
> On 26/07/2019 1:07 AM, Rémi Denis-Courmont wrote:
>> Le torstaina 25. heinäkuuta 2019, 20.51.09 EEST Roland Bewick a écrit :
>>> On Fri, 26 Jul 2019 at 12:15 AM, Rémi Denis-Courmont <remi at remlab.net>
>>>
>>> wrote:
>>>> Le torstaina 25. heinäkuuta 2019, 8.39.11 EEST Roland Bewick a écrit :
>>>>> On 24/07/2019 11:18 PM, Rémi Denis-Courmont wrote:
>>>>>> Le keskiviikkona 24. heinäkuuta 2019, 19.10.50 EEST Roland Bewick a
>>>> écrit
>>>>
>>>>>>> Hi Rémi,
>>>>>>>
>>>>>>> Thanks for the reply.
>>>>>>>
>>>>>>> On 24/07/2019 10:59 PM, Rémi Denis-Courmont wrote:
>>>>>>>> Le keskiviikkona 24. heinäkuuta 2019, 18.43.49 EEST Roland 
>>>>>>>> Bewick a
>>>>>>>> écrit
>>>>>>>>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>     modules/services_discovery/microdns.c | 62
>>>>>>>>>
>>>>>>>>> +++++++++++++++++++++++++++++++++-- 1 file changed, 59
>>>> insertions(+), 3
>>>>
>>>>>>>>> deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/modules/services_discovery/microdns.c
>>>>>>>>> b/modules/services_discovery/microdns.c index
>>>>>>>>> c460126b58..fa34ffa8d8
>>>>>>>>> 100644
>>>>>>>>> --- a/modules/services_discovery/microdns.c
>>>>>>>>> +++ b/modules/services_discovery/microdns.c
>>>>>>>>> @@ -28,6 +28,12 @@
>>>>>>>>>
>>>>>>>>>     #include <stdatomic.h>
>>>>>>>>>     #include <assert.h>
>>>>>>>>>
>>>>>>>>> +#if defined( _WIN32 )
>>>>>>>>> +
>>>>>>>>> +#else
>>>>>>>>> +    #include <ifaddrs.h>
>>>>>>>>> +#endif
>>>>>>>>> +
>>>>>>>>>
>>>>>>>>>     #include <vlc_common.h>
>>>>>>>>>     #include <vlc_plugin.h>
>>>>>>>>>     #include <vlc_modules.h>
>>>>>>>>>
>>>>>>>>> @@ -868,9 +874,59 @@ CloseRD( vlc_object_t *p_this )
>>>>>>>>>
>>>>>>>>>         CleanDiscoveryCommon( p_sys );
>>>>>>>>>         }
>>>>>>>>>
>>>>>>>>> -static char * detect_ip_address()
>>>>>>>>> +static char * detect_ip_address( vlc_object_t *p_obj )
>>>>>>>>>
>>>>>>>>>     {
>>>>>>>>>
>>>>>>>>> -    return strdup("127.0.0.1"); /* TODO: actually detect ip
>>>> address */
>>>>
>>>>>>>>> +    char result[16] = "127.0.0.1";
>>>>>>>>> +    bool found = false;
>>>>>>>>> +#if defined( _WIN32 )
>>>>>>>>> +
>>>>>>>>> +#else
>>>>>>>>> +    struct ifaddrs *id;
>>>>>>>>> +    if( getifaddrs( &id ) == 0 )
>>>>>>>>> +    {
>>>>>>>>> +        int selected_priority = 0;
>>>>>>>>> +
>>>>>>>>> +        for( ; id != NULL; id = id->ifa_next )
>>>>>>>>> +        {
>>>>>>>>> +            if( id->ifa_addr->sa_family == AF_INET )
>>>>>>>>> +            {
>>>>>>>>> +                struct sockaddr_in *addr_in = (struct 
>>>>>>>>> sockaddr_in
>>>>>>>>> *)id->ifa_addr; +
>>>>>>>>> +                char *ip_address = inet_ntoa( 
>>>>>>>>> addr_in->sin_addr );
>>>>>>>>> +
>>>>>>>>> +                /* TODO: smarter priority function. */
>>>>>>>>> +                int priority = 0;
>>>>>>>>> +                if( strncmp( "eth0", id->ifa_name, strlen( 
>>>>>>>>> "eth0"
>>>> ) )
>>>>
>>>>>>>>> ==
>>>>>>>>> 0
>>>>>>>>> ) +                {
>>>>>>>>> +                    priority = 1000;
>>>>>>>>> +                }
>>>>>>>>> +                else if( strncmp( "wifi0", id->ifa_name, strlen(
>>>>>>>>> "wifi0"
>>>>>>>>> )
>>>>>>>>> ) == 0 ) +                {
>>>>>>>>> +                    priority = 2000;
>>>>>>>>> +                }
>>>>>>>>> +
>>>>>>>>> +                msg_Dbg( p_obj, "Assigned interface %s priority
>>>> %d",
>>>>
>>>>>>>>> + id->ifa_name, priority);
>>>>>>>>> +
>>>>>>>>> +                if( priority > selected_priority )
>>>>>>>>> +                {
>>>>>>>>> +                    strcpy( result, ip_address );
>>>>>>>>> +                    selected_priority = priority;
>>>>>>>>> +                    found = true;
>>>>>>>>> +                }
>>>>>>>>> +            }
>>>>>>>>> +        }
>>>>>>>>> +    }
>>>>>>>>> +    else
>>>>>>>>> +    {
>>>>>>>>> +        msg_Err( p_obj, "Couldn't get interface addresses");
>>>>>>>>> +    }
>>>>>>>> That could have worked in the nineties. Machines, especially
>>>>>>>> laptops,
>>>>>>>> change IP addresses and have multiple addresses routinely 
>>>>>>>> nowadays,
>>>> and
>>>>
>>>>>>>> Linux desktops interfaces are not always called eth0 or wifi0.
>>>>>>> I will take Alexandre's suggestion.
>>>>>>>
>>>>>>>> Besides, you need to respond with the correct IP address for the
>>>>>>>> interface
>>>>>>>> on which the request came, usually with IP_PKTINFO/IP6_PKTINFO if
>>>>>>>> you
>>>>>>>> use
>>>>>>>> UDP.
>>>>>>> Respond to what? I don't understand.
>>>>>> Typically, discovery protocols wait for inbound client requests.
>>>>> Oh - so I don't even have to do the detection myself. Great!
>>>> You should never do detection.
>>>>
>>>> For unsolicited announcements, you should either enumerate all 
>>>> eligible
>>>> interfaces and spam all of them, or leave the IP stack routing 
>>>> table to
>>>> pick
>>>> the correct one at a given time.
>>>>
>>>> For responses to unicast, you should copy the destination IP of the
>>>> request as
>>>> the source IP of the response.
>>>>
>>>> And for responses to multicast, you should again let the IP stack 
>>>> routing
>>>> table pick the correct source address for the inbound interface.
>>>>
>>>> In any case, that can only work with packet info ancillary data. 
>>>> There are
>>>> no
>>>> reason to write OS-specific code here.
>>> In this case we are responding to multicast (works currently as you
>>> mentioned above I think) BUT also we are providing a unicast ip 
>>> address in
>>> the A/AAAA record in the response packet.
>>> The relationship between the unicast address and the multicast 
>>> response is
>>> that they share the same interface. Right?
>> Multicast addresses do not have an interface; you can have the same 
>> multicast
>> addresses on different interfaces referring to the same or to 
>> different actual
>> group(s). AFAIK, you have to get the correct interface index from the 
>> incoming
>> packet ancillary data.
> Ok, thanks for the information.
>
> libmicrodns scans all interface addresses on initialisation and for 
> each valid one opens a socket.
>
> Each socket and interface [address (linux) / index (Windows)] are 
> paired so when we get an incoming packet we know what interface it is. 
> Therefore, I don't think we have to deal with packet ancillary data?
>
> The problem though is getting the unicast address for that interface 
> so we can put it in the response body (A/AAAA record).
>
> If I'm correct, this needs to be calculated on Windows which will 
> require OS-specific code. Then I will update libmicrodns to pass the 
> IP address string to the callback so VLC can use it to populate the 
> A/AAAA record.
>
> Does this sound like a valid approach?
>
> Thanks,
>
> Roland


Hi,

I've made the necessary changes to libmicrodns. My pull request is here: 
https://github.com/videolabs/libmicrodns/pull/20/commits/fb58b2fffdce80be6381431845fc8e743920e054

Please have a look and let me know if it's a viable solution.

Thanks,

Roland