[feat] Prefix on NIC v6 windows support #4060

NihaNallappagari · 2025-10-01T14:36:24Z

Reason for Change:
This PR includes the required changes for Windows to support prefix on NIC NIC Swift v2 scenario. The HNS team requires certain values to be added to the registry, and this PR sets those values under the appropriate registry key.

Issue Fixed:

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
relevant PR labels added

Notes:
This PR is extension of Prefix on nic linux support

Verifying registry keys...
Reading all values from SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config\PrefixOnNic
Registry verification - enabled: 1
Registry verification - infra_nic_mac_address: 60:45:bd:75:e3:73
Registry verification - infra_nic_ifname: vEthernet (Ethernet 3)
Reading all values from SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config
Registry verification - EnableSNAT: 0

* add iptables block to signed image * fix syntax * mariner version

* Prefix on NIC v6 support * update dedup comment in ipam * add log for a gatway error scenario * handle synchostversion sync on pon swiftv2 nic * handle gatewayip parse failure or nil scenario * remove unused code Address PR comments remove unnecessary logger update synhost skip test scenario fix linting error updated lint error fix linting error . update synhost skip test scenario fix linting error updated lint error fix linting error . * add test scenario * update test to handle gatway nil case * remove synchost skip and single ip skip . . * Add missing test scenarios * fix ipam_test conflicts * fix conflits small fixes fix lint errors lint fix . Delete examples/imds_nc_lookup.go Signed-off-by: NihaNallappagari <[email protected]> fix lint . * fix linting * add todo and remove multiple interface scenario * Skip add primary ip's to secondary config for swiftv2 pon scenarios fix lint error . * code review comment * Update gateway ipv6 to use default value, that auto detects and adds to neigh table * remove unwanted changes * address ipfamily comment * fix test failure * Handle nilgateway scenario * remove unnedded else block * comments changes --------- Co-authored-by: Kaushik Vuligonda <[email protected]> Co-authored-by: nn052161 <[email protected]>

…re-container-networking into ponv6WindowsSupport

github-actions · 2025-11-01T00:01:52Z

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

github-actions · 2025-11-09T00:01:54Z

Pull request closed due to inactivity.

github-actions · 2025-11-28T00:01:56Z

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

Copilot

Pull request overview

This PR adds Windows support for the SwiftV2 PrefixOnNic scenario by implementing Windows registry configuration functionality. The changes enable CNS to detect SwiftV2 VNETBlock network containers and configure appropriate registry settings for HNS (Host Network Service) on Windows platforms.

Key Changes:

Added Windows registry manipulation functions to configure PrefixOnNic settings
Introduced SwiftV2PrefixOnNic field to track SwiftV2 VNETBlock containers
Modified IMDS NC retrieval to handle Windows SwiftV2 scenarios and trigger registry configuration

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
cns/restserver/internalapi_windows.go	Implements registry manipulation functions for configuring Windows HNS PrefixOnNic settings
cns/restserver/internalapi_test.go	Adds test coverage for Windows SwiftV2 PrefixOnNic scenario in SyncHostNCVersion
cns/restserver/internalapi_linux.go	Provides no-op implementation of setPrefixOnNICRegistry for Linux platform
cns/restserver/internalapi.go	Refactors getIMDSNCs to detect and configure Windows SwiftV2 scenarios, adds isPrefixonNicSwiftV2 helper
cns/kubecontroller/nodenetworkconfig/conversion_linux.go	Sets SwiftV2PrefixOnNic flag based on NC type for static NCs
cns/kubecontroller/nodenetworkconfig/conversion.go	Explicitly sets SwiftV2PrefixOnNic to false for dynamic NCs
cns/NetworkContainerContract.go	Adds SwiftV2PrefixOnNic field to CreateNetworkContainerRequest

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tamilmani1989 · 2025-12-23T17:56:27Z

cns/restserver/internalapi_windows.go

