-
Notifications
You must be signed in to change notification settings - Fork 5.5k
quiche: add epoll_server for testing #6650
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 10 commits
1068b8e
30a1dd6
79570f3
fa174ac
f44663a
35c09c6
8105de3
4c18507
05fb83b
2a6b9f8
ea66710
93f08e6
751b7a9
3883e6d
499ccb1
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 |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ load(":genrule_cmd.bzl", "genrule_cmd") | |
| load( | ||
| "@envoy//bazel:envoy_build_system.bzl", | ||
| "envoy_cc_test", | ||
| "envoy_copts", | ||
| "envoy_select_quiche", | ||
| ) | ||
|
|
||
|
|
@@ -48,6 +49,8 @@ genrule( | |
| visibility = ["//visibility:private"], | ||
| ) | ||
|
|
||
| quiche_copt = ["-Wno-unused-parameter"] | ||
|
|
||
| cc_library( | ||
| name = "http2_platform", | ||
| hdrs = [ | ||
|
|
@@ -253,6 +256,48 @@ cc_library( | |
| deps = [":quic_platform_export"], | ||
| ) | ||
|
|
||
| cc_library( | ||
| name = "epoll_server_platform", | ||
| testonly = 1, | ||
| hdrs = [ | ||
| "quiche/epoll_server/platform/api/epoll_address_test_utils.h", | ||
| "quiche/epoll_server/platform/api/epoll_bug.h", | ||
| "quiche/epoll_server/platform/api/epoll_expect_bug.h", | ||
| "quiche/epoll_server/platform/api/epoll_export.h", | ||
| "quiche/epoll_server/platform/api/epoll_logging.h", | ||
| "quiche/epoll_server/platform/api/epoll_ptr_util.h", | ||
| "quiche/epoll_server/platform/api/epoll_test.h", | ||
| "quiche/epoll_server/platform/api/epoll_thread.h", | ||
| "quiche/epoll_server/platform/api/epoll_time.h", | ||
| ], | ||
| visibility = ["//visibility:public"], | ||
| deps = ["@envoy//test/extensions/quic_listeners/quiche/platform:epoll_server_platform_impl_lib"], | ||
| ) | ||
|
|
||
| cc_library( | ||
| name = "epoll_server_lib", | ||
| testonly = 1, | ||
| srcs = [ | ||
| "quiche/epoll_server/fake_simple_epoll_server.cc", | ||
| "quiche/epoll_server/simple_epoll_server.cc", | ||
| ], | ||
| hdrs = [ | ||
| "quiche/epoll_server/fake_simple_epoll_server.h", | ||
| "quiche/epoll_server/simple_epoll_server.h", | ||
| ], | ||
| copts = envoy_copts("@envoy") + quiche_copt, | ||
|
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. Do you need to explicitly add the
Contributor
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. Not needed actually. Removed. |
||
| visibility = ["//visibility:public"], | ||
| deps = [":epoll_server_platform"], | ||
| ) | ||
|
|
||
| envoy_cc_test( | ||
| name = "epoll_server_test", | ||
|
Contributor
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. This test is also defined in test/extensions/quic_listeners/quiche/platform/BUILD, should we delete one of them?
Contributor
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. Sorry, the one is //test/ is used for debugging. I removed that one instead. |
||
| srcs = ["quiche/epoll_server/simple_epoll_server_test.cc"], | ||
| copts = quiche_copt, | ||
| repository = "@envoy", | ||
| deps = [":epoll_server_lib"], | ||
| ) | ||
|
|
||
| envoy_cc_test( | ||
| name = "http2_platform_test", | ||
| srcs = envoy_select_quiche( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #include <sys/socket.h> | ||
|
|
||
| #include <algorithm> | ||
|
|
||
| #include "envoy/network/address.h" | ||
|
|
||
| #include "test/test_common/environment.h" | ||
|
|
||
| namespace epoll_server { | ||
|
|
||
| namespace { | ||
|
|
||
| int addressFamilyUnderTestHelper() { | ||
| std::vector<Envoy::Network::Address::IpVersion> versions = | ||
| Envoy::TestEnvironment::getIpVersionsForTest(); | ||
|
Contributor
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. How about using std::find for clarity? if (std::find(versions.begin(), versions.end(), Envoy::Network::Address::IpVersion::v4) { if (std::find(versions.begin(), versions.end(), Envoy::Network::Address::IpVersion::v6) { return -1;
Contributor
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. done
Contributor
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. hmm, my code in comment doesn't actually work, it should be if (std::find(versions.begin(), versions.end(), Envoy::Network::Address::IpVersion::v4) != versions.end()) { |
||
| if (std::find(versions.begin(), versions.end(), Envoy::Network::Address::IpVersion::v4) != | ||
| versions.end()) { | ||
| return AF_INET; | ||
| } | ||
| if (std::find(versions.begin(), versions.end(), Envoy::Network::Address::IpVersion::v6) != | ||
| versions.end()) { | ||
| return AF_INET6; | ||
| } | ||
| return -1; | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| // Returns the address family to be used for test. Return v4 if the environment | ||
| // supports v4 only or both v4 and v6. Otherwise return v6 or an invalid value. | ||
| int AddressFamilyUnderTestImpl() { | ||
|
Contributor
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. Looks like this is confusingly named, but doing the right thing.
Contributor
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. Renaming requires upstream change. I added a comment here for the return value. |
||
| static const int version = addressFamilyUnderTestHelper(); | ||
| return version; | ||
| } | ||
|
|
||
| } // namespace epoll_server | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #include "extensions/quic_listeners/quiche/platform/quic_bug_tracker_impl.h" | ||
|
|
||
| #define EPOLL_BUG_IMPL QUIC_BUG_IMPL |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #include "extensions/quic_listeners/quiche/platform/quic_expect_bug_impl.h" | ||
|
|
||
| #define EXPECT_EPOLL_BUG_IMPL EXPECT_QUIC_BUG_IMPL |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #define EPOLL_EXPORT | ||
| #define EPOLL_EXPORT_PRIVATE |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #include "extensions/quic_listeners/quiche/platform/quic_logging_impl.h" | ||
|
|
||
| namespace epoll_server { | ||
|
|
||
| #define EPOLL_LOG_IMPL(severity) QUIC_LOG_IMPL(severity) | ||
| #define EPOLL_VLOG_IMPL(verbosity) QUIC_VLOG_IMPL(verbosity) | ||
|
|
||
| #define EPOLL_PLOG_IMPL(severity) QUIC_PLOG_IMPL(severity) | ||
|
|
||
| #define EPOLL_DVLOG_IMPL(verbosity) QUIC_DVLOG_IMPL(verbosity) | ||
|
|
||
| } // namespace epoll_server |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #include <memory> | ||
|
|
||
| namespace epoll_server { | ||
|
|
||
| template <typename T, typename... Args> std::unique_ptr<T> EpollMakeUniqueImpl(Args&&... args) { | ||
|
Contributor
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. nit: template <typename T, typename... Args>
Contributor
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. alias doesn't work with function template.
Contributor
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. Sorry about the wrong suggestion, and thanks for trying:) |
||
| return std::make_unique<T>(std::forward<Args>(args)...); | ||
| } | ||
|
|
||
| } // namespace epoll_server | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #include "gmock/gmock.h" | ||
| #include "gtest/gtest.h" | ||
|
|
||
| using EpollTestImpl = ::testing::Test; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #include "extensions/quic_listeners/quiche/platform/quic_thread_impl.h" | ||
|
|
||
| namespace epoll_server { | ||
|
|
||
| using EpollThreadImpl = quic::QuicThreadImpl; | ||
|
|
||
| } // namespace epoll_server |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| #pragma once | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| // This file is part of the QUICHE platform implementation, and is not to be | ||
| // consumed or referenced directly by other Envoy code. It serves purely as a | ||
| // porting layer for QUICHE. | ||
|
|
||
| #include "absl/time/clock.h" | ||
|
|
||
| namespace epoll_server { | ||
|
|
||
| inline int64_t WallTimeNowInUsecImpl() { return absl::GetCurrentTimeNanos() / 1000; } | ||
|
|
||
| } // namespace epoll_server |
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.
Why are these
cc_libraryrather thanenvoy_cc_library? The latter is preferred, it simplifies Google import and removes boilerplate.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.
I think the original intention was that these targets are not in the envoy workspace and to use envoy_cc_library we need to explicitly specify
repository = "@envoy". But since it benefits to code import, I just switched to use envoy_cc_library.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.
I guess internally we will use a different QUICHE binding actually. But, it's good either way.