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

Use LOCAL instead of CONTROLLER as the in_port of packet-out message #4992

Merged
merged 1 commit into from
May 18, 2023

Conversation

tnqn
Copy link
Member

@tnqn tnqn commented May 17, 2023

CONTROLLER cannot be used as the in_port due to a bug in Windows ovsext driver, otherwise the Windows OS would crash.

See openvswitch/ovs-issues#280.

Fixes #4990

CONTROLLER cannot be used as the in_port due to a bug in Windows ovsext
driver, otherwise the Windows OS would crash.

See openvswitch/ovs-issues#280.

Signed-off-by: Quan Tian <qtian@vmware.com>
@tnqn tnqn added area/OS/windows Issues or PRs related to the Windows operating system. area/proxy Issues or PRs related to proxy functions in Antrea action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. labels May 17, 2023
@tnqn tnqn added this to the Antrea v1.12 release milestone May 17, 2023
@tnqn tnqn requested review from wenyingd and xliuxu May 17, 2023 16:09
@tnqn
Copy link
Member Author

tnqn commented May 17, 2023

/test-all
/test-windows-all

@@ -1053,12 +1054,15 @@ func (p *proxier) HandlePacketIn(pktIn *ofctrl.PacketIn) error {
return fmt.Errorf("error when getting match field inPort")
}
outPort := inPortField.GetValue().(uint32)
// It cannot use CONTROLLER (the default value when inPort is 0) as the inPort due to a bug in Windows ovsext
// driver, otherwise the Windows OS would crash. See https://github.com/openvswitch/ovs-issues/issues/280.
inPort := uint32(openflow15.P_LOCAL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it work with value "0xfffffffe" on Windows ? What about Linux?

Copy link
Member Author

@tnqn tnqn May 17, 2023

Choose a reason for hiding this comment

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

I have verified on Linux it works (accessig the service from antrea-gw0 and Pod interface), and @xliuxu tested manually on Windows via ovs-ofctl packet-out and it didn't crash. Will need your help to verify it end to end on Windows.

Copy link
Contributor

@wenyingd wenyingd left a comment

Choose a reason for hiding this comment

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

tested with Windows docker runtime, a reject packet on a no-Endpoint Service request does not trigger BSoD. Will run more tests tomorrow.

@xliuxu
Copy link
Contributor

xliuxu commented May 18, 2023

tested with Windows docker runtime, a reject packet on a no-Endpoint Service request does not trigger BSoD. Will run more tests tomorrow.

Do we have a test case for this scenario?

@XinShuYang
Copy link
Contributor

/test-windows-containerd-networkpolicy

@tnqn
Copy link
Member Author

tnqn commented May 18, 2023

tested with Windows docker runtime, a reject packet on a no-Endpoint Service request does not trigger BSoD. Will run more tests tomorrow.

Do we have a test case for this scenario?

No, I think we should add one, but I notice none of proxy specific tests run on Windows. @hongliangl can we make these tests run on Windows?

@hongliangl
Copy link
Contributor

tested with Windows docker runtime, a reject packet on a no-Endpoint Service request does not trigger BSoD. Will run more tests tomorrow.

Do we have a test case for this scenario?

No, I think we should add one, but I notice none of proxy specific tests run on Windows. @hongliangl can we make these tests run on Windows?

Sure, I'll try to add these tests on Windows.

@tnqn
Copy link
Member Author

tnqn commented May 18, 2023

@hongliangl, as @xliuxu suggested above, please add one case for Service without Endpoints to verify we can get reject response when accessing the Service from Node and Pod.

@hongliangl
Copy link
Contributor

@hongliangl, as @xliuxu suggested above, please add one case for Service without Endpoints to verify we can get reject response when accessing the Service from Node and Pod.

Sure, I'll add it today.

@tnqn
Copy link
Member Author

tnqn commented May 18, 2023

/test-e2e
/test-conformance

@hongliangl
Copy link
Contributor

@hongliangl, as @xliuxu suggested above, please add one case for Service without Endpoints to verify we can get reject response when accessing the Service from Node and Pod.

#5000

@tnqn tnqn merged commit 8ab9b2f into antrea-io:main May 18, 2023
@tnqn tnqn deleted the fix-windows-core branch May 18, 2023 10:49
ceclinux pushed a commit to ceclinux/antrea that referenced this pull request Jun 5, 2023
…ntrea-io#4992)

CONTROLLER cannot be used as the in_port due to a bug in Windows ovsext
driver, otherwise the Windows OS would crash.

See openvswitch/ovs-issues#280.

Signed-off-by: Quan Tian <qtian@vmware.com>
@tnqn tnqn added the kind/bug Categorizes issue or PR as related to a bug. label Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. area/OS/windows Issues or PRs related to the Windows operating system. area/proxy Issues or PRs related to proxy functions in Antrea kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Accessing a Service without any Endpoints made Windows Node crash
5 participants