Skip to content

fix: IPv4 /31 gateway index in 3-tier literal is wrong - #200

Open
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:bugfix/addressing-test-ipv4-31-gateway-index-in-3-tier
Open

fix: IPv4 /31 gateway index in 3-tier literal is wrong#200
andrewwhitecdw wants to merge 1 commit into
NVIDIA:mainfrom
andrewwhitecdw:bugfix/addressing-test-ipv4-31-gateway-index-in-3-tier

Conversation

@andrewwhitecdw

@andrewwhitecdw andrewwhitecdw commented Aug 17, 2026

Copy link
Copy Markdown

This PR addresses the following issue in pkg/networkoperatorplugin/spectrumx/addressing_test.go: IPv4 /31 gateway index in 3-tier literal is wrong.

Changes

  • pkg/networkoperatorplugin/spectrumx/addressing_test.go: IPv4 /31 gateway index in 3-tier literal is wrong.

Details

--- a/pkg/networkoperatorplugin/spectrumx/addressing_test.go
+++ b/pkg/networkoperatorplugin/spectrumx/addressing_test.go
@@ -1,3 +1,3 @@
-		Name:                 "rail-0",
-		CIDR:                 "10.0.0.0/13",
-		GatewayIndex:         0,
+		Name:                 "rail-0",
+		CIDR:                 "10.0.0.0/13",
+		GatewayIndex:         1,

Tests

Let me know if you want tests added for this fix or not.

Contributor guidelines

Per this repo's CONTRIBUTING.md:

  • All commits are signed off (Signed-off-by trailer, DCO).

Update (post-Greptile review): The IPv4 poolSettings implementation has been updated to return GatewayIndex: 1, and the remaining IPv4 BuildCIDRPools test expectations (including the sibling AIR 3-tier case) have been updated to match. The Spectrum-X addressing test suite now passes (go test ./pkg/networkoperatorplugin/spectrumx/...). Commit: dc7a21164f13ea11af9a54cd75ac579433a5448c.

@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown

Greptile Summary

The PR corrects Spectrum-X IPv4 /31 pool settings so the gateway index selects the odd leaf/gateway address.

  • Changes the IPv4 gatewayIndex from 0 to 1.
  • Updates two-tier, three-tier, and AIR-based addressing expectations consistently.

Confidence Score: 5/5

The PR appears safe to merge.

The implementation and all affected test expectations now consistently use gateway index 1, so the previously reported test contradiction no longer remains.

Important Files Changed

Filename Overview
pkg/networkoperatorplugin/spectrumx/addressing.go Aligns the IPv4 pool gateway index with the leaf address at index 1 of each /31.
pkg/networkoperatorplugin/spectrumx/addressing_test.go Updates all affected IPv4 pool assertions, including the previously inconsistent three-tier and AIR cases.

Reviews (2): Last reviewed commit: "fix: IPv4 /31 gateway index in 3-tier li..." | Re-trigger Greptile

Comment thread pkg/networkoperatorplugin/spectrumx/addressing_test.go
Signed-off-by: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
@andrewwhitecdw
andrewwhitecdw force-pushed the bugfix/addressing-test-ipv4-31-gateway-index-in-3-tier branch from 30e48a2 to dc7a211 Compare August 17, 2026 22:03
@andrewwhitecdw

Copy link
Copy Markdown
Author

@greptile-apps Thanks for the review. I updated the IPv4 poolSettings implementation to return GatewayIndex: 1 and updated the remaining BuildCIDRPools test expectations (including the AIR 3-tier sibling) to match. go test ./pkg/networkoperatorplugin/spectrumx/... now passes.

@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown

The fix is complete and consistent — gatewayIndex is now 1 in poolSettings() (the source of truth), and all three affected test expectations in TestBuildCIDRPools2TierSWPLB, TestBuildCIDRPools3Tier, and TestBuildCIDRPoolsFromAIR3Tier have been updated to match. This is correct: in a /31, .1 is the gateway and .0 is the host, so GatewayIndex: 1 is the right value. LGTM.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant