From a7feb41e62bc7a96f850be2fa06f10fe51767905 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 19 Apr 2017 12:00:45 +0000 Subject: [PATCH] Replace switch statements with Bridge polymorphism --- net/bridge.go | 145 +++++++++++++++++++-------------------- net/veth.go | 27 +++----- prog/weaver/main.go | 14 ++-- prog/weaveutil/bridge.go | 10 ++- weave | 4 -- 5 files changed, 94 insertions(+), 106 deletions(-) diff --git a/net/bridge.go b/net/bridge.go index 9e6b06e9..3f8cdfa3 100644 --- a/net/bridge.go +++ b/net/bridge.go @@ -15,8 +15,6 @@ import ( "github.com/weaveworks/weave/common/odp" ) -type BridgeType int - /* This code implements three possible configurations to connect containers to the Weave Net overlay: @@ -55,51 +53,47 @@ const ( DatapathIfName = "vethwe-datapath" BridgeIfName = "vethwe-bridge" PcapIfName = "vethwe-pcap" - - None BridgeType = iota - Bridge - Fastdp - BridgedFastdp - AWSVPC - Inconsistent ) -// Returns a string that is consistent with the weave script -func (t BridgeType) String() string { - switch t { - case None: - return "none" - case Bridge: - return "bridge" - case Fastdp: - return "fastdp" - case BridgedFastdp: - return "bridged_fastdp" - case AWSVPC: - return "AWSVPC" - case Inconsistent: - return "inconsistent" - } - return "unknown" +type Bridge interface { + init(config *BridgeConfig) error // create and initialise bridge device(s) + attach(veth *netlink.Veth) error // attach veth to bridge + IsFastdp() bool // does this bridge use fastdp? + String() string // human-readable type string } -func DetectBridgeType(weaveBridgeName, datapathName string) BridgeType { +type bridgeImpl struct{ bridge netlink.Link } +type fastdpImpl struct{ datapathName string } +type bridgedFastdpImpl struct { + bridgeImpl + fastdpImpl +} + +// Returns a string that is consistent with the weave script +func (bridgeImpl) String() string { return "bridge" } +func (fastdpImpl) String() string { return "fastdp" } +func (bridgedFastdpImpl) String() string { return "bridged_fastdp" } + +// Used to decide whether to manage ODP tunnels +func (bridgeImpl) IsFastdp() bool { return false } +func (fastdpImpl) IsFastdp() bool { return true } +func (bridgedFastdpImpl) IsFastdp() bool { return true } + +func DetectBridgeType(weaveBridgeName, datapathName string) (Bridge, error) { bridge, _ := netlink.LinkByName(weaveBridgeName) datapath, _ := netlink.LinkByName(datapathName) switch { case bridge == nil && datapath == nil: - return None + return nil, nil case isBridge(bridge) && datapath == nil: - return Bridge + return bridgeImpl{bridge: bridge}, nil case isDatapath(bridge) && datapath == nil: - return Fastdp + return fastdpImpl{datapathName: datapathName}, nil case isDatapath(datapath) && isBridge(bridge): - return BridgedFastdp - // We cannot detect AWSVPC here; it looks just like Bridge. - // Anyone who cares will have to know some other way. + return bridgedFastdpImpl{bridgeImpl{bridge: bridge}, fastdpImpl{datapathName: datapathName}}, nil default: - return Inconsistent + return nil, errors.New("Inconsistent bridge state detected. Please do 'weave reset' and try again") } } @@ -219,48 +213,37 @@ type BridgeConfig struct { Port int } -func CreateBridge(procPath string, config *BridgeConfig) (BridgeType, error) { - bridgeType := DetectBridgeType(config.WeaveBridgeName, config.DatapathName) +func CreateBridge(procPath string, config *BridgeConfig) (Bridge, error) { + bridgeType, err := DetectBridgeType(config.WeaveBridgeName, config.DatapathName) + if err != nil { + return nil, err + } - if bridgeType == None { - bridgeType = Bridge + if bridgeType == nil { + bridgeType = bridgeImpl{} if !config.NoFastdp { - bridgeType = BridgedFastdp + bridgeType = bridgedFastdpImpl{} if config.NoBridgedFastdp { - bridgeType = Fastdp + bridgeType = fastdpImpl{} config.DatapathName = config.WeaveBridgeName } odpSupported, err := odp.CreateDatapath(config.DatapathName) if err != nil { - return None, errors.Wrapf(err, "creating datapath %q", config.DatapathName) + return nil, errors.Wrapf(err, "creating datapath %q", config.DatapathName) } if !odpSupported { - bridgeType = Bridge + bridgeType = bridgeImpl{} } } - - var err error - switch bridgeType { - case Bridge: - err = initBridge(config) - case Fastdp: - err = initFastdp(config) - case BridgedFastdp: - err = initBridgedFastdp(config) - default: - err = fmt.Errorf("Cannot initialise bridge type %v", bridgeType) + if err := bridgeType.init(config); err != nil { + return nil, err } - if err != nil { - return None, err - } - - if err = configureIPTables(config); err != nil { + if err := configureIPTables(config); err != nil { return bridgeType, errors.Wrap(err, "configuring iptables") } } if config.AWSVPC { - bridgeType = AWSVPC // Set proxy_arp on the bridge, so that it could accept packets destined // to containers within the same subnet but running on remote hosts. // Without it, exact routes on each container are required. @@ -275,12 +258,6 @@ func CreateBridge(procPath string, config *BridgeConfig) (BridgeType, error) { } } - if bridgeType == Bridge { - if err := EthtoolTXOff(config.WeaveBridgeName); err != nil { - return bridgeType, errors.Wrap(err, "setting tx off") - } - } - if err := linkSetUpByName(config.WeaveBridgeName); err != nil { return bridgeType, err } @@ -292,7 +269,7 @@ func CreateBridge(procPath string, config *BridgeConfig) (BridgeType, error) { return bridgeType, nil } -func initBridgePrep(config *BridgeConfig) error { +func (b bridgeImpl) initPrep(config *BridgeConfig) error { mac, err := net.ParseMAC(config.Mac) if err != nil { return errors.Wrapf(err, "parsing bridge MAC %q", config.Mac) @@ -304,11 +281,11 @@ func initBridgePrep(config *BridgeConfig) error { if config.MTU == 0 { config.MTU = 65535 } - link := &netlink.Bridge{LinkAttrs: linkAttrs} - if err = netlink.LinkAdd(link); err != nil { + b.bridge = &netlink.Bridge{LinkAttrs: linkAttrs} + if err = netlink.LinkAdd(b.bridge); err != nil { return errors.Wrapf(err, "creating bridge %q", config.WeaveBridgeName) } - if err := netlink.LinkSetHardwareAddr(link, mac); err != nil { + if err := netlink.LinkSetHardwareAddr(b.bridge, mac); err != nil { return errors.Wrapf(err, "setting bridge %q mac %v", config.WeaveBridgeName, mac) } // Attempting to set the bridge MTU to a high value directly @@ -323,7 +300,7 @@ func initBridgePrep(config *BridgeConfig) error { if err := netlink.LinkSetMTU(dummy, config.MTU); err != nil { return errors.Wrapf(err, "setting dummy interface mtu to %d", config.MTU) } - if err := netlink.LinkSetMaster(dummy, link); err != nil { + if err := netlink.LinkSetMasterByIndex(dummy, b.bridge.Attrs().Index); err != nil { return errors.Wrap(err, "setting dummy interface master") } if err := netlink.LinkDel(dummy); err != nil { @@ -333,8 +310,8 @@ func initBridgePrep(config *BridgeConfig) error { return nil } -func initBridge(config *BridgeConfig) error { - if err := initBridgePrep(config); err != nil { +func (b bridgeImpl) init(config *BridgeConfig) error { + if err := b.initPrep(config); err != nil { return err } if _, err := CreateAndAttachVeth(BridgeIfName, PcapIfName, config.WeaveBridgeName, config.MTU, true, func(veth netlink.Link) error { @@ -342,15 +319,19 @@ func initBridge(config *BridgeConfig) error { }); err != nil { return errors.Wrap(err, "creating pcap veth pair") } + if err := EthtoolTXOff(config.WeaveBridgeName); err != nil { + return errors.Wrap(err, "setting tx off") + } return nil } -func initFastdp(config *BridgeConfig) error { +func (f fastdpImpl) init(config *BridgeConfig) error { datapath, err := netlink.LinkByName(config.DatapathName) if err != nil { return errors.Wrapf(err, "finding datapath %q", config.DatapathName) } + f.datapathName = config.DatapathName if config.MTU == 0 { /* GCE has the lowest underlay network MTU we're likely to encounter on a local network, at 1460 bytes. To get the overlay MTU from that we @@ -366,11 +347,11 @@ func initFastdp(config *BridgeConfig) error { return nil } -func initBridgedFastdp(config *BridgeConfig) error { - if err := initFastdp(config); err != nil { +func (bf bridgedFastdpImpl) init(config *BridgeConfig) error { + if err := bf.fastdpImpl.init(config); err != nil { return err } - if err := initBridgePrep(config); err != nil { + if err := bf.bridgeImpl.initPrep(config); err != nil { return err } if _, err := CreateAndAttachVeth(BridgeIfName, DatapathIfName, config.WeaveBridgeName, config.MTU, true, func(veth netlink.Link) error { @@ -392,6 +373,18 @@ func initBridgedFastdp(config *BridgeConfig) error { return nil } +func (b bridgeImpl) attach(veth *netlink.Veth) error { + return netlink.LinkSetMasterByIndex(veth, b.bridge.Attrs().Index) +} + +func (bf bridgedFastdpImpl) attach(veth *netlink.Veth) error { + return bf.bridgeImpl.attach(veth) +} + +func (f fastdpImpl) attach(veth *netlink.Veth) error { + return odp.AddDatapathInterface(f.datapathName, veth.Attrs().Name) +} + func configureIPTables(config *BridgeConfig) error { ipt, err := iptables.New() if err != nil { diff --git a/net/veth.go b/net/veth.go index a95553e8..8637ac9f 100644 --- a/net/veth.go +++ b/net/veth.go @@ -11,8 +11,6 @@ import ( "github.com/j-keck/arping" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" - - "github.com/weaveworks/weave/common/odp" ) // create and attach a veth to the Weave bridge @@ -40,22 +38,17 @@ func CreateAndAttachVeth(name, peerName, bridgeName string, mtu int, keepTXOn bo return nil, fmt.Errorf(format, a...) } - switch bridgeType := DetectBridgeType(bridgeName, DatapathName); bridgeType { - case Bridge, BridgedFastdp: - if err := netlink.LinkSetMasterByIndex(veth, bridge.Attrs().Index); err != nil { - return cleanup(`unable to set master of %s: %s`, name, err) + bridgeType, err := DetectBridgeType(bridgeName, DatapathName) + if err != nil { + return cleanup("detect bridge type: %s", err) + } + if err := bridgeType.attach(veth); err != nil { + return cleanup("attaching veth %q to %q: %s", name, bridgeName, err) + } + if !bridgeType.IsFastdp() && !keepTXOn { + if err := EthtoolTXOff(veth.PeerName); err != nil { + return cleanup(`unable to set tx off on %q: %s`, peerName, err) } - if bridgeType == Bridge && !keepTXOn { - if err := EthtoolTXOff(peerName); err != nil { - return cleanup(`unable to set tx off on %q: %s`, peerName, err) - } - } - case Fastdp: - if err := odp.AddDatapathInterface(bridgeName, name); err != nil { - return cleanup(`failed to attach %s to device "%s": %s`, name, bridgeName, err) - } - default: - return cleanup(`invalid bridge configuration`) } if init != nil { diff --git a/prog/weaver/main.go b/prog/weaver/main.go index 08d2a90d..93ec293b 100644 --- a/prog/weaver/main.go +++ b/prog/weaver/main.go @@ -491,32 +491,32 @@ func (nopPacketLogging) LogPacket(string, weave.PacketKey) { func (nopPacketLogging) LogForwardPacket(string, weave.ForwardPacketKey) { } -func createOverlay(bridgeType weavenet.BridgeType, config weavenet.BridgeConfig, host string, port int, bufSzMB int, enableEncryption bool) (weave.NetworkOverlay, weave.Bridge) { +func createOverlay(bridgeType weavenet.Bridge, config weavenet.BridgeConfig, host string, port int, bufSzMB int, enableEncryption bool) (weave.NetworkOverlay, weave.Bridge) { overlay := weave.NewOverlaySwitch() var bridge weave.Bridge var ignoreSleeve bool - switch bridgeType { - case weavenet.AWSVPC: + switch { + case config.AWSVPC: vpc := weave.NewAWSVPC() overlay.Add("awsvpc", vpc) bridge = weave.NullBridge{} // Currently, we do not support any overlay with AWSVPC ignoreSleeve = true - case weavenet.Fastdp, weavenet.BridgedFastdp: + case bridgeType == nil: + bridge = weave.NullBridge{} + case bridgeType.IsFastdp(): iface, err := weavenet.EnsureInterface(config.DatapathName) checkFatal(err) fastdp, err := weave.NewFastDatapath(iface, port, enableEncryption) checkFatal(err) bridge = fastdp.Bridge() overlay.Add("fastdp", fastdp.Overlay()) - case weavenet.Bridge: + case !bridgeType.IsFastdp(): iface, err := weavenet.EnsureInterface(weavenet.PcapIfName) checkFatal(err) bridge, err = weave.NewPcap(iface, bufSzMB*1024*1024) // bufsz flag is in MB checkFatal(err) - default: - bridge = weave.NullBridge{} } if !ignoreSleeve { diff --git a/prog/weaveutil/bridge.go b/prog/weaveutil/bridge.go index de300fdb..cf87e3ab 100644 --- a/prog/weaveutil/bridge.go +++ b/prog/weaveutil/bridge.go @@ -11,8 +11,14 @@ func detectBridgeType(args []string) error { if len(args) != 2 { cmdUsage("detect-bridge-type", " ") } - bridgeType := weavenet.DetectBridgeType(args[0], args[1]) - fmt.Println(bridgeType.String()) + bridgeType, err := weavenet.DetectBridgeType(args[0], args[1]) + if err != nil { + return err + } else if bridgeType == nil { + fmt.Println("none") + } else { + fmt.Println(bridgeType.String()) + } return nil } diff --git a/weave b/weave index f5e4e6fb..a4b7547f 100755 --- a/weave +++ b/weave @@ -401,10 +401,6 @@ detect_bridge_type() { fastdp) DATAPATH="$BRIDGE" ;; - inconsistent) - echo "Inconsistent bridge state detected. Please do 'weave reset' and try again." >&2 - exit 1 - ;; *) return 1 ;;