enable dual NIC support in transparent VLAN #4057

mugeshsp · 2025-09-29T16:53:30Z

Reason for Change:
Enables dual NIC feature support in Azure Container Networking by adding the ability to skip default routes and improving ARP proxy configuration for transparent VLAN clients. This enhancement allows for better network isolation and custom routing scenarios in multi-interface container environments.

Requirements:

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

Notes:
This PR includes three main commits:

Added SkipDefaultRoutes field to network container request/response contracts
Implemented ARP proxy setting and custom subnet route addition for VLAN interfaces

…sponses

…interfaces

…in transparent VLAN client

Copilot

Pull Request Overview

This PR enables dual NIC support in transparent VLAN by adding the ability to skip default routes and improving ARP proxy configuration for transparent VLAN clients. This enhancement allows for better network isolation and custom routing scenarios in multi-interface container environments.

Added SkipDefaultRoutes field to network container request/response contracts
Implemented ARP proxy setting and custom subnet route addition for VLAN interfaces
Enabled dual NIC feature support on Linux platform

Reviewed Changes

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

Show a summary per file

File	Description
network/transparent_vlan_endpointclient_linux.go	Added ARP proxy configuration and custom routing logic with SkipDefaultRoutes support
cns/restserver/util.go	Propagated SkipDefaultRoutes field in network container responses and IP configuration
cns/NetworkContainerContract.go	Added SkipDefaultRoutes field to request/response contracts and updated string representation
cni/network/network_linux.go	Enabled dual NIC feature support for Linux platform
cni/network/multitenancy.go	Added SkipDefaultRoutes field to interface info configuration

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-29T16:54:20Z

network/transparent_vlan_endpointclient_linux.go

+	cmd := fmt.Sprintf("echo 1 > /proc/sys/net/ipv4/conf/%v/proxy_arp", ifName)
+	_, err := client.plClient.ExecuteRawCommand(cmd)


Using shell command execution with string formatting could be vulnerable to command injection if ifName contains malicious characters. Consider using a safer approach or validating the interface name against a whitelist of allowed characters.

Copilot · 2025-09-29T16:54:21Z

network/transparent_vlan_endpointclient_linux.go

