From 7a62515407a50e3aa8a00c36435439969ad739d7 Mon Sep 17 00:00:00 2001 From: Casey Callendrello Date: Fri, 25 Aug 2017 16:58:19 +0200 Subject: [PATCH] pkg/ip: Fix ipmasq teardown on v6-only interfaces --- pkg/ip/ipmasq.go | 11 ++++++++++- pkg/ip/link.go | 19 ++++++++++++++----- pkg/ip/link_test.go | 13 +++++-------- plugins/main/bridge/bridge.go | 12 ++++++++---- plugins/main/ipvlan/ipvlan.go | 2 +- plugins/main/macvlan/macvlan.go | 2 +- plugins/main/ptp/ptp.go | 10 ++++++---- plugins/main/vlan/vlan.go | 5 ++--- 8 files changed, 47 insertions(+), 27 deletions(-) diff --git a/pkg/ip/ipmasq.go b/pkg/ip/ipmasq.go index 7a549d13..ba00f133 100644 --- a/pkg/ip/ipmasq.go +++ b/pkg/ip/ipmasq.go @@ -75,7 +75,16 @@ func SetupIPMasq(ipn *net.IPNet, chain string, comment string) error { // TeardownIPMasq undoes the effects of SetupIPMasq func TeardownIPMasq(ipn *net.IPNet, chain string, comment string) error { - ipt, err := iptables.New() + isV6 := ipn.IP.To4() == nil + + var ipt *iptables.IPTables + var err error + + if isV6 { + ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv6) + } else { + ipt, err = iptables.NewWithProtocol(iptables.ProtocolIPv4) + } if err != nil { return fmt.Errorf("failed to locate iptables: %v", err) } diff --git a/pkg/ip/link.go b/pkg/ip/link.go index fb8d3d50..3cb84861 100644 --- a/pkg/ip/link.go +++ b/pkg/ip/link.go @@ -158,6 +158,9 @@ func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (net.Interface, ne func DelLinkByName(ifName string) error { iface, err := netlink.LinkByName(ifName) if err != nil { + if err.Error() == "Link not found" { + return ErrLinkNotFound + } return fmt.Errorf("failed to lookup %q: %v", ifName, err) } @@ -168,9 +171,8 @@ func DelLinkByName(ifName string) error { return nil } -// DelLinkByNameAddr remove an interface returns its IP address -// of the specified family -func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) { +// DelLinkByNameAddr remove an interface and returns its addresses +func DelLinkByNameAddr(ifName string) ([]*net.IPNet, error) { iface, err := netlink.LinkByName(ifName) if err != nil { if err != nil && err.Error() == "Link not found" { @@ -179,7 +181,7 @@ func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) { return nil, fmt.Errorf("failed to lookup %q: %v", ifName, err) } - addrs, err := netlink.AddrList(iface, family) + addrs, err := netlink.AddrList(iface, netlink.FAMILY_ALL) if err != nil || len(addrs) == 0 { return nil, fmt.Errorf("failed to get IP addresses for %q: %v", ifName, err) } @@ -188,7 +190,14 @@ func DelLinkByNameAddr(ifName string, family int) (*net.IPNet, error) { return nil, fmt.Errorf("failed to delete %q: %v", ifName, err) } - return addrs[0].IPNet, nil + out := []*net.IPNet{} + for _, addr := range addrs { + if addr.IP.IsGlobalUnicast() { + out = append(out, addr.IPNet) + } + } + + return out, nil } func SetHWAddrByIP(ifName string, ip4 net.IP, ip6 net.IP) error { diff --git a/pkg/ip/link_test.go b/pkg/ip/link_test.go index a20a1582..1049c86e 100644 --- a/pkg/ip/link_test.go +++ b/pkg/ip/link_test.go @@ -27,7 +27,6 @@ import ( "github.com/containernetworking/plugins/pkg/ns" "github.com/vishvananda/netlink" - "github.com/vishvananda/netlink/nl" ) func getHwAddr(linkname string) string { @@ -133,7 +132,7 @@ var _ = Describe("Link", func() { defer GinkgoRecover() // This string should match the expected error codes in the cmdDel functions of some of the plugins - _, err := ip.DelLinkByNameAddr("THIS_DONT_EXIST", netlink.FAMILY_V4) + _, err := ip.DelLinkByNameAddr("THIS_DONT_EXIST") Expect(err).To(Equal(ip.ErrLinkNotFound)) return nil @@ -220,16 +219,14 @@ var _ = Describe("Link", func() { }) }) - It("DelLinkByNameAddr must throw an error for configured interfaces", func() { + It("DelLinkByNameAddr should return no IPs when no IPs are configured", func() { _ = containerNetNS.Do(func(ns.NetNS) error { defer GinkgoRecover() // this will delete the host endpoint too - addr, err := ip.DelLinkByNameAddr(containerVethName, nl.FAMILY_V4) - Expect(err).To(HaveOccurred()) - - var ipNetNil *net.IPNet - Expect(addr).To(Equal(ipNetNil)) + addr, err := ip.DelLinkByNameAddr(containerVethName) + Expect(err).NotTo(HaveOccurred()) + Expect(addr).To(HaveLen(0)) return nil }) }) diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index b8a573e8..63e0d89a 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -474,10 +474,10 @@ func cmdDel(args *skel.CmdArgs) error { // There is a netns so try to clean up. Delete can be called multiple times // so don't return an error if the device is already removed. // If the device isn't there then don't try to clean up IP masq either. - var ipn *net.IPNet + var ipnets []*net.IPNet err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { var err error - ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_ALL) + ipnets, err = ip.DelLinkByNameAddr(args.IfName) if err != nil && err == ip.ErrLinkNotFound { return nil } @@ -488,10 +488,14 @@ func cmdDel(args *skel.CmdArgs) error { return err } - if ipn != nil && n.IPMasq { + if n.IPMasq { chain := utils.FormatChainName(n.Name, args.ContainerID) comment := utils.FormatComment(n.Name, args.ContainerID) - err = ip.TeardownIPMasq(ipn, chain, comment) + for _, ipn := range ipnets { + if err := ip.TeardownIPMasq(ipn, chain, comment); err != nil { + return err + } + } } return err diff --git a/plugins/main/ipvlan/ipvlan.go b/plugins/main/ipvlan/ipvlan.go index 09924ef3..74a98863 100644 --- a/plugins/main/ipvlan/ipvlan.go +++ b/plugins/main/ipvlan/ipvlan.go @@ -194,7 +194,7 @@ func cmdDel(args *skel.CmdArgs) error { // There is a netns so try to clean up. Delete can be called multiple times // so don't return an error if the device is already removed. err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { - if _, err := ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4); err != nil { + if err := ip.DelLinkByName(args.IfName); err != nil { if err != ip.ErrLinkNotFound { return err } diff --git a/plugins/main/macvlan/macvlan.go b/plugins/main/macvlan/macvlan.go index 3b69194b..0589831f 100644 --- a/plugins/main/macvlan/macvlan.go +++ b/plugins/main/macvlan/macvlan.go @@ -226,7 +226,7 @@ func cmdDel(args *skel.CmdArgs) error { // There is a netns so try to clean up. Delete can be called multiple times // so don't return an error if the device is already removed. err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { - if _, err := ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4); err != nil { + if err := ip.DelLinkByName(args.IfName); err != nil { if err != ip.ErrLinkNotFound { return err } diff --git a/plugins/main/ptp/ptp.go b/plugins/main/ptp/ptp.go index 226c0ef6..70fc102b 100644 --- a/plugins/main/ptp/ptp.go +++ b/plugins/main/ptp/ptp.go @@ -259,10 +259,10 @@ func cmdDel(args *skel.CmdArgs) error { // There is a netns so try to clean up. Delete can be called multiple times // so don't return an error if the device is already removed. // If the device isn't there then don't try to clean up IP masq either. - var ipn *net.IPNet + var ipnets []*net.IPNet err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { var err error - ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4) + ipnets, err = ip.DelLinkByNameAddr(args.IfName) if err != nil && err == ip.ErrLinkNotFound { return nil } @@ -273,10 +273,12 @@ func cmdDel(args *skel.CmdArgs) error { return err } - if ipn != nil && conf.IPMasq { + if len(ipnets) != 0 && conf.IPMasq { chain := utils.FormatChainName(conf.Name, args.ContainerID) comment := utils.FormatComment(conf.Name, args.ContainerID) - err = ip.TeardownIPMasq(ipn, chain, comment) + for _, ipn := range ipnets { + err = ip.TeardownIPMasq(ipn, chain, comment) + } } return err diff --git a/plugins/main/vlan/vlan.go b/plugins/main/vlan/vlan.go index 7f527b55..694d85ae 100644 --- a/plugins/main/vlan/vlan.go +++ b/plugins/main/vlan/vlan.go @@ -181,9 +181,8 @@ func cmdDel(args *skel.CmdArgs) error { } err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { - _, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4) - // FIXME: use ip.ErrLinkNotFound when cni is revendored - if err != nil && err.Error() == "Link not found" { + err = ip.DelLinkByName(args.IfName) + if err != nil && err != ip.ErrLinkNotFound { return nil } return err