Merge branch 'main' into support-sf

plugins/main/host-device/host-device.go

@@ -230,34 +230,112 @@ func cmdDel(args *skel.CmdArgs) error {
 	return nil
 }
 
+// setTempName sets a temporary name for netdevice, returns updated Link object or error
+// if occurred.
+func setTempName(dev netlink.Link) (netlink.Link, error) {
+	tempName := fmt.Sprintf("%s%d", "temp_", dev.Attrs().Index)
+
+	// rename to tempName
+	if err := netlink.LinkSetName(dev, tempName); err != nil {
+		return nil, fmt.Errorf("failed to rename device %q to %q: %v", dev.Attrs().Name, tempName, err)
+	}
+
+	// Get updated Link obj
+	tempDev, err := netlink.LinkByName(tempName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to find %q after rename to %q: %v", dev.Attrs().Name, tempName, err)
+	}
+
+	return tempDev, nil
+}
+
 func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, ifName string) (netlink.Link, error) {
-	if err := netlink.LinkSetNsFd(hostDev, int(containerNs.Fd())); err != nil {
+	origLinkFlags := hostDev.Attrs().Flags
-		return nil, err
+	hostDevName := hostDev.Attrs().Name
+	defaultNs, err := ns.GetCurrentNS()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get host namespace: %v", err)
+	}
+
+	// Devices can be renamed only when down
+	if err = netlink.LinkSetDown(hostDev); err != nil {
+		return nil, fmt.Errorf("failed to set %q down: %v", hostDev.Attrs().Name, err)
+	}
+
+	// restore original link state in case of error
+	defer func() {
+		if err != nil {
+			if origLinkFlags&net.FlagUp == net.FlagUp && hostDev != nil {
+				_ = netlink.LinkSetUp(hostDev)
+			}
+		}
+	}()
+
+	hostDev, err = setTempName(hostDev)
+	if err != nil {
+		return nil, fmt.Errorf("failed to rename device %q to temporary name: %v", hostDevName, err)
+	}
+
+	// restore original netdev name in case of error
+	defer func() {
+		if err != nil && hostDev != nil {
+			_ = netlink.LinkSetName(hostDev, hostDevName)
+		}
+	}()
+
+	if err = netlink.LinkSetNsFd(hostDev, int(containerNs.Fd())); err != nil {
+		return nil, fmt.Errorf("failed to move %q to container ns: %v", hostDev.Attrs().Name, err)
 	}
 
 	var contDev netlink.Link
-	if err := containerNs.Do(func(_ ns.NetNS) error {
+	tempDevName := hostDev.Attrs().Name
+	if err = containerNs.Do(func(_ ns.NetNS) error {
 		var err error
-		contDev, err = netlink.LinkByName(hostDev.Attrs().Name)
+		contDev, err = netlink.LinkByName(tempDevName)
 		if err != nil {
-			return fmt.Errorf("failed to find %q: %v", hostDev.Attrs().Name, err)
+			return fmt.Errorf("failed to find %q: %v", tempDevName, err)
-		}
-		// Devices can be renamed only when down
-		if err = netlink.LinkSetDown(contDev); err != nil {
-			return fmt.Errorf("failed to set %q down: %v", hostDev.Attrs().Name, err)
 		}
+
+		// move netdev back to host namespace in case of error
+		defer func() {
+			if err != nil {
+				_ = netlink.LinkSetNsFd(contDev, int(defaultNs.Fd()))
+				// we need to get updated link object as link was moved back to host namepsace
+				_ = defaultNs.Do(func(_ ns.NetNS) error {
+					hostDev, _ = netlink.LinkByName(tempDevName)
+					return nil
+				})
+			}
+		}()
+
 		// Save host device name into the container device's alias property
-		if err := netlink.LinkSetAlias(contDev, hostDev.Attrs().Name); err != nil {
+		if err = netlink.LinkSetAlias(contDev, hostDevName); err != nil {
-			return fmt.Errorf("failed to set alias to %q: %v", hostDev.Attrs().Name, err)
+			return fmt.Errorf("failed to set alias to %q: %v", tempDevName, err)
 		}
 		// Rename container device to respect args.IfName
-		if err := netlink.LinkSetName(contDev, ifName); err != nil {
+		if err = netlink.LinkSetName(contDev, ifName); err != nil {
-			return fmt.Errorf("failed to rename device %q to %q: %v", hostDev.Attrs().Name, ifName, err)
+			return fmt.Errorf("failed to rename device %q to %q: %v", tempDevName, ifName, err)
 		}
+
+		// restore tempDevName in case of error
+		defer func() {
+			if err != nil {
+				_ = netlink.LinkSetName(contDev, tempDevName)
+			}
+		}()
+
 		// Bring container device up
 		if err = netlink.LinkSetUp(contDev); err != nil {
 			return fmt.Errorf("failed to set %q up: %v", ifName, err)
 		}
+
+		// bring device down in case of error
+		defer func() {
+			if err != nil {
+				_ = netlink.LinkSetDown(contDev)
+			}
+		}()
+
 		// Retrieve link again to get up-to-date name and attributes
 		contDev, err = netlink.LinkByName(ifName)
 		if err != nil {
@@ -278,11 +356,14 @@ func moveLinkOut(containerNs ns.NetNS, ifName string) error {
 	}
 	defer defaultNs.Close()
 
-	return containerNs.Do(func(_ ns.NetNS) error {
+	var tempName string
+	var origDev netlink.Link
+	err = containerNs.Do(func(_ ns.NetNS) error {
 		dev, err := netlink.LinkByName(ifName)
 		if err != nil {
 			return fmt.Errorf("failed to find %q: %v", ifName, err)
 		}
+		origDev = dev
 
 		// Devices can be renamed only when down
 		if err = netlink.LinkSetDown(dev); err != nil {
@@ -300,16 +381,49 @@ func moveLinkOut(containerNs ns.NetNS, ifName string) error {
 			}
 		}()
 
-		// Rename the device to its original name from the host namespace
+		newLink, err := setTempName(dev)
-		if err = netlink.LinkSetName(dev, dev.Attrs().Alias); err != nil {
+		if err != nil {
-			return fmt.Errorf("failed to restore %q to original name %q: %v", ifName, dev.Attrs().Alias, err)
+			return fmt.Errorf("failed to rename device %q to temporary name: %v", ifName, err)
 		}
+		dev = newLink
+		tempName = dev.Attrs().Name
 
 		if err = netlink.LinkSetNsFd(dev, int(defaultNs.Fd())); err != nil {
-			return fmt.Errorf("failed to move %q to host netns: %v", dev.Attrs().Alias, err)
+			return fmt.Errorf("failed to move %q to host netns: %v", tempName, err)
 		}
 		return nil
 	})
+
+	if err != nil {
+		return err
+	}
+
+	// Rename the device to its original name from the host namespace
+	tempDev, err := netlink.LinkByName(tempName)
+	if err != nil {
+		return fmt.Errorf("failed to find %q in host namespace: %v", tempName, err)
+	}
+
+	if err = netlink.LinkSetName(tempDev, tempDev.Attrs().Alias); err != nil {
+		// move device back to container ns so it may be retired
+		defer func() {
+			_ = netlink.LinkSetNsFd(tempDev, int(containerNs.Fd()))
+			_ = containerNs.Do(func(_ ns.NetNS) error {
+				lnk, err := netlink.LinkByName(tempName)
+				if err != nil {
+					return err
+				}
+				_ = netlink.LinkSetName(lnk, ifName)
+				if origDev.Attrs().Flags&net.FlagUp == net.FlagUp {
+					_ = netlink.LinkSetUp(lnk)
+				}
+				return nil
+			})
+		}()
+		return fmt.Errorf("failed to restore %q to original name %q: %v", tempName, tempDev.Attrs().Alias, err)
+	}
+
+	return nil
 }
 
 func hasDpdkDriver(pciaddr string) (bool, error) {

plugins/main/host-device/host-device_test.go

@@ -1214,6 +1214,114 @@ var _ = Describe("base functionality", func() {
 				return nil
 			})
 		})
+
+		It(fmt.Sprintf("Test CmdAdd/Del when additioinal interface alreay exists in container ns with same name. %s config", ver), func() {
+			var (
+				origLink      netlink.Link
+				containerLink netlink.Link
+			)
+
+			hostIfname := "eth0"
+			containerAdditionalIfname := "eth0"
+
+			// prepare host device in original namespace
+			_ = originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+				err := netlink.LinkAdd(&netlink.Dummy{
+					LinkAttrs: netlink.LinkAttrs{
+						Name: hostIfname,
+					},
+				})
+				Expect(err).NotTo(HaveOccurred())
+				origLink, err = netlink.LinkByName(hostIfname)
+				Expect(err).NotTo(HaveOccurred())
+				err = netlink.LinkSetUp(origLink)
+				Expect(err).NotTo(HaveOccurred())
+				return nil
+			})
+
+			// prepare device in container namespace with same name as host device
+			_ = targetNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+				err := netlink.LinkAdd(&netlink.Dummy{
+					LinkAttrs: netlink.LinkAttrs{
+						Name: containerAdditionalIfname,
+					},
+				})
+				Expect(err).NotTo(HaveOccurred())
+				containerLink, err = netlink.LinkByName(containerAdditionalIfname)
+				Expect(err).NotTo(HaveOccurred())
+				err = netlink.LinkSetUp(containerLink)
+				Expect(err).NotTo(HaveOccurred())
+				return nil
+			})
+
+			// call CmdAdd
+			cniName := "net1"
+			conf := fmt.Sprintf(`{
+				"cniVersion": "%s",
+				"name": "cni-plugin-host-device-test",
+				"type": "host-device",
+				"device": %q
+			}`, ver, hostIfname)
+			args := &skel.CmdArgs{
+				ContainerID: "dummy",
+				Netns:       targetNS.Path(),
+				IfName:      cniName,
+				StdinData:   []byte(conf),
+			}
+			var resI types.Result
+			err := originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+				var err error
+				resI, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args) })
+				return err
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// check that the result was sane
+			t := newTesterByVersion(ver)
+			t.expectInterfaces(resI, cniName, origLink.Attrs().HardwareAddr.String(), targetNS.Path())
+
+			// assert that host device is now in the target namespace and is up
+			_ = targetNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+				link, err := netlink.LinkByName(cniName)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(link.Attrs().HardwareAddr).To(Equal(origLink.Attrs().HardwareAddr))
+				Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp))
+				return nil
+			})
+
+			// call CmdDel, expect it to succeed
+			_ = originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+				err = testutils.CmdDelWithArgs(args, func() error {
+					return cmdDel(args)
+				})
+				Expect(err).ToNot(HaveOccurred())
+				return nil
+			})
+
+			// assert container interface "eth0" still exists in target namespace and is up
+			err = targetNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+				link, err := netlink.LinkByName(containerAdditionalIfname)
+				Expect(err).NotTo(HaveOccurred())
+				Expect(link.Attrs().HardwareAddr).To(Equal(containerLink.Attrs().HardwareAddr))
+				Expect(link.Attrs().Flags & net.FlagUp).To(Equal(net.FlagUp))
+				return nil
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// assert that host device is now back in the original namespace
+			_ = originalNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
+				_, err := netlink.LinkByName(hostIfname)
+				Expect(err).NotTo(HaveOccurred())
+				return nil
+			})
+		})
 	}
 })