Skip to content
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

Update Envoy to 70a5f29 #1962

Merged
merged 19 commits into from
Dec 14, 2021
Merged

Update Envoy to 70a5f29 #1962

merged 19 commits into from
Dec 14, 2021

Conversation

jpsim
Copy link
Contributor

@jpsim jpsim commented Dec 6, 2021

Signed-off-by: JP Simard <jp@jpsim.com>
@@ -26,7 +26,7 @@ RequestHeaderMapPtr toRequestHeaders(envoy_headers headers) {
auto transformed_headers = RequestHeaderMapImpl::create();
transformed_headers->setFormatter(
std::make_unique<
Extensions::Http::HeaderFormatters::PreserveCase::PreserveCaseHeaderFormatter>());
Extensions::Http::HeaderFormatters::PreserveCase::PreserveCaseHeaderFormatter>(false));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To account for envoyproxy/envoy#18997

Considering it's designed to be off by default, I imagine that we can disable it for our use cases until we have reason to do otherwise.

@jpsim jpsim mentioned this pull request Dec 6, 2021
jpsim added 3 commits December 6, 2021 15:46
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim added 9 commits December 7, 2021 10:28
It should be rewritten now that getifaddrs works.

Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
@@ -69,16 +66,6 @@ constexpr absl::string_view WwanPrefix = "";
namespace Envoy {
namespace Network {

#if !defined(SUPPORTS_GETIFADDRS) && defined(INCLUDE_IFADDRS)
Copy link
Member

Choose a reason for hiding this comment

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

In my version of this work I had then created a network/android/utility namespace where I created a function that sets the android getifaddrs impl. Then I called that function early in the engine's lifecycle before the network configurator is constructed.

This was my implementation:

#include "library/common/network/utility.h"

#include "source/common/api/os_sys_calls_impl.h"
#include "source/common/network/address_impl.h"

namespace Envoy {
namespace Network {
namespace Android {
namespace Utility {

#if defined(INCLUDE_ANDROID_IFADDRS)
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wold-style-cast"
namespace {
#include "third_party/android/ifaddrs-android.h"
}
#pragma clang diagnostic pop
#endif

/**
 */
void setAlternateGetifaddrs() {
#if defined(INCLUDE_ANDROID_IFADDRS)
  auto& os_syscalls = Api::OsSysCallsSingleton::get();
  if (!os_syscalls.supportsGetifaddrs()) {
    Api::AlternateGetifaddrs android_getifaddrs =
        [](Api::InterfaceAddressVector& interfaces) -> Api::SysCallIntResult {
      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<Address::InstanceConstSharedPtr> address =
              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};
    };

    os_syscalls.setAlternateGetifaddrs(android_getifaddrs);
  } else {
    // FIXME: assert that this function is not called more than once?
  }
#endif
}

} // namespace Utility
} // namespace Android
} // namespace Network
} // namespace Envoy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for saving me the trouble of writing this 😄

I've pushed it up with minor modifications and addressing the TODO in eb6a57b. I'm working on running the Android sample app locally to confirm this works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've confirmed that this ifaddrs implementation is used on Android by manually adding logs and confirming they're emitted when running the sample app locally.

jpsim added 4 commits December 9, 2021 16:32
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Copy link
Member

@junr03 junr03 left a comment

Choose a reason for hiding this comment

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

lgtm, looking forward to test.

Did you run the example app to look for the interface log line in Android as a sanity check?

@jpsim
Copy link
Contributor Author

jpsim commented Dec 13, 2021

Did you run the example app to look for the interface log line in Android as a sanity check?

Yes: #1962 (comment)

PS: I can’t merge this PR myself

@jpsim jpsim changed the title Update Envoy to 70a5f29 Update Envoy to f4e3018 Dec 14, 2021
@jpsim jpsim changed the title Update Envoy to f4e3018 Update Envoy to 70a5f29 Dec 14, 2021
@alyssawilk alyssawilk merged commit bc37b81 into envoyproxy:main Dec 14, 2021
@jpsim jpsim deleted the envoy-70a5f29 branch December 14, 2021 18:34
@jpsim
Copy link
Contributor Author

jpsim commented Dec 14, 2021

Thanks for merging @alyssawilk !

junr03 pushed a commit that referenced this pull request Dec 15, 2021
Changes: envoyproxy/envoy@23e5fc2...70a5f29

Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants