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

test: Fix O(1/32k) flakiness in H2 flood tests that disable writes based on source port of outgoing connections. #14695

Merged
merged 1 commit into from
Jan 14, 2021
Merged
Changes from all 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
29 changes: 18 additions & 11 deletions test/integration/http2_flood_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,24 +36,33 @@ class SocketInterfaceSwap {
// Object of this class hold the state determining the IoHandle which
// should return EAGAIN from the `writev` call.
struct IoHandleMatcher {
bool shouldReturnEgain(uint32_t port) const {
bool shouldReturnEgain(uint32_t src_port, uint32_t dst_port) const {
absl::ReaderMutexLock lock(&mutex_);
return port == port_ && writev_returns_egain_;
return writev_returns_egain_ && (src_port == src_port_ || dst_port == dst_port_);
}

// Source port to match. The port specified should be associated with a listener.
void setSourcePort(uint32_t port) {
absl::WriterMutexLock lock(&mutex_);
port_ = port;
src_port_ = port;
}

// Destination port to match. The port specified should be associated with a listener.
void setDestinationPort(uint32_t port) {
absl::WriterMutexLock lock(&mutex_);
dst_port_ = port;
}

void setWritevReturnsEgain() {
absl::WriterMutexLock lock(&mutex_);
ASSERT(src_port_ != 0 || dst_port_ != 0);
writev_returns_egain_ = true;
}

private:
mutable absl::Mutex mutex_;
uint32_t port_ ABSL_GUARDED_BY(mutex_) = 0;
uint32_t src_port_ ABSL_GUARDED_BY(mutex_) = 0;
uint32_t dst_port_ ABSL_GUARDED_BY(mutex_) = 0;
bool writev_returns_egain_ ABSL_GUARDED_BY(mutex_) = false;
};

Expand All @@ -64,7 +73,8 @@ class SocketInterfaceSwap {
[writev_matcher = writev_matcher_](
Envoy::Network::TestIoSocketHandle* io_handle, const Buffer::RawSlice*,
uint64_t) -> absl::optional<Api::IoCallUint64Result> {
if (writev_matcher->shouldReturnEgain(io_handle->localAddress()->ip()->port())) {
if (writev_matcher->shouldReturnEgain(io_handle->localAddress()->ip()->port(),
io_handle->peerAddress()->ip()->port())) {
return Api::IoCallUint64Result(
0, Api::IoErrorPtr(Network::IoSocketError::getIoSocketEagainInstance(),
Network::IoSocketError::deleteIoError));
Expand Down Expand Up @@ -200,8 +210,7 @@ void Http2FloodMitigationTest::floodClient(const Http2Frame& frame, uint32_t num
waitForNextUpstreamRequest();

// Make Envoy's writes into the upstream connection to return EAGAIN
writev_matcher_->setSourcePort(
fake_upstream_connection_->connection().addressProvider().remoteAddress()->ip()->port());
writev_matcher_->setDestinationPort(fake_upstreams_[0]->localAddress()->ip()->port());

auto buf = serializeFrames(frame, num_frames);

Expand Down Expand Up @@ -303,8 +312,7 @@ Http2FloodMitigationTest::prefillOutboundUpstreamQueue(uint32_t frame_count) {
EXPECT_TRUE(upstream_request_->waitForData(*dispatcher_, 1));

// Make Envoy's writes into the upstream connection to return EAGAIN
writev_matcher_->setSourcePort(
fake_upstream_connection_->connection().addressProvider().remoteAddress()->ip()->port());
writev_matcher_->setDestinationPort(fake_upstreams_[0]->localAddress()->ip()->port());

auto buf = serializeFrames(Http2Frame::makePingFrame(), frame_count);

Expand Down Expand Up @@ -1526,8 +1534,7 @@ TEST_P(Http2FloodMitigationTest, RequestMetadata) {

// Make Envoy's writes into the upstream connection to return EAGAIN, preventing proxying of the
// METADATA frames
writev_matcher_->setSourcePort(
fake_upstream_connection_->connection().addressProvider().remoteAddress()->ip()->port());
writev_matcher_->setDestinationPort(fake_upstreams_[0]->localAddress()->ip()->port());

writev_matcher_->setWritevReturnsEgain();

Expand Down