diff --git a/plugins/main/host-device/host-device.go b/plugins/main/host-device/host-device.go index 8a101fe03..52eb5ad94 100644 --- a/plugins/main/host-device/host-device.go +++ b/plugins/main/host-device/host-device.go @@ -23,6 +23,7 @@ import ( "os" "path/filepath" "runtime" + "strconv" "strings" "github.com/vishvananda/netlink" @@ -41,8 +42,62 @@ import ( var ( sysBusPCI = "/sys/bus/pci/devices" sysBusAuxiliary = "/sys/bus/auxiliary/devices" + sysClassNet = "/sys/class/net" + + linkSetNsFd = netlink.LinkSetNsFd + linkSetDown = netlink.LinkSetDown + wiphySetNsFd = netlink.WiphySetNsFd ) +type moveRequest struct { + wiphy int +} + +func (r *moveRequest) move(link netlink.Link, fd int) error { + if r == nil { + return linkSetNsFd(link, fd) + } + + // NL80211 requires every interface on the PHY to be down before moving it. + if err := linkSetDown(link); err != nil { + return err + } + return wiphySetNsFd(r.wiphy, fd) +} + +func getMoveRequest(ifName string) (*moveRequest, error) { + phyPath := filepath.Join(sysClassNet, ifName, "phy80211") + wiphyBytes, err := os.ReadFile(filepath.Join(phyPath, "index")) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("read wireless PHY for %q: %v", ifName, err) + } + + wiphy, err := strconv.Atoi(strings.TrimSpace(string(wiphyBytes))) + if err != nil { + return nil, fmt.Errorf("parse wireless PHY for %q: %v", ifName, err) + } + + // Enumeration and moving the wiphy cannot be atomic. A sibling created after + // this check may move with the PHY, although the kernel should reject one + // that is already up. + interfaces, err := os.ReadDir(filepath.Join(phyPath, "device", "net")) + if err != nil { + return nil, fmt.Errorf("list interfaces associated with wireless PHY %d: %v", wiphy, err) + } + if len(interfaces) != 1 || interfaces[0].Name() != ifName { + names := make([]string, 0, len(interfaces)) + for _, entry := range interfaces { + names = append(names, entry.Name()) + } + return nil, fmt.Errorf("wireless PHY %d has associated interfaces %q; host-device can safely move only a PHY containing solely %q", wiphy, names, ifName) + } + + return &moveRequest{wiphy: wiphy}, nil +} + // Array of different linux drivers bound to network device needed for DPDK var userspaceDrivers = []string{"vfio-pci", "uio_pci_generic", "igb_uio"} @@ -237,6 +292,10 @@ func cmdDel(args *skel.CmdArgs) error { func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, containerIfName string) (netlink.Link, error) { hostDevName := hostDev.Attrs().Name + moveRequest, err := getMoveRequest(hostDevName) + if err != nil { + return nil, err + } // With recent kernels we could do all changes in a single netlink call, // but on failure the device is left in a partially modified state. @@ -266,7 +325,7 @@ func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, containerIfName stri } // Move the host device into tempNS - if err = netlink.LinkSetNsFd(hostDev, int(tempNS.Fd())); err != nil { + if err = moveRequest.move(hostDev, int(tempNS.Fd())); err != nil { return nil, fmt.Errorf("failed to move %q to tempNS: %v", hostDevName, err) } @@ -286,7 +345,7 @@ func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, containerIfName stri // so we need to actively move the device back to hostNS on error defer func() { if err != nil && tempNSDev != nil { - _ = netlink.LinkSetNsFd(tempNSDev, int(hostNS.Fd())) + _ = moveRequest.move(tempNSDev, int(hostNS.Fd())) } }() @@ -315,7 +374,7 @@ func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, containerIfName stri }() // Move the device to the containerNS - if err = netlink.LinkSetNsFd(tempNSDev, int(containerNs.Fd())); err != nil { + if err = moveRequest.move(tempNSDev, int(containerNs.Fd())); err != nil { return fmt.Errorf("failed to move %q (host: %q) to container NS: %v", containerIfName, hostDevName, err) } @@ -336,7 +395,7 @@ func moveLinkIn(hostDev netlink.Link, containerNs ns.NetNS, containerIfName stri // Move the interface back to tempNS on error defer func() { if err != nil { - _ = netlink.LinkSetNsFd(contDev, int(tempNS.Fd())) + _ = moveRequest.move(contDev, int(tempNS.Fd())) } }() @@ -368,6 +427,7 @@ func moveLinkOut(containerNs ns.NetNS, containerIfName string) error { defer tempNS.Close() var contDev netlink.Link + var moveRequest *moveRequest // Restore original up state in case of error // This must be done in the containerNS as moving @@ -397,8 +457,13 @@ func moveLinkOut(containerNs ns.NetNS, containerIfName string) error { return fmt.Errorf("failed to find original ifname for %q (alias is not set)", containerIfName) } + moveRequest, err = getMoveRequest(containerIfName) + if err != nil { + return err + } + // Move the device to the tempNS - if err = netlink.LinkSetNsFd(contDev, int(tempNS.Fd())); err != nil { + if err = moveRequest.move(contDev, int(tempNS.Fd())); err != nil { return fmt.Errorf("failed to move %q to tempNS: %v", containerIfName, err) } return nil @@ -417,7 +482,7 @@ func moveLinkOut(containerNs ns.NetNS, containerIfName string) error { // Move the device back to containerNS on error defer func() { if err != nil { - _ = netlink.LinkSetNsFd(tempNSDev, int(containerNs.Fd())) + _ = moveRequest.move(tempNSDev, int(containerNs.Fd())) } }() @@ -448,7 +513,7 @@ func moveLinkOut(containerNs ns.NetNS, containerIfName string) error { }() // Finally move the device to the hostNS - if err = netlink.LinkSetNsFd(tempNSDev, int(hostNS.Fd())); err != nil { + if err = moveRequest.move(tempNSDev, int(hostNS.Fd())); err != nil { return fmt.Errorf("failed to move %q to hostNS: %v", hostDevName, err) } diff --git a/plugins/main/host-device/host-device_test.go b/plugins/main/host-device/host-device_test.go index d1a26f720..5af82b5dd 100644 --- a/plugins/main/host-device/host-device_test.go +++ b/plugins/main/host-device/host-device_test.go @@ -16,6 +16,7 @@ package main import ( "encoding/json" + "errors" "fmt" "math/rand" "net" @@ -37,6 +38,139 @@ import ( "github.com/containernetworking/plugins/pkg/testutils" ) +var _ = Describe("wireless device moves", func() { + var originalSysClassNet string + + BeforeEach(func() { + originalSysClassNet = sysClassNet + sysClassNet = GinkgoT().TempDir() + }) + + AfterEach(func() { + sysClassNet = originalSysClassNet + }) + + makeWirelessDevice := func(ifName, index string, associatedInterfaces ...string) { + phyPath := path.Join(sysClassNet, ifName, "phy80211") + Expect(os.MkdirAll(path.Join(phyPath, "device", "net"), 0o755)).To(Succeed()) + Expect(os.WriteFile(path.Join(phyPath, "index"), []byte(index), 0o644)).To(Succeed()) + for _, name := range associatedInterfaces { + Expect(os.Mkdir(path.Join(phyPath, "device", "net", name), 0o755)).To(Succeed()) + } + } + + It("selects an ordinary link move when no wireless PHY exists", func() { + request, err := getMoveRequest("eth0") + Expect(err).NotTo(HaveOccurred()) + Expect(request).To(BeNil()) + }) + + It("selects a wiphy move for a PHY containing only the requested interface", func() { + makeWirelessDevice("wlan0", "7\n", "wlan0") + + request, err := getMoveRequest("wlan0") + Expect(err).NotTo(HaveOccurred()) + Expect(request).To(Equal(&moveRequest{wiphy: 7})) + }) + + It("rejects a malformed wireless PHY index", func() { + makeWirelessDevice("wlan0", "not-an-index", "wlan0") + + request, err := getMoveRequest("wlan0") + Expect(request).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring(`parse wireless PHY for "wlan0"`))) + }) + + It("rejects a missing device net directory", func() { + phyPath := path.Join(sysClassNet, "wlan0", "phy80211") + Expect(os.MkdirAll(phyPath, 0o755)).To(Succeed()) + Expect(os.WriteFile(path.Join(phyPath, "index"), []byte("7\n"), 0o644)).To(Succeed()) + + request, err := getMoveRequest("wlan0") + Expect(request).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("list interfaces associated with wireless PHY 7"))) + }) + + It("rejects an unreadable device net path", func() { + phyPath := path.Join(sysClassNet, "wlan0", "phy80211") + Expect(os.MkdirAll(path.Join(phyPath, "device"), 0o755)).To(Succeed()) + Expect(os.WriteFile(path.Join(phyPath, "index"), []byte("7\n"), 0o644)).To(Succeed()) + Expect(os.WriteFile(path.Join(phyPath, "device", "net"), nil, 0o644)).To(Succeed()) + + request, err := getMoveRequest("wlan0") + Expect(request).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring("list interfaces associated with wireless PHY 7"))) + }) + + DescribeTable("rejects a PHY without exactly the requested interface", + func(associatedInterfaces ...string) { + makeWirelessDevice("wlan0", "7\n", associatedInterfaces...) + + request, err := getMoveRequest("wlan0") + Expect(request).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring(`host-device can safely move only a PHY containing solely "wlan0"`))) + }, + Entry("with no associated interfaces"), + Entry("with a differently named sole interface", "wlan1"), + ) + + It("rejects moving a PHY with sibling interfaces", func() { + makeWirelessDevice("wlan0", "7\n", "p2p0", "wlan0") + + request, err := getMoveRequest("wlan0") + Expect(request).To(BeNil()) + Expect(err).To(MatchError(ContainSubstring(`wireless PHY 7 has associated interfaces ["p2p0" "wlan0"]`))) + }) + + It("preserves ordinary-link dispatch and propagates wireless move failures", func() { + originalLinkSetNsFd := linkSetNsFd + originalLinkSetDown := linkSetDown + originalWiphySetNsFd := wiphySetNsFd + defer func() { + linkSetNsFd = originalLinkSetNsFd + linkSetDown = originalLinkSetDown + wiphySetNsFd = originalWiphySetNsFd + }() + + link := &netlink.Dummy{LinkAttrs: netlink.LinkAttrs{Name: "test0"}} + linkMoved := false + wiphyMoved := false + movedWiphy := 0 + movedFD := 0 + linkSetNsFd = func(netlink.Link, int) error { + linkMoved = true + return nil + } + linkSetDown = func(netlink.Link) error { return errors.New("down failed") } + wiphySetNsFd = func(wiphy, fd int) error { + wiphyMoved = true + movedWiphy = wiphy + movedFD = fd + return nil + } + + Expect((*moveRequest)(nil).move(link, 11)).To(Succeed()) + Expect(linkMoved).To(BeTrue()) + + err := (&moveRequest{wiphy: 7}).move(link, 11) + Expect(err).To(MatchError("down failed")) + Expect(wiphyMoved).To(BeFalse()) + + linkSetDown = func(netlink.Link) error { return nil } + wiphySetNsFd = func(wiphy, fd int) error { + wiphyMoved = true + movedWiphy = wiphy + movedFD = fd + return errors.New("wiphy move failed") + } + err = (&moveRequest{wiphy: 7}).move(link, 11) + Expect(err).To(MatchError("wiphy move failed")) + Expect(wiphyMoved).To(BeTrue()) + Expect(movedWiphy).To(Equal(7)) + Expect(movedFD).To(Equal(11)) + }) +}) + type Net struct { Name string `json:"name"` CNIVersion string `json:"cniVersion"`