Skip to content

Commit

Permalink
network: create SocketSelectionFilter (#1797)
Browse files Browse the repository at this point in the history
Description: Removes network-specific clusters and replaces with a filter that sets unique socket options based on current preferred network (via Reachability) status. Envoy internally creates a different socket pool for a unique set of socket options, so the effect of this is that we still use different sets of connections per interface, but no longer rely on a large set of clusters to do so. This PR also lays the groundwork for us to experiment with interface-bound sockets.
Risk Level: High
Testing: Existing unit and integration coverage

Signed-off-by: Mike Schore <[email protected]>
Signed-off-by: JP Simard <[email protected]>
  • Loading branch information
goaway authored and jpsim committed Nov 29, 2022
1 parent 92db440 commit 0f1b14e
Show file tree
Hide file tree
Showing 22 changed files with 283 additions and 264 deletions.
1 change: 1 addition & 0 deletions mobile/envoy_build_config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ envoy_cc_library(
"@envoy_mobile//library/common/extensions/filters/http/local_error:config",
"@envoy_mobile//library/common/extensions/filters/http/platform_bridge:config",
"@envoy_mobile//library/common/extensions/filters/http/route_cache_reset:config",
"@envoy_mobile//library/common/extensions/filters/http/socket_selection:config",
],
)
1 change: 1 addition & 0 deletions mobile/envoy_build_config/extension_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ void ExtensionRegistry::registerFactories() {
Envoy::Extensions::HttpFilters::PlatformBridge::forceRegisterPlatformBridgeFilterFactory();
Envoy::Extensions::HttpFilters::RouteCacheReset::forceRegisterRouteCacheResetFilterFactory();
Envoy::Extensions::HttpFilters::RouterFilter::forceRegisterRouterFilterConfig();
Envoy::Extensions::HttpFilters::SocketSelection::forceRegisterSocketSelectionFilterFactory();
Envoy::Extensions::NetworkFilters::HttpConnectionManager::
forceRegisterHttpConnectionManagerFilterConfigFactory();
Envoy::Extensions::StatSinks::MetricsService::forceRegisterMetricsServiceSinkFactory();
Expand Down
1 change: 1 addition & 0 deletions mobile/envoy_build_config/extension_registry.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "library/common/extensions/filters/http/local_error/config.h"
#include "library/common/extensions/filters/http/platform_bridge/config.h"
#include "library/common/extensions/filters/http/route_cache_reset/config.h"
#include "library/common/extensions/filters/http/socket_selection/config.h"

namespace Envoy {
class ExtensionRegistry {
Expand Down
1 change: 1 addition & 0 deletions mobile/envoy_build_config/extensions_build_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ EXTENSIONS = {
"envoy.filters.http.dynamic_forward_proxy": "//source/extensions/filters/http/dynamic_forward_proxy:config",
"envoy.filters.http.local_error": "@envoy_mobile//library/common/extensions/filters/http/local_error:config",
"envoy.filters.http.platform_bridge": "@envoy_mobile//library/common/extensions/filters/http/platform_bridge:config",
"envoy.filters.http.socket_selection": "@envoy_mobile//library/common/extensions/filters/http/socket_selection:config",
"envoy.filters.http.route_cache_reset": "@envoy_mobile//library/common/extensions/filters/http/route_cache_reset:config",
"envoy.filters.http.router": "//source/extensions/filters/http/router:config",
"envoy.filters.network.http_connection_manager": "//source/extensions/filters/network/http_connection_manager:config",
Expand Down
79 changes: 3 additions & 76 deletions mobile/library/common/config/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,9 @@ const char* config_template = R"(
max_interval: 60s
http_filters:
#{custom_filters}
- name: envoy.filters.http.socket_selection
typed_config:
"@type": type.googleapis.com/envoymobile.extensions.filters.http.socket_selection.SocketSelection
- name: envoy.filters.http.local_error
typed_config:
"@type": type.googleapis.com/envoymobile.extensions.filters.http.local_error.LocalError
Expand Down Expand Up @@ -318,24 +321,6 @@ R"(
max_ejection_time: 0.001s
interval: 1s
typed_extension_protocol_options: *http1_protocol_options_defs
- name: base_wlan
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
cluster_type: *base_cluster_type
transport_socket: *base_tls_socket
upstream_connection_options: *upstream_opts
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *http1_protocol_options_defs
- name: base_wwan
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
cluster_type: *base_cluster_type
transport_socket: *base_tls_socket
upstream_connection_options: *upstream_opts
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *http1_protocol_options_defs
- name: base_clear
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
Expand All @@ -345,24 +330,6 @@ R"(
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *http1_protocol_options_defs
- name: base_wlan_clear
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
cluster_type: *base_cluster_type
transport_socket: { name: envoy.transport_sockets.raw_buffer }
upstream_connection_options: *upstream_opts
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *http1_protocol_options_defs
- name: base_wwan_clear
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
cluster_type: *base_cluster_type
transport_socket: { name: envoy.transport_sockets.raw_buffer }
upstream_connection_options: *upstream_opts
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *http1_protocol_options_defs
- name: base_h2
http2_protocol_options: {}
connect_timeout: *connect_timeout
Expand All @@ -373,26 +340,6 @@ R"(
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *base_protocol_options
- name: base_wlan_h2
http2_protocol_options: {}
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
cluster_type: *base_cluster_type
transport_socket: *base_tls_h2_socket
upstream_connection_options: *upstream_opts
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *base_protocol_options
- name: base_wwan_h2
http2_protocol_options: {}
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
cluster_type: *base_cluster_type
transport_socket: *base_tls_h2_socket
upstream_connection_options: *upstream_opts
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *base_protocol_options
- name: base_alpn
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
Expand All @@ -402,26 +349,6 @@ R"(
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *base_protocol_options
- name: base_wlan_alpn
http2_protocol_options: {}
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
cluster_type: *base_cluster_type
transport_socket: *base_tls_socket
upstream_connection_options: *upstream_opts
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *base_protocol_options
- name: base_wwan_alpn
http2_protocol_options: {}
connect_timeout: *connect_timeout
lb_policy: CLUSTER_PROVIDED
cluster_type: *base_cluster_type
transport_socket: *base_tls_socket
upstream_connection_options: *upstream_opts
circuit_breakers: *circuit_breakers_settings
outlier_detection: *base_outlier_detection
typed_extension_protocol_options: *base_protocol_options
stats_flush_interval: *stats_flush_interval
stats_sinks: *stats_sinks
stats_config:
Expand Down
11 changes: 5 additions & 6 deletions mobile/library/common/engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,9 @@
namespace Envoy {

Engine::Engine(envoy_engine_callbacks callbacks, envoy_logger logger,
envoy_event_tracker event_tracker, std::atomic<envoy_network_t>& preferred_network)
envoy_event_tracker event_tracker)
: callbacks_(callbacks), logger_(logger), event_tracker_(event_tracker),
dispatcher_(std::make_unique<Event::ProvisionalDispatcher>()),
preferred_network_(preferred_network) {
dispatcher_(std::make_unique<Event::ProvisionalDispatcher>()) {
// Ensure static factory registration occurs on time.
// TODO: ensure this is only called one time once multiple Engine objects can be allocated.
// https://github.com/lyft/envoy-mobile/issues/332
Expand Down Expand Up @@ -114,9 +113,9 @@ envoy_status_t Engine::main(const std::string config, const std::string log_leve
stat_name_set_ = client_scope_->symbolTable().makeSet("pulse");
auto api_listener = server_->listenerManager().apiListener()->get().http();
ASSERT(api_listener.has_value());
http_client_ = std::make_unique<Http::Client>(
api_listener.value(), *dispatcher_, server_->serverFactoryContext().scope(),
preferred_network_, server_->api().randomGenerator());
http_client_ = std::make_unique<Http::Client>(api_listener.value(), *dispatcher_,
server_->serverFactoryContext().scope(),
server_->api().randomGenerator());
dispatcher_->drain(server_->dispatcher());
if (callbacks_.on_engine_running != nullptr) {
callbacks_.on_engine_running(callbacks_.context);
Expand Down
5 changes: 1 addition & 4 deletions mobile/library/common/engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,8 @@ class Engine : public Logger::Loggable<Logger::Id::main> {
* @param callbacks, the callbacks to use for engine lifecycle monitoring.
* @param logger, the callbacks to use for engine logging.
* @param event_tracker, the event tracker to use for the emission of events.
* @param preferred_network, hook to obtain the preferred network for new streams.
*/
Engine(envoy_engine_callbacks callbacks, envoy_logger logger, envoy_event_tracker event_tracker,
std::atomic<envoy_network_t>& preferred_network);
Engine(envoy_engine_callbacks callbacks, envoy_logger logger, envoy_event_tracker event_tracker);

/**
* Engine destructor.
Expand Down Expand Up @@ -143,7 +141,6 @@ class Engine : public Logger::Loggable<Logger::Id::main> {
Logger::EventTrackingDelegatePtr log_delegate_ptr_{};
Server::Instance* server_{};
Server::ServerLifecycleNotifier::HandlePtr postinit_callback_handler_;
std::atomic<envoy_network_t>& preferred_network_;
// main_thread_ should be destroyed first, hence it is the last member variable. Objects with
// instructions scheduled on the main_thread_ need to have a longer lifetime.
std::thread main_thread_{}; // Empty placeholder to be populated later.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
load(
"@envoy//bazel:envoy_build_system.bzl",
"envoy_cc_extension",
"envoy_extension_package",
"envoy_proto_library",
)

licenses(["notice"]) # Apache 2

envoy_extension_package()

envoy_proto_library(
name = "filter",
srcs = ["filter.proto"],
)

envoy_cc_extension(
name = "socket_selection_filter_lib",
srcs = ["filter.cc"],
hdrs = ["filter.h"],
repository = "@envoy",
deps = [
":filter_cc_proto",
"//library/common/http:internal_headers_lib",
"//library/common/network:mobile_utility_lib",
"//library/common/types:c_types_lib",
"@envoy//envoy/http:codes_interface",
"@envoy//envoy/http:filter_interface",
"@envoy//source/common/grpc:common_lib",
"@envoy//source/common/grpc:status_lib",
"@envoy//source/common/http:codes_lib",
"@envoy//source/common/http:header_map_lib",
"@envoy//source/common/http:headers_lib",
"@envoy//source/common/http:utility_lib",
"@envoy//source/extensions/filters/http/common:pass_through_filter_lib",
],
)

envoy_cc_extension(
name = "config",
srcs = ["config.cc"],
hdrs = ["config.h"],
repository = "@envoy",
deps = [
":socket_selection_filter_lib",
"@envoy//source/extensions/filters/http/common:factory_base_lib",
],
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#include "library/common/extensions/filters/http/socket_selection/config.h"

#include "library/common/extensions/filters/http/socket_selection/filter.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace SocketSelection {

Http::FilterFactoryCb SocketSelectionFilterFactory::createFilterFactoryFromProtoTyped(
const envoymobile::extensions::filters::http::socket_selection::SocketSelection&,
const std::string&, Server::Configuration::FactoryContext&) {

return [](Http::FilterChainFactoryCallbacks& callbacks) -> void {
callbacks.addStreamFilter(std::make_shared<SocketSelectionFilter>());
};
}

/**
* Static registration for the SocketSelection filter. @see NamedHttpFilterConfigFactory.
*/
REGISTER_FACTORY(SocketSelectionFilterFactory, Server::Configuration::NamedHttpFilterConfigFactory);

} // namespace SocketSelection
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#include <string>

#include "source/extensions/filters/http/common/factory_base.h"

#include "library/common/extensions/filters/http/socket_selection/filter.pb.h"
#include "library/common/extensions/filters/http/socket_selection/filter.pb.validate.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace SocketSelection {

/**
* Config registration for the socket_selection filter. @see NamedHttpFilterConfigFactory.
*/
class SocketSelectionFilterFactory
: public Common::FactoryBase<
envoymobile::extensions::filters::http::socket_selection::SocketSelection> {
public:
SocketSelectionFilterFactory() : FactoryBase("socket_selection") {}

private:
::Envoy::Http::FilterFactoryCb createFilterFactoryFromProtoTyped(
const envoymobile::extensions::filters::http::socket_selection::SocketSelection& config,
const std::string& stats_prefix, Server::Configuration::FactoryContext& context) override;
};

DECLARE_FACTORY(SocketSelectionFilterFactory);

} // namespace SocketSelection
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#include "library/common/extensions/filters/http/socket_selection/filter.h"

#include "envoy/server/filter_config.h"

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

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace SocketSelection {

Http::FilterHeadersStatus SocketSelectionFilter::decodeHeaders(Http::RequestHeaderMap&, bool) {
ENVOY_LOG(debug, "SocketSelectionFilter::decodeHeaders");

envoy_network_t network = Network::MobileUtility::getPreferredNetwork();
ENVOY_LOG(debug, "current preferred network: {}", network);

auto connection_options = Network::MobileUtility::getUpstreamSocketOptions(network);
decoder_callbacks_->addUpstreamSocketOptions(connection_options);

return Http::FilterHeadersStatus::Continue;
}

} // namespace SocketSelection
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#pragma once

#include "envoy/http/filter.h"

#include "source/common/common/logger.h"
#include "source/extensions/filters/http/common/pass_through_filter.h"

#include "library/common/extensions/filters/http/socket_selection/filter.pb.h"
#include "library/common/types/c_types.h"

namespace Envoy {
namespace Extensions {
namespace HttpFilters {
namespace SocketSelection {

/**
* Filter to set upstream socket options based on network conditions.
*/
class SocketSelectionFilter final : public Http::PassThroughFilter,
public Logger::Loggable<Logger::Id::filter> {
public:
// Http::StreamDecoderFilter
Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers,
bool end_stream) override;
};

} // namespace SocketSelection
} // namespace HttpFilters
} // namespace Extensions
} // namespace Envoy
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
syntax = "proto3";

package envoymobile.extensions.filters.http.socket_selection;

message SocketSelection {
}
2 changes: 2 additions & 0 deletions mobile/library/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ envoy_cc_library(
"//library/common/data:utility_lib",
"//library/common/event:provisional_dispatcher_lib",
"//library/common/extensions/filters/http/local_error:local_error_filter_lib",
"//library/common/extensions/filters/http/socket_selection:socket_selection_filter_lib",
"//library/common/http:header_utility_lib",
"//library/common/network:mobile_utility_lib",
"//library/common/network:synthetic_address_lib",
"//library/common/types:c_types_lib",
"@envoy//envoy/buffer:buffer_interface",
Expand Down
Loading

0 comments on commit 0f1b14e

Please sign in to comment.