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

Add the no-wait argument to ovs-vsctl commands for hw-offload #418

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

wizhaoredhat
Copy link
Contributor

The "no-wait" argument is needed for ovs-vsctl commands because there is a chance for ovs-vsctl to timeout.

Adding "--no-wait" to the command line would make it such that ovs-vsctl doesn't wait for ovs-vswitchd to update "cur_cfg"

An example of this timeout is shown here:
ovs-vsctl[3265]: ovs|00001|vsctl|INFO|Called as /bin/ovs-vsctl set Open_vSwitch . other_config:hw-offload=true systemd[1]: ovs-vswitchd.service: start-pre operation timed out. Terminating. ovs-vsctl[3265]: 2023-02-22T22:00:20Z|00002|fatal_signal|WARN|terminating with signal 15 (Terminated) ovs-vsctl[3265]: ovs|00002|fatal_signal|WARN|terminating with signal 15 (Terminated) systemd[1]: ovs-vswitchd.service: Failed with result 'timeout'. systemd[1]: Failed to start Open vSwitch Forwarding Unit.

The "no-wait" argument is needed for ovs-vsctl commands because
there is a chance for ovs-vsctl to timeout.

Adding "--no-wait" to the command line would make it such that
ovs-vsctl doesn't wait for ovs-vswitchd to update "cur_cfg"

An example of this timeout is shown here:
ovs-vsctl[3265]: ovs|00001|vsctl|INFO|Called as /bin/ovs-vsctl set Open_vSwitch . other_config:hw-offload=true
systemd[1]: ovs-vswitchd.service: start-pre operation timed out. Terminating.
ovs-vsctl[3265]: 2023-02-22T22:00:20Z|00002|fatal_signal|WARN|terminating with signal 15 (Terminated)
ovs-vsctl[3265]: ovs|00002|fatal_signal|WARN|terminating with signal 15 (Terminated)
systemd[1]: ovs-vswitchd.service: Failed with result 'timeout'.
systemd[1]: Failed to start Open vSwitch Forwarding Unit.
@github-actions
Copy link

github-actions bot commented Mar 9, 2023

Thanks for your PR,
To run vendors CIs use one of:

  • /test-all: To run all tests for all vendors.
  • /test-e2e-all: To run all E2E tests for all vendors.
  • /test-e2e-nvidia-all: To run all E2E tests for NVIDIA vendor.

To skip the vendors CIs use one of:

  • /skip-all: To skip all tests for all vendors.
  • /skip-e2e-all: To skip all E2E tests for all vendors.
  • /skip-e2e-nvidia-all: To skip all E2E tests for NVIDIA vendor.
    Best regards.

@wizhaoredhat
Copy link
Contributor Author

/cc @bn222
/cc @zshi-redhat
/cc @adrianchiris

@github-actions github-actions bot requested a review from adrianchiris March 9, 2023 20:13
@coveralls
Copy link

Pull Request Test Coverage Report for Build 4378389223

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.09%) to 25.767%

Totals Coverage Status
Change from base Build 4363097788: 0.09%
Covered Lines: 1949
Relevant Lines: 7564

💛 - Coveralls

@bn222
Copy link
Collaborator

bn222 commented Mar 9, 2023

This is a great find. Thanks for figuring this one out. I like the idea of adding no-wait we avoid a timeout from systemd. However, I'm wondering what other implications this has. Afaict, not waiting to finish setting hwol to true isn't going to have an impact elsewhere.

/lgtm

@github-actions github-actions bot added the lgtm label Mar 9, 2023
@bn222 bn222 merged commit 3f31e0b into k8snetworkplumbingwg:master Mar 13, 2023
@zshi-redhat
Copy link
Collaborator

@wizhaoredhat Is this the fix to the ovs-configuration service failure you found on openshift 4.13?

@wizhaoredhat
Copy link
Contributor Author

@wizhaoredhat Is this the fix to the ovs-configuration service failure you found on openshift 4.13?

Yes it was in OCP 4.12.

@wizhaoredhat wizhaoredhat deleted the add_no_wait_ovs branch March 14, 2023 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants