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

Optimize getting routes function for Windows #5335

Merged

Conversation

hongliangl
Copy link
Contributor

No description provided.

@hongliangl hongliangl requested a review from qiyueyao July 31, 2023 09:37
@hongliangl hongliangl force-pushed the 20230731-optimize-get-routes-on-windows branch 2 times, most recently from e79c30a to c1ced70 Compare August 1, 2023 04:08
pkg/agent/agent_windows.go Show resolved Hide resolved
pkg/agent/util/net_windows.go Show resolved Hide resolved
@hongliangl hongliangl force-pushed the 20230731-optimize-get-routes-on-windows branch from c1ced70 to ce96c68 Compare August 1, 2023 07:03
@hongliangl hongliangl requested a review from tnqn August 1, 2023 07:11
@hongliangl hongliangl added the area/OS/windows Issues or PRs related to the Windows operating system. label Aug 1, 2023
@hongliangl hongliangl added this to the Antrea v1.14 release milestone Aug 9, 2023
pkg/agent/agent_windows.go Show resolved Hide resolved
pkg/agent/route/route_windows.go Outdated Show resolved Hide resolved
@@ -1296,3 +1290,16 @@ func adapterAddresses() ([]*windows.IpAdapterAddresses, error) {
}
return aas, nil
}

// ipNetEqual returns true iff both IPNet are equal
func ipNetEqual(ipn1 *net.IPNet, ipn2 *net.IPNet) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use IPNetEqual defined in pkg/util/ip/ip.go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can use IPNetEqual in pkg/util/ip/ip.go in this PR, but in #5159, we might still need this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, maybe I am missing some context. Aren't the 2 supposed to do the same thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll verify that. I remember that when I made some tests in #5159, IPNetEqual in pkg/util/ip/ip.go cannot be called by the unit tests for Windows code.

pkg/agent/route/route_windows.go Outdated Show resolved Hide resolved
@@ -623,17 +623,6 @@ func TestReplaceNetRoute(t *testing.T) {
}
}

func TestGetNetRoutesAll(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should there be a replacement test for RouteListFiltered?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add the unit test in #5159

@hongliangl hongliangl force-pushed the 20230731-optimize-get-routes-on-windows branch from ce96c68 to 759c5d2 Compare August 23, 2023 02:23
antoninbas
antoninbas previously approved these changes Aug 23, 2023
pkg/agent/agent_windows.go Outdated Show resolved Hide resolved
pkg/agent/route/route_windows.go Outdated Show resolved Hide resolved
@hongliangl
Copy link
Contributor Author

/test-all
/test-windows-all

antoninbas
antoninbas previously approved these changes Aug 24, 2023
@antoninbas
Copy link
Contributor

/test-all
/test-windows-all

@antoninbas antoninbas merged commit f5c34dd into antrea-io:main Aug 25, 2023
42 of 44 checks passed
@hongliangl hongliangl deleted the 20230731-optimize-get-routes-on-windows branch August 27, 2023 00:19
hongliangl added a commit to hongliangl/antrea that referenced this pull request Sep 1, 2023
hongliangl added a commit to hongliangl/antrea that referenced this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/OS/windows Issues or PRs related to the Windows operating system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants