From c820f2b1cfeaff01dbae2f0948f63a4ac04d81bd Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Mon, 30 Jun 2014 13:37:09 -0700 Subject: [PATCH 1/3] bonding: Add support for probing the bonding module with parameters Until support for bonding params is added to networkd, this will be neccessary in order to use bonding parameters (i.e. miimon, mode). This also makes it such that the 8012q module will only be loaded if the network config makes use of VLANs. --- network/interface.go | 42 ++++++++++++++++++++-- network/interface_test.go | 76 ++++++++++++++++++++++++++++++++++++--- network/stanza.go | 1 - network/stanza_test.go | 5 +-- system/networkd.go | 25 ++++++++++--- 5 files changed, 132 insertions(+), 17 deletions(-) diff --git a/network/interface.go b/network/interface.go index be0b04b..e8e4752 100644 --- a/network/interface.go +++ b/network/interface.go @@ -3,6 +3,7 @@ package network import ( "fmt" "strconv" + "strings" ) type InterfaceGenerator interface { @@ -11,6 +12,8 @@ type InterfaceGenerator interface { Netdev() string Link() string Network() string + Type() string + ModprobeParams() string } type networkInterface interface { @@ -68,6 +71,10 @@ func (i *logicalInterface) Children() []networkInterface { return i.children } +func (i *logicalInterface) ModprobeParams() string { + return "" +} + func (i *logicalInterface) setConfigDepth(depth int) { i.configDepth = depth } @@ -84,9 +91,14 @@ func (p *physicalInterface) Netdev() string { return "" } +func (p *physicalInterface) Type() string { + return "physical" +} + type bondInterface struct { logicalInterface - slaves []string + slaves []string + options map[string]string } func (b *bondInterface) Name() string { @@ -97,6 +109,19 @@ func (b *bondInterface) Netdev() string { return fmt.Sprintf("[NetDev]\nKind=bond\nName=%s\n", b.name) } +func (b *bondInterface) Type() string { + return "bond" +} + +func (b *bondInterface) ModprobeParams() string { + params := "" + for name, val := range b.options { + params += fmt.Sprintf("%s=%s ", name, val) + } + params = strings.TrimSuffix(params, " ") + return params +} + type vlanInterface struct { logicalInterface id int @@ -123,6 +148,10 @@ func (v *vlanInterface) Netdev() string { return config } +func (v *vlanInterface) Type() string { + return "vlan" +} + func buildInterfaces(stanzas []*stanzaInterface) []InterfaceGenerator { interfaceMap := createInterfaces(stanzas) linkAncestors(interfaceMap) @@ -141,15 +170,22 @@ func createInterfaces(stanzas []*stanzaInterface) map[string]networkInterface { for _, iface := range stanzas { switch iface.kind { case interfaceBond: + bondOptions := make(map[string]string) + for _, k := range []string{"mode", "miimon", "lacp-rate"} { + if v, ok := iface.options["bond-"+k]; ok && len(v) > 0 { + bondOptions[k] = v[0] + } + } interfaceMap[iface.name] = &bondInterface{ logicalInterface{ name: iface.name, config: iface.configMethod, children: []networkInterface{}, }, - iface.options["slaves"], + iface.options["bond-slaves"], + bondOptions, } - for _, slave := range iface.options["slaves"] { + for _, slave := range iface.options["bond-slaves"] { if _, ok := interfaceMap[slave]; !ok { interfaceMap[slave] = &physicalInterface{ logicalInterface{ diff --git a/network/interface_test.go b/network/interface_test.go index c98bf60..eaf1aff 100644 --- a/network/interface_test.go +++ b/network/interface_test.go @@ -36,6 +36,7 @@ func TestPhysicalInterfaceNetwork(t *testing.T) { name: "testbond1", }, nil, + nil, }, &vlanInterface{ logicalInterface{ @@ -67,14 +68,14 @@ VLAN=testvlan2 } func TestBondInterfaceName(t *testing.T) { - b := bondInterface{logicalInterface{name: "testname"}, nil} + b := bondInterface{logicalInterface{name: "testname"}, nil, nil} if b.Name() != "testname" { t.FailNow() } } func TestBondInterfaceNetdev(t *testing.T) { - b := bondInterface{logicalInterface{name: "testname"}, nil} + b := bondInterface{logicalInterface{name: "testname"}, nil, nil} netdev := `[NetDev] Kind=bond Name=testname @@ -102,6 +103,7 @@ func TestBondInterfaceNetwork(t *testing.T) { name: "testbond1", }, nil, + nil, }, &vlanInterface{ logicalInterface{ @@ -120,6 +122,7 @@ func TestBondInterfaceNetwork(t *testing.T) { }, }, nil, + nil, } network := `[Match] Name=testname @@ -218,6 +221,61 @@ Gateway=1.2.3.4 } } +func TestType(t *testing.T) { + for _, tt := range []struct { + i InterfaceGenerator + t string + }{ + { + i: &physicalInterface{}, + t: "physical", + }, + { + i: &vlanInterface{}, + t: "vlan", + }, + { + i: &bondInterface{}, + t: "bond", + }, + } { + if tp := tt.i.Type(); tp != tt.t { + t.Fatalf("bad type (%q): got %s, want %s", tt.i, tp, tt.t) + } + } +} + +func TestModprobeParams(t *testing.T) { + for _, tt := range []struct { + i InterfaceGenerator + p string + }{ + { + i: &physicalInterface{}, + p: "", + }, + { + i: &vlanInterface{}, + p: "", + }, + { + i: &bondInterface{ + logicalInterface{}, + nil, + map[string]string{ + "a": "1", + "b": "2", + }, + }, + p: "a=1 b=2", + }, + } { + if p := tt.i.ModprobeParams(); p != tt.p { + t.Fatalf("bad params (%q): got %s, want %s", tt.i, p, tt.p) + } + } +} + func TestBuildInterfacesLo(t *testing.T) { stanzas := []*stanzaInterface{ &stanzaInterface{ @@ -242,7 +300,7 @@ func TestBuildInterfacesBlindBond(t *testing.T) { auto: false, configMethod: configMethodManual{}, options: map[string][]string{ - "slaves": []string{"eth0"}, + "bond-slaves": []string{"eth0"}, }, }, } @@ -255,6 +313,7 @@ func TestBuildInterfacesBlindBond(t *testing.T) { configDepth: 1, }, []string{"eth0"}, + map[string]string{}, } eth0 := &physicalInterface{ logicalInterface{ @@ -323,7 +382,9 @@ func TestBuildInterfaces(t *testing.T) { auto: false, configMethod: configMethodManual{}, options: map[string][]string{ - "slaves": []string{"eth0"}, + "bond-slaves": []string{"eth0"}, + "bond-mode": []string{"4"}, + "bond-miimon": []string{"100"}, }, }, &stanzaInterface{ @@ -332,7 +393,7 @@ func TestBuildInterfaces(t *testing.T) { auto: false, configMethod: configMethodManual{}, options: map[string][]string{ - "slaves": []string{"bond0"}, + "bond-slaves": []string{"bond0"}, }, }, &stanzaInterface{ @@ -385,6 +446,7 @@ func TestBuildInterfaces(t *testing.T) { configDepth: 2, }, []string{"bond0"}, + map[string]string{}, } bond0 := &bondInterface{ logicalInterface{ @@ -394,6 +456,10 @@ func TestBuildInterfaces(t *testing.T) { configDepth: 1, }, []string{"eth0"}, + map[string]string{ + "mode": "4", + "miimon": "100", + }, } eth0 := &physicalInterface{ logicalInterface{ diff --git a/network/stanza.go b/network/stanza.go index 005231b..3319ce7 100644 --- a/network/stanza.go +++ b/network/stanza.go @@ -293,7 +293,6 @@ func parseHwaddress(options map[string][]string, iface string) (net.HardwareAddr } func parseBondStanza(iface string, conf configMethod, attributes []string, options map[string][]string) (*stanzaInterface, error) { - options["slaves"] = options["bond-slaves"] return &stanzaInterface{name: iface, kind: interfaceBond, configMethod: conf, options: options}, nil } diff --git a/network/stanza_test.go b/network/stanza_test.go index 440dcf8..02cc006 100644 --- a/network/stanza_test.go +++ b/network/stanza_test.go @@ -129,7 +129,7 @@ func TestParseBondStanzaNoSlaves(t *testing.T) { if err != nil { t.FailNow() } - if bond.options["slaves"] != nil { + if bond.options["bond-slaves"] != nil { t.FailNow() } } @@ -152,9 +152,6 @@ func TestParseBondStanza(t *testing.T) { if bond.configMethod != conf { t.FailNow() } - if !reflect.DeepEqual(bond.options["slaves"], options["bond-slaves"]) { - t.FailNow() - } } func TestParsePhysicalStanza(t *testing.T) { diff --git a/system/networkd.go b/system/networkd.go index a982869..9562789 100644 --- a/system/networkd.go +++ b/system/networkd.go @@ -6,6 +6,7 @@ import ( "net" "os/exec" "path" + "strings" "github.com/coreos/coreos-cloudinit/network" "github.com/coreos/coreos-cloudinit/third_party/github.com/dotcloud/docker/pkg/netlink" @@ -26,10 +27,10 @@ func RestartNetwork(interfaces []network.InterfaceGenerator) (err error) { return } - if err = probe8012q(); err != nil { + if err = maybeProbe8012q(interfaces); err != nil { return } - return + return maybeProbeBonding(interfaces) } func downNetworkInterfaces(interfaces []network.InterfaceGenerator) error { @@ -55,8 +56,24 @@ func downNetworkInterfaces(interfaces []network.InterfaceGenerator) error { return nil } -func probe8012q() error { - return exec.Command("modprobe", "8021q").Run() +func maybeProbe8012q(interfaces []network.InterfaceGenerator) error { + for _, iface := range interfaces { + if iface.Type() == "vlan" { + return exec.Command("modprobe", "8021q").Run() + } + } + return nil +} + +func maybeProbeBonding(interfaces []network.InterfaceGenerator) error { + args := []string{"bonding"} + for _, iface := range interfaces { + if iface.Type() == "bond" { + args = append(args, strings.Split(iface.ModprobeParams(), " ")...) + break + } + } + return exec.Command("modprobe", args...).Run() } func restartNetworkd() error { From fe388a3ab6eeec0f7b9f91d44d90ac2b1f455ba2 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Tue, 8 Jul 2014 16:48:42 -0700 Subject: [PATCH 2/3] networkd: Create config directory before writing config --- system/networkd.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/system/networkd.go b/system/networkd.go index 9562789..f9d59ce 100644 --- a/system/networkd.go +++ b/system/networkd.go @@ -4,6 +4,7 @@ import ( "fmt" "io/ioutil" "net" + "os" "os/exec" "path" "strings" @@ -103,6 +104,8 @@ func writeConfig(filename string, config string) error { if config == "" { return nil } - + if err := os.MkdirAll(path.Dir(filename), 0755); err != nil { + return err + } return ioutil.WriteFile(filename, []byte(config), 0444) } From e3037f18a6e96454e3c6e1804a50a8492db77fa0 Mon Sep 17 00:00:00 2001 From: Alex Crawford Date: Thu, 12 Jun 2014 19:47:05 -0500 Subject: [PATCH 3/3] networkd: Restart networkd twice to work around race https://bugs.freedesktop.org/show_bug.cgi?id=76077 --- system/networkd.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/system/networkd.go b/system/networkd.go index f9d59ce..9ef7e39 100644 --- a/system/networkd.go +++ b/system/networkd.go @@ -8,6 +8,7 @@ import ( "os/exec" "path" "strings" + "time" "github.com/coreos/coreos-cloudinit/network" "github.com/coreos/coreos-cloudinit/third_party/github.com/dotcloud/docker/pkg/netlink" @@ -19,6 +20,13 @@ const ( func RestartNetwork(interfaces []network.InterfaceGenerator) (err error) { defer func() { + if e := restartNetworkd(); e != nil { + err = e + return + } + // TODO(crawford): Get rid of this once networkd fixes the race + // https://bugs.freedesktop.org/show_bug.cgi?id=76077 + time.Sleep(5 * time.Second) if e := restartNetworkd(); e != nil { err = e }