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

…ger (#3184)

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

go.mod

@@ -15,7 +15,7 @@ require (
 	github.com/golang/protobuf v1.5.2
 	github.com/greenpau/ovsdb v0.0.0-20181114004433-3582b85e8968
 	github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.1.1-0.20210510153419-66a699ae3b05
-	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/neverlee/keymutex v0.0.0-20171121013845-f593aa834bf9
 	github.com/oilbeater/go-ping v0.0.0-20200413021620-332b7197c5b5
@@ -70,7 +70,7 @@ require (
 	github.com/golang/mock v1.5.0 // indirect
 	github.com/google/go-cmp v0.5.9 // indirect
 	github.com/google/gofuzz v1.1.0 // indirect
-	github.com/google/uuid v1.3.0 // indirect
+	github.com/google/uuid v1.3.1 // indirect
 	github.com/googleapis/gnostic v0.5.5 // indirect
 	github.com/gopherjs/gopherjs v0.0.0-20200217142428-fce0ec30dd00 // indirect
 	github.com/gorilla/websocket v1.4.2 // indirect

go.sum

@@ -541,8 +541,8 @@ 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/gax-go v2.0.2+incompatible/go.mod h1:SFVmujtThgffbyetf+mdk2eWhX2bMyUtNHzFKcPA9HY=
 github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+vpHVxEJEs9eg=
 github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
@@ -693,8 +693,8 @@ github.com/kubeovn/arp v0.0.0-20230101053045-8a0772d9c34c h1:AcOKlV+lInNlGO3o3+1
 github.com/kubeovn/arp v0.0.0-20230101053045-8a0772d9c34c/go.mod h1:Ce8lvkopTGXfPmeb5AY3/umEOmoFVV3HlCPGfGk0+Y0=
 github.com/kubeovn/felix v0.0.0-20220325073257-c8a0f705d139 h1:MaLC8/dohKHU8nkfglfE2oikefB6urJG75yZDOcKTRU=
 github.com/kubeovn/felix v0.0.0-20220325073257-c8a0f705d139/go.mod h1:ulxnUH9cbIOtCH+exhJPeV2mleh+bDv67WKsl/MVU/g=
-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/libovsdb v0.0.0-20221125061852-8b910935f8e4 h1:S/R2b2/S7L3l68Y2YwV/0zDGEVDargfRVqYu6989fIw=
 github.com/kubeovn/libovsdb v0.0.0-20221125061852-8b910935f8e4/go.mod h1:N20zsElkDpTm57hVDosiZVghprt9Y4Vfqsi1HBXOzr4=
 github.com/kubeovn/netlink v0.0.0-20230322092337-960188369daf h1:inZiuUjcQaX0O0Sdki38TWzCl0+wJty+vaQKEr47by8=

pkg/daemon/init.go

@@ -243,7 +243,7 @@ func (c *Controller) ovsInitProviderNetwork(provider, nic string, exchangeLinkNa
 		}
 	}
 
-	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
@@ -305,7 +305,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

@@ -11,6 +11,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
@@ -59,18 +67,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) {
-				continue
-			}
-			state, ok := event.Body[0].(uint32)
-			if !ok {
-				klog.Warningf("failed to convert %#v to uint32", event.Body[0])
+			if event.Name != nmIP4ConfigInterfacePropertiesChanged &&
+				event.Name != nmIP6ConfigInterfacePropertiesChanged {
 				continue
 			}
-			if gonetworkmanager.NmDeviceState(state) != gonetworkmanager.NmDeviceStateActivated {
+			path := string(event.Path)
+			if !strings.HasPrefix(path, nmDBusObjectPathIPConfig4) &&
+				!strings.HasPrefix(path, nmDBusObjectPathIPConfig6) {
 				continue
 			}
 
@@ -82,13 +87,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
 			}
 
@@ -105,7 +126,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)
 		}
 	}()
@@ -151,17 +172,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

@@ -643,7 +643,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)
@@ -673,7 +673,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)
 					}
 				}
@@ -752,11 +752,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)
@@ -766,18 +768,22 @@ func (c *Controller) transferAddrsAndRoutes(nicName, brName string, delNonExiste
 		klog.Infof("address %q has been removed from link %s", addr.String(), nicName)
 
 		addr.Label = ""
+		addr.PreferedLft, addr.ValidLft = 0, 0
 		if err = netlink.AddrReplace(bridge, &addr); err != nil {
 			return 0, fmt.Errorf("failed to replace address %q on OVS bridge %s: %v", addr.String(), brName, err)
 		}
 		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,
@@ -818,7 +824,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
@@ -911,7 +917,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 {