Set constant mac for host veth interface in transparent vlan mode (#1906)

* set constant mac for host veth interface * fixed a race issue in transparent-vlan where delete can happen after add and removes route add by ADD call * moved log to place where its executed * enable proxy arp on bridge to allow public connectivity from apipa interface * validate newly created namespace is not same as host namespace * addressed comments and added UTs * fixed cni delete call for linux multitenancy * lint fixes * windows lint fixes * lint fixes * fix issues with network namespace creation and vlan interface creation * Removed deletehostveth flag and delete host veth on delete endpoint trigger * lint fix * address comment
2023-04-17 14:26:00 -07:00 · 2023-04-17 14:26:00 -07:00 · a82b312995
--- a/cni/network/network.go
+++ b/cni/network/network.go
@ -466,10 +466,11 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {

 	// iterate ipamAddResults and program the endpoint
 	for i := 0; i < len(ipamAddResults); i++ {
+		var networkID string
 		ipamAddResult = ipamAddResults[i]

 		options := make(map[string]any)
-		networkID, err := plugin.getNetworkName(args.Netns, &ipamAddResult, nwCfg)
+		networkID, err = plugin.getNetworkName(args.Netns, &ipamAddResult, nwCfg)

 		endpointID := GetEndpointID(args)
 		policies := cni.GetPoliciesFromNwCfg(nwCfg.AdditionalArgs)
@ -557,7 +558,8 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error {
 			natInfo:          natInfo,
 		}

-		epInfo, err := plugin.createEndpointInternal(&createEndpointInternalOpt)
+		var epInfo network.EndpointInfo
+		epInfo, err = plugin.createEndpointInternal(&createEndpointInternalOpt)
 		if err != nil {
 			log.Errorf("Endpoint creation failed:%w", err)
 			return err
@ -953,7 +955,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
 	numEndpointsToDelete := 1
 	// only get number of endpoints if it's multitenancy mode
 	if nwCfg.MultiTenancy {
-		numEndpointsToDelete = plugin.nm.GetNumEndpointsInNetNs(args.Netns)
+		numEndpointsToDelete = plugin.nm.GetNumEndpointsByContainerID(args.ContainerID)
 	}

 	log.Printf("[cni-net] number of endpoints to be deleted %d", numEndpointsToDelete)
@ -973,23 +975,21 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
 		}
 		// Query the network.
 		if nwInfo, err = plugin.nm.GetNetworkInfo(networkID); err != nil {
-			if !nwCfg.MultiTenancy {
-				log.Printf("[cni-net] Failed to query network:%s: %v", networkID, err)
 			// Log the error but return success if the network is not found.
 			// if cni hits this, mostly state file would be missing and it can be reboot scenario where
 			// container runtime tries to delete and create pods which existed before reboot.
+			log.Printf("[cni-net] Failed to query network:%s: %v", networkID, err)
 			err = nil
 			return err
 		}
-		}

 		endpointID := GetEndpointID(args)
 		// Query the endpoint.
 		if epInfo, err = plugin.nm.GetEndpointInfo(networkID, endpointID); err != nil {
+			log.Printf("[cni-net] GetEndpoint for endpointID: %s returns: %v", endpointID, err)
 			if !nwCfg.MultiTenancy {
 				// attempt to release address associated with this Endpoint id
 				// This is to ensure clean up is done even in failure cases
-				log.Printf("[cni-net] Failed to query endpoint %s: %v", endpointID, err)
 				logAndSendEvent(plugin, fmt.Sprintf("Release ip by ContainerID (endpoint not found):%v", args.ContainerID))
 				if err = plugin.ipamInvoker.Delete(nil, nwCfg, args, nwInfo.Options); err != nil {
 					return plugin.RetriableError(fmt.Errorf("failed to release address(no endpoint): %w", err))
--- a/netio/mocknetio.go
+++ b/netio/mocknetio.go
@ -6,10 +6,13 @@ import (
 	"net"
 )

+type getInterfaceValidationFn func(name string) (*net.Interface, error)
+
 type MockNetIO struct {
 	fail           bool
 	failAttempt    int
 	numTimesCalled int
+	getInterfaceFn getInterfaceValidationFn
 }

 // ErrMockNetIOFail - mock netio error
@ -22,6 +25,10 @@ func NewMockNetIO(fail bool, failAttempt int) *MockNetIO {
 	}
 }

+func (netshim *MockNetIO) SetGetInterfaceValidatonFn(fn getInterfaceValidationFn) {
+	netshim.getInterfaceFn = fn
+}
+
 func (netshim *MockNetIO) GetNetworkInterfaceByName(name string) (*net.Interface, error) {
 	netshim.numTimesCalled++

@ -29,6 +36,10 @@ func (netshim *MockNetIO) GetNetworkInterfaceByName(name string) (*net.Interface
 		return nil, fmt.Errorf("%w:%s", ErrMockNetIOFail, name)
 	}

+	if netshim.getInterfaceFn != nil {
+		return netshim.getInterfaceFn(name)
+	}
+
 	hwAddr, _ := net.ParseMAC("ab:cd:ef:12:34:56")

 	return &net.Interface{
--- a/netlink/mocknetlink.go
+++ b/netlink/mocknetlink.go
@ -13,9 +13,13 @@ func newErrorMockNetlink(errStr string) error {
 	return fmt.Errorf("%w : %s", ErrorMockNetlink, errStr)
 }

+type routeValidateFn func(route *Route) error
+
 type MockNetlink struct {
 	returnError   bool
 	errorString   string
+	deleteRouteFn routeValidateFn
+	addRouteFn    routeValidateFn
 }

 func NewMockNetlink(returnError bool, errorString string) *MockNetlink {
@ -25,6 +29,14 @@ func NewMockNetlink(returnError bool, errorString string) *MockNetlink {
 	}
 }

+func (f *MockNetlink) SetDeleteRouteValidationFn(fn routeValidateFn) {
+	f.deleteRouteFn = fn
+}
+
+func (f *MockNetlink) SetAddRouteValidationFn(fn routeValidateFn) {
+	f.addRouteFn = fn
+}
+
 func (f *MockNetlink) error() error {
 	if f.returnError {
 		return newErrorMockNetlink(f.errorString)
@ -88,10 +100,16 @@ func (f *MockNetlink) GetIPRoute(*Route) ([]*Route, error) {
 	return nil, f.error()
 }

-func (f *MockNetlink) AddIPRoute(*Route) error {
+func (f *MockNetlink) AddIPRoute(r *Route) error {
+	if f.addRouteFn != nil {
+		return f.addRouteFn(r)
+	}
 	return f.error()
 }

-func (f *MockNetlink) DeleteIPRoute(*Route) error {
+func (f *MockNetlink) DeleteIPRoute(r *Route) error {
+	if f.deleteRouteFn != nil {
+		return f.deleteRouteFn(r)
+	}
 	return f.error()
 }
--- a/netns/netns.go
+++ b/netns/netns.go
@ -36,3 +36,11 @@ func (f *Netns) NewNamed(name string) (int, error) {
 func (f *Netns) DeleteNamed(name string) error {
 	return errors.Wrap(netns.DeleteNamed(name), "netns impl")
 }
+
+func (f *Netns) IsNamespaceEqual(fd1, fd2 int) bool {
+	return netns.NsHandle(fd1).Equal(netns.NsHandle(fd2))
+}
+
+func (f *Netns) NamespaceUniqueID(fd int) string {
+	return netns.NsHandle(fd).UniqueId()
+}
--- a/network/bridge_endpointclient_linux.go
+++ b/network/bridge_endpointclient_linux.go
@ -210,7 +210,7 @@ func (client *LinuxBridgeEndpointClient) ConfigureContainerInterfacesAndRoutes(e
 	return nil
 }

-func (client *LinuxBridgeEndpointClient) DeleteEndpoints(ep *endpoint, _ bool) error {
+func (client *LinuxBridgeEndpointClient) DeleteEndpoints(ep *endpoint) error {
 	log.Printf("[net] Deleting veth pair %v %v.", ep.HostIfName, ep.IfName)
 	err := client.netlink.DeleteLink(ep.HostIfName)
 	if err != nil {
--- a/network/endpoint_linux.go
+++ b/network/endpoint_linux.go
@ -158,7 +158,7 @@ func (nw *network) newEndpointImpl(
 			}
 			// set deleteHostVeth to true to cleanup host veth interface if created
 			//nolint:errcheck // ignore error
-			epClient.DeleteEndpoints(endpt, true)
+			epClient.DeleteEndpoints(endpt)
 		}
 	}()

@ -284,7 +284,7 @@ func (nw *network) deleteEndpointImpl(nl netlink.NetlinkInterface, plc platform.
 	// deleteHostVeth set to false not to delete veth as CRI will remove network namespace and
 	// veth will get removed as part of that.
 	//nolint:errcheck // ignore error
-	epClient.DeleteEndpoints(ep, false)
+	epClient.DeleteEndpoints(ep)

 	return nil
 }
@ -297,8 +297,6 @@ func addRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, inte
 	ifIndex := 0

 	for _, route := range routes {
-		log.Printf("[net] Adding IP route %+v to link %v.", route, interfaceName)
-
 		if route.DevName != "" {
 			devIf, _ := netioshim.GetNetworkInterfaceByName(route.DevName)
 			ifIndex = devIf.Index
@ -327,6 +325,7 @@ func addRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, inte
 			Table:     route.Table,
 		}

+		log.Printf("[net] Adding IP route %+v to link %v.", route, interfaceName)
 		if err := nl.AddIPRoute(nlRoute); err != nil {
 			if !strings.Contains(strings.ToLower(err.Error()), "file exists") {
 				return err
@ -343,8 +342,6 @@ func deleteRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, i
 	ifIndex := 0

 	for _, route := range routes {
-		log.Printf("[net] Deleting IP route %+v from link %v.", route, interfaceName)
-
 		if route.DevName != "" {
 			devIf, _ := netioshim.GetNetworkInterfaceByName(route.DevName)
 			if devIf == nil {
@ -353,15 +350,15 @@ func deleteRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, i
 			}

 			ifIndex = devIf.Index
-		} else {
+		} else if interfaceName != "" {
 			interfaceIf, _ := netioshim.GetNetworkInterfaceByName(interfaceName)
 			if interfaceIf == nil {
 				log.Printf("[net] Not deleting route. Interface %v doesn't exist", interfaceName)
 				continue
 			}
-
 			ifIndex = interfaceIf.Index
 		}
+
 		family := netlink.GetIPAddressFamily(route.Gw)
 		if route.Gw == nil {
 			family = netlink.GetIPAddressFamily(route.Dst.IP)
@ -370,12 +367,13 @@ func deleteRoutes(nl netlink.NetlinkInterface, netioshim netio.NetIOInterface, i
 		nlRoute := &netlink.Route{
 			Family:    family,
 			Dst:       &route.Dst,
-			Gw:        route.Gw,
 			LinkIndex: ifIndex,
+			Gw:        route.Gw,
 			Protocol:  route.Protocol,
 			Scope:     route.Scope,
 		}

+		log.Printf("[net] Deleting IP route %+v from link %v.", route, interfaceName)
 		if err := nl.DeleteIPRoute(nlRoute); err != nil {
 			return err
 		}
--- a/network/endpoint_linux_test.go
+++ b/network/endpoint_linux_test.go
@ -0,0 +1,137 @@
+package network
+
+import (
+	"net"
+	"testing"
+
+	"github.com/Azure/azure-container-networking/netio"
+	"github.com/Azure/azure-container-networking/netlink"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+	"github.com/pkg/errors"
+)
+
+func TestEndpointLinux(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Endpoint Suite")
+}
+
+var _ = Describe("Test TestEndpointLinux", func() {
+	Describe("Test deleteRoutes", func() {
+		_, dst, _ := net.ParseCIDR("192.168.0.0/16")
+
+		It("DeleteRoute with interfacename explicit", func() {
+			nlc := netlink.NewMockNetlink(false, "")
+			nlc.SetDeleteRouteValidationFn(func(r *netlink.Route) error {
+				Expect(r.LinkIndex).To(Equal(5))
+				return nil
+			})
+
+			netiocl := netio.NewMockNetIO(false, 0)
+			netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
+				Expect(ifName).To(Equal("eth0"))
+				return &net.Interface{
+					Index: 5,
+				}, nil
+			})
+
+			err := deleteRoutes(nlc, netiocl, "eth0", []RouteInfo{{Dst: *dst, DevName: ""}})
+			Expect(err).To(BeNil())
+		})
+		It("DeleteRoute with interfacename set in Route", func() {
+			nlc := netlink.NewMockNetlink(false, "")
+			nlc.SetDeleteRouteValidationFn(func(r *netlink.Route) error {
+				Expect(r.LinkIndex).To(Equal(6))
+				return nil
+			})
+
+			netiocl := netio.NewMockNetIO(false, 0)
+			netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
+				Expect(ifName).To(Equal("eth1"))
+				return &net.Interface{
+					Index: 6,
+				}, nil
+			})
+
+			err := deleteRoutes(nlc, netiocl, "", []RouteInfo{{Dst: *dst, DevName: "eth1"}})
+			Expect(err).To(BeNil())
+		})
+		It("DeleteRoute with no ifindex", func() {
+			nlc := netlink.NewMockNetlink(false, "")
+			nlc.SetDeleteRouteValidationFn(func(r *netlink.Route) error {
+				Expect(r.LinkIndex).To(Equal(0))
+				return nil
+			})
+
+			netiocl := netio.NewMockNetIO(false, 0)
+			netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
+				Expect(ifName).To(Equal("eth1"))
+				return &net.Interface{
+					Index: 6,
+				}, nil
+			})
+
+			err := deleteRoutes(nlc, netiocl, "", []RouteInfo{{Dst: *dst, DevName: ""}})
+			Expect(err).To(BeNil())
+		})
+	})
+	Describe("Test addRoutes", func() {
+		_, dst, _ := net.ParseCIDR("192.168.0.0/16")
+		It("AddRoute with interfacename explicit", func() {
+			nlc := netlink.NewMockNetlink(false, "")
+			nlc.SetAddRouteValidationFn(func(r *netlink.Route) error {
+				Expect(r).NotTo(BeNil())
+				Expect(r.LinkIndex).To(Equal(5))
+				return nil
+			})
+
+			netiocl := netio.NewMockNetIO(false, 0)
+			netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
+				Expect(ifName).To(Equal("eth0"))
+				return &net.Interface{
+					Index: 5,
+				}, nil
+			})
+
+			err := addRoutes(nlc, netiocl, "eth0", []RouteInfo{{Dst: *dst, DevName: ""}})
+			Expect(err).To(BeNil())
+		})
+		It("AddRoute with interfacename set in route", func() {
+			nlc := netlink.NewMockNetlink(false, "")
+			nlc.SetAddRouteValidationFn(func(r *netlink.Route) error {
+				Expect(r.LinkIndex).To(Equal(6))
+				return nil
+			})
+
+			netiocl := netio.NewMockNetIO(false, 0)
+			netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
+				Expect(ifName).To(Equal("eth1"))
+				return &net.Interface{
+					Index: 6,
+				}, nil
+			})
+
+			err := addRoutes(nlc, netiocl, "", []RouteInfo{{Dst: *dst, DevName: "eth1"}})
+			Expect(err).To(BeNil())
+		})
+		It("AddRoute with interfacename not set should return error", func() {
+			nlc := netlink.NewMockNetlink(false, "")
+			nlc.SetAddRouteValidationFn(func(r *netlink.Route) error {
+				Expect(r.LinkIndex).To(Equal(0))
+				//nolint:goerr113 // for testing
+				return errors.New("Cannot add route")
+			})
+
+			netiocl := netio.NewMockNetIO(false, 0)
+			netiocl.SetGetInterfaceValidatonFn(func(ifName string) (*net.Interface, error) {
+				Expect(ifName).To(Equal(""))
+				return &net.Interface{
+					Index: 0,
+				}, errors.Wrapf(netio.ErrInterfaceNil, "Cannot get interface")
+			})
+
+			err := addRoutes(nlc, netiocl, "", []RouteInfo{{Dst: *dst, DevName: ""}})
+			Expect(err).ToNot(BeNil())
+		})
+	})
+})
--- a/network/endpoint_windows.go
+++ b/network/endpoint_windows.go
@ -157,6 +157,7 @@ func (nw *network) newEndpointImplHnsV1(epInfo *EndpointInfo) (*endpoint, error)
 		VlanID:           vlanid,
 		EnableSnatOnHost: epInfo.EnableSnatOnHost,
 		NetNs:            epInfo.NetNsPath,
+		ContainerID:      epInfo.ContainerID,
 	}

 	for _, route := range epInfo.Routes {
--- a/network/manager.go
+++ b/network/manager.go
@ -49,7 +49,7 @@ type EndpointClient interface {
 	MoveEndpointsToContainerNS(epInfo *EndpointInfo, nsID uintptr) error
 	SetupContainerInterfaces(epInfo *EndpointInfo) error
 	ConfigureContainerInterfacesAndRoutes(epInfo *EndpointInfo) error
-	DeleteEndpoints(ep *endpoint, deleteHostVeth bool) error
+	DeleteEndpoints(ep *endpoint) error
 }

 // NetworkManager manages the set of container networking resources.
@ -76,7 +76,7 @@ type NetworkManager interface {
 	GetNetworkInfo(networkID string) (NetworkInfo, error)
 	// FindNetworkIDFromNetNs returns the network name that contains an endpoint created for this netNS, errNetworkNotFound if no network is found
 	FindNetworkIDFromNetNs(netNs string) (string, error)
-	GetNumEndpointsInNetNs(netNs string) int
+	GetNumEndpointsByContainerID(containerID string) int

 	CreateEndpoint(client apipaClient, networkID string, epInfo *EndpointInfo) error
 	DeleteEndpoint(networkID string, endpointID string) error
--- a/network/manager_mock.go
+++ b/network/manager_mock.go
@ -115,8 +115,8 @@ func (nm *MockNetworkManager) FindNetworkIDFromNetNs(netNs string) (string, erro
 	return "", errNetworkNotFound
 }

-// GetNumEndpointsInNetNs mock
-func (nm *MockNetworkManager) GetNumEndpointsInNetNs(netNs string) int {
+// GetNumEndpointsByContainerID mock
+func (nm *MockNetworkManager) GetNumEndpointsByContainerID(_ string) int {
 	// based on the GetAllEndpoints func above, it seems that this mock is only intended to be used with
 	// one network, so just return the number of endpoints if network exists
 	numEndpoints := 0
--- a/network/mock_endpointclient.go
+++ b/network/mock_endpointclient.go
@ -58,7 +58,7 @@ func (client *MockEndpointClient) ConfigureContainerInterfacesAndRoutes(_ *Endpo
 	return nil
 }

-func (client *MockEndpointClient) DeleteEndpoints(ep *endpoint, _ bool) error {
+func (client *MockEndpointClient) DeleteEndpoints(ep *endpoint) error {
 	delete(client.endpoints, ep.Id)
 	return nil
 }
--- a/network/network.go
+++ b/network/network.go
@ -276,7 +276,7 @@ func (nm *networkManager) FindNetworkIDFromNetNs(netNs string) (string, error) {
 }

 // GetNumEndpointsInNetNs returns number of endpoints
-func (nm *networkManager) GetNumEndpointsInNetNs(netNs string) int {
+func (nm *networkManager) GetNumEndpointsByContainerID(containerID string) int {
 	numEndpoints := 0
 	// Look through the external interfaces
 	for _, iface := range nm.ExternalInterfaces {
@ -285,8 +285,8 @@ func (nm *networkManager) GetNumEndpointsInNetNs(netNs string) int {
 			// Network may have multiple endpoints, so look through all of them
 			for _, endpoint := range network.Endpoints {
 				// If the netNs matches for this endpoint, return the network ID (which is the name)
-				if endpoint.NetNs == netNs {
-					log.Printf("Found endpoint [%s] for NetNS [%s]", endpoint.Id, netNs)
+				if endpoint.ContainerID == containerID {
+					log.Printf("Found endpoint [%s] for containerID [%s]", endpoint.Id, containerID)
 					numEndpoints++
 				}
 			}
--- a/network/network_test.go
+++ b/network/network_test.go
@ -258,10 +258,10 @@ var _ = Describe("Test Network", func() {
 		})
 	})

-	Describe("Test GetNumEndpointsInNetNs", func() {
+	Describe("Test GetNumEndpointsByContainerID", func() {
 		Context("When one network has one endpoint and another network has two endpoints", func() {
 			It("Should return three endpoints", func() {
-				netNs := "989c079b-45a6-485f-8f9e-88b05d6c55c5"
+				containerID := "989c079b-45a6-485f-8f9e-88b05d6c55c5"
 				networkOneID := "byovnetbridge-vlan1-10-128-8-0_23"
 				networkTwoID := "byovnetbridge-vlan2-20-128-8-0_23"

@ -275,14 +275,13 @@ var _ = Describe("Test Network", func() {
 									Endpoints: map[string]*endpoint{
 										"a591be2a-eth0": {
 											Id:          "a591be2a-eth0",
-											NetNs: netNs,
+											ContainerID: containerID,
 										},
 										"a691be2b-eth0": {
 											Id:          "a691be2b-eth0",
-											NetNs: netNs,
+											ContainerID: containerID,
 										},
 									},
-									NetNs: "aaac079b-45a6-485f-8f9e-88b05d6c55c5",
 								},
 							},
 						},
@ -294,29 +293,29 @@ var _ = Describe("Test Network", func() {
 									Endpoints: map[string]*endpoint{
 										"a591be2b-eth0": {
 											Id:          "a591be2b-eth0",
-											NetNs: netNs,
+											ContainerID: containerID,
 										},
 									},
-									NetNs: "aaac079b-45a6-485f-8f9e-88b05d6c55c5",
+									NetNs: "",
 								},
 							},
 						},
 					},
 				}

-				got := nm.GetNumEndpointsInNetNs(netNs)
-				Expect(got).To(Equal(3))
+				got := nm.GetNumEndpointsByContainerID(containerID)
+				Expect(3).To(Equal(got))
 			})
 		})

 		Context("When network does not exist", func() {
 			It("Should return zero endpoints", func() {
-				netNs := "989c079b-45a6-485f-8f9e-88b05d6c55c9"
+				containerID := "989c079b-45a6-485f-8f9e-88b05d6c55c9"
 				nm := &networkManager{
 					ExternalInterfaces: make(map[string]*externalInterface),
 				}

-				got := nm.GetNumEndpointsInNetNs(netNs)
+				got := nm.GetNumEndpointsByContainerID(containerID)
 				Expect(got).To(Equal(0))
 			})
 		})
--- a/network/networkutils/networkutils_linux.go
+++ b/network/networkutils/networkutils_linux.go
@ -4,7 +4,6 @@
 package networkutils

 import (
-	"errors"
 	"fmt"
 	"net"

@ -12,6 +11,7 @@ import (
 	"github.com/Azure/azure-container-networking/log"
 	"github.com/Azure/azure-container-networking/netlink"
 	"github.com/Azure/azure-container-networking/platform"
+	"github.com/pkg/errors"
 )

 /*RFC For Private Address Space: https://tools.ietf.org/html/rfc1918
@ -267,6 +267,12 @@ func (nu NetworkUtils) DisableRAForInterface(ifName string) error {
 	return err
 }

+func (nu NetworkUtils) SetProxyArp(ifName string) error {
+	cmd := fmt.Sprintf("echo 1 > /proc/sys/net/ipv4/conf/%v/proxy_arp", ifName)
+	_, err := nu.plClient.ExecuteCommand(cmd)
+	return errors.Wrapf(err, "failed to set proxy arp for interface %v", ifName)
+}
+
 func getPrivateIPSpace() []string {
 	privateIPAddresses := []string{"10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "169.254.0.0/16"}
 	return privateIPAddresses
--- a/network/ovs_endpoint_snatroute_linux.go
+++ b/network/ovs_endpoint_snatroute_linux.go
@ -29,6 +29,7 @@ func (client *OVSEndpointClient) NewSnatClient(snatBridgeIP, localIP string, epI
 			snatBridgeIP,
 			client.hostPrimaryMac,
 			epInfo.DNS.Servers,
+			false,
 			client.netlink,
 			client.plClient,
 		)
--- a/network/ovs_endpointclient_linux.go
+++ b/network/ovs_endpointclient_linux.go
@ -233,7 +233,7 @@ func (client *OVSEndpointClient) ConfigureContainerInterfacesAndRoutes(epInfo *E
 	return addRoutes(client.netlink, client.netioshim, client.containerVethName, epInfo.Routes)
 }

-func (client *OVSEndpointClient) DeleteEndpoints(ep *endpoint, _ bool) error {
+func (client *OVSEndpointClient) DeleteEndpoints(ep *endpoint) error {
 	log.Printf("[ovs] Deleting veth pair %v %v.", ep.HostIfName, ep.IfName)
 	err := client.netlink.DeleteLink(ep.HostIfName)
 	if err != nil {
--- a/network/snat/snat_linux.go
+++ b/network/snat/snat_linux.go
@ -4,7 +4,6 @@
 package snat

 import (
-	"errors"
 	"fmt"
 	"net"
 	"strings"
@ -15,6 +14,7 @@ import (
 	"github.com/Azure/azure-container-networking/netlink"
 	"github.com/Azure/azure-container-networking/network/networkutils"
 	"github.com/Azure/azure-container-networking/platform"
+	"github.com/pkg/errors"
 )

 const (
@ -40,8 +40,8 @@ type Client struct {
 	localIP                string
 	SnatBridgeIP           string
 	SkipAddressesFromBlock []string
+	enableProxyArpOnBridge bool
 	netlink                netlink.NetlinkInterface
-
 	plClient               platform.ExecClient
 }

@ -51,8 +51,8 @@ func NewSnatClient(hostIfName string,
 	snatBridgeIP string,
 	hostPrimaryMac string,
 	skipAddressesFromBlock []string,
+	enableProxyArpOnBridge bool,
 	nl netlink.NetlinkInterface,
-
 	plClient platform.ExecClient,
 ) Client {
 	log.Printf("Initialize new snat client")
@ -62,8 +62,8 @@ func NewSnatClient(hostIfName string,
 		localIP:                localIP,
 		SnatBridgeIP:           snatBridgeIP,
 		hostPrimaryMac:         hostPrimaryMac,
+		enableProxyArpOnBridge: enableProxyArpOnBridge,
 		netlink:                nl,
-
 		plClient:               plClient,
 	}

@ -81,6 +81,16 @@ func (client *Client) CreateSnatEndpoint() error {
 		return err
 	}

+	nuc := networkutils.NewNetworkUtils(client.netlink, client.plClient)
+	// Enabling proxy arp on bridge allows bridge to respond to arp requests it receives with its own mac otherwise arp requests are not getting forwarded and responded.
+	if client.enableProxyArpOnBridge {
+		// Enable proxy arp on bridge
+		if err := nuc.SetProxyArp(SnatBridgeName); err != nil {
+			log.Printf("Enabling proxy arp failed with error %v", err)
+			return errors.Wrap(err, "")
+		}
+	}
+
 	// SNAT Rule to masquerade packets destined to non-vnet ip
 	if err := client.addMasqueradeRule(client.SnatBridgeIP); err != nil {
 		log.Printf("Adding snat rule failed with error %v", err)
@ -93,9 +103,8 @@ func (client *Client) CreateSnatEndpoint() error {
 		return err
 	}

-	epc := networkutils.NewNetworkUtils(client.netlink, client.plClient)
 	// Create veth pair to tie one end to container and other end to linux bridge
-	if err := epc.CreateEndpoint(client.hostSnatVethName, client.containerSnatVethName, nil); err != nil {
+	if err := nuc.CreateEndpoint(client.hostSnatVethName, client.containerSnatVethName, nil); err != nil {
 		log.Printf("Creating Snat Endpoint failed with error %v", err)
 		return newErrorSnatClient(err.Error())
 	}
@ -127,9 +136,7 @@ func (client *Client) BlockIPAddressesOnSnatBridge() error {
 	return nil
 }

-/**
-	Move container veth inside container network namespace
-**/
+// Move container veth inside container network namespace
 func (client *Client) MoveSnatEndpointToContainerNS(netnsPath string, nsID uintptr) error {
 	log.Printf("[snat] Setting link %v netns %v.", client.containerSnatVethName, netnsPath)
 	err := client.netlink.SetLinkNetNs(client.containerSnatVethName, nsID)
@ -139,9 +146,7 @@ func (client *Client) MoveSnatEndpointToContainerNS(netnsPath string, nsID uintp
 	return nil
 }

-/**
-	Configure Routes and setup name for container veth
-**/
+// Configure Routes and setup name for container veth
 func (client *Client) SetupSnatContainerInterface() error {
 	epc := networkutils.NewNetworkUtils(client.netlink, client.plClient)
 	if err := epc.SetupContainerInterface(client.containerSnatVethName, azureSnatIfName); err != nil {
@ -159,9 +164,7 @@ func getNCLocalAndGatewayIP(client *Client) (brIP, contIP net.IP) {
 	return bridgeIP, containerIP
 }

-/**
-	This function adds iptables rules that allows only host to NC communication and not the other way
-**/
+// This function adds iptables rules that allows only host to NC communication and not the other way
 func (client *Client) AllowInboundFromHostToNC() error {
 	bridgeIP, containerIP := getNCLocalAndGatewayIP(client)

@ -250,9 +253,7 @@ func (client *Client) DeleteInboundFromHostToNC() error {
 	return err
 }

-/**
-	This function adds iptables rules that allows only NC to Host communication and not the other way
-**/
+// This function adds iptables rules that allows only NC to Host communication and not the other way
 func (client *Client) AllowInboundFromNCToHost() error {
 	bridgeIP, containerIP := getNCLocalAndGatewayIP(client)

@ -340,10 +341,7 @@ func (client *Client) DeleteInboundFromNCToHost() error {
 	return err
 }

-/**
-	Configures Local IP Address for container Veth
-**/
-
+// Configures Local IP Address for container Veth
 func (client *Client) ConfigureSnatContainerInterface() error {
 	log.Printf("[snat] Adding IP address %v to link %v.", client.localIP, client.containerSnatVethName)
 	ip, intIpAddr, _ := net.ParseCIDR(client.localIP)
@ -388,9 +386,7 @@ func (client *Client) DropArpForSnatBridgeApipaRange(snatBridgeIP, azSnatVethIfN
 	return err
 }

-/**
-	This function creates linux bridge which will be used for outbound connectivity by NCs
-**/
+// This function creates linux bridge which will be used for outbound connectivity by NCs
 func (client *Client) createSnatBridge(snatBridgeIP, hostPrimaryMac string) error {
 	_, err := net.InterfaceByName(SnatBridgeName)
 	if err == nil {
@ -437,18 +433,14 @@ func (client *Client) createSnatBridge(snatBridgeIP, hostPrimaryMac string) erro
 	return nil
 }

-/**
-	This function adds iptable rules that will snat all traffic that has source ip in apipa range and coming via linux bridge
-**/
+// This function adds iptable rules that will snat all traffic that has source ip in apipa range and coming via linux bridge
 func (client *Client) addMasqueradeRule(snatBridgeIPWithPrefix string) error {
 	_, ipNet, _ := net.ParseCIDR(snatBridgeIPWithPrefix)
 	matchCondition := fmt.Sprintf("-s %s", ipNet.String())
 	return iptables.InsertIptableRule(iptables.V4, iptables.Nat, iptables.Postrouting, matchCondition, iptables.Masquerade)
 }

-/**
-	Drop all vlan traffic on linux bridge
-**/
+// Drop all vlan traffic on linux bridge
 func (client *Client) addVlanDropRule() error {
 	out, err := client.plClient.ExecuteCommand(l2PreroutingEntries)
 	if err != nil {
--- a/network/transparent_endpointclient_linux.go
+++ b/network/transparent_endpointclient_linux.go
@ -314,6 +314,6 @@ func (client *TransparentEndpointClient) setIPV6NeighEntry() error {
 	return nil
 }

-func (client *TransparentEndpointClient) DeleteEndpoints(_ *endpoint, _ bool) error {
+func (client *TransparentEndpointClient) DeleteEndpoints(_ *endpoint) error {
 	return nil
 }
--- a/network/transparent_vlan_endpoint_snatroute_linux.go
+++ b/network/transparent_vlan_endpoint_snatroute_linux.go
@ -17,6 +17,7 @@ func (client *TransparentVlanEndpointClient) NewSnatClient(snatBridgeIP, localIP
 			snatBridgeIP,
 			client.hostPrimaryMac.String(),
 			epInfo.DNS.Servers,
+			true,
 			client.netlink,
 			client.plClient,
 		)
--- a/network/transparent_vlan_endpointclient_linux.go
+++ b/network/transparent_vlan_endpointclient_linux.go
@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net"
 	"strings"
+	"time"

 	"github.com/Azure/azure-container-networking/iptables"
 	"github.com/Azure/azure-container-networking/log"
@ -26,12 +27,21 @@ const (
 	DisableRPFilterCmd = "sysctl -w net.ipv4.conf.all.rp_filter=0" // Command to disable the rp filter for tunneling
 )

+var errNamespaceCreation = fmt.Errorf("Network namespace creation error")
+
+var (
+	numRetries = 5
+	sleepInMs  = 100
+)
+
 type netnsClient interface {
 	Get() (fileDescriptor int, err error)
 	GetFromName(name string) (fileDescriptor int, err error)
 	Set(fileDescriptor int) (err error)
 	NewNamed(name string) (fileDescriptor int, err error)
 	DeleteNamed(name string) (err error)
+	IsNamespaceEqual(fd1, fd2 int) bool
+	NamespaceUniqueID(fd int) string
 }
 type TransparentVlanEndpointClient struct {
 	primaryHostIfName string // So like eth0
@ -112,6 +122,35 @@ func (client *TransparentVlanEndpointClient) AddEndpoints(epInfo *EndpointInfo)
 	})
 }

+func (client *TransparentVlanEndpointClient) createNetworkNamespace(vmNS, numRetries int) error {
+	var isNamespaceUnique bool
+
+	for i := 0; i < numRetries; i++ {
+		vnetNS, err := client.netnsClient.NewNamed(client.vnetNSName)
+		if err != nil {
+			return errors.Wrap(err, "failed to create vnet ns")
+		}
+		log.Printf("Vnet Namespace created: %s", client.netnsClient.NamespaceUniqueID(vnetNS))
+		if !client.netnsClient.IsNamespaceEqual(vnetNS, vmNS) {
+			client.vnetNSFileDescriptor = vnetNS
+			isNamespaceUnique = true
+			break
+		}
+		log.Printf("Vnet Namespace is the same as VM namespace. Deleting and retrying...")
+		delErr := client.netnsClient.DeleteNamed(client.vnetNSName)
+		if delErr != nil {
+			log.Errorf("failed to cleanup/delete ns after failing to create vlan veth:%v", delErr)
+		}
+		time.Sleep(time.Duration(sleepInMs) * time.Millisecond)
+	}
+
+	if !isNamespaceUnique {
+		return errors.Wrap(errNamespaceCreation, "vnet and vm namespace are same")
+	}
+
+	return nil
+}
+
 // Called from AddEndpoints, Namespace: VM
 func (client *TransparentVlanEndpointClient) PopulateVM(epInfo *EndpointInfo) error {
 	vmNS, err := client.netnsClient.Get()
@ -120,17 +159,18 @@ func (client *TransparentVlanEndpointClient) PopulateVM(epInfo *EndpointInfo) er
 	}

 	log.Printf("[transparent vlan] Checking if NS exists...")
-	vnetNS, existingErr := client.netnsClient.GetFromName(client.vnetNSName)
+	var existingErr error
+	client.vnetNSFileDescriptor, existingErr = client.netnsClient.GetFromName(client.vnetNSName)
 	// If the ns does not exist, the below code will trigger to create it
 	// This will also (we assume) mean the vlan veth does not exist
 	if existingErr != nil {
 		// We assume the only possible error is that the namespace doesn't exist
 		log.Printf("[transparent vlan] No existing NS detected. Creating the vnet namespace and switching to it")
-		vnetNS, err = client.netnsClient.NewNamed(client.vnetNSName)
-		if err != nil {
-			return errors.Wrap(err, "failed to create vnet ns")
+
+		if err = client.createNetworkNamespace(vmNS, numRetries); err != nil {
+			return errors.Wrap(err, "")
 		}
-		client.vnetNSFileDescriptor = vnetNS
+
 		deleteNSIfNotNilErr := client.netnsClient.Set(vmNS)
 		// Any failure will trigger removing the namespace created
 		defer func() {
@ -165,9 +205,12 @@ func (client *TransparentVlanEndpointClient) PopulateVM(epInfo *EndpointInfo) er
 		// Create vlan veth
 		deleteNSIfNotNilErr = vishnetlink.LinkAdd(link)
 		if deleteNSIfNotNilErr != nil {
+			// ignore link already exists error
+			if !strings.Contains(deleteNSIfNotNilErr.Error(), "file exists") {
 				// Any failure to add the link should error (auto delete NS)
 				return errors.Wrap(deleteNSIfNotNilErr, "failed to create vlan vnet link after making new ns")
 			}
+		}
 		defer func() {
 			if deleteNSIfNotNilErr != nil {
 				log.Logf("[transparent vlan] removing vlan veth due to failure...")
@ -176,6 +219,19 @@ func (client *TransparentVlanEndpointClient) PopulateVM(epInfo *EndpointInfo) er
 				}
 			}
 		}()
+
+		// sometimes there is slight delay in interface creation. check if it exists
+		for i := 0; i < numRetries; i++ {
+			if _, err = client.netioshim.GetNetworkInterfaceByName(client.vlanIfName); err == nil {
+				break
+			}
+			time.Sleep(time.Duration(sleepInMs) * time.Millisecond)
+		}
+		if err != nil {
+			deleteNSIfNotNilErr = errors.Wrapf(err, "failed to get vlan veth interface:%s", client.vlanIfName)
+			return deleteNSIfNotNilErr
+		}
+
 		deleteNSIfNotNilErr = client.netUtilsClient.DisableRAForInterface(client.vlanIfName)
 		if deleteNSIfNotNilErr != nil {
 			return errors.Wrap(deleteNSIfNotNilErr, "failed to disable router advertisements for vlan vnet link")
@ -189,9 +245,15 @@ func (client *TransparentVlanEndpointClient) PopulateVM(epInfo *EndpointInfo) er
 	} else {
 		log.Printf("[transparent vlan] Existing NS (%s) detected. Assuming %s exists too", client.vnetNSName, client.vlanIfName)
 	}
-	client.vnetNSFileDescriptor = vnetNS

-	if err = client.netUtilsClient.CreateEndpoint(client.vnetVethName, client.containerVethName, nil); err != nil {
+	// Get the default constant host veth mac
+	mac, err := net.ParseMAC(defaultHostVethHwAddr)
+	if err != nil {
+		log.Printf("[net] Failed to parse the mac addrress %v", defaultHostVethHwAddr)
+	}
+
+	// Create veth pair
+	if err = client.netUtilsClient.CreateEndpoint(client.vnetVethName, client.containerVethName, mac); err != nil {
 		return errors.Wrap(err, "failed to create veth pair")
 	}
 	// Disable RA for veth pair, and delete if any failure
@ -361,7 +423,7 @@ func (client *TransparentVlanEndpointClient) ConfigureContainerInterfacesAndRout
 		}
 	}

-	if err := client.AddDefaultRoutes(client.containerVethName, 0); err != nil {
+	if err := client.addDefaultRoutes(client.containerVethName, 0); err != nil {
 		return errors.Wrap(err, "failed container ns add default routes")
 	}
 	if err := client.AddDefaultArp(client.containerVethName, client.vnetMac.String()); err != nil {
@ -379,16 +441,21 @@ func (client *TransparentVlanEndpointClient) ConfigureVnetInterfacesAndRoutesImp

 	// Add route specifying which device the pod ip(s) are on
 	routeInfoList := client.GetVnetRoutes(epInfo.IPAddresses)
-	if err = client.AddDefaultRoutes(client.vlanIfName, 0); err != nil {
+	if err = client.addDefaultRoutes(client.vlanIfName, 0); err != nil {
 		return errors.Wrap(err, "failed vnet ns add default/gateway routes (idempotent)")
 	}
 	if err = client.AddDefaultArp(client.vlanIfName, azureMac); err != nil {
 		return errors.Wrap(err, "failed vnet ns add default arp entry (idempotent)")
 	}
+
+	// Delete old route if any for this IP
+	err = deleteRoutes(client.netlink, client.netioshim, "", routeInfoList)
+	log.Printf("[transparent-vlan] Deleting old routes returned:%v", err)
+
 	if err = addRoutes(client.netlink, client.netioshim, client.vnetVethName, routeInfoList); err != nil {
 		return errors.Wrap(err, "failed adding routes to vnet specific to this container")
 	}
-	if err = client.AddDefaultRoutes(client.vlanIfName, tunnelingTable); err != nil {
+	if err = client.addDefaultRoutes(client.vlanIfName, tunnelingTable); err != nil {
 		return errors.Wrap(err, "failed vnet ns add outbound routing table routes for tunneling (idempotent)")
 	}
 	// Return to ConfigureContainerInterfacesAndRoutes
@ -411,7 +478,7 @@ func (client *TransparentVlanEndpointClient) GetVnetRoutes(ipAddresses []net.IPN
 		} else {
 			ipNet = net.IPNet{IP: ipAddr.IP, Mask: net.CIDRMask(ipv6FullMask, ipv6Bits)}
 		}
-		log.Printf("[net] transparent vlan client adding route for the ip %v", ipNet.String())
+		log.Printf("[net] Getting route for this ip %v", ipNet.String())
 		routeInfo.Dst = ipNet
 		routeInfoList = append(routeInfoList, routeInfo)

@ -423,7 +490,7 @@ func (client *TransparentVlanEndpointClient) GetVnetRoutes(ipAddresses []net.IPN
 // to the virtual gateway ip on linkToName device interface
 // Route 1: 169.254.1.1 dev <linkToName>
 // Route 2: default via 169.254.1.1 dev <linkToName>
-func (client *TransparentVlanEndpointClient) AddDefaultRoutes(linkToName string, table int) error {
+func (client *TransparentVlanEndpointClient) addDefaultRoutes(linkToName string, table int) error {
 	// Add route for virtualgwip (ip route add 169.254.1.1/32 dev eth0)
 	virtualGwIP, virtualGwNet, _ := net.ParseCIDR(virtualGwIPString)
 	routeInfo := RouteInfo{
@ -474,7 +541,7 @@ func (client *TransparentVlanEndpointClient) AddDefaultArp(interfaceName, destMa
 	return nil
 }

-func (client *TransparentVlanEndpointClient) DeleteEndpoints(ep *endpoint, deleteHosVeth bool) error {
+func (client *TransparentVlanEndpointClient) DeleteEndpoints(ep *endpoint) error {
 	// Vnet NS
 	err := ExecuteInNS(client.vnetNSName, func() error {
 		// Passing in functionality to get number of routes after deletion
@ -486,7 +553,7 @@ func (client *TransparentVlanEndpointClient) DeleteEndpoints(ep *endpoint, delet
 			return len(routes), nil
 		}

-		return client.DeleteEndpointsImpl(ep, getNumRoutesLeft, deleteHosVeth)
+		return client.DeleteEndpointsImpl(ep, getNumRoutesLeft)
 	})
 	if err != nil {
 		return err
@ -500,24 +567,16 @@ func (client *TransparentVlanEndpointClient) DeleteEndpoints(ep *endpoint, delet
 }

 // getNumRoutesLeft is a function which gets the current number of routes in the namespace. Namespace: Vnet
-func (client *TransparentVlanEndpointClient) DeleteEndpointsImpl(ep *endpoint, getNumRoutesLeft func() (int, error), deleteHostVeth bool) error {
+func (client *TransparentVlanEndpointClient) DeleteEndpointsImpl(ep *endpoint, _ func() (int, error)) error {
 	routeInfoList := client.GetVnetRoutes(ep.IPAddresses)
 	if err := deleteRoutes(client.netlink, client.netioshim, client.vnetVethName, routeInfoList); err != nil {
 		return errors.Wrap(err, "failed to remove routes")
 	}

-	routesLeft, err := getNumRoutesLeft()
-	if err != nil {
-		return err
-	}
-
-	log.Printf("[transparent vlan] There are %d routes remaining after deletion", routesLeft)
-
+	log.Printf("Deleting host veth %v", client.vnetVethName)
 	// Delete Host Veth
-	if deleteHostVeth {
 	if err := client.netlink.DeleteLink(client.vnetVethName); err != nil {
-			return errors.Wrap(err, "failed to delete host veth")
-		}
+		return errors.Wrapf(err, "deleteLink for %v failed", client.vnetVethName)
 	}

 	// TODO: revist if this require in future.
--- a/network/transparent_vlan_endpointclient_linux_test.go
+++ b/network/transparent_vlan_endpointclient_linux_test.go
@ -49,6 +49,14 @@ func (netns *mockNetns) DeleteNamed(name string) (err error) {
 	return netns.deleteNamed(name)
 }

+func (netns *mockNetns) IsNamespaceEqual(_, _ int) bool {
+	return false
+}
+
+func (netns *mockNetns) NamespaceUniqueID(_ int) string {
+	return "nsid"
+}
+
 func defaultGet() (int, error) {
 	return 1, nil
 }
@ -465,7 +473,7 @@ func TestTransparentVlanDeleteEndpoints(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			err := tt.client.DeleteEndpointsImpl(tt.ep, tt.routesLeft, false)
+			err := tt.client.DeleteEndpointsImpl(tt.ep, tt.routesLeft)
 			if tt.wantErr {
 				require.Error(t, err)
 				require.Contains(t, err.Error(), tt.wantErrMsg, "Expected:%v actual:%v", tt.wantErrMsg, err.Error())