Skip to content

Commit 0aa39e8

Browse files
zhangzujianzhangzujian
committed
underlay: fix ip/route tranfer when the nic is managed by NetworkManager (#3184)
Signed-off-by: 张祖建 <zhangzujian.7@gmail.com>
1 parent 47f475b commit 0aa39e8

File tree

7 files changed

+62
-35
lines changed

7 files changed

+62
-35
lines changed

‎go.mod

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ require (
2020
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.4.0
2121
github.com/k8snetworkplumbingwg/sriovnet v1.2.0
2222
github.com/kubeovn/go-iptables v0.0.0-20230322103850-8619a8ab3dca
23-
github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230327064018-0b27f88874f7
23+
github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230905082151-e28c4d73a589
2424
github.com/mdlayher/arp v0.0.0-20220512170110-6706a2966875
2525
github.com/moby/sys/mountinfo v0.6.2
2626
github.com/onsi/ginkgo/v2 v2.11.0
@@ -133,7 +133,7 @@ require (
133133
github.com/google/gofuzz v1.2.0 // indirect
134134
github.com/google/pprof v0.0.0-20230510103437-eeec1cb781c3 // indirect
135135
github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
136-
github.com/google/uuid v1.3.0 // indirect
136+
github.com/google/uuid v1.3.1 // indirect
137137
github.com/googleapis/enterprise-certificate-proxy v0.2.3 // indirect
138138
github.com/googleapis/gax-go/v2 v2.7.1 // indirect
139139
github.com/gopherjs/gopherjs v1.17.2 // indirect

‎go.sum

+4-3
Original file line numberDiff line numberDiff line change
@@ -591,8 +591,9 @@ github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+
591591
github.com/google/uuid v1.1.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
592592
github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
593593
github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
594-
github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
595594
github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
595+
github.com/google/uuid v1.3.1 h1:KjJaJ9iWZ3jOFZIf1Lqf4laDRCasjl0BCmnEGxkdLb4=
596+
github.com/google/uuid v1.3.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
596597
github.com/googleapis/enterprise-certificate-proxy v0.0.0-20220520183353-fd19c99a87aa/go.mod h1:17drOmN3MwGY7t0e+Ei9b45FFGA3fBs3x36SsCg1hq8=
597598
github.com/googleapis/enterprise-certificate-proxy v0.1.0/go.mod h1:17drOmN3MwGY7t0e+Ei9b45FFGA3fBs3x36SsCg1hq8=
598599
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
757758
github.com/kubeovn/felix v0.0.0-20220325073257-c8a0f705d139/go.mod h1:ulxnUH9cbIOtCH+exhJPeV2mleh+bDv67WKsl/MVU/g=
758759
github.com/kubeovn/go-iptables v0.0.0-20230322103850-8619a8ab3dca h1:fTMjoho2et9nKVOFrjzVEWVd9XD1zzxOYrlxxZpO0fU=
759760
github.com/kubeovn/go-iptables v0.0.0-20230322103850-8619a8ab3dca/go.mod h1:jY1XeGzkx8ASNJ+SqQSxTESNXARkjvt+I6IJOTnzIjw=
760-
github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230327064018-0b27f88874f7 h1:X5/DAYXXe8p3mUz3Z+j0dsgpIUPiNhaq0f7D1Z9/8CY=
761-
github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230327064018-0b27f88874f7/go.mod h1:rNwHas8aX9k/BEz5dwObhRvfV7KEd0MnrTTDd4gQ3D0=
761+
github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230905082151-e28c4d73a589 h1:y9exo1hjCsq7jsGUzt11kxhTiEGrGSQ0ZqibAiZk2PQ=
762+
github.com/kubeovn/gonetworkmanager/v2 v2.0.0-20230905082151-e28c4d73a589/go.mod h1:49upX+/hUyppWIqu58cumojyIwXdkA8k6reA/mQlKuI=
762763
github.com/kubeovn/kubevirt-client-go v0.0.0-20230517062539-8dd832f39ec5 h1:3tygbNMsdRZpFXEofMq8MLkTlWTRd4tDomEZdXxrbUA=
763764
github.com/kubeovn/kubevirt-client-go v0.0.0-20230517062539-8dd832f39ec5/go.mod h1:GhMrao/zkhyDt9T4Azt3/2zxztchdSecAumECdFi+XQ=
764765
github.com/kubeovn/libovsdb v0.0.0-20230517064328-9d5a1383643f h1:HDjnbJZN+2T3XH7usjtO2+PYDA2fyrLGYjypEA/87pM=

‎pkg/daemon/init.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func (c *Controller) ovsInitProviderNetwork(provider, nic string, exchangeLinkNa
108108
}
109109

110110
klog.V(3).Infof("configure external bridge %s", brName)
111-
if err := configExternalBridge(provider, brName, nic, exchangeLinkName, macLearningFallback); err != nil {
111+
if err := c.configExternalBridge(provider, brName, nic, exchangeLinkName, macLearningFallback); err != nil {
112112
errMsg := fmt.Errorf("failed to create and configure external bridge %s: %v", brName, err)
113113
klog.Error(errMsg)
114114
return 0, errMsg
@@ -170,7 +170,7 @@ func (c *Controller) ovsCleanProviderNetwork(provider string) error {
170170
continue
171171
}
172172
klog.V(3).Infof("removing ovs port %s from bridge %s", port, brName)
173-
if err = removeProviderNic(port, brName); err != nil {
173+
if err = c.removeProviderNic(port, brName); err != nil {
174174
errMsg := fmt.Errorf("failed to remove port %s from external bridge %s: %v", port, brName, err)
175175
klog.Error(errMsg)
176176
return errMsg

‎pkg/daemon/nm_linux.go

+36-17
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ import (
1212
"k8s.io/klog/v2"
1313
)
1414

15+
const (
16+
nmIP4ConfigInterfacePropertiesChanged = gonetworkmanager.IP4ConfigInterface + ".PropertiesChanged"
17+
nmIP6ConfigInterfacePropertiesChanged = gonetworkmanager.IP6ConfigInterface + ".PropertiesChanged"
18+
19+
nmDBusObjectPathIPConfig4 = gonetworkmanager.NetworkManagerObjectPath + "/IP4Config/"
20+
nmDBusObjectPathIPConfig6 = gonetworkmanager.NetworkManagerObjectPath + "/IP6Config/"
21+
)
22+
1523
type networkManagerSyncer struct {
1624
manager gonetworkmanager.NetworkManager
1725
workqueue workqueue.Interface
@@ -60,18 +68,15 @@ func (n *networkManagerSyncer) Run(handler func(nic, bridge string, delNonExiste
6068
ch := n.manager.Subscribe()
6169
defer n.manager.Unsubscribe()
6270

63-
suffix := "." + gonetworkmanager.ActiveConnectionSignalStateChanged
6471
for {
6572
event := <-ch
66-
if len(event.Body) == 0 || !strings.HasSuffix(event.Name, suffix) {
73+
if event.Name != nmIP4ConfigInterfacePropertiesChanged &&
74+
event.Name != nmIP6ConfigInterfacePropertiesChanged {
6775
continue
6876
}
69-
state, ok := event.Body[0].(uint32)
70-
if !ok {
71-
klog.Warningf("failed to convert %#v to uint32", event.Body[0])
72-
continue
73-
}
74-
if gonetworkmanager.NmDeviceState(state) != gonetworkmanager.NmDeviceStateActivated {
77+
path := string(event.Path)
78+
if !strings.HasPrefix(path, nmDBusObjectPathIPConfig4) &&
79+
!strings.HasPrefix(path, nmDBusObjectPathIPConfig6) {
7580
continue
7681
}
7782

@@ -83,13 +88,29 @@ func (n *networkManagerSyncer) Run(handler func(nic, bridge string, delNonExiste
8388

8489
var device gonetworkmanager.Device
8590
for _, dev := range devices {
86-
if dev.GetPath() == event.Path {
87-
device = dev
88-
break
91+
if event.Name == nmIP4ConfigInterfacePropertiesChanged {
92+
config, err := dev.GetPropertyIP4Config()
93+
if err != nil {
94+
klog.Errorf("failed to get ipv4 config of device %s: %v", dev.GetPath(), err)
95+
break
96+
}
97+
if config != nil && config.Path() == event.Path {
98+
device = dev
99+
break
100+
}
101+
} else {
102+
config, err := dev.GetPropertyIP6Config()
103+
if err != nil {
104+
klog.Errorf("failed to get ipv6 config of device %s: %v", dev.GetPath(), err)
105+
break
106+
}
107+
if config != nil && config.Path() == event.Path {
108+
device = dev
109+
break
110+
}
89111
}
90112
}
91113
if device == nil {
92-
klog.Warningf("NetworkManager device %s not found", event.Path)
93114
continue
94115
}
95116

@@ -106,7 +127,7 @@ func (n *networkManagerSyncer) Run(handler func(nic, bridge string, delNonExiste
106127
}
107128
n.lock.Unlock()
108129

109-
klog.Infof("adding device %s to workqueue", name)
130+
klog.Infof("ip address change detected on device %q, adding to workqueue", name)
110131
n.workqueue.Add(name)
111132
}
112133
}()
@@ -152,17 +173,15 @@ func (n *networkManagerSyncer) AddDevice(nicName, bridge string) error {
152173
return nil
153174
}
154175

155-
func (n *networkManagerSyncer) RemoveDevice(nicName string) error {
176+
func (n *networkManagerSyncer) RemoveDevice(nicName string) {
156177
if n.manager == nil {
157-
return nil
178+
return
158179
}
159180

160181
n.lock.Lock()
161182
n.devices.Remove(nicName)
162183
delete(n.bridgeMap, nicName)
163184
n.lock.Unlock()
164-
165-
return nil
166185
}
167186

168187
func (n *networkManagerSyncer) SetManaged(name string, managed bool) error {

‎pkg/daemon/ovs.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ func removeOvnMapping(name, key string) error {
166166
return nil
167167
}
168168

169-
func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLearningFallback bool) error {
169+
func (c *Controller) configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLearningFallback bool) error {
170170
brExists, err := ovs.BridgeExists(bridge)
171171
if err != nil {
172172
return fmt.Errorf("failed to check OVS bridge existence: %v", err)
@@ -196,7 +196,7 @@ func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLea
196196
return fmt.Errorf("failed to check vendor of port %s: %v", port, err)
197197
}
198198
if ok {
199-
if err = removeProviderNic(port, bridge); err != nil {
199+
if err = c.removeProviderNic(port, bridge); err != nil {
200200
return fmt.Errorf("failed to remove port %s from OVS bridge %s: %v", port, bridge, err)
201201
}
202202
}

‎pkg/daemon/ovs_linux.go

+15-8
Original file line numberDiff line numberDiff line change
@@ -941,11 +941,13 @@ func (c *Controller) transferAddrsAndRoutes(nicName, brName string, delNonExiste
941941
return 0, err
942942
}
943943

944+
var count int
944945
for _, addr := range addrs {
945946
if addr.IP.IsLinkLocalUnicast() {
946947
// skip 169.254.0.0/16 and fe80::/10
947948
continue
948949
}
950+
count++
949951

950952
if err = netlink.AddrDel(nic, &addr); err != nil {
951953
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
961963
}
962964
klog.Infof("address %q has been added/replaced to link %s", addr.String(), brName)
963965
}
964-
for _, addr := range delAddrs {
965-
if err = netlink.AddrDel(bridge, &addr); err != nil {
966-
errMsg := fmt.Errorf("failed to delete address %q on OVS bridge %s: %v", addr.String(), brName, err)
967-
klog.Error(errMsg)
968-
return 0, errMsg
966+
967+
if count != 0 {
968+
for _, addr := range delAddrs {
969+
if err = netlink.AddrDel(bridge, &addr); err != nil {
970+
errMsg := fmt.Errorf("failed to delete address %q on OVS bridge %s: %v", addr.String(), brName, err)
971+
klog.Error(errMsg)
972+
return 0, errMsg
973+
}
974+
klog.Infof("address %q has been removed from OVS bridge %s", addr.String(), brName)
969975
}
970-
klog.Infof("address %q has been removed from OVS bridge %s", addr.String(), brName)
971976
}
972977

973978
// keep mac address the same with the provider nic,
@@ -1008,7 +1013,7 @@ func (c *Controller) transferAddrsAndRoutes(nicName, brName string, delNonExiste
10081013
}
10091014

10101015
var delRoutes []netlink.Route
1011-
if delNonExistent {
1016+
if delNonExistent && count != 0 {
10121017
for _, route := range brRoutes {
10131018
if route.Gw == nil && route.Dst != nil && route.Dst.IP.IsLinkLocalUnicast() {
10141019
// skip 169.254.0.0/16 and fe80::/10
@@ -1101,7 +1106,9 @@ func linkIsAlbBond(link netlink.Link) (bool, error) {
11011106

11021107
// Remove host nic from external bridge
11031108
// IP addresses & routes will be transferred to the host nic
1104-
func removeProviderNic(nicName, brName string) error {
1109+
func (c *Controller) removeProviderNic(nicName, brName string) error {
1110+
c.nmSyncer.RemoveDevice(nicName)
1111+
11051112
nic, err := netlink.LinkByName(nicName)
11061113
if err != nil {
11071114
if _, ok := err.(netlink.LinkNotFoundError); ok {

‎pkg/daemon/ovs_windows.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ func (c *Controller) configProviderNic(nicName, brName string) (int, error) {
325325
return 0, nil
326326
}
327327

328-
func removeProviderNic(nicName, brName string) error {
328+
func (c *Controller) removeProviderNic(nicName, brName string) error {
329329
// nothing to do on Windows
330330
return nil
331331
}

0 commit comments

Comments
 (0)