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

perf: add config to enable looking up local interface name for upstream connections #19758

Merged
merged 22 commits into from
Mar 16, 2022

Conversation

junr03
Copy link
Member

@junr03 junr03 commented Jan 31, 2022

Commit Message: add config to enable looking up local interface name for upstream connections
Risk Level: low, bug fix
Testing: integration test
Release Notes: added
Runtime guard: envoy.reloadable_features.disable_local_interface_name_for_upstream_connection
Fixes #19717

Jose Nino added 2 commits January 31, 2022 15:04
…ame for upstream connections

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
@yanavlasov
Copy link
Contributor

/wait-any

Signed-off-by: Jose Nino <jnino@lyft.com>
@repokitteh-read-only
Copy link

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #19758 was synchronize by junr03.

see: more, trace.

Signed-off-by: Jose Nino <jnino@lyft.com>
Copy link
Member Author

@junr03 junr03 left a comment

Choose a reason for hiding this comment

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

@alyssawilk I managed to string config through all the way. It took a moment because there was no clear way to do so outside of the pattern established for SOCKET_OPTIONS. Lmk what you think.

envoy/network/connection.h Outdated Show resolved Hide resolved
source/common/network/connection_impl.cc Show resolved Hide resolved
Jose Nino added 3 commits February 4, 2022 09:59
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
api/envoy/config/cluster/v3/cluster.proto Outdated Show resolved Hide resolved
api/envoy/config/cluster/v3/cluster.proto Outdated Show resolved Hide resolved
api/envoy/config/cluster/v3/cluster.proto Show resolved Hide resolved
@junr03 junr03 changed the title perf: add runtime guard to disable looking up local interface name for upstream connections perf: add config to enable looking up local interface name for upstream connections Feb 7, 2022
fmt
Signed-off-by: Jose Nino <jnino@lyft.com>
@yanavlasov
Copy link
Contributor

/wait-any

Signed-off-by: Jose Nino <jnino@lyft.com>
@junr03
Copy link
Member Author

junr03 commented Feb 9, 2022

@yanavlasov @alyssawilk @markdroth updated!

Signed-off-by: Jose Nino <jnino@lyft.com>
@yanavlasov
Copy link
Contributor

Looks like CI is not happy.

/wait

Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: José Ulises Niño Rivera <junr03@users.noreply.github.com>
alyssawilk
alyssawilk previously approved these changes Mar 14, 2022
Copy link
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

LGTM pending windows CI :-)

Jose Nino added 3 commits March 14, 2022 15:34
fix
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
Signed-off-by: Jose Nino <jnino@lyft.com>
@alyssawilk
Copy link
Contributor

Looks like windows still needs a pass
/wait

Signed-off-by: Jose Nino <jnino@lyft.com>
@yanavlasov yanavlasov merged commit 8cbbf66 into main Mar 16, 2022
@mattklein123 mattklein123 deleted the perf-fix branch March 17, 2022 17:38
@mattklein123 mattklein123 added the backport/approved Approved backports to stable releases label Mar 18, 2022
@mattklein123
Copy link
Member

cc @oschaaf we should backport this.

@cboitel
Copy link

cboitel commented Mar 29, 2022

No news on a backport ?

@phlax
Copy link
Member

phlax commented Mar 30, 2022

cc @pradeepcrao

@pradeepcrao
Copy link
Contributor

I will get to this end of this week.

pradeepcrao added a commit to pradeepcrao/envoy that referenced this pull request Apr 16, 2022
name for upstream connections (envoyproxy#19758)

Signed-off-by: Pradeep Rao <pcrao@google.com>
htuch pushed a commit that referenced this pull request May 6, 2022
… name for upstream connections (#19758) (#20860)

Backport #19758 to 1.21

Risk Level:
Testing: //test/...
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A

Signed-off-by: Pradeep Rao <pcrao@google.com>
ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022
…am connections (envoyproxy#19758)

* connection: add runtime guard to disable looking up local interface name for upstream connections

Signed-off-by: Jose Nino <jnino@lyft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api backport/approved Approved backports to stable releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Critical perf regression in getifaddrs syscall under load