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

Bind to a list of Network Interfaces #471

Merged
merged 33 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
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
10 changes: 10 additions & 0 deletions include/aws/http/connection_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,16 @@ struct aws_http_connection_manager_options {
* timeout will be closed automatically.
*/
uint64_t max_connection_idle_in_milliseconds;

/**
* (Optional)
* An `aws_array_list<struct aws_byte_cursor>` of network interface names. The manager will distribute the
* connections across network interface names provided in this list. If any interface name is invalid, goes down, or
* has any issues like network access, you will see connection failures. If `socket_options.network_interface_name`
* is set, this list will be ignored and all the connections will go to that network interface. This option is only
* supported on Linux and macOS and will be ignored on other platforms.
*/
const struct aws_array_list *network_interface_names_list;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just have users pass a normal-ass C-array, and separate length.

Suggested change
const struct aws_array_list *network_interface_names_list;
const struct aws_byte_cursor *network_interface_names_array;
size_t num_network_interface_names;

There's already precedent for this, see a few lines up:

const struct aws_http2_setting *initial_settings_array;
size_t num_initial_settings;

It's simpler for everyone to just use vanilla C arrays. Most uses of this will be in language bindings, and most other languages have their own easier-to-use way to create an array. C++ users can do: std::vector<aws_byte_cursor> names and pass it like names.data(), names.size(). C users can still use aws_array_list if they want, and pass a pointer to the first item. Or simply use aws_mem_calloc().

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, in S3, the precedence is array list for checksum algorithms list. I debated between char *[16] and aws_array_arraylist, but aws_byte_cursor * seems much nicer. I will update it to that.

};

AWS_EXTERN_C_BEGIN
Expand Down
55 changes: 54 additions & 1 deletion source/connection_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,17 @@ struct aws_http_connection_manager {
*/
struct aws_task *cull_task;
struct aws_event_loop *cull_event_loop;

/*
* An aws_array_list<struct aws_string *> of network interface names to distribute the connections using the
* round-robin algorithm. We picked round-robin because it is trivial to implement and good enough. We can later
* update to a more complex distribution algorithm if required.
*/
struct aws_array_list network_interface_names_list;
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: _list makes me think aws_linked_list

Suggested change
struct aws_array_list network_interface_names_list;
struct aws_array_list network_interface_names;

/*
* Current index in the network_interface_names list.
*/
size_t network_interface_names_list_index;
};

