From 58c834c4f3eb090c2c64e4dbc6c69f46f1baf483 Mon Sep 17 00:00:00 2001 From: Jay Dunkelberger Date: Thu, 9 Mar 2017 13:55:15 -0800 Subject: [PATCH 1/3] pkg/ip: do not leak types from vendored netlink package The exported function SetupVeth now returns a package-defined type. Signed-off-by: Gabe Rosenhouse --- pkg/ip/link.go | 46 ++++++++++++++++++++++++++++++----------- pkg/ip/link_test.go | 4 ++-- plugins/main/ptp/ptp.go | 10 ++++----- 3 files changed, 41 insertions(+), 19 deletions(-) diff --git a/pkg/ip/link.go b/pkg/ip/link.go index 6431bb41..8900f653 100644 --- a/pkg/ip/link.go +++ b/pkg/ip/link.go @@ -98,30 +98,49 @@ func RenameLink(curName, newName string) error { return err } +type LinkAttrs struct { + Name string + HardwareAddr net.HardwareAddr + Index int +} + +type link struct { + netlink.Link +} + +func (l *link) Attrs() LinkAttrs { + a := l.Link.Attrs() + return LinkAttrs{ + Name: a.Name, + HardwareAddr: a.HardwareAddr, + Index: a.Index, + } +} + +type Link interface { + Attrs() LinkAttrs +} + // SetupVeth sets up a virtual ethernet link. // Should be in container netns, and will switch back to hostNS to set the host // veth end up. -func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (hostVeth, contVeth netlink.Link, err error) { - var hostVethName string - hostVethName, contVeth, err = makeVeth(contVethName, mtu) +func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (Link, Link, error) { + hostVethName, contVeth, err := makeVeth(contVethName, mtu) if err != nil { - return + return nil, nil, err } if err = netlink.LinkSetUp(contVeth); err != nil { - err = fmt.Errorf("failed to set %q up: %v", contVethName, err) - return + return nil, nil, fmt.Errorf("failed to set %q up: %v", contVethName, err) } - hostVeth, err = netlink.LinkByName(hostVethName) + hostVeth, err := netlink.LinkByName(hostVethName) if err != nil { - err = fmt.Errorf("failed to lookup %q: %v", hostVethName, err) - return + return nil, nil, fmt.Errorf("failed to lookup %q: %v", hostVethName, err) } if err = netlink.LinkSetNsFd(hostVeth, int(hostNS.Fd())); err != nil { - err = fmt.Errorf("failed to move veth to host netns: %v", err) - return + return nil, nil, fmt.Errorf("failed to move veth to host netns: %v", err) } err = hostNS.Do(func(_ ns.NetNS) error { @@ -135,7 +154,10 @@ func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (hostVeth, contVet } return nil }) - return + if err != nil { + return nil, nil, err + } + return &link{hostVeth}, &link{contVeth}, nil } // DelLinkByName removes an interface link. diff --git a/pkg/ip/link_test.go b/pkg/ip/link_test.go index 3df9ab8f..2c9ae400 100644 --- a/pkg/ip/link_test.go +++ b/pkg/ip/link_test.go @@ -46,8 +46,8 @@ var _ = Describe("Link", func() { hostNetNS ns.NetNS containerNetNS ns.NetNS ifaceCounter int = 0 - hostVeth netlink.Link - containerVeth netlink.Link + hostVeth ip.Link + containerVeth ip.Link hostVethName string containerVethName string diff --git a/plugins/main/ptp/ptp.go b/plugins/main/ptp/ptp.go index 87e86a59..3e88515c 100644 --- a/plugins/main/ptp/ptp.go +++ b/plugins/main/ptp/ptp.go @@ -63,14 +63,14 @@ func setupContainerVeth(netns ns.NetNS, ifName string, mtu int, pr *current.Resu containerInterface := ¤t.Interface{} err := netns.Do(func(hostNS ns.NetNS) error { - hostVeth, contVeth, err := ip.SetupVeth(ifName, mtu, hostNS) + hostVeth, contVeth0, err := ip.SetupVeth(ifName, mtu, hostNS) if err != nil { return err } hostInterface.Name = hostVeth.Attrs().Name hostInterface.Mac = hostVeth.Attrs().HardwareAddr.String() - containerInterface.Name = contVeth.Attrs().Name - containerInterface.Mac = contVeth.Attrs().HardwareAddr.String() + containerInterface.Name = contVeth0.Attrs().Name + containerInterface.Mac = contVeth0.Attrs().HardwareAddr.String() containerInterface.Sandbox = netns.Path() var firstV4Addr net.IP @@ -103,12 +103,12 @@ func setupContainerVeth(netns ns.NetNS, ifName string, mtu int, pr *current.Resu return err } - if err := ip.SetHWAddrByIP(contVeth.Attrs().Name, firstV4Addr, nil /* TODO IPv6 */); err != nil { + if err := ip.SetHWAddrByIP(contVeth0.Attrs().Name, firstV4Addr, nil /* TODO IPv6 */); err != nil { return fmt.Errorf("failed to set hardware addr by IP: %v", err) } // Re-fetch container veth to update attributes - contVeth, err = netlink.LinkByName(ifName) + contVeth, err := netlink.LinkByName(ifName) if err != nil { return fmt.Errorf("failed to look up %q: %v", ifName, err) } From e4a0583d7aa1f7ccc8bf01fd4b3d51487e26065f Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Fri, 10 Mar 2017 09:49:46 -0800 Subject: [PATCH 2/3] pkg/ip: SetupVeth returns net.Interface --- pkg/ip/link.go | 38 ++++++++++++----------------------- pkg/ip/link_test.go | 14 ++++++------- plugins/main/bridge/bridge.go | 6 +++--- plugins/main/ptp/ptp.go | 12 +++++------ 4 files changed, 29 insertions(+), 41 deletions(-) diff --git a/pkg/ip/link.go b/pkg/ip/link.go index 8900f653..52f6aae2 100644 --- a/pkg/ip/link.go +++ b/pkg/ip/link.go @@ -98,49 +98,37 @@ func RenameLink(curName, newName string) error { return err } -type LinkAttrs struct { - Name string - HardwareAddr net.HardwareAddr - Index int -} - -type link struct { - netlink.Link -} - -func (l *link) Attrs() LinkAttrs { - a := l.Link.Attrs() - return LinkAttrs{ +func ifaceFromNetlinkLink(l netlink.Link) net.Interface { + a := l.Attrs() + return net.Interface{ + Index: a.Index, + MTU: a.MTU, Name: a.Name, HardwareAddr: a.HardwareAddr, - Index: a.Index, + Flags: a.Flags, } } -type Link interface { - Attrs() LinkAttrs -} - // SetupVeth sets up a virtual ethernet link. // Should be in container netns, and will switch back to hostNS to set the host // veth end up. -func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (Link, Link, error) { +func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (net.Interface, net.Interface, error) { hostVethName, contVeth, err := makeVeth(contVethName, mtu) if err != nil { - return nil, nil, err + return net.Interface{}, net.Interface{}, err } if err = netlink.LinkSetUp(contVeth); err != nil { - return nil, nil, fmt.Errorf("failed to set %q up: %v", contVethName, err) + return net.Interface{}, net.Interface{}, fmt.Errorf("failed to set %q up: %v", contVethName, err) } hostVeth, err := netlink.LinkByName(hostVethName) if err != nil { - return nil, nil, fmt.Errorf("failed to lookup %q: %v", hostVethName, err) + return net.Interface{}, net.Interface{}, fmt.Errorf("failed to lookup %q: %v", hostVethName, err) } if err = netlink.LinkSetNsFd(hostVeth, int(hostNS.Fd())); err != nil { - return nil, nil, fmt.Errorf("failed to move veth to host netns: %v", err) + return net.Interface{}, net.Interface{}, fmt.Errorf("failed to move veth to host netns: %v", err) } err = hostNS.Do(func(_ ns.NetNS) error { @@ -155,9 +143,9 @@ func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (Link, Link, error return nil }) if err != nil { - return nil, nil, err + return net.Interface{}, net.Interface{}, err } - return &link{hostVeth}, &link{contVeth}, nil + return ifaceFromNetlinkLink(hostVeth), ifaceFromNetlinkLink(contVeth), nil } // DelLinkByName removes an interface link. diff --git a/pkg/ip/link_test.go b/pkg/ip/link_test.go index 2c9ae400..85d8b944 100644 --- a/pkg/ip/link_test.go +++ b/pkg/ip/link_test.go @@ -46,8 +46,8 @@ var _ = Describe("Link", func() { hostNetNS ns.NetNS containerNetNS ns.NetNS ifaceCounter int = 0 - hostVeth ip.Link - containerVeth ip.Link + hostVeth net.Interface + containerVeth net.Interface hostVethName string containerVethName string @@ -78,8 +78,8 @@ var _ = Describe("Link", func() { } Expect(err).NotTo(HaveOccurred()) - hostVethName = hostVeth.Attrs().Name - containerVethName = containerVeth.Attrs().Name + hostVethName = hostVeth.Name + containerVethName = containerVeth.Name return nil }) @@ -98,7 +98,7 @@ var _ = Describe("Link", func() { containerVethFromName, err := netlink.LinkByName(containerVethName) Expect(err).NotTo(HaveOccurred()) - Expect(containerVethFromName.Attrs().Index).To(Equal(containerVeth.Attrs().Index)) + Expect(containerVethFromName.Attrs().Index).To(Equal(containerVeth.Index)) return nil }) @@ -108,7 +108,7 @@ var _ = Describe("Link", func() { hostVethFromName, err := netlink.LinkByName(hostVethName) Expect(err).NotTo(HaveOccurred()) - Expect(hostVethFromName.Attrs().Index).To(Equal(hostVeth.Attrs().Index)) + Expect(hostVethFromName.Attrs().Index).To(Equal(hostVeth.Index)) return nil }) @@ -156,7 +156,7 @@ var _ = Describe("Link", func() { hostVeth, _, err := ip.SetupVeth(containerVethName, mtu, hostNetNS) Expect(err).NotTo(HaveOccurred()) - hostVethName = hostVeth.Attrs().Name + hostVethName = hostVeth.Name return nil }) diff --git a/plugins/main/bridge/bridge.go b/plugins/main/bridge/bridge.go index ac959791..82334918 100644 --- a/plugins/main/bridge/bridge.go +++ b/plugins/main/bridge/bridge.go @@ -168,10 +168,10 @@ func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairp if err != nil { return err } - contIface.Name = containerVeth.Attrs().Name - contIface.Mac = containerVeth.Attrs().HardwareAddr.String() + contIface.Name = containerVeth.Name + contIface.Mac = containerVeth.HardwareAddr.String() contIface.Sandbox = netns.Path() - hostIface.Name = hostVeth.Attrs().Name + hostIface.Name = hostVeth.Name return nil }) if err != nil { diff --git a/plugins/main/ptp/ptp.go b/plugins/main/ptp/ptp.go index 3e88515c..00109154 100644 --- a/plugins/main/ptp/ptp.go +++ b/plugins/main/ptp/ptp.go @@ -67,10 +67,10 @@ func setupContainerVeth(netns ns.NetNS, ifName string, mtu int, pr *current.Resu if err != nil { return err } - hostInterface.Name = hostVeth.Attrs().Name - hostInterface.Mac = hostVeth.Attrs().HardwareAddr.String() - containerInterface.Name = contVeth0.Attrs().Name - containerInterface.Mac = contVeth0.Attrs().HardwareAddr.String() + hostInterface.Name = hostVeth.Name + hostInterface.Mac = hostVeth.HardwareAddr.String() + containerInterface.Name = contVeth0.Name + containerInterface.Mac = contVeth0.HardwareAddr.String() containerInterface.Sandbox = netns.Path() var firstV4Addr net.IP @@ -87,7 +87,7 @@ func setupContainerVeth(netns ns.NetNS, ifName string, mtu int, pr *current.Resu if firstV4Addr != nil { err = hostNS.Do(func(_ ns.NetNS) error { - hostVethName := hostVeth.Attrs().Name + hostVethName := hostVeth.Name if err := ip.SetHWAddrByIP(hostVethName, firstV4Addr, nil /* TODO IPv6 */); err != nil { return fmt.Errorf("failed to set hardware addr by IP: %v", err) } @@ -103,7 +103,7 @@ func setupContainerVeth(netns ns.NetNS, ifName string, mtu int, pr *current.Resu return err } - if err := ip.SetHWAddrByIP(contVeth0.Attrs().Name, firstV4Addr, nil /* TODO IPv6 */); err != nil { + if err := ip.SetHWAddrByIP(contVeth0.Name, firstV4Addr, nil /* TODO IPv6 */); err != nil { return fmt.Errorf("failed to set hardware addr by IP: %v", err) } From f51dd618c5eab4f4b3bb8489242941fdeec17057 Mon Sep 17 00:00:00 2001 From: Gabe Rosenhouse Date: Mon, 13 Mar 2017 11:27:12 -0700 Subject: [PATCH 3/3] pkg/ip: improve docstring for SetupVeth --- pkg/ip/link.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/ip/link.go b/pkg/ip/link.go index 52f6aae2..ff0bdb29 100644 --- a/pkg/ip/link.go +++ b/pkg/ip/link.go @@ -109,9 +109,10 @@ func ifaceFromNetlinkLink(l netlink.Link) net.Interface { } } -// SetupVeth sets up a virtual ethernet link. -// Should be in container netns, and will switch back to hostNS to set the host -// veth end up. +// SetupVeth sets up a pair of virtual ethernet devices. +// Call SetupVeth from inside the container netns. It will create both veth +// devices and move the host-side veth into the provided hostNS namespace. +// On success, SetupVeth returns (hostVeth, containerVeth, nil) func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (net.Interface, net.Interface, error) { hostVethName, contVeth, err := makeVeth(contVethName, mtu) if err != nil {