From c952693b82e23ebb0a6729743268392d3047ac4a Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Thu, 16 Jan 2020 18:08:27 +0800 Subject: [PATCH 1/6] hostman: fix removing addresses on slave ifaces Previously it was done with "ifconfig xxx 0 up" Fixes 4d8d20f ("hostman: use package iproute2") --- pkg/hostman/hostinfo/hostbridge/hostbridge.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/hostman/hostinfo/hostbridge/hostbridge.go b/pkg/hostman/hostinfo/hostbridge/hostbridge.go index 81a2be52a4..8d64aa880d 100644 --- a/pkg/hostman/hostinfo/hostbridge/hostbridge.go +++ b/pkg/hostman/hostinfo/hostbridge/hostbridge.go @@ -201,6 +201,9 @@ func (d *SBaseBridgeDriver) SetupAddresses(mask net.IPMask) error { } if d.inter != nil { ifname := d.inter.String() + if err := iproute2.NewAddress(ifname).Exact().Err(); err != nil { + return errors.Wrapf(err, "remove addresses on slave ifname: %s", ifname) + } if err := iproute2.NewLink(ifname).Up().Err(); err != nil { return errors.Wrapf(err, "setting bridge %s ifname %s up", br, ifname) } @@ -210,9 +213,6 @@ func (d *SBaseBridgeDriver) SetupAddresses(mask net.IPMask) error { func (d *SBaseBridgeDriver) SetupSlaveAddresses(slaveAddrs [][]string) error { for _, slaveAddr := range slaveAddrs { - if err := iproute2.NewAddress(d.inter.String()).Exact().Err(); err != nil { - return errors.Wrap(err, "remove address on slave interface") - } addr := fmt.Sprintf("%s/%s", slaveAddr[0], slaveAddr[1]) if err := iproute2.NewAddress(d.bridge.String(), addr).Add().Err(); err != nil { return errors.Wrap(err, "move address to bridge interface") From 697c441acf5d2a2debde6cc809f5c93c012400e1 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Thu, 16 Jan 2020 20:26:02 +0800 Subject: [PATCH 2/6] hostman: up bridge interface in SetupAddresses Previously it was done implicitly by ifconfig addr config command --- pkg/hostman/hostinfo/hostbridge/hostbridge.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/hostman/hostinfo/hostbridge/hostbridge.go b/pkg/hostman/hostinfo/hostbridge/hostbridge.go index 8d64aa880d..a8c581d0ee 100644 --- a/pkg/hostman/hostinfo/hostbridge/hostbridge.go +++ b/pkg/hostman/hostinfo/hostbridge/hostbridge.go @@ -193,10 +193,14 @@ func (d *SBaseBridgeDriver) SetupAddresses(mask net.IPMask) error { return errors.Wrapf(err, "set bridge %s address", br) } } - if options.HostOptions.TunnelPaddingBytes > 0 { - mtu := 1500 + int(options.HostOptions.TunnelPaddingBytes) - if err := iproute2.NewLink(br).MTU(mtu).Err(); err != nil { - return errors.Wrapf(err, "setting bridge %s mtu %d", br, mtu) + { + brLink := iproute2.NewLink(br).Up() + if options.HostOptions.TunnelPaddingBytes > 0 { + mtu := 1500 + int(options.HostOptions.TunnelPaddingBytes) + brLink.MTU(mtu) + } + if err := brLink.Err(); err != nil { + return errors.Wrapf(err, "setting bridge %s up", br) } } if d.inter != nil { From 8b5b6bc59146eed62a31186395bfb925fb300af5 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Thu, 16 Jan 2020 18:11:35 +0800 Subject: [PATCH 3/6] hostman: add secondary addresses with one iproute2 call --- pkg/hostman/hostinfo/hostbridge/hostbridge.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pkg/hostman/hostinfo/hostbridge/hostbridge.go b/pkg/hostman/hostinfo/hostbridge/hostbridge.go index a8c581d0ee..c7b9c1f2da 100644 --- a/pkg/hostman/hostinfo/hostbridge/hostbridge.go +++ b/pkg/hostman/hostinfo/hostbridge/hostbridge.go @@ -216,11 +216,13 @@ func (d *SBaseBridgeDriver) SetupAddresses(mask net.IPMask) error { } func (d *SBaseBridgeDriver) SetupSlaveAddresses(slaveAddrs [][]string) error { - for _, slaveAddr := range slaveAddrs { - addr := fmt.Sprintf("%s/%s", slaveAddr[0], slaveAddr[1]) - if err := iproute2.NewAddress(d.bridge.String(), addr).Add().Err(); err != nil { - return errors.Wrap(err, "move address to bridge interface") - } + br := d.bridge.String() + addrs := make([]string, len(slaveAddrs)) + for i, slaveAddr := range slaveAddrs { + addrs[i] = fmt.Sprintf("%s/%s", slaveAddr[0], slaveAddr[1]) + } + if err := iproute2.NewAddress(br, addrs...).Add().Err(); err != nil { + return errors.Wrap(err, "move secondary addresses to bridge interface") } return nil } From 1a5f6ee0f619495c5b635fdf6a366c6790906348 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Thu, 16 Jan 2020 18:04:58 +0800 Subject: [PATCH 4/6] iproute2: route operations --- pkg/util/iproute2/route.go | 136 +++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 pkg/util/iproute2/route.go diff --git a/pkg/util/iproute2/route.go b/pkg/util/iproute2/route.go new file mode 100644 index 0000000000..fdc1a44a62 --- /dev/null +++ b/pkg/util/iproute2/route.go @@ -0,0 +1,136 @@ +package iproute2 + +import ( + "net" + + "github.com/vishvananda/netlink" + + "yunion.io/x/pkg/errors" +) + +const ( + errBadIP = errors.Error("bad ip") +) + +type Route struct { + *Link +} + +func NewRoute(ifname string) *Route { + l := NewLink(ifname) + route := &Route{ + Link: l, + } + return route +} + +func (route *Route) link() (link netlink.Link, ok bool) { + link = route.Link.link + if link != nil { + ok = true + } + return +} + +func (route *Route) List4() ([]netlink.Route, error) { + link, ok := route.link() + if !ok { + return nil, route.Err() + } + + rs, err := netlink.RouteList(link, netlink.FAMILY_V4) + if err != nil { + route.addErr(err, "route list") + return nil, route.Err() + } + for i := range rs { + if rs[i].Dst == nil { + // make the return value easier to work with + rs[i].Dst = &net.IPNet{ + IP: net.IPv4zero, + Mask: net.IPMask(net.IPv4zero), + } + } + } + return rs, nil +} + +func (route *Route) AddByIPNet(ipnet *net.IPNet, gw net.IP) *Route { + link, ok := route.link() + if !ok { + return route + } + + r := &netlink.Route{ + LinkIndex: link.Attrs().Index, + Dst: ipnet, + } + if len(gw) > 0 { + r.Gw = gw + } + if err := netlink.RouteReplace(r); err != nil { + route.addErr(err, "RouteReplace %s", r.String()) + } + return route +} + +func (route *Route) AddByCidr(cidr string, gwStr string) *Route { + var ( + dst *net.IPNet + gw net.IP + err error + ) + if _, dst, err = net.ParseCIDR(cidr); err != nil { + route.addErr(err, "parse cidr") + return route + } + + if gwStr != "" { + gw = net.ParseIP(gwStr) + if len(gw) == 0 { + route.addErr(errBadIP, "gwStr: %s", gwStr) + return route + } + } + + return route.AddByIPNet(dst, gw) +} + +func (route *Route) Add(netStr, maskStr, gwStr string) *Route { + var ( + ip net.IP + mask net.IPMask + gw net.IP + ) + + if ip = net.ParseIP(netStr); len(ip) == 0 { + route.addErr(errBadIP, "netStr %s", netStr) + return route + } + if maskIp := net.ParseIP(maskStr); len(maskIp) == 0 { + route.addErr(errBadIP, "maskStr %s", maskStr) + return route + } else { + if ip := maskIp.To4(); len(ip) > 0 { + maskIp = ip + } + mask = net.IPMask(maskIp) + ones, bits := mask.Size() + if ones == 0 && bits == 0 { + route.addErr(errBadIP, "bad mask %s", maskStr) + return route + } + } + if gwStr != "" { + if gw = net.ParseIP(gwStr); len(gw) == 0 { + route.addErr(errBadIP, "gwStr %s", gwStr) + return route + } + } + + ipnet := &net.IPNet{ + IP: ip, + Mask: mask, + } + return route.AddByIPNet(ipnet, gw) +} From 5cd60db1b45f7d60c713c87cb853b9beea376c52 Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Thu, 16 Jan 2020 18:05:23 +0800 Subject: [PATCH 5/6] netutils2: replace GetRoutes with netlink call --- pkg/util/netutils2/netutils.go | 24 ------------------------ pkg/util/netutils2/netutils_linux.go | 24 ++++++++++++++++++++++++ pkg/util/netutils2/netutils_test.go | 17 ----------------- 3 files changed, 24 insertions(+), 41 deletions(-) diff --git a/pkg/util/netutils2/netutils.go b/pkg/util/netutils2/netutils.go index cbd6d68162..fe9b6ea08e 100644 --- a/pkg/util/netutils2/netutils.go +++ b/pkg/util/netutils2/netutils.go @@ -19,7 +19,6 @@ import ( "fmt" "net" "reflect" - "regexp" "strconv" "strings" "unicode" @@ -32,7 +31,6 @@ import ( "yunion.io/x/onecloud/pkg/cloudcommon/types" "yunion.io/x/onecloud/pkg/util/procutils" - "yunion.io/x/onecloud/pkg/util/regutils2" ) var PSEUDO_VIP = "169.254.169.231" @@ -370,28 +368,6 @@ func GetSecretInterfaceAddress() (string, int) { return addr, SECRET_MASK_LEN } -func (n *SNetInterface) GetRoutes(gwOnly bool) [][]string { - output, err := procutils.NewCommand("route", "-n").Output() - if err != nil { - return nil - } - return n.getRoutes(gwOnly, strings.Split(string(output), "\n")) -} - -func (n *SNetInterface) getRoutes(gwOnly bool, outputs []string) [][]string { - re := regexp.MustCompile(`(?P[0-9.]+)\s+(?P[0-9.]+)\s+(?P[0-9.]+)` + - `\s+[A-Z!]+\s+[0-9]+\s+[0-9]+\s+[0-9]+\s+` + n.name) - - var res [][]string = make([][]string, 0) - for _, line := range outputs { - m := regutils2.GetParams(re, line) - if len(m) > 0 && (!gwOnly || m["gw"] != "0.0.0.0") { - res = append(res, []string{m["dest"], m["gw"], m["mask"]}) - } - } - return res -} - func (n *SNetInterface) GetSlaveAddresses() [][]string { addrs := n.GetAddresses() var slaves = make([][]string, 0) diff --git a/pkg/util/netutils2/netutils_linux.go b/pkg/util/netutils2/netutils_linux.go index bfbd406d93..63c5aeca0d 100644 --- a/pkg/util/netutils2/netutils_linux.go +++ b/pkg/util/netutils2/netutils_linux.go @@ -30,6 +30,30 @@ func (n *SNetInterface) GetAddresses() [][]string { return r } +func (n *SNetInterface) GetRoutes(gwOnly bool) [][]string { + rs, err := iproute2.NewRoute(n.name).List4() + if err != nil { + return nil + } + + res := [][]string{} + for i := range rs { + r := &rs[i] + ok := true + if masklen, _ := r.Dst.Mask.Size(); gwOnly && masklen != 0 { + ok = false + } + if ok { + res = append(res, []string{ + r.Dst.IP.String(), + r.Gw.String(), + net.IP(r.Dst.Mask).String(), + }) + } + } + return res +} + func DefaultSrcIpDev() (srcIp net.IP, ifname string, err error) { destIp := net.ParseIP("114.114.114.114") routes, err := netlink.RouteGet(destIp) diff --git a/pkg/util/netutils2/netutils_test.go b/pkg/util/netutils2/netutils_test.go index b62301a5eb..479ecb45f0 100644 --- a/pkg/util/netutils2/netutils_test.go +++ b/pkg/util/netutils2/netutils_test.go @@ -15,26 +15,9 @@ package netutils2 import ( - "reflect" "testing" ) -func TestSNetInterface_getRoutes(t *testing.T) { - n := SNetInterface{name: "br0"} - routes := []string{"Kernel IP routing table", - "Destination Gateway Genmask Flags Metric Ref Use Iface", - "0.0.0.0 10.168.222.1 0.0.0.0 UG 0 0 0 br0", - "10.168.222.0 0.0.0.0 255.255.255.0 U 0 0 0 br0", - "169.254.169.254 10.168.222.1 255.255.255.255 UGH 0 0 0 br0", - ""} - - want := [][]string{{"0.0.0.0", "10.168.222.1", "0.0.0.0"}, {"169.254.169.254", "10.168.222.1", "255.255.255.255"}} - - if got := n.getRoutes(true, routes); !reflect.DeepEqual(got, want) { - t.Errorf("getParams() = %v, want %v", got, want) - } -} - func TestNetlen2Mask(t *testing.T) { type args struct { netmasklen int From 167bca193103b3825816c1cd09b1fb9d7e78521c Mon Sep 17 00:00:00 2001 From: Yousong Zhou Date: Thu, 16 Jan 2020 18:28:54 +0800 Subject: [PATCH 6/6] hostman: replace route command dep with netlink call --- pkg/hostman/hostinfo/hostbridge/hostbridge.go | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/pkg/hostman/hostinfo/hostbridge/hostbridge.go b/pkg/hostman/hostinfo/hostbridge/hostbridge.go index c7b9c1f2da..b19902d356 100644 --- a/pkg/hostman/hostinfo/hostbridge/hostbridge.go +++ b/pkg/hostman/hostinfo/hostbridge/hostbridge.go @@ -29,7 +29,6 @@ import ( "yunion.io/x/onecloud/pkg/util/fileutils2" "yunion.io/x/onecloud/pkg/util/iproute2" "yunion.io/x/onecloud/pkg/util/netutils2" - "yunion.io/x/onecloud/pkg/util/procutils" ) type IBridgeDriver interface { @@ -228,17 +227,14 @@ func (d *SBaseBridgeDriver) SetupSlaveAddresses(slaveAddrs [][]string) error { } func (d *SBaseBridgeDriver) SetupRoutes(routes [][]string) error { - for _, r := range routes { - var cmd []string - if r[2] == "0.0.0.0" { - cmd = []string{"route", "add", "default", "gw", r[1], "dev", d.bridge.String()} - } else { - cmd = []string{"route", "add", "-net", r[0], "netmask", r[2], "gw", r[1], "dev", d.bridge.String()} - } - if _, err := procutils.NewCommand(cmd[0], cmd[1:]...).Output(); err != nil { - log.Errorln(err) - return fmt.Errorf("Failed to add slave address to bridge %s", d.bridge) - } + br := d.bridge.String() + r := iproute2.NewRoute(br) + for _, route := range routes { + netStr, maskStr, gwStr := route[0], route[2], route[1] + r.Add(netStr, maskStr, gwStr) + } + if err := r.Err(); err != nil { + return errors.Wrapf(err, "set routes on %s", br) } return nil }