struct aws_http_connection_manager_snapshot {
Expand Down Expand Up @@ -703,6 +714,13 @@ static void s_aws_http_connection_manager_finish_destroy(struct aws_http_connect
aws_http_proxy_config_destroy(manager->proxy_config);
}

for (size_t i = 0; i < aws_array_list_length(&manager->network_interface_names_list); i++) {
struct aws_string *interface_name = NULL;
aws_array_list_get_at(&manager->network_interface_names_list, &interface_name, i);
aws_string_destroy(interface_name);
}
aws_array_list_clean_up(&manager->network_interface_names_list);

/*
* If this task exists then we are actually in the corresponding event loop running the final destruction task.
* In that case, we've already cancelled this task and when you cancel, it runs synchronously. So in that
Expand Down Expand Up @@ -896,6 +914,25 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(
manager->max_closed_streams = options->max_closed_streams;
manager->http2_conn_manual_window_management = options->http2_conn_manual_window_management;

manager->network_interface_names_list_index = 0;
size_t network_interface_names_list_length = options->network_interface_names_list != NULL
? aws_array_list_length(options->network_interface_names_list)
: 0;
/* Ignore the network_interface_names_list if socket_options already contains a network_interface_name. */
if (manager->socket_options.network_interface_name[0] == '\0' && network_interface_names_list_length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

debatable: maybe raise an error if both are set? instead of silently ignoring one of them?

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, updated.

aws_array_list_init_dynamic(
&manager->network_interface_names_list,
allocator,
network_interface_names_list_length,
sizeof(struct aws_string *));
for (size_t i = 0; i < network_interface_names_list_length; i++) {
struct aws_byte_cursor interface_name;
aws_array_list_get_at(options->network_interface_names_list, &interface_name, i);
struct aws_string *interface_name_str = aws_string_new_from_cursor(allocator, &interface_name);
aws_array_list_push_back(&manager->network_interface_names_list, &interface_name_str);
}
}

/* NOTHING can fail after here */
s_schedule_connection_culling(manager);

Expand Down Expand Up @@ -990,7 +1027,23 @@ static int s_aws_http_connection_manager_new_connection(struct aws_http_connecti
options.host_name = aws_byte_cursor_from_string(manager->host);
options.port = manager->port;
options.initial_window_size = manager->initial_window_size;
options.socket_options = &manager->socket_options;
struct aws_socket_options socket_options = manager->socket_options;
if (aws_array_list_length(&manager->network_interface_names_list)) {
struct aws_string *interface_name = NULL;
aws_array_list_get_at(
&manager->network_interface_names_list, &interface_name, manager->network_interface_names_list_index);
manager->network_interface_names_list_index = (manager->network_interface_names_list_index + 1) %
aws_array_list_length(&manager->network_interface_names_list);
#if defined(_MSC_VER)
# pragma warning(push)
# pragma warning(disable : 4996) /* deprecation */
waahm7 marked this conversation as resolved.
Show resolved Hide resolved
#endif
strncpy(socket_options.network_interface_name, aws_string_c_str(interface_name), AWS_NETWORK_INTERFACE_MAX_LEN);
graebm marked this conversation as resolved.
Show resolved Hide resolved
#if defined(_MSC_VER)
# pragma warning(pop)
#endif
}
options.socket_options = &socket_options;
options.on_setup = s_aws_http_connection_manager_on_connection_setup;
options.on_shutdown = s_aws_http_connection_manager_on_connection_shutdown;
options.manual_window_management = manager->enable_read_back_pressure;
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ add_net_test_case(test_connection_manager_idle_culling_single)
add_net_test_case(test_connection_manager_idle_culling_many)
add_net_test_case(test_connection_manager_idle_culling_mixture)
add_net_test_case(test_connection_manager_idle_culling_refcount)
add_net_test_case(test_connection_manager_with_network_interface_list)

# tests where we establish real connections
add_net_test_case(test_connection_manager_single_connection)
Expand Down
51 changes: 51 additions & 0 deletions tests/test_connection_manager.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ struct cm_tester_options {
struct aws_http2_setting *initial_settings_array;
size_t num_initial_settings;
bool self_lib_init;
struct aws_array_list *verify_network_interface_names;
};

struct cm_tester {
Expand All @@ -73,6 +74,7 @@ struct cm_tester {
struct aws_tls_ctx_options tls_ctx_options;
struct aws_tls_connection_options tls_connection_options;
struct aws_http_proxy_options *verify_proxy_options;
struct aws_array_list *verify_network_interface_names;

struct aws_mutex lock;
struct aws_condition_variable signal;
Expand Down Expand Up @@ -219,6 +221,7 @@ static int s_cm_tester_init(struct cm_tester_options *options) {
.http2_prior_knowledge = !options->use_tls && options->http2,
.initial_settings_array = options->initial_settings_array,
.num_initial_settings = options->num_initial_settings,
.network_interface_names_list = options->verify_network_interface_names,
};

if (options->mock_table) {
Expand All @@ -234,6 +237,7 @@ static int s_cm_tester_init(struct cm_tester_options *options) {
}

tester->mock_table = options->mock_table;
tester->verify_network_interface_names = options->verify_network_interface_names;

aws_atomic_store_int(&tester->next_connection_id, 0);

Expand Down Expand Up @@ -729,6 +733,16 @@ static int s_aws_http_connection_manager_create_connection_sync_mock(
const struct aws_http_client_connection_options *options) {
struct cm_tester *tester = &s_tester;

if (tester->verify_network_interface_names) {
struct aws_byte_cursor interface_name;
aws_array_list_get_at(
tester->verify_network_interface_names,
&interface_name,
aws_atomic_load_int(&tester->next_connection_id) %
aws_array_list_length(tester->verify_network_interface_names));
ASSERT_TRUE(aws_byte_cursor_eq_c_str(&interface_name, options->socket_options->network_interface_name));
}

size_t next_connection_id = aws_atomic_fetch_add(&tester->next_connection_id, 1);

ASSERT_SUCCESS(aws_mutex_lock(&tester->lock));
Expand Down Expand Up @@ -819,6 +833,43 @@ static struct aws_http_connection_manager_system_vtable s_synchronous_mocks = {
.aws_http_connection_get_version = s_aws_http_connection_manager_connection_get_version_sync_mock,
};

static int s_test_connection_manager_with_network_interface_list(struct aws_allocator *allocator, void *ctx) {
(void)ctx;
struct aws_array_list interface_names_list;
aws_array_list_init_dynamic(&interface_names_list, allocator, 3, sizeof(struct aws_byte_cursor));
struct aws_byte_cursor ens32 = aws_byte_cursor_from_c_str("ens32");
struct aws_byte_cursor ens64 = aws_byte_cursor_from_c_str("ens64");
struct aws_byte_cursor ens96 = aws_byte_cursor_from_c_str("ens96");
aws_array_list_push_back(&interface_names_list, &ens32);
aws_array_list_push_back(&interface_names_list, &ens64);
aws_array_list_push_back(&interface_names_list, &ens96);

struct cm_tester_options options = {
.allocator = allocator,
.max_connections = 20,
.mock_table = &s_synchronous_mocks,
.verify_network_interface_names = &interface_names_list,
};

ASSERT_SUCCESS(s_cm_tester_init(&options));
size_t num_connections = 6;
for (size_t i = 0; i < num_connections; ++i) {
s_add_mock_connections(1, AWS_NCRT_SUCCESS, i % 1 == 0);
}
s_acquire_connections(num_connections);

ASSERT_SUCCESS(s_wait_on_connection_reply_count(num_connections));
ASSERT_SUCCESS(s_release_connections(num_connections, false));
ASSERT_UINT_EQUALS(0, s_tester.connection_errors);

ASSERT_SUCCESS(s_cm_tester_clean_up());
aws_array_list_clean_up(&interface_names_list);
return AWS_OP_SUCCESS;
}
AWS_TEST_CASE(
test_connection_manager_with_network_interface_list,
s_test_connection_manager_with_network_interface_list);

static int s_test_connection_manager_acquire_release_mix_synchronous(struct aws_allocator *allocator, void *ctx) {
(void)ctx;

Expand Down
Loading