-
Notifications
You must be signed in to change notification settings - Fork 5.3k
os syscalls: add getifaddrs to singleton #18821
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
Changes from all commits
5efec7d
1e8748e
5a2e99f
348a1a6
1ce7abc
c94001f
a46ce52
5f53e43
6c808fe
0ff1451
65c8b63
43b9f76
9c64f51
a7b646d
a8c6293
dd9cef3
b829f31
7cceeff
c578f3e
8e3ae6a
5d14dd1
8ab24f5
45bd77f
c46de52
d702b9d
ce9d5f1
97d837e
f6ada1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| #include <string> | ||
|
|
||
| #include "source/common/api/os_sys_calls_impl.h" | ||
| #include "source/common/network/address_impl.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Api { | ||
|
|
@@ -296,5 +297,68 @@ SysCallBoolResult OsSysCallsImpl::socketTcpInfo([[maybe_unused]] os_fd_t sockfd, | |
| return {false, EOPNOTSUPP}; | ||
| } | ||
|
|
||
| bool OsSysCallsImpl::supportsGetifaddrs() const { | ||
| // TODO: eliminate this branching by upstreaming an alternative Android implementation | ||
| // e.g.: https://github.com/envoyproxy/envoy-mobile/blob/main/third_party/android/ifaddrs-android.h | ||
| #if defined(__ANDROID_API__) && __ANDROID_API__ < 24 | ||
junr03 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (alternate_getifaddrs_.has_value()) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't you be doing this in all cases otherwise the interface docs are misleading?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mattklein123 I think I might be misunderstanding your point. Where else should I be checking this that is not checked?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should check this outside of the #ifdef ?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see what you mean now @mattklein123. My thought was that in posix the class would default to true for supporting getifaddrs regardless of if an alternate implementation is given. Only for Android<24 do we need to check for existence of an alternate implementation.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK I see. Makes sense. Can you just add a small comment? /wait |
||
| return true; | ||
| } | ||
| return false; | ||
| #else | ||
| // Note: posix defaults to true regardless of whether an alternate getifaddrs has been set or not. | ||
| // This is because as far as we are aware only Android<24 lacks an implementation and thus another | ||
| // posix based platform that lacks a native getifaddrs implementation should be a programming | ||
| // error. | ||
| // | ||
| // That being said, if an alternate getifaddrs impl is set, that will be used in calls to | ||
| // OsSysCallsImpl::getifaddrs as seen below. | ||
| return true; | ||
| #endif | ||
| } | ||
|
|
||
| SysCallIntResult OsSysCallsImpl::getifaddrs([[maybe_unused]] InterfaceAddressVector& interfaces) { | ||
| if (alternate_getifaddrs_.has_value()) { | ||
| return alternate_getifaddrs_.value()(interfaces); | ||
| } | ||
|
|
||
| // TODO: eliminate this branching by upstreaming an alternative Android implementation | ||
| // e.g.: https://github.com/envoyproxy/envoy-mobile/blob/main/third_party/android/ifaddrs-android.h | ||
| #if defined(__ANDROID_API__) && __ANDROID_API__ < 24 | ||
| NOT_IMPLEMENTED_GCOVR_EXCL_LINE; | ||
| #else | ||
| struct ifaddrs* ifaddr; | ||
| struct ifaddrs* ifa; | ||
|
|
||
| const int rc = ::getifaddrs(&ifaddr); | ||
| if (rc == -1) { | ||
| return {rc, errno}; | ||
| } | ||
|
|
||
| for (ifa = ifaddr; ifa != nullptr; ifa = ifa->ifa_next) { | ||
| if (ifa->ifa_addr == nullptr) { | ||
| continue; | ||
| } | ||
|
|
||
| if (ifa->ifa_addr->sa_family == AF_INET || ifa->ifa_addr->sa_family == AF_INET6) { | ||
| const sockaddr_storage* ss = reinterpret_cast<sockaddr_storage*>(ifa->ifa_addr); | ||
| size_t ss_len = | ||
| ifa->ifa_addr->sa_family == AF_INET ? sizeof(sockaddr_in) : sizeof(sockaddr_in6); | ||
| StatusOr<Network::Address::InstanceConstSharedPtr> address = | ||
| Network::Address::addressFromSockAddr(*ss, ss_len, ifa->ifa_addr->sa_family == AF_INET6); | ||
| if (address.ok()) { | ||
| interfaces.emplace_back(ifa->ifa_name, ifa->ifa_flags, *address); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (ifaddr) { | ||
| ::freeifaddrs(ifaddr); | ||
| } | ||
|
|
||
| return {rc, 0}; | ||
| #endif | ||
| } | ||
|
|
||
| } // namespace Api | ||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| load( | ||
| "//bazel:envoy_build_system.bzl", | ||
| "envoy_cc_test", | ||
| "envoy_package", | ||
| ) | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| envoy_package() | ||
|
|
||
| envoy_cc_test( | ||
| name = "os_sys_calls_test", | ||
| srcs = ["os_sys_calls_test.cc"], | ||
| deps = [ | ||
| "//source/common/api:os_sys_calls_lib", | ||
| ], | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| #include "source/common/api/os_sys_calls_impl.h" | ||
|
|
||
| #include "gtest/gtest.h" | ||
|
|
||
| namespace Envoy { | ||
|
|
||
| TEST(OsSyscallsTest, SetAlternateGetifaddrs) { | ||
| auto& os_syscalls = Api::OsSysCallsSingleton::get(); | ||
| const bool pre_alternate_support = os_syscalls.supportsGetifaddrs(); | ||
| Api::InterfaceAddressVector interfaces{}; | ||
| #if defined(WIN32) || (defined(__ANDROID_API__) && __ANDROID_API__ < 24) | ||
| EXPECT_FALSE(pre_alternate_support); | ||
| EXPECT_DEATH(os_syscalls.getifaddrs(interfaces), "not implemented"); | ||
| #else | ||
| EXPECT_TRUE(pre_alternate_support); | ||
| const auto pre_alternate_rc = os_syscalls.getifaddrs(interfaces); | ||
| EXPECT_EQ(0, pre_alternate_rc.return_value_); | ||
| EXPECT_FALSE(interfaces.empty()); | ||
| #endif | ||
|
|
||
| os_syscalls.setAlternateGetifaddrs( | ||
| [](Api::InterfaceAddressVector& interfaces) -> Api::SysCallIntResult { | ||
| interfaces.emplace_back("made_up_if", 0, nullptr); | ||
| return {0, 0}; | ||
| }); | ||
| interfaces.clear(); | ||
|
|
||
| const bool post_alternate_support = os_syscalls.supportsGetifaddrs(); | ||
| EXPECT_TRUE(post_alternate_support); | ||
| EXPECT_EQ(0, os_syscalls.getifaddrs(interfaces).return_value_); | ||
| EXPECT_EQ(1, interfaces.size()); | ||
| EXPECT_EQ("made_up_if", interfaces.front().interface_name_); | ||
| } | ||
| } // namespace Envoy |
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.
happy to move this to address.h if we think it makes more sense there. I couldn't decide.