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

Reapply "udp: set Don't Fragment(DF) bit in IP packet header (#36341)" #36437

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

danzh2010
Copy link
Contributor

@danzh2010 danzh2010 commented Oct 3, 2024

Commit Message: removed the static_assert as it is expected that some platform, especially some iOS versions supports neither of the socket options. In this case, Envoy wont' set DF bit.

Additional Description: reland #36341
Risk Level: low
Testing: new unit tests
Docs Changes: N/A
Release Notes: Yes
Platform Specific Features: N/A
Runtime guard: envoy.reloadable_features.udp_set_do_not_fragment

…oxy#36341)"

This reverts commit ea1d345.

Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Signed-off-by: Dan Zhang <danzh@google.com>
Copy link

CC @envoyproxy/runtime-guard-changes: FYI only for changes made to (source/common/runtime/runtime_features.cc).

🐱

Caused by: #36437 was opened by danzh2010.

see: more, trace.

@@ -1330,7 +1330,7 @@ TEST_P(ClientIntegrationTest, TestProxyResolutionApi) {

// This test is simply to test the IPv6 connectivity check and DNS refresh and make sure the code
// doesn't crash. It doesn't really test the actual network change event.
TEST_P(ClientIntegrationTest, OnNetworkChanged) {
TEST_P(ClientIntegrationTest, OnNetworkChanged1) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: Revert this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to trigger mobile tests. Looks like the tests are still failing due to bulk transfer error: https://github.com/envoyproxy/envoy/actions/runs/11166604324/job/31041663403

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha! It's all good then :)

Copy link
Contributor Author

@danzh2010 danzh2010 Oct 3, 2024

Choose a reason for hiding this comment

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

In fact, those failures look weird, can you take a look?

Copy link
Member

Choose a reason for hiding this comment

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

I believe it's related to #36426 (the issue, not the PR). The flakiness is still there caused by AndroidEngineExplicitFlowTest.post_multipleRequests_randomBehavior. Let me take a closer look at it further. For the time being, you just may need to retest.

@danzh2010
Copy link
Contributor Author

/retest

@danzh2010 danzh2010 changed the title Reapply "udp: set Don't Fragment(DF) bit in IP packet header (#36341) Reapply "udp: set Don't Fragment(DF) bit in IP packet header (#36341)" Oct 3, 2024
@fredyw
Copy link
Member

fredyw commented Oct 3, 2024

/retest

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