From 8eb52538de7fbac24342b20f62a8f205f90b358e Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Wed, 30 Mar 2016 14:01:57 +0100 Subject: [PATCH 1/5] wait for datapath device so that when the router container restarts after a reboot it waits for the proxy to create the datapath device. Fixes #2113 --- prog/weaver/main.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/prog/weaver/main.go b/prog/weaver/main.go index 77ead51e..a406b543 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -348,6 +348,8 @@ func createOverlay(datapathName string, ifaceName string, host string, port int, case datapathName != "" && ifaceName != "": Log.Fatal("At most one of --datapath and --iface must be specified.") case datapathName != "": + _, err := weavenet.EnsureInterface(datapathName) + checkFatal(err) fastdp, err := weave.NewFastDatapath(datapathName, port) checkFatal(err) bridge = fastdp.Bridge() From 0f89f8562f9cb53af30b0378dbae16ff41318d26 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Wed, 30 Mar 2016 14:07:33 +0100 Subject: [PATCH 2/5] refactor(ish): parameterise dp creation by iface, not name --- prog/weaver/main.go | 4 ++-- router/fastdp.go | 11 +++-------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/prog/weaver/main.go b/prog/weaver/main.go index a406b543..cb297117 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -348,9 +348,9 @@ func createOverlay(datapathName string, ifaceName string, host string, port int, case datapathName != "" && ifaceName != "": Log.Fatal("At most one of --datapath and --iface must be specified.") case datapathName != "": - _, err := weavenet.EnsureInterface(datapathName) + iface, err := weavenet.EnsureInterface(datapathName) checkFatal(err) - fastdp, err := weave.NewFastDatapath(datapathName, port) + fastdp, err := weave.NewFastDatapath(iface, port) checkFatal(err) bridge = fastdp.Bridge() overlay.Add("fastdp", fastdp.Overlay()) diff --git a/router/fastdp.go b/router/fastdp.go index 3c1efa7d..c4d8c775 100644 --- a/router/fastdp.go +++ b/router/fastdp.go @@ -61,7 +61,7 @@ type FastDatapath struct { forwarders map[mesh.PeerName]*fastDatapathForwarder } -func NewFastDatapath(dpName string, port int) (*FastDatapath, error) { +func NewFastDatapath(iface *net.Interface, port int) (*FastDatapath, error) { dpif, err := odp.NewDpif() if err != nil { return nil, err @@ -74,18 +74,13 @@ func NewFastDatapath(dpName string, port int) (*FastDatapath, error) { } }() - dp, err := dpif.LookupDatapath(dpName) - if err != nil { - return nil, err - } - - iface, err := net.InterfaceByName(dpName) + dp, err := dpif.LookupDatapath(iface.Name) if err != nil { return nil, err } fastdp := &FastDatapath{ - dpname: dpName, + dpname: iface.Name, dpif: dpif, dp: dp, iface: iface, From f24f38be9c2a5316c0a5bb2b6a9ae67525163e10 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Wed, 30 Mar 2016 14:10:03 +0100 Subject: [PATCH 3/5] refactor: get rid of a superfluous fastdp member --- router/fastdp.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/router/fastdp.go b/router/fastdp.go index c4d8c775..f0861927 100644 --- a/router/fastdp.go +++ b/router/fastdp.go @@ -28,7 +28,6 @@ type bridgeSender func(key PacketKey, lock *fastDatapathLock) FlowOp type missHandler func(fks odp.FlowKeys, lock *fastDatapathLock) FlowOp type FastDatapath struct { - dpname string lock sync.Mutex // guards state and synchronises use of dpif dpif *odp.Dpif dp odp.DatapathHandle @@ -80,7 +79,6 @@ func NewFastDatapath(iface *net.Interface, port int) (*FastDatapath, error) { } fastdp := &FastDatapath{ - dpname: iface.Name, dpif: dpif, dp: dp, iface: iface, @@ -204,7 +202,7 @@ func (fastdp fastDatapathBridge) Interface() *net.Interface { } func (fastdp fastDatapathBridge) String() string { - return fmt.Sprint(fastdp.dpname, " (via ODP)") + return fmt.Sprint(fastdp.iface.Name, " (via ODP)") } func (fastdp fastDatapathBridge) Stats() map[string]int { From ec167554c50fc48c9cfffe9de464919356a59c8f Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Wed, 30 Mar 2016 14:13:41 +0100 Subject: [PATCH 4/5] refactor: consistent order --- router/fastdp.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/router/fastdp.go b/router/fastdp.go index f0861927..1b50b03d 100644 --- a/router/fastdp.go +++ b/router/fastdp.go @@ -29,9 +29,9 @@ type missHandler func(fks odp.FlowKeys, lock *fastDatapathLock) FlowOp type FastDatapath struct { lock sync.Mutex // guards state and synchronises use of dpif + iface *net.Interface dpif *odp.Dpif dp odp.DatapathHandle - iface *net.Interface deleteFlowsCount uint64 missCount uint64 missHandlers map[odp.VportID]missHandler @@ -79,9 +79,9 @@ func NewFastDatapath(iface *net.Interface, port int) (*FastDatapath, error) { } fastdp := &FastDatapath{ + iface: iface, dpif: dpif, dp: dp, - iface: iface, missHandlers: make(map[odp.VportID]missHandler), sendToPort: nil, sendToMAC: make(map[MAC]bridgeSender), From 495a35a6a6dea090cce0a85e6635204074617137 Mon Sep 17 00:00:00 2001 From: Matthias Radestock Date: Fri, 1 Apr 2016 09:04:14 +0100 Subject: [PATCH 5/5] work around fd re-use bug in vishvananda/netlink Closing a subscription can cause the associated fd to be re-used while the subscription goroutine is also still trying to use it. Which can have disastrous consequences. So as a work around we don't close subscriptions. This means we leak an fd, some buffer space, and goroutine. The latter will eventually block in a channel send of a subscription event, since nothing is left reading ont the other end. So at least it won't be consuming cycles. --- net/if.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/net/if.go b/net/if.go index f107433e..d4972cad 100644 --- a/net/if.go +++ b/net/if.go @@ -19,9 +19,10 @@ func EnsureInterface(ifaceName string) (*net.Interface, error) { func ensureInterface(ifaceName string) (*net.Interface, error) { ch := make(chan netlink.LinkUpdate) - done := make(chan struct{}) - defer close(done) - if err := netlink.LinkSubscribe(ch, done); err != nil { + // NB: We do not supply (and eventually close) a 'done' channel + // here since that can cause incorrect file descriptor + // re-usage. See https://github.com/weaveworks/weave/issues/2120 + if err := netlink.LinkSubscribe(ch, nil); err != nil { return nil, err } // check for currently-existing interface after subscribing, to avoid race @@ -46,9 +47,7 @@ func EnsureInterfaceAndMcastRoute(ifaceName string) (*net.Interface, error) { return nil, err } ch := make(chan netlink.RouteUpdate) - done := make(chan struct{}) - defer close(done) - if err := netlink.RouteSubscribe(ch, done); err != nil { + if err := netlink.RouteSubscribe(ch, nil); err != nil { return nil, err } dest := net.IPv4(224, 0, 0, 0)