+	return service.setRegistryValue(hnsRegistryPath, "EnableSNAT", isEnabled)
+}
+
+func (service *HTTPRestService) setPrefixOnNICRegistry(enablePrefixOnNic bool, infraNicMacAddress string) error {


why this is required for hns?

HNS team need to know thinks like infra nic, delegated nic, swiftv2 prefix on nic scenario to probably add some routes specific to the scenario. HNS team might answer better, tagging @surajkrishnan14

Yes, this is required for HNS to setup appropriate rules to forward packets correctly between the 2 interfaces and container in various scenarios, setup SNAT etc.

tamilmani1989 · 2025-12-23T18:00:12Z

cns/restserver/internalapi.go


 		if ncID != "" {
 			ncs[ncID] = PrefixOnNicNCVersion // for prefix on nic version scenario nc version is 1
+		} else if runtime.GOOS == "windows" && service.isPrefixonNicSwiftV2() {


GOOS=windows not required since you are defining in windows specific file, linux specific calls would be dummy.. i think its better we expose all regkey apis through interface.. @behzad-mir / @rbtr to comment on this

Good feedback. IMO any explicit check to GOOS or stubbing an interface in an *_<os>.go file indicates that the abstraction is in the wrong place.
There was a similar discussion here #3286 (comment) which you should read so that I don't have to repeat it all.

The tl;dr is that our abstraction should live at the last boundary of common code before we start splitting things up by OS. E.g. here: if getIMDSNCs is the final call possible before we have to do OS specific things, that is what lives in both the *_linux.go and the *_windows.go.

However - I haven't done a full review of this code, but at first glance getIMDSNCs is probably not the place to be doing OS specific things at all. That advertises that it just getting NCs.
And if we think about duplicating this func in OS-specific files, we end up copying a bunch of common code to both locations that doesn't care about the OS at all. This probably all just means that the current func is overloaded and poorly defined. Consider how to separate things in to the minimum necessary complete behaviors.

removed the os check and moved windowskey process call form getIMDSNCs . Thanks for the feedback

@rbtr I’ve refactored the code based on your suggestions. Please review it and let me know if any of the changes don’t fully address your feedback. Thanks

tamilmani1989 · 2025-12-23T18:03:47Z

cns/restserver/internalapi_windows.go

+	return service.setRegistryValue(hnsRegistryPath, "EnableSNAT", isEnabled)
+}
+
+func (service *HTTPRestService) setPrefixOnNICRegistry(enablePrefixOnNic bool, infraNicMacAddress string) error {


since we are setting multiple regkeys under same function, can function name be setRegistryKeysForPrefixOnNic and then expose all registry key functions in interface, so that unit test can be written mocking it

Refactored as per suggestion

QxBytes

could you also add to the pr description some background on prefix on nic (pon, particularly v6-- what PON is meant for/aims to accomplish), what this feature is allowing us to do that we couldn't do before, and what scenarios this affects (ex: standalone, multi/single tenant)

cns/restserver/restserver.go

cns/restserver/internalapi_linux.go

cns/restserver/internalapi.go

cns/kubecontroller/nodenetworkconfig/conversion_windows_test.go

cns/restserver/internalapi.go

cns/restserver/internalapi_windows.go

cns/restserver/internalapi_linux.go

rbtr

latest updates to the OS abstraction lgtm. only one question

rbtr · 2026-01-15T22:32:53Z

cns/restserver/internalapi_windows.go

+		go func() {
+			// Get the interface name from the MAC address
+			infraNicIfName, err := service.getInterfaceNameFromMAC(infraNicMacAddress)
+			if err != nil {
+				//nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated
+				logger.Errorf("[Windows] Failed to get interface name from MAC address to set for windows registry: %v", err)
+				return
+			}
+			// Process Windows registry keys with the retrieved MAC address and interface name. It is required for HNS team to configure cilium routes specific to windows nodes
+			if err := service.setRegistryKeysForPrefixOnNic(isSwiftv2PrefixOnNic, infraNicMacAddress, infraNicIfName); err != nil {
+				//nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated
+				logger.Errorf("[Windows] Failed to set registry keys: %v", err)
+				return
+			}
+		}()


is it safe for this to be async? what if it happens much much later than the NCs that we return begin to be used?

I initially thought the keys were needed to send data to Cilium/CNI to configure routes, which happens later in the process. However, there is a risk incorrect details could be sent to Cilium. I updated this to a synchronous approach to ensure correct ordering. Please let me know if you think there’s a more optimal way to handle this.

@rbtr Could you please review the changes and let me know if any further updates are needed? Thanks

addressed

QxBytes

also note that we have external dependencies in our unit tests (registry and getting interface by mac) which might cause the same test to fail on one computer but succeed on another

cns/restserver/internalapi.go

cns/restserver/internalapi_windows.go

NihaNallappagari · 2026-01-23T18:59:27Z

also note that we have external dependencies in our unit tests (registry and getting interface by mac) which might cause the same test to fail on one computer but succeed on another
@QxBytes Could you please elaborate on which test case you’re referring to? I needed to introduce dependency injection for the interface MAC address, which required production code changes. To handle this, I refactored the code and added test scenarios that don’t require mocking NetIO. The runners will cover the end-to-end testing.

QxBytes · 2026-01-26T23:37:04Z

also note that we have external dependencies in our unit tests (registry and getting interface by mac) which might cause the same test to fail on one computer but succeed on another
@QxBytes Could you please elaborate on which test case you’re referring to? I needed to introduce dependency injection for the interface MAC address, which required production code changes. To handle this, I refactored the code and added test scenarios that don’t require mocking NetIO. The runners will cover the end-to-end testing.

I believe you fixed this in your most recent push

QxBytes · 2026-01-31T00:06:48Z

niha to rename getIMDSNCDetails to better reflect that it has side-effects as per side discussion

NihaNallappagari and others added 8 commits September 30, 2025 15:41

Add registry keys for prefix on nic scenario

01e4ea0

update tests

942e890

ci: add iptables block signed image (Azure#4049)

eed5080

* add iptables block to signed image * fix syntax * mariner version

Add registry keys for prefix on nic scenario

5ca999b

update tests

23a756b

Merge branch 'ponv6WindowsSupport' of github.com:NihaNallappagari/azu…

bc3cad8

…re-container-networking into ponv6WindowsSupport

remove unused code

4395f17

NihaNallappagari force-pushed the ponv6WindowsSupport branch from 875c0f0 to 4395f17 Compare October 14, 2025 18:20

github-actions bot added the stale Stale due to inactivity. label Nov 1, 2025

github-actions bot closed this Nov 9, 2025

NihaNallappagari reopened this Nov 13, 2025

github-actions bot removed the stale Stale due to inactivity. label Nov 14, 2025

github-actions bot added the stale Stale due to inactivity. label Nov 28, 2025

surajkrishnan14 approved these changes Dec 1, 2025

View reviewed changes

github-actions bot removed the stale Stale due to inactivity. label Dec 2, 2025

Merge branch 'master' into ponv6WindowsSupport

8e673c3

NihaNallappagari marked this pull request as ready for review December 16, 2025 16:25

NihaNallappagari requested a review from a team as a code owner December 16, 2025 16:25

NihaNallappagari requested review from Copilot and nairashu December 16, 2025 16:25

Copilot AI reviewed Dec 16, 2025

View reviewed changes

NihaNallappagari changed the title ~~[Draft PR-no review needed] Ponv6 windows support~~ [feat] Ponv6 windows support Dec 16, 2025

Azure deleted a comment from Copilot AI Dec 16, 2025

NihaNallappagari force-pushed the ponv6WindowsSupport branch from ca1cb7d to 19a8a62 Compare December 16, 2025 17:38

NihaNallappagari removed the request for review from nairashu December 16, 2025 18:19

NihaNallappagari requested review from rbtr and tamilmani1989 December 16, 2025 18:19

NihaNallappagari force-pushed the ponv6WindowsSupport branch 2 times, most recently from 39289be to 5e76f7b Compare December 17, 2025 17:01

update linting error

aef689e

NihaNallappagari force-pushed the ponv6WindowsSupport branch from 5e76f7b to aef689e Compare December 17, 2025 18:07

tamilmani1989 reviewed Dec 23, 2025

View reviewed changes

Azure deleted a comment from Copilot AI Dec 26, 2025

nihanMicrosoft added 2 commits December 31, 2025 11:58

address code review comments

d37a407

fix linting error

15e66d7

QxBytes reviewed Jan 7, 2026

View reviewed changes

rbtr previously requested changes Jan 8, 2026

View reviewed changes

cns/restserver/internalapi_linux.go Outdated Show resolved Hide resolved

NihaNallappagari changed the title ~~[feat] Ponv6 windows support~~ [feat] Prefix on NIC v6 windows support Jan 8, 2026

update set registry keys

ca2d074

NihaNallappagari force-pushed the ponv6WindowsSupport branch from 383045c to ca2d074 Compare January 12, 2026 23:23

nihanMicrosoft added 2 commits January 14, 2026 09:34

Update registry keys to match HNS expected keywords

f23d975

fetch infra nic network interface name based on mac address

2dcaad2

NihaNallappagari force-pushed the ponv6WindowsSupport branch from c78e3b8 to 2dcaad2 Compare January 15, 2026 21:15

rbtr reviewed Jan 15, 2026

View reviewed changes

update setRegistryKeysForPrefixOnNic to sync function

260c467

QxBytes reviewed Jan 22, 2026

View reviewed changes

cns/restserver/internalapi.go Outdated Show resolved Hide resolved

cns/restserver/internalapi_windows.go Outdated Show resolved Hide resolved

update registry key error cases

e61e52b

[feat] Prefix on NIC v6 windows support #4060

Are you sure you want to change the base?

[feat] Prefix on NIC v6 windows support #4060

Conversation

NihaNallappagari commented Oct 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 1, 2025

Uh oh!

github-actions bot commented Nov 9, 2025

Uh oh!

github-actions bot commented Nov 28, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rbtr Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QxBytes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rbtr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NihaNallappagari Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QxBytes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

NihaNallappagari commented Jan 23, 2026

Uh oh!

QxBytes commented Jan 26, 2026

Uh oh!

QxBytes commented Jan 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

NihaNallappagari commented Oct 1, 2025 •

edited

Loading

rbtr Dec 23, 2025 •

edited

Loading

NihaNallappagari Jan 16, 2026 •

edited

Loading