+// Route 2: default via 169.254.2.1 dev <linkToName>
+func (client *TransparentVlanEndpointClient) addCustomRoutes(linkToName string, gatewayIP net.IP, subnetCIDR net.IPNet, table int) error {
+	// Add route for virtualgwip (ip route add <gatewayIP> dev <linkToName>)
+	gWIP, gwNet, _ := net.ParseCIDR(gatewayIP.String() + "/32")


Error values from net.ParseCIDR are being ignored. These parsing operations could fail and should be handled to prevent potential runtime panics or incorrect network configuration.

Suggested change

gWIP, gwNet, _ := net.ParseCIDR(gatewayIP.String() + "/32")

gWIP, gwNet, err := net.ParseCIDR(gatewayIP.String() + "/32")

if err != nil {

return errors.Wrapf(err, "failed to parse CIDR for gatewayIP %s", gatewayIP.String())

}

network/transparent_vlan_endpointclient_linux.go

…equests

tamilmani1989 · 2025-10-01T04:49:02Z

network/transparent_vlan_endpointclient_linux.go

+			return errors.Wrap(err, "failed container ns add custom routes")
+		}
+		return nil
+	} else {


can remove else

Removed else

tamilmani1989 · 2025-10-01T04:49:51Z

network/transparent_vlan_endpointclient_linux.go

-		return errors.Wrap(err, "failed container ns add default routes")
+	if epInfo.SkipDefaultRoutes {
+		logger.Info("Skipping adding default routes in container ns as requested")
+		if err := client.addCustomRoutes(client.containerVethName, epInfo.Subnets[0].Gateway, epInfo.Subnets[0].Prefix, 0); err != nil {


i think we decided not to add any custom routes. let the orchestrator add it

Removed addition of custom routes.

tamilmani1989 · 2025-10-03T16:28:52Z

network/transparent_vlan_endpointclient_linux.go

+	if err != nil {
+		logger.Error("Failed to set ARP proxy", zap.String("interface", ifName), zap.Error(err))
+	} else {
+		logger.Info("ARP proxy enabled", zap.String("interface", ifName))
+	}
+	return err


Suggested change

if err != nil {

logger.Error("Failed to set ARP proxy", zap.String("interface", ifName), zap.Error(err))

} else {

logger.Info("ARP proxy enabled", zap.String("interface", ifName))

}

return err

if err != nil {

logger.Error("Failed to set ARP proxy", zap.String("interface", ifName), zap.Error(err))

return err

}

return nil

tamilmani1989 · 2025-10-03T17:17:55Z

@mugeshsp please fix lint errors

mugeshsp · 2025-10-06T15:51:16Z

@microsoft-github-policy-service agree company="Microsoft"

mugeshsp · 2025-10-06T15:52:12Z

@mugeshsp please fix lint errors

Fixed the lint errors

tamilmani1989 · 2025-10-07T16:14:06Z

network/transparent_vlan_endpointclient_linux.go

+		if err := client.setArpProxy(client.vnetVethName); err != nil {
+			logger.Error("setArpProxy failed with", zap.Error(err))
+			return err
+		}
+
+		return nil


Suggested change

if err := client.setArpProxy(client.vnetVethName); err != nil {

logger.Error("setArpProxy failed with", zap.Error(err))

return err

}

return nil

return client.setArpProxy(client.vnetVethName)

paulyufan2 · 2025-10-09T16:08:42Z

/azp run Azure Container Networking PR

azure-pipelines · 2025-10-09T16:08:54Z

Azure Pipelines successfully started running 1 pipeline(s).

QxBytes · 2025-09-30T16:34:30Z

cni/network/network_linux.go


 func platformInit(cniConfig *cni.NetworkConfig) {}

 // isDualNicFeatureSupported returns if the dual nic feature is supported. Currently it's only supported for windows hnsv2 path


nit: could you also update the comment here if linux now supports dual nic?

QxBytes · 2025-09-30T16:40:21Z

network/transparent_vlan_endpointclient_linux.go

 }

+// Set ARP proxy on the vlan interface to respond to ARP requests for the gateway IP
+func (client *TransparentVlanEndpointClient) setArpProxy(ifName string) error {


nit: for the comment, the function itself is not necessarily on the vlan interface, just whatever interface is passed in

mugeshsp and others added 3 commits September 5, 2025 14:12

feat: add SkipDefaultRoutes field to network container request and re…

2ddbf89

…sponses

feat: implement ARP proxy setting and custom route addition for VLAN …

2f33a42

…interfaces

feat: enable dual NIC feature support and improve ARP proxy handling …

776430c

…in transparent VLAN client

mugeshsp requested review from a team as code owners September 29, 2025 16:53

Copilot AI review requested due to automatic review settings September 29, 2025 16:53

mugeshsp requested a review from a team as a code owner September 29, 2025 16:53

mugeshsp requested review from QxBytes and timraymond September 29, 2025 16:53

Copilot AI reviewed Sep 29, 2025

View reviewed changes

mugeshsp requested review from QxBytes and tamilmani1989 and removed request for QxBytes September 30, 2025 16:18

feat: add tests for SkipDefaultRoutes handling in network container r…

7cc5010

…equests

tamilmani1989 reviewed Oct 1, 2025

View reviewed changes

feat: remove addition of custom routes

c31176e

tamilmani1989 reviewed Oct 3, 2025

View reviewed changes

fix: improve ARP proxy error handling

4441787

fix: Lint Errors

6fde3f8

mugeshsp force-pushed the mugeshsp/pg-dual-nic branch from 850cadc to 6fde3f8 Compare October 6, 2025 11:02

tamilmani1989 reviewed Oct 7, 2025

View reviewed changes

refactor: streamline ARP proxy setup in AddEndpointRules

3ddc19e

tamilmani1989 approved these changes Oct 7, 2025

View reviewed changes

paulyufan2 approved these changes Oct 9, 2025

View reviewed changes

paulyufan2 enabled auto-merge October 9, 2025 16:08

QxBytes reviewed Oct 9, 2025

View reviewed changes

		cmd := fmt.Sprintf("echo 1 > /proc/sys/net/ipv4/conf/%v/proxy_arp", ifName)
		_, err := client.plClient.ExecuteRawCommand(cmd)


		func platformInit(cniConfig *cni.NetworkConfig) {}

		// isDualNicFeatureSupported returns if the dual nic feature is supported. Currently it's only supported for windows hnsv2 path

enable dual NIC support in transparent VLAN #4057

Are you sure you want to change the base?

enable dual NIC support in transparent VLAN #4057

Conversation

mugeshsp commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

tamilmani1989 commented Oct 3, 2025

Uh oh!

mugeshsp commented Oct 6, 2025

Uh oh!

mugeshsp commented Oct 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paulyufan2 commented Oct 9, 2025

Uh oh!

azure-pipelines bot commented Oct 9, 2025

Uh oh!

QxBytes Sep 30, 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!

QxBytes Sep 30, 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!

Uh oh!

mugeshsp commented Sep 29, 2025 •

edited

Loading

QxBytes Sep 30, 2025 •

edited

Loading

QxBytes Sep 30, 2025 •

edited

Loading