Skip to content

Bug 1935539: vSphere: Disable tx checksum offload #2471

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.

Sign up for GitHub

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

Jump to bottom
Closed
jcpowermac wants to merge 1 commit into from
Closed

Bug 1935539: vSphere: Disable tx checksum offload #2471

jcpowermac wants to merge 1 commit into from

Conversation

@jcpowermac
Copy link
Contributor

@jcpowermac jcpowermac commented Mar 19, 2021

Based on customer reports and internal testing
checksum offload introduced with VMXNET34 v4 is
causing checksum issues with openshift-apiserver and
various other components causing installations and upgrades
to fail when using greater than guest hardware version 14.

@jcpowermac
vSphere: Disable tx checksum offload
4a126b2 
Based on customer reports and internal testing
checksum offload introduced with VMXNET34 v4 is
causing checksum issues with openshift-apiserver and
various other components causing installations and upgrades
to fail when using greater than guest hardware version 14.
@openshift-ci-robot openshift-ci-robot added the bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. label Mar 19, 2021
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 19, 2021

@jcpowermac: This pull request references Bugzilla bug 1935539, which is invalid:

  • expected the bug to target the "4.8.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

Details

In response to this:

Bug 1935539: vSphere: Disable tx checksum offload

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/test-infra repository.

@openshift-ci-robot openshift-ci-robot added the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Mar 19, 2021
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 19, 2021
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Mar 19, 2021

This would be a work around only this shouldn't be the long term fix. Also this change needs to be tested in vSphere 6.7 and hw15

@ashcrow ashcrow requested a review from yuqi-zhang March 19, 2021 20:28
@wking
Copy link
Member

wking commented Mar 19, 2021

/bugzilla refresh

@openshift-ci-robot openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Mar 19, 2021
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 19, 2021

@wking: This pull request references Bugzilla bug 1935539, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.8.0) matches configured target release for branch (4.8.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @zhaozhanqi

Details

In response to this:

/bugzilla refresh

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/test-infra repository.

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Mar 19, 2021

/test e2e-vsphere
/test e2e-vsphere-upi

(though this really doesn't test the exact issue since it will be HW 13 in CI)

@kikisdeliveryservice
Copy link
Contributor

kikisdeliveryservice commented Mar 19, 2021

/assign @patrickdillon

@wking
Copy link
Member

wking commented Mar 19, 2021

We were running this same code before after #1606, so it should be pretty safe.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 19, 2021
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 19, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcpowermac, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wking
Copy link
Member

wking commented Mar 19, 2021

/hold

So we see how the non-required vSphere CI works out. Seems like a low risk, but still worth looking at results.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 19, 2021
@trozet
Copy link
Contributor

trozet commented Mar 19, 2021 •
edited
Loading

@jcpowermac is this targeted for OVN deployments or openshift-sdn as well? I'm asking because we do some vmxnet3 driver workaround in configure-ovs:
https://github.com/openshift/machine-config-operator/blob/master/templates/common/_base/files/configure-ovs-network.yaml#L37

We also modify NM conns, so just wondering if there is any impact here.

@wking
Copy link
Member

wking commented Mar 19, 2021

Presubmit jobs look fine to me. I'm going to let this in so it can cook in our release informer CI over the weekend.

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 19, 2021
@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 19, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@jcpowermac jcpowermac mentioned this pull request Mar 20, 2021
Merged
@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

5 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@romfreiman
Copy link

romfreiman commented Mar 20, 2021

Happens on openshift-sdn

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

11 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Mar 20, 2021

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 20, 2021

@jcpowermac: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-gcp-op 4a126b2 link /test e2e-gcp-op
ci/prow/e2e-aws-workers-rhel7 4a126b2 link /test e2e-aws-workers-rhel7
ci/prow/okd-e2e-aws 4a126b2 link /test okd-e2e-aws

Full PR test history. Your PR dashboard.

Details

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/test-infra repository. I understand the commands that are listed here.

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Mar 20, 2021

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 20, 2021
@jcpowermac
Copy link
Contributor Author

jcpowermac commented Mar 20, 2021

Moving forward with #2472 inplace of this.

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Mar 20, 2021

/close

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 20, 2021

@jcpowermac: Closed this PR.

Details

In response to this:

/close

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/test-infra repository.

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Mar 20, 2021

@jcpowermac: This pull request references Bugzilla bug 1935539. The bug has been updated to no longer refer to the pull request using the external bug tracker.

Details

In response to this:

Bug 1935539: vSphere: Disable tx checksum offload

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/test-infra repository.

@abhat
Copy link
Contributor

abhat commented Mar 22, 2021

@jcpowermac is this targeted for OVN deployments or openshift-sdn as well? I'm asking because we do some vmxnet3 driver workaround in configure-ovs:
https://github.com/openshift/machine-config-operator/blob/master/templates/common/_base/files/configure-ovs-network.yaml#L37

We also modify NM conns, so just wondering if there is any impact here.

Anything we do here will be additive I think. The vsphere drop-in fix that @jcpowermac added to #2472 is similarly relevant. But I hope the drop-in does its thing and then when ovs-configuration service sees the platform as vsphere, it simply sets the multicast option correctly.

@abhat
Copy link
Contributor

abhat commented Mar 22, 2021

@jcpowermac here is the relevant comment that explains why we need the ovs-configuration change to set the nic to receive all multicast traffic.

https://bugzilla.redhat.com/show_bug.cgi?id=1854355#c15

@jcpowermac
Copy link
Contributor Author

jcpowermac commented Mar 22, 2021

@jcpowermac here is the relevant comment that explains why we need the ovs-configuration change to set the nic to receive all multicast traffic.

https://bugzilla.redhat.com/show_bug.cgi?id=1854355#c15

@abhat based on the BZ details the CI test would have failed if allmulti wasn't still configured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kikisdeliveryservice kikisdeliveryservice Awaiting requested review from kikisdeliveryservice

@patrickdillon patrickdillon Awaiting requested review from patrickdillon

@yuqi-zhang yuqi-zhang Awaiting requested review from yuqi-zhang

@zhaozhanqi zhaozhanqi Awaiting requested review from zhaozhanqi

Assignees

@wking wking

@patrickdillon patrickdillon

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-urgent Referenced Bugzilla bug's severity is urgent for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@jcpowermac @openshift-ci-robot @wking @kikisdeliveryservice @trozet @openshift-bot @romfreiman @abhat @patrickdillon