Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Return all local IPs on Linux #41764

Merged
merged 18 commits into from
Nov 4, 2019
Merged

Return all local IPs on Linux #41764

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8713443
Added interop for getfiaddrs.
ManickaP Oct 1, 2019
c69c736
HostInterfaces parsed in C# Unix PAL.
ManickaP Oct 1, 2019
99de10f
Fixed linked files in csproj.
ManickaP Oct 1, 2019
c767b71
Fixed compilation.
ManickaP Oct 1, 2019
a2bb451
Fixed test project.
ManickaP Oct 2, 2019
64d6052
Merged getifaddrs into GetHostEntryForName
ManickaP Oct 8, 2019
dba4a46
pal_networking returns IPAddress array instead of original lists.
ManickaP Oct 8, 2019
4a9a7d4
GetHostName to C pal, fixed missing free.
ManickaP Oct 9, 2019
6698096
Reverted accidental formatting change, clean up.
ManickaP Oct 10, 2019
a9719af
Formatting
ManickaP Oct 13, 2019
c6fbee4
Merge branch 'master' into mapichov/32611_getifaddrs
ManickaP Oct 13, 2019
21eb284
Fixed NameResolution tests.
ManickaP Oct 14, 2019
cdc83f3
Fixed OSX compilation
ManickaP Oct 14, 2019
4d1682f
Addressing PR comments
ManickaP Oct 16, 2019
110a24e
Addressed PR comments
ManickaP Oct 17, 2019
f06697e
Try to mimic mono behavior.
ManickaP Oct 21, 2019
5142544
Merge branch 'master' into mapichov/32611_getifaddrs
ManickaP Oct 29, 2019
4b411c5
Addressed PR comments
ManickaP Oct 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 4 additions & 13 deletions src/Common/src/Interop/Unix/System.Native/Interop.HostEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,27 +23,18 @@ internal enum GetAddrInfoErrorFlags : int
EAI_NOMORE = 7, // No more entries are present in the list.
}

//opaque structure to maintain consistency with native function signature
internal unsafe struct addrinfo
{

}

[StructLayout(LayoutKind.Sequential)]
internal unsafe struct HostEntry
{
internal byte* CanonicalName; // Canonical Name of the Host
internal byte** Aliases; // List of aliases for the host
internal addrinfo* AddressListHandle; // Handle for socket address list
internal int IPAddressCount; // Number of IP addresses in the list
internal byte* CanonicalName; // Canonical Name of the Host
internal byte** Aliases; // List of aliases for the host
internal IPAddress* AddressList; // Handle for socket address list
internal uint AddressCount; // Number of IP addresses in the list
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
}

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetHostEntryForName")]
internal static extern unsafe int GetHostEntryForName(string address, HostEntry* entry);

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetNextIPAddress")]
internal static extern unsafe int GetNextIPAddress(HostEntry* entry, addrinfo** addressListHandle, IPAddress* endPoint);

[DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_FreeHostEntry")]
internal static extern unsafe void FreeHostEntry(HostEntry* entry);
}
Expand Down
130 changes: 87 additions & 43 deletions src/Native/Unix/System.Native/pal_networking.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,9 @@
#elif HAVE_SENDFILE_6
#include <sys/uio.h>
#endif
#if !HAVE_IN_PKTINFO
#if HAVE_GETIFADDRS
#include <ifaddrs.h>
#endif
#endif
#ifdef AF_CAN
#include <linux/can.h>
#endif
Expand Down Expand Up @@ -140,6 +138,8 @@ enum
INET6_ADDRSTRLEN_MANAGED = 65 // Managed code has a longer max IPv6 string length
};

#define MAX_HOST_NAME 255
ManickaP marked this conversation as resolved.
Show resolved Hide resolved

c_static_assert(GetHostErrorCodes_HOST_NOT_FOUND == HOST_NOT_FOUND);
c_static_assert(GetHostErrorCodes_TRY_AGAIN == TRY_AGAIN);
c_static_assert(GetHostErrorCodes_NO_RECOVERY == NO_RECOVERY);
Expand Down Expand Up @@ -236,7 +236,7 @@ int32_t SystemNative_GetHostEntryForName(const uint8_t* address, HostEntry* entr
{
return GetAddrInfoErrorFlags_EAI_BADARG;
}

ManickaP marked this conversation as resolved.
Show resolved Hide resolved
// Get all address families and the canonical name
struct addrinfo hint;
memset(&hint, 0, sizeof(struct addrinfo));
Expand All @@ -252,82 +252,126 @@ int32_t SystemNative_GetHostEntryForName(const uint8_t* address, HostEntry* entr

entry->CanonicalName = NULL;
entry->Aliases = NULL;
entry->AddressListHandle = info;
entry->IPAddressCount = 0;
entry->AddressList = NULL;
entry->AddressCount = 0;

// Find the canonical name for this host (if any) and count the number of IP end points.
for (struct addrinfo* ai = info; ai != NULL; ai = ai->ai_next)
{
// If we haven't found a canonical name yet and this addrinfo has one, copy it
if ((entry->CanonicalName == NULL) && (ai->ai_canonname != NULL))
{
entry->CanonicalName = (uint8_t*)ai->ai_canonname;
entry->CanonicalName = (uint8_t*)strdup(ai->ai_canonname);
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}

if (ai->ai_family == AF_INET || ai->ai_family == AF_INET6)
{
entry->IPAddressCount++;
entry->AddressCount++;
}
}

return GetAddrInfoErrorFlags_EAI_SUCCESS;
}
#if HAVE_GETIFADDRS
char name[MAX_HOST_NAME + 1];
result = gethostname((char*)name, MAX_HOST_NAME);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will call this for every lookup, right? e.g. we will slow down common case when trying to resolve names other than ours. Since it is unlikely this will change during process run it may be worth of some caching.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same behavior, without caching, is on Mono.

struct ifaddrs* addrs = NULL;
if (result == 0 && strcasecmp((const char*)address, name) == 0)
{
// Get all interface addresses if the host name corresponds to the local host.
result = getifaddrs(&addrs);

static int32_t GetNextIPAddressFromAddrInfo(struct addrinfo** info, IPAddress* endPoint)
{
assert(info != NULL);
assert(endPoint != NULL);
// If getifaddrs fails, just skip it, the data are no crucial for the result.
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
if (result == 0)
{
// Count the number of IP end points.
for (struct ifaddrs* ifa = addrs; ifa != NULL; ifa = ifa->ifa_next)
{
if (ifa->ifa_addr == NULL)
{
continue;
}

if (ifa->ifa_addr->sa_family == AF_INET || ifa->ifa_addr->sa_family == AF_INET6)
{
entry->AddressCount++;
}
}
}
}
#endif

for (struct addrinfo* ai = *info; ai != NULL; ai = ai->ai_next)
if (entry->AddressCount > 0)
{
switch (ai->ai_family)
entry->AddressList = (IPAddress*)calloc(entry->AddressCount, sizeof(IPAddress));
ManickaP marked this conversation as resolved.
Show resolved Hide resolved

IPAddress* addressList = entry->AddressList;

for (struct addrinfo* ai = info; ai != NULL; ai = ai->ai_next)
{
case AF_INET:
if (ai->ai_family == AF_INET)
{
struct sockaddr_in* inetSockAddr = (struct sockaddr_in*)ai->ai_addr;

ConvertInAddrToByteArray(endPoint->Address, NUM_BYTES_IN_IPV4_ADDRESS, &inetSockAddr->sin_addr);
endPoint->IsIPv6 = 0;
break;
ConvertInAddrToByteArray(addressList->Address, NUM_BYTES_IN_IPV4_ADDRESS, &inetSockAddr->sin_addr);
addressList->IsIPv6 = 0;
++addressList;
continue;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use continue; in both of these blocks rather than just making the next if into an else if?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I like the symmetricity of it and I'm not a fan of if {} else if {} without final else {}. However, if you think that if {} else if {} is generally more readable I'll gladly change it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like the block are very similar. You may get away with ternary operator in few places where it differs.

}

case AF_INET6:
if (ai->ai_family == AF_INET6)
{
struct sockaddr_in6* inet6SockAddr = (struct sockaddr_in6*)ai->ai_addr;

ConvertIn6AddrToByteArray(endPoint->Address, NUM_BYTES_IN_IPV6_ADDRESS, &inet6SockAddr->sin6_addr);
endPoint->IsIPv6 = 1;
endPoint->ScopeId = inet6SockAddr->sin6_scope_id;
break;
}

default:
// Skip non-IPv4 and non-IPv6 addresses
ConvertIn6AddrToByteArray(addressList->Address, NUM_BYTES_IN_IPV6_ADDRESS, &inet6SockAddr->sin6_addr);
addressList->IsIPv6 = 1;
addressList->ScopeId = inet6SockAddr->sin6_scope_id;
++addressList;
continue;
}
}
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
freeaddrinfo(info);
ManickaP marked this conversation as resolved.
Show resolved Hide resolved

*info = ai->ai_next;
return GetAddrInfoErrorFlags_EAI_SUCCESS;
}
#if HAVE_GETIFADDRS
if (addrs != NULL)
{
for (struct ifaddrs* ifa = addrs; ifa != NULL; ifa = ifa->ifa_next)
{
if (ifa->ifa_addr == NULL)
continue;

return GetAddrInfoErrorFlags_EAI_NOMORE;
}
if (ifa->ifa_addr->sa_family == AF_INET)
{
struct sockaddr_in* inetSockAddr = (struct sockaddr_in*)ifa->ifa_addr;

int32_t SystemNative_GetNextIPAddress(const HostEntry* hostEntry, struct addrinfo** addressListHandle, IPAddress* endPoint)
{
if (hostEntry == NULL || addressListHandle == NULL || endPoint == NULL)
{
return GetAddrInfoErrorFlags_EAI_BADARG;
ConvertInAddrToByteArray(addressList->Address, NUM_BYTES_IN_IPV4_ADDRESS, &inetSockAddr->sin_addr);
addressList->IsIPv6 = 0;
++addressList;
continue;
}

if (ifa->ifa_addr->sa_family == AF_INET6)
{
struct sockaddr_in6* inet6SockAddr = (struct sockaddr_in6*)ifa->ifa_addr;

ConvertIn6AddrToByteArray(addressList->Address, NUM_BYTES_IN_IPV6_ADDRESS, &inet6SockAddr->sin6_addr);
addressList->IsIPv6 = 1;
addressList->ScopeId = inet6SockAddr->sin6_scope_id;
++addressList;
continue;
}
}
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
freeifaddrs(addrs);
}
#endif
}
return GetNextIPAddressFromAddrInfo(addressListHandle, endPoint);

return GetAddrInfoErrorFlags_EAI_SUCCESS;
}

