Skip to content

xds: replace net with netip in xds/xdsclient and xds/server#8909

Merged
easwars merged 7 commits intogrpc:masterfrom
chengxilo:replace-net-with-netip
Mar 24, 2026
Merged

xds: replace net with netip in xds/xdsclient and xds/server#8909
easwars merged 7 commits intogrpc:masterfrom
chengxilo:replace-net-with-netip

Conversation

@chengxilo
Copy link
Contributor

@chengxilo chengxilo commented Feb 18, 2026 •
edited
Loading

Updates #8884

This PR replace net.IP and net.IPNet with netip.Addr and netip.Prefix in directories internal/xds/xdsclient and internal/xds/server

RELEASE NOTES:

  • Updated error messages in addFilterChainsForDestPrefixes and addFilterChainsForSourcePrefixes when an invalid prefix or address is encountered.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 18, 2026 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Feb 18, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.06%. Comparing base (12e91dd) to head (e9014b9).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8909      +/-   ##
==========================================
+ Coverage   83.04%   83.06%   +0.02%     
==========================================
  Files         411      411              
  Lines       32892    32893       +1     
==========================================
+ Hits        27316    27324       +8     
+ Misses       4181     4179       -2     
+ Partials     1395     1390       -5
Files with missing lines Coverage Δ
internal/xds/server/filter_chain_manager.go 87.03% <100.00%> (ø)
internal/xds/server/listener_wrapper.go 76.88% <100.00%> (-1.21%) ⬇️
internal/xds/xdsclient/xdsresource/filter_chain.go 92.45% <ø> (ø)
...nternal/xds/xdsclient/xdsresource/unmarshal_lds.go 93.39% <100.00%> (-0.02%) ⬇️

... and 22 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chengxilo chengxilo force-pushed the replace-net-with-netip branch from f6044c3 to 3ab95f0 Compare February 18, 2026 03:30
@chengxilo chengxilo marked this pull request as draft February 18, 2026 03:37
@chengxilo chengxilo marked this pull request as ready for review February 18, 2026 03:51
@Pranjali-2501 Pranjali-2501 self-requested a review February 19, 2026 05:22
@Pranjali-2501 Pranjali-2501 self-assigned this Feb 19, 2026
@Pranjali-2501 Pranjali-2501 added this to the 1.80 Release milestone Feb 19, 2026
@Pranjali-2501 Pranjali-2501 added the Type: Internal Cleanup Refactors, etc label Feb 19, 2026
@Pranjali-2501
Copy link
Contributor

Pranjali-2501 commented Feb 19, 2026

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Feb 19, 2026
View reviewed changes
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request successfully transitions the xDS client and server components from using net.IP and net.IPNet to the more efficient netip.Addr and netip.Prefix. This is a positive change for performance and memory usage. I have identified one important issue in listener_wrapper.go where incoming addresses should be normalized using .Unmap() to ensure consistent matching against stored prefixes, especially since the stored prefixes are normalized during unmarshaling in unmarshal_lds.go.

Pranjali-2501
Pranjali-2501 reviewed Feb 19, 2026
View reviewed changes
@veeceey veeceey mentioned this pull request Feb 20, 2026
Merged
@chengxilo
Copy link
Contributor Author

chengxilo commented Feb 21, 2026 •
edited
Loading

Since the approach was verbose and duplicated across addFilterChainsForDestPrefixes and addFilterChainsForSourcePrefixes, I extracted the shared logic into a separate function parsePrefixRanges. I think it looks better now.

Ready for review.

@arjan-bal arjan-bal assigned mbissa and unassigned arjan-bal Mar 2, 2026
@arjan-bal arjan-bal requested a review from mbissa March 2, 2026 07:09
@mbissa mbissa modified the milestones: 1.80 Release, 1.81 Release Mar 6, 2026
mbissa
mbissa approved these changes Mar 10, 2026
View reviewed changes
Copy link
Contributor

@mbissa mbissa left a comment

Choose a reason for hiding this comment

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

LGTM

@mbissa mbissa assigned Pranjali-2501 and unassigned mbissa Mar 10, 2026
@arjan-bal arjan-bal assigned mbissa and unassigned Pranjali-2501 Mar 17, 2026
arjan-bal pushed a commit that referenced this pull request Mar 17, 2026
@veeceey
grpclb: replace net.IP with netip.Addr (#8918)
36e13de 
Contributes to #8884

This PR replaces deprecated `net.IP` usage with the modern `netip.Addr`
API in two packages:

- **grpclb**: Replace `net.IP(s.IpAddress).String()` with
`netip.AddrFromSlice()` in `processServerList`, adding proper error
handling for invalid IP addresses instead of silently producing `"?"`
from `net.IP.String()`.

Note: `x509.Certificate.IPAddresses` fields in test files
(`internal/credentials/xds/handshake_info_test.go`,
`security/advancedtls/crl_test.go`) are typed as `[]net.IP` by the
standard library, so those cannot be migrated. The xds/server and
xds/xdsclient packages are being addressed in #8909.

RELEASE NOTES: N/A
@chengxilo
Copy link
Contributor Author

chengxilo commented Mar 21, 2026 •
edited
Loading

conflicts solved, sorry for the delay.

@easwars easwars merged commit 4b786ff into grpc:master Mar 24, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbissa mbissa mbissa approved these changes

@Pranjali-2501 Pranjali-2501 Pranjali-2501 approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@mbissa mbissa

Labels

Type: Internal Cleanup Refactors, etc

Projects

None yet

Milestone

1.81 Release

Development

Successfully merging this pull request may close these issues.

5 participants

@chengxilo @Pranjali-2501 @mbissa @easwars @arjan-bal