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

[Roland Bewick]

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