void SystemNative_FreeHostEntry(HostEntry* entry)
{
if (entry != NULL)
{
freeaddrinfo(entry->AddressListHandle);
{
free(entry->AddressList);
free(entry->CanonicalName);
}
}

Expand Down
7 changes: 3 additions & 4 deletions src/Native/Unix/System.Native/pal_networking.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,8 +251,8 @@ typedef struct
{
uint8_t* CanonicalName; // Canonical name of the host
uint8_t** Aliases; // List of aliases for the host
struct addrinfo* AddressListHandle; // Handle for host socket addresses
int32_t IPAddressCount; // Number of IP end points in the list
IPAddress* AddressList; // Handle for host socket addresses
uint32_t AddressCount; // Number of IP end points in the socket address list
} HostEntry;

typedef struct
Expand Down Expand Up @@ -311,10 +311,9 @@ typedef struct

DLLEXPORT int32_t SystemNative_GetHostEntryForName(const uint8_t* address, HostEntry* entry);

DLLEXPORT int32_t SystemNative_GetNextIPAddress(const HostEntry* entry, struct addrinfo** addressListHandle, IPAddress* endPoint);

DLLEXPORT void SystemNative_FreeHostEntry(HostEntry* entry);


DLLEXPORT int32_t SystemNative_GetNameInfo(const uint8_t* address,
int32_t addressLength,
int8_t isIPv6,
Expand Down
2 changes: 1 addition & 1 deletion src/System.Net.NameResolution/src/System.Net.NameResolution.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
<Link>Interop\Unix\System.Native\Interop.HostEntries.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.IPAddress.cs">
<Link>Interop\Unix\System.Native\Interop.HostEntries.cs</Link>
<Link>Interop\Unix\System.Native\Interop.IPAddress.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Socket.cs">
<Link>Interop\Unix\System.Native\Interop.Socket.cs</Link>
Expand Down
16 changes: 6 additions & 10 deletions src/System.Net.NameResolution/src/System/Net/NameResolutionPal.Unix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private static unsafe void ParseHostEntry(Interop.Sys.HostEntry hostEntry, bool
null;

IPAddress[] localAddresses;
if (hostEntry.IPAddressCount == 0)
if (hostEntry.AddressCount == 0)
{
localAddresses = Array.Empty<IPAddress>();
}
Expand All @@ -70,19 +70,15 @@ private static unsafe void ParseHostEntry(Interop.Sys.HostEntry hostEntry, bool
// is likely to involve extra allocations, hashing, etc., and so will probably be more expensive than
// this one in the typical (short list) case.

var nativeAddresses = new Interop.Sys.IPAddress[hostEntry.IPAddressCount];
Interop.Sys.IPAddress* nativeAddresses = stackalloc Interop.Sys.IPAddress[(int)hostEntry.AddressCount];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use spans instead of adding unsafe code?

Also, how do we know that the AddressCount will be small enough for this to go on the stack?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use spans instead of adding unsafe code?

I agree we shouldn't be needing to add unsafe code anymore now that Span<T> and friends are available.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a mistake, I lost some changes somewhere in between the multiple repo clones I have. Here should have been Span and lower Slice.IndexOf.

Anyway, I've changed it back to dynamic allocation. According to DNS RFC, it may use TCP (ch 4.2.2) to transmit the data, thus effectively bypassing the UDP limit for the datagram size (ch 2.3.4). So I cannot safely say that it will be small enough to fit on the stack.

int nativeAddressCount = 0;

Interop.Sys.addrinfo* addressListHandle = hostEntry.AddressListHandle;
for (int i = 0; i < hostEntry.IPAddressCount; i++)
Interop.Sys.IPAddress* addressHandle = hostEntry.AddressList;
for (int i = 0; i < hostEntry.AddressCount; i++)
{
Interop.Sys.IPAddress nativeIPAddress = default;
int err = Interop.Sys.GetNextIPAddress(&hostEntry, &addressListHandle, &nativeIPAddress);
Debug.Assert(err == 0);

if (Array.IndexOf(nativeAddresses, nativeIPAddress, 0, nativeAddressCount) == -1)
if (new Span<Interop.Sys.IPAddress>(addressHandle, nativeAddressCount).IndexOf(addressHandle[i]) == -1)
ManickaP marked this conversation as resolved.
Show resolved Hide resolved
{
nativeAddresses[nativeAddressCount++] = nativeIPAddress;
nativeAddresses[nativeAddressCount++] = addressHandle[i];
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down