From 569b576ac4e4c42890721909996071147ca0fcd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=A5=96=E5=BB=BA?= Date: Wed, 22 Mar 2023 11:44:03 +0800 Subject: [PATCH] underlay: fix link name exchange (#2516) --- dist/images/start-ovs.sh | 147 ++++----------------------------------- pkg/daemon/controller.go | 4 ++ pkg/daemon/init.go | 37 ++++------ pkg/daemon/init_linux.go | 53 ++++++++++++-- pkg/daemon/ovs.go | 68 ++++++++++++------ pkg/daemon/ovs_linux.go | 57 ++++++++------- 6 files changed, 151 insertions(+), 215 deletions(-) diff --git a/dist/images/start-ovs.sh b/dist/images/start-ovs.sh index a7ea06e07c0..0b122113e26 100755 --- a/dist/images/start-ovs.sh +++ b/dist/images/start-ovs.sh @@ -81,151 +81,32 @@ ovsdb_server_ctl="/var/run/openvswitch/ovsdb-server.$(cat /var/run/openvswitch/o ovs-appctl -t "$ovsdb_server_ctl" vlog/set jsonrpc:file:err ovs-appctl -t "$ovsdb_server_ctl" vlog/set reconnect:file:err -function exchange_link_names() { - mappings=($(ovs-vsctl --if-exists get open . external-ids:ovn-bridge-mappings | tr -d '"' | tr ',' ' ')) - bridges=($(ovs-vsctl --no-heading --columns=name find bridge external-ids:vendor=kube-ovn external-ids:exchange-link-name=true)) +function handle_underlay_bridges() { + bridges=($(ovs-vsctl --no-heading --columns=name find bridge external-ids:vendor=kube-ovn)) for br in ${bridges[@]}; do - provider="" - for m in ${mappings[*]}; do - if echo $m | grep -q ":$br"'$'; then - provider=${m%:$br} - break - fi - done - if [ "x$provider" = "x" ]; then - echo "error: failed to get provider name for bridge $br" - continue + if ! ip link show $br >/dev/null; then + # the bridge does not exist, leave it to be handled by kube-ovn-cni + echo "deleting ovs bridge $br" + ovs-vsctl --no-wait del-br $br fi + done - port="br-$provider" - if ip link show $port 2>/dev/null; then - echo "link $port already exists" - continue - fi - if ! ip link show $br 2>/dev/null; then - echo "link $br does not exists" - continue + bridges=($(ovs-vsctl --no-heading --columns=name find bridge external-ids:vendor=kube-ovn external-ids:exchange-link-name=true)) + for br in ${bridges[@]}; do + if [ -z $(ip link show $br type openvswitch 2>/dev/null || true) ]; then + # the bridge does not exist, leave it to be handled by kube-ovn-cni + echo "deleting ovs bridge $br" + ovs-vsctl --no-wait del-br $br fi - - echo "change link name from $br to $port" - ipv4_routes=($(ip -4 route show dev $br | tr ' ' '#')) - ipv6_routes=($(ip -6 route show dev $br | tr ' ' '#')) - ip link set $br down - ip link set $br name $port - ip link set $port up - - # transfer IPv4 routes - default_ipv4_routes=() - for route in ${ipv4_routes[@]}; do - r=$(echo $route | tr '#' ' ') - if echo $r | grep -q -w 'scope link'; then - printf "add/replace IPv4 route $r to $port\n" - ip -4 route replace $r dev $port - else - default_ipv4_routes=(${default_ipv4_routes[@]} $route) - fi - done - for route in ${default_ipv4_routes[@]}; do - r=$(echo $route | tr '#' ' ') - printf "add/replace IPv4 route $r to $port\n" - ip -4 route replace $r dev $port - done - - # transfer IPv6 routes - default_ipv6_routes=() - for route in ${ipv6_routes[@]}; do - r=$(echo $route | tr '#' ' ') - if echo $r | grep -q -w 'scope link'; then - printf "add/replace IPv6 route $r to $port\n" - ip -6 route replace $r dev $port - else - default_ipv6_routes=(${default_ipv6_routes[@]} $route) - fi - done - for route in ${default_ipv6_routes[@]}; do - r=$(echo $route | tr '#' ' ') - printf "add/replace IPv6 route $r to $port\n" - ip -6 route replace $r dev $port - done done } -exchange_link_names +handle_underlay_bridges # Start vswitchd. restart will automatically set/unset flow-restore-wait which is not what we want /usr/share/openvswitch/scripts/ovs-ctl restart --no-ovsdb-server --system-id=random --no-mlockall /usr/share/openvswitch/scripts/ovs-ctl --protocol=udp --dport=6081 enable-protocol -sleep 1 - -function handle_underlay_bridges() { - bridges=($(ovs-vsctl --no-heading --columns=name find bridge external-ids:vendor=kube-ovn)) - for br in ${bridges[@]}; do - echo "handle bridge $br" - ip link set $br up - - ports=($(ovs-vsctl list-ports $br)) - for port in ${ports[@]}; do - port_type=$(ovs-vsctl --no-heading --columns=type find interface name=$port) - if [ ! "x$port_type" = 'x""' ]; then - continue - fi - - echo "handle port $port on bridge $br" - ipv4_routes=($(ip -4 route show dev $port | tr ' ' '#')) - ipv6_routes=($(ip -6 route show dev $port | tr ' ' '#')) - - set +o pipefail - addresses=($(ip addr show dev $port | grep -E '^\s*inet[6]?\s+' | grep -w global | awk '{print $2}')) - set -o pipefail - - # transfer IP addresses - for addr in ${addresses[@]}; do - printf "delete address $addr on $port\n" - ip addr del $addr dev $port || true - printf "add/replace address $addr to $br\n" - ip addr replace $addr dev $br - done - - # transfer IPv4 routes - default_ipv4_routes=() - for route in ${ipv4_routes[@]}; do - r=$(echo $route | tr '#' ' ') - if echo $r | grep -q -w 'scope link'; then - printf "add/replace IPv4 route $r to $br\n" - ip -4 route replace $r dev $br - else - default_ipv4_routes=(${default_ipv4_routes[@]} $route) - fi - done - for route in ${default_ipv4_routes[@]}; do - r=$(echo $route | tr '#' ' ') - printf "add/replace IPv4 route $r to $br\n" - ip -4 route replace $r dev $br - done - - # transfer IPv6 routes - default_ipv6_routes=() - for route in ${ipv6_routes[@]}; do - r=$(echo $route | tr '#' ' ') - if echo $r | grep -q -w 'scope link'; then - printf "add/replace IPv6 route $r to $br\n" - ip -6 route replace $r dev $br - else - default_ipv6_routes=(${default_ipv6_routes[@]} $route) - fi - done - for route in ${default_ipv6_routes[@]}; do - r=$(echo $route | tr '#' ' ') - printf "add/replace IPv6 route $r to $br\n" - ip -6 route replace $r dev $br - done - done - done -} - -handle_underlay_bridges - function gen_conn_str { if [[ -z "${OVN_DB_IPS}" ]]; then if [[ "$ENABLE_SSL" == "false" ]]; then diff --git a/pkg/daemon/controller.go b/pkg/daemon/controller.go index 8d7c1946bde..64744b35e2a 100644 --- a/pkg/daemon/controller.go +++ b/pkg/daemon/controller.go @@ -323,6 +323,10 @@ func (c *Controller) recordProviderNetworkErr(providerNetwork string, errMsg str break } } + if currentPod == nil { + klog.Warning("failed to get self pod") + return + } } else { if currentPod, err = c.config.KubeClient.CoreV1().Pods(c.localNamespace).Get(context.Background(), c.localPodName, metav1.GetOptions{}); err != nil { klog.Errorf("failed to get pod %s, %v", c.localPodName, err) diff --git a/pkg/daemon/init.go b/pkg/daemon/init.go index 47a048d4ed6..ce24a98f894 100644 --- a/pkg/daemon/init.go +++ b/pkg/daemon/init.go @@ -148,34 +148,11 @@ func ovsCleanProviderNetwork(provider string) error { for idx, m = range brMappings { if strings.HasPrefix(m, mappingPrefix) { brName = m[len(mappingPrefix):] + klog.V(3).Infof("found bridge name for provider %s: %s", provider, brName) break } } - if output, err = ovs.Exec("list-br"); err != nil { - return fmt.Errorf("failed to list OVS bridge %v: %q", err, output) - } - - if !util.ContainsString(strings.Split(output, "\n"), brName) { - return nil - } - - // get host nic - if output, err = ovs.Exec("list-ports", brName); err != nil { - return fmt.Errorf("failed to list ports of OVS bridge %s, %v: %q", brName, err, output) - } - - // remove host nic from the external bridge - if output != "" { - for _, port := range strings.Split(output, "\n") { - if err = 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 - } - } - } - if idx != len(brMappings) { brMappings = append(brMappings[:idx], brMappings[idx+1:]...) if len(brMappings) == 0 { @@ -207,6 +184,15 @@ func ovsCleanProviderNetwork(provider string) error { return fmt.Errorf("failed to set ovn-chassis-mac-mappings, %v: %q", err, output) } + if output, err = ovs.Exec("list-br"); err != nil { + return fmt.Errorf("failed to list OVS bridge %v: %q", err, output) + } + + if !util.ContainsString(strings.Split(output, "\n"), brName) { + klog.V(3).Infof("ovs bridge %s not found", brName) + return nil + } + // get host nic if output, err = ovs.Exec("list-ports", brName); err != nil { return fmt.Errorf("failed to list ports of OVS bridge %s, %v: %q", brName, err, output) @@ -215,11 +201,13 @@ func ovsCleanProviderNetwork(provider string) error { // remove host nic from the external bridge if output != "" { for _, port := range strings.Split(output, "\n") { + klog.V(3).Infof("removing ovs port %s from bridge %s", port, brName) if err = 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 } + klog.V(3).Infof("ovs port %s has been removed from bridge %s", port, brName) } } @@ -228,6 +216,7 @@ func ovsCleanProviderNetwork(provider string) error { if output, err = ovs.Exec(ovs.IfExists, "del-br", brName); err != nil { return fmt.Errorf("failed to remove OVS bridge %s, %v: %q", brName, err, output) } + klog.V(3).Infof("ovs bridge %s has been deleted", brName) if br := util.ExternalBridgeName(provider); br != brName { if _, err = changeProvideNicName(br, brName); err != nil { diff --git a/pkg/daemon/init_linux.go b/pkg/daemon/init_linux.go index 624898ac41b..8a841dbda15 100644 --- a/pkg/daemon/init_linux.go +++ b/pkg/daemon/init_linux.go @@ -1,7 +1,7 @@ package daemon import ( - "syscall" + "time" "k8s.io/klog/v2" @@ -45,6 +45,39 @@ func nmSetManaged(device string, managed bool) error { return nil } +// wait systemd-networkd to finish interface configuration +func waitNetworkdConfiguration(linkIndex int) { + done := make(chan struct{}) + ch := make(chan netlink.RouteUpdate) + if err := netlink.RouteSubscribe(ch, done); err != nil { + klog.Warningf("failed to subscribe route update events: %v", err) + klog.Info("Waiting 100ms ...") + time.Sleep(100 * time.Millisecond) + return + } + + // wait route event on the link for 50ms + timer := time.NewTimer(50 * time.Millisecond) + for { + select { + case <-timer.C: + // timeout, interface configuration is expected to be completed + done <- struct{}{} + return + case event := <-ch: + if event.LinkIndex == linkIndex { + // received a route event on the link + // stop the timer + if !timer.Stop() { + <-timer.C + } + // reset the timer, wait for another 50ms + timer.Reset(50 * time.Millisecond) + } + } + } +} + func changeProvideNicName(current, target string) (bool, error) { link, err := netlink.LinkByName(current) if err != nil { @@ -56,17 +89,17 @@ func changeProvideNicName(current, target string) (bool, error) { return false, err } if link.Type() == "openvswitch" { - klog.Infof("%s is an openvswitch interface, skip", current) + klog.V(3).Infof("%s is an openvswitch interface, skip", current) return true, nil } - // set link unmanaged by NetworkManager to avoid getting new IP by DHCP + // set link unmanaged by NetworkManager if err = nmSetManaged(current, false); err != nil { klog.Errorf("failed set device %s to unmanaged by NetworkManager: %v", current, err) return false, err } - klog.Infof("change nic name from %s to %s", current, target) + klog.Infof("renaming link %s as %s", current, target) addresses, err := netlink.AddrList(link, netlink.FAMILY_ALL) if err != nil { klog.Errorf("failed to list addresses of link %s: %v", current, err) @@ -90,15 +123,20 @@ func changeProvideNicName(current, target string) (bool, error) { klog.Errorf("failed to set link %s up: %v", target, err) return false, err } + klog.Infof("link %s has been renamed as %s", current, target) + + waitNetworkdConfiguration(link.Attrs().Index) for _, addr := range addresses { if addr.IP.IsLinkLocalUnicast() { continue } + addr.Label = "" if err = netlink.AddrReplace(link, &addr); err != nil { - klog.Errorf("failed to replace address %s: %v", addr.String(), err) + klog.Errorf("failed to replace address %q: %v", addr.String(), err) return false, err } + klog.Infof("address %q has been added/replaced to link %s", addr.String(), target) } for _, scope := range routeScopeOrders { @@ -107,10 +145,11 @@ func changeProvideNicName(current, target string) (bool, error) { continue } if route.Scope == scope { - if err = netlink.RouteReplace(&route); err != nil && err != syscall.EEXIST { - klog.Errorf("failed to replace route %s: %v", route.String(), err) + if err = netlink.RouteReplace(&route); err != nil { + klog.Errorf("failed to replace route %q to %s: %v", route.String(), target, err) return false, err } + klog.Infof("route %q has been added/replaced to link %s", route.String(), target) } } } diff --git a/pkg/daemon/ovs.go b/pkg/daemon/ovs.go index bffcc5ab78a..dc92e6ad2f5 100644 --- a/pkg/daemon/ovs.go +++ b/pkg/daemon/ovs.go @@ -83,25 +83,59 @@ func configureEmptyMirror(portName string, mtu int) error { return configureMirrorLink(portName, mtu) } +func updateOvnMapping(name, key, value string) error { + output, err := ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:"+name) + if err != nil { + return fmt.Errorf("failed to get %s, %v: %q", name, err, output) + } + + fields := strings.Split(output, ",") + mappings := make(map[string]string, len(fields)+1) + for _, f := range fields { + idx := strings.IndexRune(f, ':') + if idx <= 0 || idx == len(f)-1 { + klog.Warningf("invalid mapping entry: %s", f) + continue + } + mappings[f[:idx]] = f[idx+1:] + } + mappings[key] = value + + fields = make([]string, 0, len(mappings)) + for k, v := range mappings { + fields = append(fields, fmt.Sprintf("%s:%s", k, v)) + } + + if len(fields) == 0 { + output, err = ovs.Exec(ovs.IfExists, "remove", "open", ".", "external-ids", name) + } else { + output, err = ovs.Exec("set", "open", ".", fmt.Sprintf("external-ids:%s=%s", name, strings.Join(fields, ","))) + } + if err != nil { + return fmt.Errorf("failed to set %s, %v: %q", name, err, output) + } + + return nil +} + func 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) } - output, err := ovs.Exec(ovs.MayExist, "add-br", bridge, + cmd := []string{ + ovs.MayExist, "add-br", bridge, "--", "set", "bridge", bridge, fmt.Sprintf("other_config:mac-learning-fallback=%v", macLearningFallback), - "--", "set", "bridge", bridge, "external_ids:vendor="+util.CniTypeName, + "--", "set", "bridge", bridge, "external_ids:vendor=" + util.CniTypeName, "--", "set", "bridge", bridge, fmt.Sprintf("external_ids:exchange-link-name=%v", exchangeLinkName), - ) - if err != nil { - return fmt.Errorf("failed to create OVS bridge %s, %v: %q", bridge, err, output) } if !brExists { // assign a new generated mac address only when the bridge is newly created - output, err = ovs.Exec("set", "bridge", bridge, fmt.Sprintf(`other-config:hwaddr="%s"`, util.GenerateMac())) - if err != nil { - return fmt.Errorf("failed to set hwaddr of OVS bridge %s, %v: %q", bridge, err, output) - } + cmd = append(cmd, "--", "set", "bridge", bridge, fmt.Sprintf(`other-config:hwaddr="%s"`, util.GenerateMac())) + } + output, err := ovs.Exec(cmd...) + if err != nil { + return fmt.Errorf("failed to create OVS bridge %s, %v: %q", bridge, err, output) } if output, err = ovs.Exec("list-ports", bridge); err != nil { return fmt.Errorf("failed to list ports of OVS bridge %s, %v: %q", bridge, err, output) @@ -122,19 +156,9 @@ func configExternalBridge(provider, bridge, nic string, exchangeLinkName, macLea } } - if output, err = ovs.Exec(ovs.IfExists, "get", "open", ".", "external-ids:ovn-bridge-mappings"); err != nil { - return fmt.Errorf("failed to get ovn-bridge-mappings, %v", err) - } - - bridgeMappings := fmt.Sprintf("%s:%s", provider, bridge) - if util.IsStringIn(bridgeMappings, strings.Split(output, ",")) { - return nil - } - if output != "" { - bridgeMappings = fmt.Sprintf("%s,%s", output, bridgeMappings) - } - if output, err = ovs.Exec("set", "open", ".", "external-ids:ovn-bridge-mappings="+bridgeMappings); err != nil { - return fmt.Errorf("failed to set ovn-bridge-mappings, %v: %q", err, output) + if err = updateOvnMapping("ovn-bridge-mappings", provider, bridge); err != nil { + klog.Error(err) + return err } return nil diff --git a/pkg/daemon/ovs_linux.go b/pkg/daemon/ovs_linux.go index 67002ad426d..1ecde1618df 100644 --- a/pkg/daemon/ovs_linux.go +++ b/pkg/daemon/ovs_linux.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "encoding/json" - "errors" "fmt" "net" "os" @@ -13,7 +12,6 @@ import ( "path/filepath" "regexp" "strings" - "syscall" "time" "github.com/Mellanox/sriovnet" @@ -906,35 +904,30 @@ func configProviderNic(nicName, brName string) (int, error) { return 0, fmt.Errorf("failed to get routes on nic %s: %v", nicName, err) } + // set link unmanaged by NetworkManager + if err = nmSetManaged(nicName, false); err != nil { + klog.Errorf("failed set device %s to unmanaged by NetworkManager: %v", nicName, err) + return 0, err + } + for _, addr := range addrs { if addr.IP.IsLinkLocalUnicast() { // skip 169.254.0.0/16 and fe80::/10 continue } - if !strings.HasPrefix(addr.Label, nicName) { - if strings.HasPrefix(addr.Label, brName) { - addr.Label = nicName + addr.Label[len(brName):] - } else { - addr.Label = nicName - } - } if err = netlink.AddrDel(nic, &addr); err != nil { errMsg := fmt.Errorf("failed to delete address %q on nic %s: %v", addr.String(), nicName, err) - if errors.Is(err, syscall.EADDRNOTAVAIL) { - // the IP address does not exist now - klog.Warning(errMsg) - continue - } + klog.Error(errMsg) return 0, errMsg } + klog.Infof("address %q has been removed from link %s", addr.String(), nicName) - if addr.Label != "" { - addr.Label = brName + addr.Label[len(nicName):] - } + addr.Label = "" 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) } // keep mac address the same with the provider nic, @@ -964,6 +957,7 @@ func configProviderNic(nicName, brName string) (int, error) { if err = netlink.RouteReplace(&route); err != nil { return 0, fmt.Errorf("failed to add/replace route %s: %v", route.String(), err) } + klog.Infof("route %q has been added/replaced to link %s", route.String(), brName) } } } @@ -972,6 +966,7 @@ func configProviderNic(nicName, brName string) (int, error) { "--", "set", "port", nicName, "external_ids:vendor="+util.CniTypeName); err != nil { return 0, fmt.Errorf("failed to add %s to OVS bridge %s: %v", nicName, brName, err) } + klog.V(3).Infof("ovs port %s has been added to bridge %s", nicName, brName) if err = netlink.LinkSetUp(nic); err != nil { return 0, fmt.Errorf("failed to set link %s up: %v", nicName, err) @@ -1031,6 +1026,7 @@ func removeProviderNic(nicName, brName string) error { if _, err = ovs.Exec(ovs.IfExists, "del-port", brName, nicName); err != nil { return fmt.Errorf("failed to remove %s from OVS bridge %s: %v", nicName, brName, err) } + klog.V(3).Infof("ovs port %s has been removed from bridge %s", nicName, brName) for _, addr := range addrs { if addr.IP.IsLinkLocalUnicast() { @@ -1039,25 +1035,22 @@ func removeProviderNic(nicName, brName string) error { } if err = netlink.AddrDel(bridge, &addr); err != nil { - errMsg := fmt.Errorf("failed to delete address %s on OVS bridge %s: %v", addr.String(), brName, err) - if errors.Is(err, syscall.EADDRNOTAVAIL) { - // the IP address does not exist now - klog.Warning(errMsg) - continue - } + errMsg := fmt.Errorf("failed to delete address %q on OVS bridge %s: %v", addr.String(), brName, err) + klog.Error(errMsg) return errMsg } + klog.Infof("address %q has been deleted from link %s", addr.String(), brName) - if addr.Label != "" { - addr.Label = nicName + strings.TrimPrefix(addr.Label, brName) - } + addr.Label = "" if err = netlink.AddrReplace(nic, &addr); err != nil { - return fmt.Errorf("failed to replace address %s on nic %s: %v", addr.String(), nicName, err) + return fmt.Errorf("failed to replace address %q on nic %s: %v", addr.String(), nicName, err) } + klog.Infof("address %q has been added/replaced to link %s", addr.String(), nicName) } - if err = netlink.LinkSetDown(bridge); err != nil { - return fmt.Errorf("failed to set OVS bridge %s down: %v", brName, err) + if err = netlink.LinkSetUp(nic); err != nil { + klog.Error("failed to set link %s up: %v", nicName, err) + return err } scopeOrders := [...]netlink.Scope{ @@ -1077,10 +1070,16 @@ func removeProviderNic(nicName, brName string) error { if err = netlink.RouteReplace(&route); err != nil { return fmt.Errorf("failed to add/replace route %s: %v", route.String(), err) } + klog.Infof("route %q has been added/replaced to link %s", route.String(), nicName) } } } + if err = netlink.LinkSetDown(bridge); err != nil { + return fmt.Errorf("failed to set OVS bridge %s down: %v", brName, err) + } + klog.V(3).Infof("link %s has been set down", brName) + return nil }