-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[Android] Port getifaddrs implementation from Xamarin.Android #71943
[Android] Port getifaddrs implementation from Xamarin.Android #71943
Conversation
/azp run runtime-extra-platforms |
Tagging subscribers to this area: @dotnet/ncl Issue DetailsIn recent Android versions the data returned by Fixes dotnet/android#6973 /cc @grendello
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
||
#include <android/log.h> | ||
#define LOG(level, fmt, ...) __android_log_print(level, "DOTNET_NETLINK", fmt, ## __VA_ARGS__) | ||
#define LOG_DEBUG(fmt, ...) LOG(ANDROID_LOG_DEBUG, fmt, ## __VA_ARGS__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calls to LOG_DEBUG
should ideally be hidden behind some sort of flag, and off by default. Logging on Android is very costly and should be avoided as much as possible, especially where there might be a lot of data logged.
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
The failing tests are unrelated to this PR (on Android x86 they are all I suggest checking the output of |
Extra-platforms failures are known and not caused by this change |
@@ -517,9 +517,6 @@ | |||
/* Have getifaddrs */ | |||
#cmakedefine HAVE_GETIFADDRS 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simonrozsival we can also remove this one, it is unused in mono now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HAVE_GETIFADDRS
is still used, for example here: https://github.com/dotnet/runtime/pull/71943/files#diff-404dd4500e99144ae93b6753065902099e05f7fc7ef19834d0d6e152ed0e6b45R253
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simonrozsival that is part of System.Native, not mono. It has its own check:
runtime/src/native/libs/configure.cmake
Lines 139 to 141 in f249dc7
check_function_exists( | |
getifaddrs | |
HAVE_GETIFADDRS) |
/backport to release/6.0 |
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/2664136084 |
In recent Android versions the data returned by
getifaddrs
is invalid and doesn't allow us to implementSystem.Net.NetworkInformation.GetAllNetworkInterfaces()
. It's possible to reimplementgetifaddrs
using Netlink and there's already an existing implementation in Xamarin.Android.Fixes dotnet/android#6973
Ref #62780
Ref #51303
/cc @grendello