underlay: fix ip/route tranfer when the nic is managed by NetworkMana…

…ger (#3184)

Signed-off-by: 张祖建 <zhangzujian.7@gmail.com>

go.mod

@@ -20,7 +20,7 @@ require (
 	github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0
 	github.com/k8snetworkplumbingwg/sriovnet v1.2.0
 	github.com/kubeovn/go-iptables v0.0.0-20230322103850-8619a8ab3dca
-	github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230327064018-0b27f88874f7
+	github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230905082151-e28c4d73a589
 	github.com/mdlayher/arp v0.0.0-20220512170110-6706a2966875
 	github.com/moby/sys/mountinfo v0.6.2
 	github.com/onsi/ginkgo/v2 v2.11.0
@@ -133,7 +133,7 @@ require (
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/pprof v0.0.0-20230510103437-eeec1cb781c3 // indirect
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
-	github.com/google/uuid v1.3.0 // indirect
+	github.com/google/uuid v1.3.1 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.2.3 // indirect
 	github.com/googleapis/gax-go/v2 v2.7.1 // indirect
 	github.com/gopherjs/gopherjs v1.17.2 // indirect

go.sum

@@ -591,8 +591,9 @@ github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+
 github.com/google/uuid v1.1.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
-github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
 github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+github.com/google/uuid v1.3.1 h1:KjJaJ9iWZ3jOFZIf1Lqf4laDRCasjl0BCmnEGxkdLb4=
+github.com/google/uuid v1.3.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/googleapis/enterprise-certificate-proxy v0.0.0-20220520183353-fd19c99a87aa/go.mod h1:17drOmN3MwGY7t0e+Ei9b45FFGA3fBs3x36SsCg1hq8=
 github.com/googleapis/enterprise-certificate-proxy v0.1.0/go.mod h1:17drOmN3MwGY7t0e+Ei9b45FFGA3fBs3x36SsCg1hq8=
 github.com/googleapis/enterprise-certificate-proxy v0.2.3 h1:yk9/cqRKtT9wXZSsRH9aurXEpJX+U6FLtpYTdC3R06k=
@@ -757,8 +758,8 @@ github.com/kubeovn/felix v0.0.0-20220325073257-c8a0f705d139 h1:MaLC8/dohKHU8nkfg
 github.com/kubeovn/felix v0.0.0-20220325073257-c8a0f705d139/go.mod h1:ulxnUH9cbIOtCH+exhJPeV2mleh+bDv67WKsl/MVU/g=
 github.com/kubeovn/go-iptables v0.0.0-20230322103850-8619a8ab3dca h1:fTMjoho2et9nKVOFrjzVEWVd9XD1zzxOYrlxxZpO0fU=
 github.com/kubeovn/go-iptables v0.0.0-20230322103850-8619a8ab3dca/go.mod h1:jY1XeGzkx8ASNJ+SqQSxTESNXARkjvt+I6IJOTnzIjw=
-github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230327064018-0b27f88874f7 h1:X5/DAYXXe8p3mUz3Z+j0dsgpIUPiNhaq0f7D1Z9/8CY=
-github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230327064018-0b27f88874f7/go.mod h1:rNwHas8aX9k/BEz5dwObhRvfV7KEd0MnrTTDd4gQ3D0=
+github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230905082151-e28c4d73a589 h1:y9exo1hjCsq7jsGUzt11kxhTiEGrGSQ0ZqibAiZk2PQ=
+github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230905082151-e28c4d73a589/go.mod h1:49upX+/hUyppWIqu58cumojyIwXdkA8k6reA/mQlKuI=
 github.com/kubeovn/kubevirt-client-go v0.0.0-20230517062539-8dd832f39ec5 h1:3tygbNMsdRZpFXEofMq8MLkTlWTRd4tDomEZdXxrbUA=
 github.com/kubeovn/kubevirt-client-go v0.0.0-20230517062539-8dd832f39ec5/go.mod h1:GhMrao/zkhyDt9T4Azt3/2zxztchdSecAumECdFi+XQ=
 github.com/kubeovn/libovsdb v0.0.0-20230517064328-9d5a1383643f h1:HDjnbJZN+2T3XH7usjtO2+PYDA2fyrLGYjypEA/87pM=

pkg/daemon/init.go

@@ -108,7 +108,7 @@ func (c *Controller) ovsInitProviderNetwork(provider, nic string, exchangeLinkNa
 	}
 
 	klog.V(3).Infof("configure external bridge %s", brName)
-	if err := configExternalBridge(provider, brName, nic, exchangeLinkName, macLearningFallback); err != nil {
+	if err := c.configExternalBridge(provider, brName, nic, exchangeLinkName, macLearningFallback); err != nil {
 		errMsg := fmt.Errorf("failed to create and configure external bridge %s: %v", brName, err)
 		klog.Error(errMsg)
 		return 0, errMsg
@@ -170,7 +170,7 @@ func (c *Controller) ovsCleanProviderNetwork(provider string) error {
 				continue
 			}
 			klog.V(3).Infof("removing ovs port %s from bridge %s", port, brName)
-			if err = removeProviderNic(port, brName); err != nil {
+			if err = c.removeProviderNic(port, brName); err != nil {
 				errMsg := fmt.Errorf("failed to remove port %s from external bridge %s: %v", port, brName, err)
 				klog.Error(errMsg)
 				return errMsg

pkg/daemon/nm_linux.go

@@ -12,6 +12,14 @@ import (
 	"k8s.io/klog/v2"
 )
 
+const (
+	nmIP4ConfigInterfacePropertiesChanged = gonetworkmanager.IP4ConfigInterface + ".PropertiesChanged"
+	nmIP6ConfigInterfacePropertiesChanged = gonetworkmanager.IP6ConfigInterface + ".PropertiesChanged"
+
+	nmDBusObjectPathIPConfig4 = gonetworkmanager.NetworkManagerObjectPath + "/IP4Config/"
+	nmDBusObjectPathIPConfig6 = gonetworkmanager.NetworkManagerObjectPath + "/IP6Config/"
+)
+
 type networkManagerSyncer struct {
 	manager   gonetworkmanager.NetworkManager
 	workqueue workqueue.Interface
@@ -60,18 +68,15 @@ func (n *networkManagerSyncer) Run(handler func(nic, bridge string, delNonExiste
 		ch := n.manager.Subscribe()
 		defer n.manager.Unsubscribe()
 
-		suffix := "." + gonetworkmanager.ActiveConnectionSignalStateChanged
 		for {
 			event := <-ch
-			if len(event.Body) == 0 || !strings.HasSuffix(event.Name, suffix) {
+			if event.Name != nmIP4ConfigInterfacePropertiesChanged &&
+				event.Name != nmIP6ConfigInterfacePropertiesChanged {
 				continue
 			}
-			state, ok := event.Body[0].(uint32)
-			if !ok {
-				klog.Warningf("failed to convert %#v to uint32", event.Body[0])
-				continue
-			}
-			if gonetworkmanager.NmDeviceState(state) != gonetworkmanager.NmDeviceStateActivated {
+			path := string(event.Path)
+			if !strings.HasPrefix(path, nmDBusObjectPathIPConfig4) &&
+				!strings.HasPrefix(path, nmDBusObjectPathIPConfig6) {
 				continue
 			}
 
@@ -83,13 +88,29 @@ func (n *networkManagerSyncer) Run(handler func(nic, bridge string, delNonExiste
 
 			var device gonetworkmanager.Device
 			for _, dev := range devices {
-				if dev.GetPath() == event.Path {
-					device = dev
-					break
+				if event.Name == nmIP4ConfigInterfacePropertiesChanged {
+					config, err := dev.GetPropertyIP4Config()
+					if err != nil {
+						klog.Errorf("failed to get ipv4 config of device %s: %v", dev.GetPath(), err)
+						break
+					}
+					if config != nil && config.Path() == event.Path {
+						device = dev
+						break
+					}
+				} else {
+					config, err := dev.GetPropertyIP6Config()
+					if err != nil {
+						klog.Errorf("failed to get ipv6 config of device %s: %v", dev.GetPath(), err)
+						break
+					}
+					if config != nil && config.Path() == event.Path {
+						device = dev
+						break
+					}
 				}
 			}
 			if device == nil {
-				klog.Warningf("NetworkManager device %s not found", event.Path)
 				continue
 			}
 
@@ -106,7 +127,7 @@ func (n *networkManagerSyncer) Run(handler func(nic, bridge string, delNonExiste
 			}
 			n.lock.Unlock()
 
-			klog.Infof("adding device %s to workqueue", name)
+			klog.Infof("ip address change detected on device %q, adding to workqueue", name)
 			n.workqueue.Add(name)
 		}
 	}()
@@ -152,17 +173,15 @@ func (n *networkManagerSyncer) AddDevice(nicName, bridge string) error {
 	return nil
 }
 
-func (n *networkManagerSyncer) RemoveDevice(nicName string) error {
+func (n *networkManagerSyncer) RemoveDevice(nicName string) {
 	if n.manager == nil {
-		return nil
+		return
 	}
 
 	n.lock.Lock()
 	n.devices.Remove(nicName)
 	delete(n.bridgeMap, nicName)
 	n.lock.Unlock()
-
-	return nil
 }
 
 func (n *networkManagerSyncer) SetManaged(name string, managed bool) error {

pkg/daemon/ovs.go

@@ -166,7 +166,7 @@ func removeOvnMapping(name, key string) error {
 	return nil
 }
 
-func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLearningFallback bool) error {
+func (c *Controller) configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLearningFallback bool) error {
 	brExists, err := ovs.BridgeExists(bridge)
 	if err != nil {
 		return fmt.Errorf("failed to check OVS bridge existence: %v", err)
@@ -196,7 +196,7 @@ func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLea
 					return fmt.Errorf("failed to check vendor of port %s: %v", port, err)
 				}
 				if ok {
-					if err = removeProviderNic(port, bridge); err != nil {
+					if err = c.removeProviderNic(port, bridge); err != nil {
 						return fmt.Errorf("failed to remove port %s from OVS bridge %s: %v", port, bridge, err)
 					}
 				}

pkg/daemon/ovs_linux.go

@@ -941,11 +941,13 @@ func (c *Controller) transferAddrsAndRoutes(nicName, brName string, delNonExiste
 		return 0, err
 	}
 
+	var count int
 	for _, addr := range addrs {
 		if addr.IP.IsLinkLocalUnicast() {
 			// skip 169.254.0.0/16 and fe80::/10
 			continue
 		}
+		count++
 
 		if err = netlink.AddrDel(nic, &addr); err != nil {
 			errMsg := fmt.Errorf("failed to delete address %q on nic %s: %v", addr.String(), nicName, err)
@@ -961,13 +963,16 @@ func (c *Controller) transferAddrsAndRoutes(nicName, brName string, delNonExiste
 		}
 		klog.Infof("address %q has been added/replaced to link %s", addr.String(), brName)
 	}
-	for _, addr := range delAddrs {
-		if err = netlink.AddrDel(bridge, &addr); err != nil {
-			errMsg := fmt.Errorf("failed to delete address %q on OVS bridge %s: %v", addr.String(), brName, err)
-			klog.Error(errMsg)
-			return 0, errMsg
+
+	if count != 0 {
+		for _, addr := range delAddrs {
+			if err = netlink.AddrDel(bridge, &addr); err != nil {
+				errMsg := fmt.Errorf("failed to delete address %q on OVS bridge %s: %v", addr.String(), brName, err)
+				klog.Error(errMsg)
+				return 0, errMsg
+			}
+			klog.Infof("address %q has been removed from OVS bridge %s", addr.String(), brName)
 		}
-		klog.Infof("address %q has been removed from OVS bridge %s", addr.String(), brName)
 	}
 
 	// keep mac address the same with the provider nic,
@@ -1008,7 +1013,7 @@ func (c *Controller) transferAddrsAndRoutes(nicName, brName string, delNonExiste
 	}
 
 	var delRoutes []netlink.Route
-	if delNonExistent {
+	if delNonExistent && count != 0 {
 		for _, route := range brRoutes {
 			if route.Gw == nil && route.Dst != nil && route.Dst.IP.IsLinkLocalUnicast() {
 				// skip 169.254.0.0/16 and fe80::/10
@@ -1101,7 +1106,9 @@ func linkIsAlbBond(link netlink.Link) (bool, error) {
 
 // Remove host nic from external bridge
 // IP addresses & routes will be transferred to the host nic
-func removeProviderNic(nicName, brName string) error {
+func (c *Controller) removeProviderNic(nicName, brName string) error {
+	c.nmSyncer.RemoveDevice(nicName)
+
 	nic, err := netlink.LinkByName(nicName)
 	if err != nil {
 		if _, ok := err.(netlink.LinkNotFoundError); ok {

pkg/daemon/ovs_windows.go

@@ -325,7 +325,7 @@ func (c *Controller) configProviderNic(nicName, brName string) (int, error) {
 	return 0, nil
 }
 
-func removeProviderNic(nicName, brName string) error {
+func (c *Controller) removeProviderNic(nicName, brName string) error {
 	// nothing to do on Windows
 	return nil
 }