Replace vishvananda/netlink with jsimonetti/rtnetlink (#271)

Fixes #257

And saves ~100kb.

Signed-off-by: Andrea Barberio <insomniac@slackware.it>

5 files changed, 315 insertions, 38 deletions

diff --git a/.travis/tests.sh b/.travis/tests.sh
index 1503547..a3389fe 100755
--- a/.travis/tests.sh
+++ b/.travis/tests.sh
@@ -13,13 +13,13 @@ for d in $(go list ./... | grep -v vendor); do
         rm profile.out
     fi
     # integration tests
-    go test -c -tags=integration -race -coverprofile=profile.out -covermode=atomic $d
+    go test -c -cover -tags=integration -race -covermode=atomic $d
     testbin="./$(basename $d).test"
     # only run it if it was built - i.e. if there are integ tests
-    test -x "${testbin}" && sudo "./${testbin}"
+    test -x "${testbin}" && sudo "./${testbin}" -test.coverprofile=profile.out
     if [ -f profile.out ]; then
         cat profile.out >> coverage.txt
-        rm profile.out
+        rm -f profile.out
     fi
 done
 
diff --git a/netboot/netconf.go b/netboot/netconf.go
index 0f40e26..2e11d38 100644
--- a/netboot/netconf.go
+++ b/netboot/netconf.go
@@ -7,11 +7,13 @@ import (
 	"net"
 	"os"
 	"strings"
+	"syscall"
 	"time"
 
 	"github.com/insomniacslk/dhcp/dhcpv4"
 	"github.com/insomniacslk/dhcp/dhcpv6"
-	"github.com/vishvananda/netlink"
+	"github.com/jsimonetti/rtnetlink"
+	"github.com/mdlayher/netlink"
 )
 
 // AddrConf holds a single IP address configuration for a NIC
@@ -136,31 +138,36 @@ func GetNetConfFromPacketv4(d *dhcpv4.DHCPv4) (*NetConf, error) {
 }
 
 // IfUp brings up an interface by name, and waits for it to come up until a timeout expires
-func IfUp(ifname string, timeout time.Duration) (netlink.Link, error) {
+func IfUp(ifname string, timeout time.Duration) (*net.Interface, error) {
 	start := time.Now()
+	var rt RTNL
+	defer rt.Close()
 	for time.Since(start) < timeout {
-		iface, err := netlink.LinkByName(ifname)
+		iface, err := net.InterfaceByName(ifname)
 		if err != nil {
-			return nil, fmt.Errorf("cannot get interface %q by name: %v", ifname, err)
+			return nil, err
 		}
-
 		// If the interface is up, return. According to kernel documentation OperState may
 		// be either Up or Unknown:
 		//   Interface is in RFC2863 operational state UP or UNKNOWN. This is for
 		//   backward compatibility, routing daemons, dhcp clients can use this
 		//   flag to determine whether they should use the interface.
 		// Source: https://www.kernel.org/doc/Documentation/networking/operstates.txt
-		operState := iface.Attrs().OperState
-		if operState == netlink.OperUp || operState == netlink.OperUnknown {
+		operState, err := rt.GetLinkState(iface.Index)
+		if err != nil {
+			return nil, err
+		}
+		if operState == rtnetlink.OperStateUp || operState == rtnetlink.OperStateUnknown {
 			// XXX despite the OperUp state, upon the first attempt I
-			// consistently get a "cannot assign requested address" error. This
-			// may be a bug in the netlink library. Need to investigate more.
+			// consistently get a "cannot assign requested address" error. Need
+			// to investigate more.
 			time.Sleep(time.Second)
 			return iface, nil
 		}
 		// otherwise try to bring it up
-		if err := netlink.LinkSetUp(iface); err != nil {
-			return nil, fmt.Errorf("interface %q: %v can't make it up: %v", ifname, iface, err)
+		err = rt.SetLinkState(iface.Index, true)
+		if err != nil {
+			return nil, fmt.Errorf("interface %q: %v can't bring it up: %v", ifname, iface, err)
 		}
 		time.Sleep(10 * time.Millisecond)
 	}
@@ -172,21 +179,16 @@ func IfUp(ifname string, timeout time.Duration) (netlink.Link, error) {
 // ConfigureInterface configures a network interface with the configuration held by a
 // NetConf structure
 func ConfigureInterface(ifname string, netconf *NetConf) error {
-	iface, err := netlink.LinkByName(ifname)
+	iface, err := net.InterfaceByName(ifname)
 	if err != nil {
-		return fmt.Errorf("error getting interface information for %s: %v", ifname, err)
+		return err
 	}
+	var rt RTNL
+	defer rt.Close()
 	// configure interfaces
 	for _, addr := range netconf.Addresses {
-		dest := &netlink.Addr{
-			IPNet:       &addr.IPNet,
-			PreferedLft: addr.PreferredLifetime,
-			ValidLft:    addr.ValidLifetime,
-		}
-		if err := netlink.AddrReplace(iface, dest); err != nil {
-			if os.IsExist(err) {
-				return fmt.Errorf("cannot configure %s on %s,%d,%d: %v", ifname, addr.IPNet, addr.PreferredLifetime, addr.ValidLifetime, err)
-			}
+		if err := rt.SetAddr(iface.Index, addr.IPNet); err != nil {
+			return fmt.Errorf("cannot configure %s on %s: %v", ifname, addr.IPNet, err)
 		}
 	}
 	// configure /etc/resolv.conf
@@ -201,32 +203,37 @@ func ConfigureInterface(ifname string, netconf *NetConf) error {
 		return fmt.Errorf("could not write resolv.conf file %v", err)
 	}
 
+	// FIXME wut? No IPv6 here?
 	// add default route information for v4 space. only one default route is allowed
 	// so ignore the others if there are multiple ones
 	if len(netconf.Routers) > 0 {
-		iface, err = netlink.LinkByName(ifname)
-		if err != nil {
-			return fmt.Errorf("could not obtain interface when adding default route: %v", err)
-		}
 		// if there is a default v4 route, remove it, as we want to add the one we just got during
 		// the dhcp transaction. if the route is not present, which is the final state we want,
 		// an error is returned so ignore it
-		dst := &net.IPNet{
-			IP:   net.IPv4(0, 0, 0, 0),
+		dst := net.IPNet{
+			IP:   net.IPv4zero,
 			Mask: net.CIDRMask(0, 32),
 		}
 		// Remove a possible default route (dst 0.0.0.0) to the L2 domain (gw: 0.0.0.0), which is what
 		// a client would want to add before initiating the DHCP transaction in order not to fail with
 		// ENETUNREACH. If this default route has a specific metric assigned, it doesn't get removed.
 		// The code doesn't remove any other default route (i.e. gw != 0.0.0.0).
-		route := netlink.Route{LinkIndex: iface.Attrs().Index, Dst: dst, Src: net.IPv4(0, 0, 0, 0)}
-		netlink.RouteDel(&route)
+		if err := rt.RouteDel(net.IPv4zero); err != nil {
+			switch err := err.(type) {
+			case *netlink.OpError:
+				// ignore the error if it's -EEXIST or -ESRCH
+				if !os.IsExist(err.Err) && err.Err != syscall.ESRCH {
+					return fmt.Errorf("could not delete default route on interface %s: %v", ifname, err)
+				}
+			default:
+				return fmt.Errorf("could not delete default route on interface %s: %v", ifname, err)
+			}
+		}
 
-		src := netconf.Addresses[0].IPNet.IP
-		route = netlink.Route{LinkIndex: iface.Attrs().Index, Dst: dst, Src: src, Gw: netconf.Routers[0]}
-		err = netlink.RouteAdd(&route)
-		if err != nil {
-			return fmt.Errorf("could not add default route (%+v) to interface %s: %v", route, iface.Attrs().Name, err)
+		src := netconf.Addresses[0].IPNet
+		// TODO handle the remaining Routers if more than one
+		if err := rt.RouteAdd(iface.Index, dst, src, netconf.Routers[0]); err != nil {
+			return fmt.Errorf("could not add gateway %s for src %s dst %s to interface %s: %v", netconf.Routers[0], src, dst, ifname, err)
 		}
 	}
 
diff --git a/netboot/netconf_integ_test.go b/netboot/netconf_integ_test.go
new file mode 100644
index 0000000..e4bad74
--- /dev/null
+++ b/netboot/netconf_integ_test.go
@@ -0,0 +1,26 @@
+// +build integration
+
+package netboot
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIfUp(t *testing.T) {
+	// this assumes that eth0 exists and is configurable
+	ifname := "eth0"
+	iface, err := IfUp(ifname, 2*time.Second)
+	require.NoError(t, err)
+	assert.Equal(t, ifname, iface.Name)
+}
+
+func TestIfUpTimeout(t *testing.T) {
+	// this assumes that eth0 exists and is configurable
+	ifname := "eth0"
+	_, err := IfUp(ifname, 0*time.Second)
+	require.Error(t, err)
+}
diff --git a/netboot/rtnetlink_linux.go b/netboot/rtnetlink_linux.go
new file mode 100644
index 0000000..949f7e5
--- /dev/null
+++ b/netboot/rtnetlink_linux.go
@@ -0,0 +1,162 @@
+package netboot
+
+import (
+	"encoding/binary"
+	"net"
+
+	"github.com/jsimonetti/rtnetlink"
+	"golang.org/x/sys/unix"
+)
+
+// RTNL is a rtnetlink object with a high-level interface.
+type RTNL struct {
+	conn *rtnetlink.Conn
+}
+
+func (r *RTNL) init() error {
+	if r.conn != nil {
+		return nil
+	}
+	conn, err := rtnetlink.Dial(nil)
+	if err != nil {
+		return err
+	}
+	r.conn = conn
+	return nil
+}
+
+// Close closes the netlink connection. Must be called to avoid leaks!
+func (r *RTNL) Close() {
+	if r.conn != nil {
+		r.conn.Close()
+		r.conn = nil
+	}
+}
+
+// GetLinkState returns the operational state for the given interface index.
+func (r *RTNL) GetLinkState(iface int) (rtnetlink.OperationalState, error) {
+	if err := r.init(); err != nil {
+		return 0, err
+	}
+	msg, err := r.conn.Link.Get(uint32(iface))
+	if err != nil {
+		return 0, err
+	}
+	return msg.Attributes.OperationalState, nil
+}
+
+// SetLinkState sets the operational state up or down for the given interface
+// index.
+func (r *RTNL) SetLinkState(iface int, up bool) error {
+	if err := r.init(); err != nil {
+		return err
+	}
+	var state uint32
+	if up {
+		state = unix.IFF_UP
+	}
+	msg := rtnetlink.LinkMessage{
+		Family: unix.AF_UNSPEC,
+		Type:   unix.ARPHRD_NETROM,
+		Index:  uint32(iface),
+		Flags:  state,
+		Change: unix.IFF_UP,
+	}
+	if err := r.conn.Link.Set(&msg); err != nil {
+		return err
+	}
+	return nil
+}
+
+func getFamily(ip net.IP) int {
+	if ip.To4() != nil {
+		return unix.AF_INET
+	}
+	return unix.AF_INET6
+}
+
+// SetAddr sets the interface address.
+func (r *RTNL) SetAddr(iface int, a net.IPNet) error {
+	if err := r.init(); err != nil {
+		return err
+	}
+	ones, _ := a.Mask.Size()
+	msg := rtnetlink.AddressMessage{
+		Family:       uint8(getFamily(a.IP)),
+		PrefixLength: uint8(ones),
+		// TODO detect the right scope to set, or get it as input argument
+		Scope: unix.RT_SCOPE_UNIVERSE,
+		Index: uint32(iface),
+		Attributes: rtnetlink.AddressAttributes{
+			Address: a.IP,
+			Local:   a.IP,
+		},
+	}
+	if a.IP.To4() != nil {
+		// Broadcast is only required for IPv4
+		ip := make(net.IP, net.IPv4len)
+		binary.BigEndian.PutUint32(
+			ip,
+			binary.BigEndian.Uint32(a.IP.To4())|
+				^binary.BigEndian.Uint32(net.IP(a.Mask).To4()))
+		msg.Attributes.Broadcast = ip
+	}
+	if err := r.conn.Address.New(&msg); err != nil {
+		return err
+	}
+	return nil
+}
+
+// RouteDel deletes a route to the given destination
+func (r *RTNL) RouteDel(dst net.IP) error {
+	if err := r.init(); err != nil {
+		return err
+	}
+	msg := rtnetlink.RouteMessage{
+		Family: uint8(getFamily(dst)),
+		Table:  unix.RT_TABLE_MAIN,
+		// TODO make this configurable?
+		Protocol: unix.RTPROT_UNSPEC,
+		// TODO make this configurable?
+		Scope: unix.RT_SCOPE_NOWHERE,
+		Type:  unix.RTN_UNSPEC,
+		Attributes: rtnetlink.RouteAttributes{
+			Dst: dst,
+		},
+	}
+	if err := r.conn.Route.Delete(&msg); err != nil {
+		return err
+	}
+	return nil
+}
+
+// RouteAdd adds a route to dst, from src (if set), via gw.
+func (r *RTNL) RouteAdd(iface int, dst, src net.IPNet, gw net.IP) error {
+	if err := r.init(); err != nil {
+		return err
+	}
+	dstLen, _ := dst.Mask.Size()
+	srcLen, _ := src.Mask.Size()
+	msg := rtnetlink.RouteMessage{
+		Family: uint8(getFamily(dst.IP)),
+		Table:  unix.RT_TABLE_MAIN,
+		// TODO make this configurable?
+		Protocol: unix.RTPROT_BOOT,
+		// TODO make this configurable?
+		Scope:     unix.RT_SCOPE_UNIVERSE,
+		Type:      unix.RTN_UNICAST,
+		DstLength: uint8(dstLen),
+		SrcLength: uint8(srcLen),
+		Attributes: rtnetlink.RouteAttributes{
+			Dst:      dst.IP,
+			Src:      src.IP,
+			Gateway:  gw,
+			OutIface: uint32(iface),
+		},
+	}
+	if err := r.conn.Route.Add(&msg); err != nil {
+		return err
+	}
+	return nil
+
+}
diff --git a/netboot/rtnetlink_linux_test.go b/netboot/rtnetlink_linux_test.go
new file mode 100644
index 0000000..860ea0f
--- /dev/null
+++ b/netboot/rtnetlink_linux_test.go
@@ -0,0 +1,82 @@
+//+build integration
+
+package netboot
+
+import (
+	"net"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
+)
+
+// integration tests that require Linux with a properly working rtnetlink
+// interface, and the existence of an "eth0" interface.
+// WARNING: these tests may improperly configure your network interfaces and
+// routing, so be careful before running them. Privileged access and integration
+// build tag required to run them.
+
+var (
+	testIfname = "eth0"
+)
+
+func TestInit(t *testing.T) {
+	var r RTNL
+	err := r.init()
+	assert.NoError(t, err)
+	require.NotNil(t, r.conn)
+	r.Close()
+}
+
+func TestClose(t *testing.T) {
+	var r RTNL
+	err := r.init()
+	assert.NoError(t, err)
+	require.NotNil(t, r.conn)
+	r.Close()
+	require.Nil(t, r.conn)
+}
+
+func TestGetLinkState(t *testing.T) {
+	var r RTNL
+	defer r.Close()
+
+	iface, err := net.InterfaceByName(testIfname)
+	require.NoError(t, err)
+	_, err = r.GetLinkState(iface.Index)
+	require.NoError(t, err)
+}
+
+func TestSetLinkState(t *testing.T) {
+	var r RTNL
+	defer r.Close()
+
+	iface, err := net.InterfaceByName(testIfname)
+	require.NoError(t, err)
+	err = r.SetLinkState(iface.Index, true)
+	require.NoError(t, err)
+}
+
+func Test_getFamily(t *testing.T) {
+	require.Equal(t, unix.AF_INET, getFamily(net.IPv4zero))
+	require.Equal(t, unix.AF_INET, getFamily(net.IPv4bcast))
+	require.Equal(t, unix.AF_INET, getFamily(net.IPv4allrouter))
+
+	require.Equal(t, unix.AF_INET6, getFamily(net.IPv6zero))
+	require.Equal(t, unix.AF_INET6, getFamily(net.IPv6loopback))
+	require.Equal(t, unix.AF_INET6, getFamily(net.IPv6linklocalallrouters))
+}
+
+func TestSetAddr(t *testing.T) {
+	var r RTNL
+	defer r.Close()
+	iface, err := net.InterfaceByName(testIfname)
+	require.NoError(t, err)
+
+	a := net.IPNet{IP: net.ParseIP("10.0.123.1"), Mask: net.IPv4Mask(255, 255, 255, 0)}
+	err = r.SetAddr(iface.Index, a)
+	require.NoError(t, err)
+	// TODO implement GetAddr to further validate this, and minimize the effect
+	// of concurrent tests that may invalidate this check.
+}