-
Notifications
You must be signed in to change notification settings - Fork 204
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
OCPBUGS-38468: use default route mtu #4315
Conversation
Skipping CI for Draft Pull Request. |
@eslutsky: This pull request references Jira Issue OCPBUGS-38468, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. In response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
/test metal-periodic-test |
@eslutsky: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
/test e2e-aws-tests-periodic |
c934c8f
to
c044607
Compare
/test e2e-aws-tests-periodic |
84c39e3
to
97da4ad
Compare
@eslutsky: This pull request references Jira Issue OCPBUGS-38468, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
/test e2e-aws-tests-periodic |
97da4ad
to
5c5bb6e
Compare
/test verify |
/retest |
5c5bb6e
to
cd512cc
Compare
/retest |
logs related to those failures ,
|
/test e2e-aws-tests-arm |
cd512cc
to
46423ba
Compare
eef83e2
to
89b0f65
Compare
pkg/util/net.go
Outdated
@@ -266,3 +274,55 @@ func GetHostIPv6(ipHint string) (string, error) { | |||
|
|||
return "", fmt.Errorf("unable to find host IPv6 address") | |||
} | |||
|
|||
// Find the Default route Interface based on ipv4 or ipv6 routes. | |||
func FindDefaultRouteIface() (iface *tcpnet.Interface, err error) { |
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 we need to pass the configuration here to determine if MicroShift is running on ipv4, ipv6, or both.
Imagine you have different NICs in the host for each IP family. Default routes would be configured on different devices, therefore they could have different MTUs.
If MicroShift is running on ipv6 check only ipv6 routes.
If MicroShift is running on ipv4 check only ipv4 routes.
If MicroShift is running on both, check both and pick the smallest.
Or is there something I am missing?
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'm not sure its a common scenario for edge devices when you have different Default routes set across different IP families .
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.
we are calling FindDefaultRouteIface from the ovn package which have its own configuration. the IPv4/6 detection logic is in the config struct .
6722393
to
864423c
Compare
/retest |
864423c
to
8e384fb
Compare
5f436e7
to
f7fd5ec
Compare
/test e2e-aws-tests-bootc |
/test e2e-aws-tests-bootc-arm |
/test e2e-aws-tests-bootc |
f7fd5ec
to
b7935e7
Compare
Signed-off-by: Evgeny Slutsky <[email protected]>
b7935e7
to
fb380ce
Compare
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eslutsky, pacevedom The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@eslutsky: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
@eslutsky: Jira Issue OCPBUGS-38468 is in an unrecognized state (ON_QA) and will not be moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
determine if the host has a default gateway based on IPv4 or IPv6 routes, then use its MTU,
the original solution was based on net.GetHostIP which didn't cover cases when we dont have any network connection (hardcoded nodeIP with loopback) .