From 125b5c5c6ed1c466bb4abc966f2c4d76a9e3a661 Mon Sep 17 00:00:00 2001 From: steiler Date: Fri, 13 Nov 2020 16:04:21 +0000 Subject: [PATCH 1/5] removed subshelling from veth between containers Added MTU capability Had to remove concurrency -> further investigation required --- clab/netlink.go | 109 ++++++++++++++++++++++++++++++++++-------------- cmd/deploy.go | 13 ++---- 2 files changed, 82 insertions(+), 40 deletions(-) diff --git a/clab/netlink.go b/clab/netlink.go index ac726126c..016ec7929 100644 --- a/clab/netlink.go +++ b/clab/netlink.go @@ -6,11 +6,11 @@ import ( "os" "os/exec" "strings" - "sync" "github.com/google/uuid" log "github.com/sirupsen/logrus" "github.com/vishvananda/netlink" + "github.com/vishvananda/netns" ) func (c *cLab) InitVirtualWiring() { @@ -51,56 +51,100 @@ func (c *cLab) createAToBveth(l *Link) error { interfaceA := fmt.Sprintf("clab-%s", genIfName()) interfaceB := fmt.Sprintf("clab-%s", genIfName()) - cmd := exec.Command("sudo", "ip", "link", "add", interfaceA, "type", "veth", "peer", "name", interfaceB) - err := runCmd(cmd) + nllA := &netlink.Veth{PeerName: interfaceB, LinkAttrs: netlink.LinkAttrs{Name: interfaceA}} + + err := netlink.LinkAdd(nllA) if err != nil { return err } - wg := new(sync.WaitGroup) - wg.Add(2) - go func() { - defer wg.Done() - err := c.configVeth(interfaceA, l.A.EndpointName, l.A.Node.LongName) - if err != nil { - log.Fatalf("failed to config interface '%s' in container %s: %v", l.A.EndpointName, l.A.Node.LongName, err) - } - }() - go func() { - defer wg.Done() - err = c.configVeth(interfaceB, l.B.EndpointName, l.B.Node.LongName) - if err != nil { - log.Fatalf("failed to config interface '%s' in container %s: %v", l.B.EndpointName, l.B.Node.LongName, err) - } - }() - wg.Wait() + + la := c.newLinkAttributes() + la.Name = l.A.EndpointName + err = c.configVeth(interfaceA, la, l.A.Node.LongName) + if err != nil { + log.Fatalf("failed to config interface '%s' in container %s: %v", l.A.EndpointName, l.A.Node.LongName, err) + } + + la = c.newLinkAttributes() + la.Name = l.B.EndpointName + err = c.configVeth(interfaceB, la, l.B.Node.LongName) + if err != nil { + log.Fatalf("failed to config interface '%s' in container %s: %v", l.B.EndpointName, l.B.Node.LongName, err) + } + return nil } -func (c *cLab) configVeth(dummyInterface, endpointName, ns string) error { - var cmd *exec.Cmd +// Type for the adjustment of the Link Attributes +type LinkAttributes struct { + Name string + MTU int + LinkUp bool +} + +func (c *cLab) newLinkAttributes() LinkAttributes { + return LinkAttributes{Name: "", MTU: 1500, LinkUp: true} +} + +func (c *cLab) configVeth(dummyInterface string, la LinkAttributes, ns string) error { + log.Debugf("Disabling TX checksum offloading for the %s interface...", dummyInterface) err := EthtoolTXOff(dummyInterface) if err != nil { return err } + + netNS, err := netns.GetFromName(ns) + if err != nil { + return err + } + + netLink, err := netlink.LinkByName(dummyInterface) + if err != nil { + return err + } + log.Debugf("map dummy interface '%s' to container %s", dummyInterface, ns) - cmd = exec.Command("sudo", "ip", "link", "set", dummyInterface, "netns", ns) - err = runCmd(cmd) + err = netlink.LinkSetNsFd(netLink, int(netNS)) if err != nil { return err } - log.Debugf("rename interface %s to %s", dummyInterface, endpointName) - cmd = exec.Command("sudo", "ip", "netns", "exec", ns, "ip", "link", "set", dummyInterface, "name", endpointName) - err = runCmd(cmd) + + err = c.setLinkAttributes(ns, netNS, dummyInterface, la) if err != nil { return err } - log.Debugf("set interface %s state to up in NS %s", endpointName, ns) - cmd = exec.Command("sudo", "ip", "netns", "exec", ns, "ip", "link", "set", endpointName, "up") - err = runCmd(cmd) + + return nil +} + +func (c *cLab) setLinkAttributes(namespaceName string, cnamespace netns.NsHandle, oldLinkName string, la LinkAttributes) error { + hostNetNs, _ := netns.Get() + netns.Set(cnamespace) + link, err := netlink.LinkByName(oldLinkName) if err != nil { return err } + if la.Name != "" { + log.Debugf("rename interface %s to %s", oldLinkName, la.Name) + err = netlink.LinkSetName(link, la.Name) + if err != nil { + return err + } + } + if la.LinkUp { + log.Debugf("set interface %s state to up in NS %s", la.Name, namespaceName) + err = netlink.LinkSetUp(link) + if err != nil { + return err + } + } + log.Debugf("setting interface %s MTU to %d", la.Name, la.MTU) + netlink.LinkSetMTU(link, la.MTU) + if err != nil { + return err + } + netns.Set(hostNetNs) return nil } @@ -130,7 +174,10 @@ func (c *cLab) createvethToBridge(l *Link) error { if err != nil { return err } - err = c.configVeth(dummyIface, containerIfName, containerNS) + + la := c.newLinkAttributes() + la.Name = containerIfName + err = c.configVeth(dummyIface, la, containerNS) if err != nil { return err } diff --git a/cmd/deploy.go b/cmd/deploy.go index c93fa62cd..dbe7ba339 100644 --- a/cmd/deploy.go +++ b/cmd/deploy.go @@ -112,18 +112,13 @@ var deployCmd = &cobra.Command{ wg.Wait() // cleanup hanging resources if a deployment failed before c.InitVirtualWiring() - wg = new(sync.WaitGroup) - wg.Add(len(c.Links)) + // wire the links between the nodes based on cabling plan for _, link := range c.Links { - go func(link *clab.Link) { - defer wg.Done() - if err = c.CreateVirtualWiring(link); err != nil { - log.Error(err) - } - }(link) + if err = c.CreateVirtualWiring(link); err != nil { + log.Error(err) + } } - wg.Wait() // generate graph of the lab topology if graph { if err = c.GenerateGraph(topo); err != nil { From ab7895683b038c47c2c1ec612d33781e87c47f7c Mon Sep 17 00:00:00 2001 From: steiler Date: Thu, 17 Dec 2020 09:55:54 +0000 Subject: [PATCH 2/5] Added runtime.LockOSThread() adjusted netlink bridge attachment --- clab/netlink.go | 63 ++++++++++++++++++++++++------------------------- go.mod | 2 +- 2 files changed, 32 insertions(+), 33 deletions(-) diff --git a/clab/netlink.go b/clab/netlink.go index 016ec7929..d290881f3 100644 --- a/clab/netlink.go +++ b/clab/netlink.go @@ -4,7 +4,7 @@ import ( "fmt" "net" "os" - "os/exec" + "runtime" "strings" "github.com/google/uuid" @@ -80,21 +80,24 @@ type LinkAttributes struct { Name string MTU int LinkUp bool + // Master is just used for interfaces meant to be attached to bridges + Master string } +// Initialize and instantiate a new LinkAttributes with default values func (c *cLab) newLinkAttributes() LinkAttributes { - return LinkAttributes{Name: "", MTU: 1500, LinkUp: true} + return LinkAttributes{Name: "", MTU: 1500, LinkUp: true, Master: ""} } func (c *cLab) configVeth(dummyInterface string, la LinkAttributes, ns string) error { - log.Debugf("Disabling TX checksum offloading for the %s interface...", dummyInterface) - err := EthtoolTXOff(dummyInterface) + netNS, err := netns.GetFromName(ns) if err != nil { return err } - netNS, err := netns.GetFromName(ns) + log.Debugf("Disabling TX checksum offloading for the %s interface...", dummyInterface) + err = EthtoolTXOff(dummyInterface) if err != nil { return err } @@ -114,13 +117,15 @@ func (c *cLab) configVeth(dummyInterface string, la LinkAttributes, ns string) e if err != nil { return err } - return nil + } func (c *cLab) setLinkAttributes(namespaceName string, cnamespace netns.NsHandle, oldLinkName string, la LinkAttributes) error { hostNetNs, _ := netns.Get() netns.Set(cnamespace) + runtime.LockOSThread() + link, err := netlink.LinkByName(oldLinkName) if err != nil { return err @@ -139,6 +144,21 @@ func (c *cLab) setLinkAttributes(namespaceName string, cnamespace netns.NsHandle return err } } + if la.Master != "" { + // if interface should be attached to a bridge, the Master is set to bridge name + log.Debugf("attache interface %s to bridge %s", la.Name, la.Master) + + // get handleto master link + master, err := netlink.LinkByName(la.Master) + if err != nil { + return err + } + // assigne master for link + err = netlink.LinkSetMaster(link, master) + if err != nil { + return err + } + } log.Debugf("setting interface %s MTU to %d", la.Name, la.MTU) netlink.LinkSetMTU(link, la.MTU) if err != nil { @@ -149,7 +169,6 @@ func (c *cLab) setLinkAttributes(namespaceName string, cnamespace netns.NsHandle } func (c *cLab) createvethToBridge(l *Link) error { - var cmd *exec.Cmd var err error log.Debugf("Create veth to bridge wire: %s <--> %s", l.A.EndpointName, l.B.EndpointName) dummyIface := fmt.Sprintf("clab-%s", genIfName()) @@ -169,30 +188,21 @@ func (c *cLab) createvethToBridge(l *Link) error { } log.Debugf("create dummy veth pair '%s'<-->'%s'", dummyIface, bridgeIfname) - cmd = exec.Command("sudo", "ip", "link", "add", dummyIface, "type", "veth", "peer", "name", bridgeIfname) - err = runCmd(cmd) + nllA := &netlink.Veth{PeerName: bridgeIfname, LinkAttrs: netlink.LinkAttrs{Name: dummyIface}} + + err = netlink.LinkAdd(nllA) if err != nil { return err } la := c.newLinkAttributes() la.Name = containerIfName + la.Master = bridgeName err = c.configVeth(dummyIface, la, containerNS) if err != nil { return err } - log.Debugf("map veth pair %s to bridge %s", bridgeIfname, bridgeName) - cmd = exec.Command("sudo", "ip", "link", "set", bridgeIfname, "master", bridgeName) - err = runCmd(cmd) - if err != nil { - return err - } - log.Debugf("set interface '%s' state to up", bridgeIfname) - cmd = exec.Command("sudo", "ip", "link", "set", bridgeIfname, "up") - err = runCmd(cmd) - if err != nil { - return err - } + log.Debugf("Disabling TX checksum offloading for the %s interface...", bridgeIfname) err = EthtoolTXOff(bridgeIfname) if err != nil { @@ -216,17 +226,6 @@ func (c *cLab) DeleteNetnsSymlinks() (err error) { return nil } -func runCmd(cmd *exec.Cmd) error { - b, err := cmd.CombinedOutput() - if err != nil { - log.Debugf("'%s' failed with: %v", cmd.String(), err) - log.Debugf("'%s' failed output: %v", cmd.String(), string(b)) - return err - } - log.Debugf("'%s' output: %v", cmd.String(), string(b)) - return nil -} - func genIfName() string { s, _ := uuid.New().MarshalText() // .MarshalText() always return a nil error return string(s[:8]) diff --git a/go.mod b/go.mod index 0819635a7..6847ab079 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/spf13/cobra v1.0.0 github.com/spf13/pflag v1.0.5 // indirect github.com/vishvananda/netlink v1.1.0 - github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae // indirect + github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae golang.org/x/net v0.0.0-20200822124328-c89045814202 // indirect gopkg.in/yaml.v2 v2.3.0 ) From c73c87b52d78961a34d97c81e5789ec1a3066089 Mon Sep 17 00:00:00 2001 From: steiler Date: Thu, 17 Dec 2020 13:59:41 +0000 Subject: [PATCH 3/5] proper handling of bridges --- clab/netlink.go | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/clab/netlink.go b/clab/netlink.go index d290881f3..f98e70b5f 100644 --- a/clab/netlink.go +++ b/clab/netlink.go @@ -75,7 +75,7 @@ func (c *cLab) createAToBveth(l *Link) error { return nil } -// Type for the adjustment of the Link Attributes +// LinkAttributes Type for the adjustment of the Link Attributes type LinkAttributes struct { Name string MTU int @@ -148,22 +148,44 @@ func (c *cLab) setLinkAttributes(namespaceName string, cnamespace netns.NsHandle // if interface should be attached to a bridge, the Master is set to bridge name log.Debugf("attache interface %s to bridge %s", la.Name, la.Master) + parentIndex := link.Attrs().ParentIndex + + runtime.UnlockOSThread() + netns.Set(hostNetNs) + runtime.LockOSThread() + // get handleto master link - master, err := netlink.LinkByName(la.Master) + rootNSvethLink, err := netlink.LinkByIndex(parentIndex) + if err != nil { + return err + } + + bridgeIf, err := netlink.LinkByName(la.Master) if err != nil { return err } + // assigne master for link - err = netlink.LinkSetMaster(link, master) + err = netlink.LinkSetMaster(rootNSvethLink, bridgeIf) + if err != nil { + return err + } + + // Finally set rootNSvethLink interface up + err = netlink.LinkSetUp(rootNSvethLink) if err != nil { return err } + runtime.UnlockOSThread() + netns.Set(cnamespace) + runtime.LockOSThread() } log.Debugf("setting interface %s MTU to %d", la.Name, la.MTU) netlink.LinkSetMTU(link, la.MTU) if err != nil { return err } + runtime.UnlockOSThread() netns.Set(hostNetNs) return nil } @@ -188,7 +210,7 @@ func (c *cLab) createvethToBridge(l *Link) error { } log.Debugf("create dummy veth pair '%s'<-->'%s'", dummyIface, bridgeIfname) - nllA := &netlink.Veth{PeerName: bridgeIfname, LinkAttrs: netlink.LinkAttrs{Name: dummyIface}} + nllA := &netlink.Veth{PeerName: bridgeIfname, LinkAttrs: netlink.LinkAttrs{Name: dummyIface, OperState: netlink.OperUp}} err = netlink.LinkAdd(nllA) if err != nil { From 037b5bbcb7dc11625b79a3ecf1f5bd98d327592a Mon Sep 17 00:00:00 2001 From: steiler Date: Thu, 17 Dec 2020 14:17:50 +0000 Subject: [PATCH 4/5] re-added concurrency --- clab/netlink.go | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/clab/netlink.go b/clab/netlink.go index f98e70b5f..b0412cb3e 100644 --- a/clab/netlink.go +++ b/clab/netlink.go @@ -6,6 +6,7 @@ import ( "os" "runtime" "strings" + "sync" "github.com/google/uuid" log "github.com/sirupsen/logrus" @@ -58,20 +59,28 @@ func (c *cLab) createAToBveth(l *Link) error { return err } - la := c.newLinkAttributes() - la.Name = l.A.EndpointName - err = c.configVeth(interfaceA, la, l.A.Node.LongName) - if err != nil { - log.Fatalf("failed to config interface '%s' in container %s: %v", l.A.EndpointName, l.A.Node.LongName, err) - } - - la = c.newLinkAttributes() - la.Name = l.B.EndpointName - err = c.configVeth(interfaceB, la, l.B.Node.LongName) - if err != nil { - log.Fatalf("failed to config interface '%s' in container %s: %v", l.B.EndpointName, l.B.Node.LongName, err) - } + wg := new(sync.WaitGroup) + wg.Add(2) + go func() { + defer wg.Done() + la := c.newLinkAttributes() + la.Name = l.A.EndpointName + err = c.configVeth(interfaceA, la, l.A.Node.LongName) + if err != nil { + log.Fatalf("failed to config interface '%s' in container %s: %v", l.A.EndpointName, l.A.Node.LongName, err) + } + }() + go func() { + defer wg.Done() + la := c.newLinkAttributes() + la.Name = l.B.EndpointName + err = c.configVeth(interfaceB, la, l.B.Node.LongName) + if err != nil { + log.Fatalf("failed to config interface '%s' in container %s: %v", l.B.EndpointName, l.B.Node.LongName, err) + } + }() + wg.Wait() return nil } From 94189506554c2ae51066c9bf91ea971058adad81 Mon Sep 17 00:00:00 2001 From: steiler Date: Thu, 17 Dec 2020 14:33:10 +0000 Subject: [PATCH 5/5] added adjusting the root-ns veth mtu as well --- clab/netlink.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/clab/netlink.go b/clab/netlink.go index b0412cb3e..83b5dcedb 100644 --- a/clab/netlink.go +++ b/clab/netlink.go @@ -185,6 +185,12 @@ func (c *cLab) setLinkAttributes(namespaceName string, cnamespace netns.NsHandle if err != nil { return err } + + log.Debugf("setting root-ns interface %s MTU to %d", la.Name, la.MTU) + netlink.LinkSetMTU(rootNSvethLink, la.MTU) + if err != nil { + return err + } runtime.UnlockOSThread() netns.Set(cnamespace) runtime.LockOSThread()