-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
System.Net.Dns.GetHostAddresses() does not return all local ip addresses on Linux/.NET Core (it works in .NET Framework) #27534
Comments
It looks like something is wrong in
which in turn makes a call to
from This function makes use of the standard I've created a small C code snippet that tries to get local ip addresses using #include <stdio.h>
#include <stdlib.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include <netdb.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <sys/un.h>
#include <unistd.h>
#define HOSTNAME_LEN 255
void main()
{
char *my_hostname = malloc(HOSTNAME_LEN);
struct addrinfo hint;
struct addrinfo* info = NULL;
int result;
result = gethostname(my_hostname, HOSTNAME_LEN);
if (result != 0)
{
printf("Error in gethostname() %d\n", result);
return;
}
printf("My hostname is %s\n", my_hostname);
memset(&hint, 0, sizeof(struct addrinfo));
hint.ai_family = AF_UNSPEC;
hint.ai_flags = AI_CANONNAME;
result = getaddrinfo((const char*)my_hostname, NULL, &hint, &info);
if (result != 0)
{
printf("Error in getaddrinfo() %d\n", result);
return;
}
printf("My IP addresses are:\n");
for (struct addrinfo *ai = info; ai != NULL; ai = ai->ai_next)
{
if (ai->ai_family != AF_INET) continue;
printf("\t%s (0x%08X)\n", inet_ntoa(((struct sockaddr_in *)ai->ai_addr)->sin_addr), ((struct sockaddr_in *)ai->ai_addr)->sin_addr.s_addr);
}
} This is the result on my test server:
I will dig further into it. |
The Mono implementation of If the hostname passed to the function matches the local hostname, then the function ends up calling a specific internal routine which enumerates local ipaddresses through a I believe that the same approach should be taken here, or at least make use of |
GetHostXXX will get incorrect result.You can use System.Net.NetworkInformation.NetworkInterface.GetAllNetworkInterfaces() to get ip address. |
Thanks, unfortunately in my project GetHostXXX is called inside an external library which I have no control over, I'll ask for a fix to the authors. |
I'm seeing the same problem in .Net Core 2.2.5. Was there ever a fix for this? |
So I'm seeing the same problem on Ubuntu WSL, however on Debian 9 (VM in Azure) it fails with:
The exception is on dotnet core 2.1.802 as well as 3.0.100 preview 9. I don't know the root cause for the exception yet. |
This is because it depends on network configuration @ManickaP. Local hostname may or may not be in DNS. And it also depends on what DNS server you are pointing at.
So I'm not sure if it is reasonable to expect all local addresses if they are not in DNS. If we want to change this, I think we should take path @bpress suggested. That would make it more consistent across OSes. Also as Mono will blend in in 5.0 it would be reasonable to preserve their behavior. That should probably also work on OSX. |
BTW do you have any recommendation here @davidsh? |
@wfurt Yeah you're right, it fails on Mono as well (in the Azure VM), though it gives much better message:
And
So my testing environment is not exactly the same as in the original problem of the issue. But I was able to get the same problem on WSL, so might continue testing there. |
Are you talking about changing our implemention in .NET Core to do the below?
The bottom line is that we should try to get the best and consistent behavior across all platforms (Windows, Linux, Mac). That "best" behavior appears to be to have all the loopback adapter IP addresses (i.e. 127.0.0.1) as well as the other IP addresses assigned to all network interfaces in the system. @stephentoub @geoffkizer Comments? |
I have did quick test on Windows 10 system with 2 interfaces. My second NIC is private network and the address is NOT in DNS. Running example above gives that IP address as well. Interestingly, IPv4 loopback (127.0.0.1) is not in the list. (nor ::1) If we want consistent behavior, adding special case for own name makes sense to me. (instead of relying on external DNS) And perhaps updating documentation to be explicit about it. |
FYI, Windows doesn't just use DNS to get IP addresses from names. It uses NETBIOS and LLMNR and other protocols. |
Please correct me if I'm wrong, but the desired behavior on all OSes is:
Unless someone express disagreement here I'll try to look into it. |
We should have our APIs be able return all IP addresses which include all of those bound to network interfaces as well as "127.0.0.1" and "::1" (loopback adapter addresses). Note that on systems supporting IPv6 this will include the link-local addresses as well, i.e: from my "ipconfig /all" on my Windows machine:
|
I'm theoretically fine with such a change, especially if it improves our behavior across OSes. One thing to be cautious of is performance; we'll want to pay careful attention to that, as iterating through all such adapters could be non-trivially expensive if we had to do it on every call. We might need to think about some kind of caching strategy. |
true. But going to external DNS server is not fast either IMHO. Going through interface list will burn more CPU. We could listen to address updates but that would require background task. |
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Collections.Generic;
using System.Net;
using System.Net.NetworkInformation;
public class Program
{
static void Main(string[] args) => BenchmarkSwitcher.FromTypes(new[] { typeof(Program) }).Run(args);
[Benchmark]
public IPAddress[] GetHostAddresses() => Dns.GetHostAddresses(Dns.GetHostName());
[Benchmark]
public IPAddress[] Adapters()
{
var addresses = new List<IPAddress>();
foreach (NetworkInterface ni in NetworkInterface.GetAllNetworkInterfaces())
{
foreach (UnicastIPAddressInformation ip in ni.GetIPProperties().UnicastAddresses)
{
addresses.Add(ip.Address);
}
}
return addresses.ToArray();
}
} |
GetAllNetworkInterfaces() (and many other NetworkInfo functions) are bloated ;( |
If we can improve those, that'd be excellent. |
You got me hooked up @stephentoub. I did run your test, and got ~ I made some proof of concept and I get this on my 4core VM with 4 interfaces and 10 addresses.
I repeated test on physical machine with 12 interfaces and 21 L3 addresses (2 physical, and bunch of Docker virtual interfaces)
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System.Collections.Generic;
using System.Net;
using System.Net.NetworkInformation;
using System;
using System.Runtime.InteropServices;
public class Program
{
[StructLayout (LayoutKind.Sequential)]
internal unsafe struct sockaddr {
internal short sa_family;
internal short sa_port;
internal fixed byte data[20];
internal uint scope_id;
}
[StructLayout (LayoutKind.Sequential)]
internal unsafe struct ifaddrs {
internal readonly void *ifa_next;
internal readonly char *ifa_name;
internal readonly uint ifa_flags;
internal readonly sockaddr *ifa_addr;
}
[DllImport("libc", EntryPoint = "getifaddrs")]
unsafe static extern int getifaddrs(ref void * addrs);
[DllImport("libc")]
unsafe static extern void freeifaddrs(void* ifa);
static void Main(string[] args) => BenchmarkSwitcher.FromTypes(new[] { typeof(Program) }).Run(args);
[Benchmark]
public IPAddress[] GetHostAddresses() => Dns.GetHostAddresses(Dns.GetHostName());
[Benchmark]
public IPAddress[] Adapters()
{
var addresses = new List<IPAddress>();
foreach (NetworkInterface ni in NetworkInterface.GetAllNetworkInterfaces())
{
foreach (UnicastIPAddressInformation ip in ni.GetIPProperties().UnicastAddresses)
{
addresses.Add(ip.Address);
}
}
return addresses.ToArray();
}
[Benchmark]
public IPAddress[] GetAllNetworkInterfaces()
{
var addresses = new IPAddress[0];
var list = NetworkInterface.GetAllNetworkInterfaces();
return addresses;
}
[Benchmark]
public unsafe IPAddress[] getifaddrs()
{
void * pinput = null;
int ret = getifaddrs(ref pinput);
int count = 0;
ifaddrs * entry = (ifaddrs*)pinput;
while (entry != null)
{
if (entry->ifa_addr->sa_family == 2 || entry->ifa_addr->sa_family == 10)
{
count ++;
}
entry = (ifaddrs*)entry->ifa_next;
}
var addresses = new IPAddress[count];
entry = (ifaddrs*)pinput;
count = 0;
while (entry != null)
{
if (entry->ifa_addr->sa_family == 2){
addresses[count] = new IPAddress(new ReadOnlySpan<byte>(entry->ifa_addr->data, 4));
count++;
}
else if (entry->ifa_addr->sa_family == 10)
{
addresses[count] = new IPAddress(new ReadOnlySpan<byte>(entry->ifa_addr->data + 4, 16));
addresses[count].ScopeId = entry->ifa_addr->scope_id;
count++;
}
entry = (ifaddrs*)entry->ifa_next;
}
freeifaddrs(pinput);
return addresses;
}
} I'll take a look if we can improve |
As far as the LL and lo addresses: This can certainly surprise anybody who is using
and the link-local addresses are certainly tricky to use. |
😄 |
we chatted about this offline with @stephentoub. It seems like it is not practical for purpose of the to make massive change to
|
I've tested
@wfurt: I suppose we don't want to break backward compatibility with .NET Framework and prune the list on Windows, but rather extend the list on other OSes? If so, I'll prepare a fix leveraging Also, I can provide the detailed results of the tests I did. I wasn't sure about sharing them here. |
yes, I think so @ManickaP e.g. make Windows/Framework/MONO behavior on other platforms. I think flow in documentation is that it does not explain that for host it self the behavior is different and it returns addresses without querying DNS or using host file. |
I have the benchmark results of using
New values:
So originally 7,789.7 ns and after change 52,723.5 ns. I might tweak it to achieve some minor improvements but nothing magical. After all it introduces another system call so it inherently is slower... Unless you want to forgo the I will measure it on Windows on my machine as well that might give us some more info. |
I did try your code @ManickaP (compared to my work-in-progress on GetAllNetworkInterfaces). I also add one more test to lookup www.mictrosoft.com to get sense how fast GetHostAddresses would be if it does not come from /etc/host. It may be worth for you to try it with and without entry matching local system.
|
Another round of benchmarks (
Linux (WSL2) before (the time unit is
Linux (WSL2) after (the time unit is
So my fix slowed the linux version ~6-7 times. However, the original linux version is ~40 times faster than Win and my fix is still ~8-9 times faster than win version. On the other hand, with DNS query, Linux is ~80 times slower since there's no DNS caching on OS level. Me personally, I would proceed with the change since the numbers are still better than their Win counterparts. There's still some room for improvements (performance-wise) in my version, but nothing which would make it as fast as it originally was. @stephentoub @wfurt I'm interested in your opinions on the numbers. Should I proceed or not? |
I did more work on Friday on tuning
It is obvious that some of the numbers depend on particular setup and performance of DNS server. I think it still make sense to proceed unless @stephentoub objects. One thing he pointed out in private chat is that if we call GetHostName() to determine if resolved name is our system or not, we will slow down "normal" lookup path. (assuming most often we try to resolve other systems) |
A call to
System.Net.Dns.GetHostAddresses(System.Net.Dns.GetHostName())
returns only the first local IP address when running on Linux.
Tested with .NET Core 2.1.3
Here is the repro code:
Here is the (wrong) output I get on my Linux Debian 9:
If I run the very same code under Mono, it works, and this is the correct output I get:
On Windows both .NET Core and .NET Framework versions work well.
The text was updated successfully, but these errors were encountered: