server: use Dialer Control method instead of hack to modify connection before dialing

Use the feature of go 1.11 to remove the hack.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

7 files changed, 89 insertions, 308 deletions

diff --git a/pkg/server/fsm.go b/pkg/server/fsm.go
index f4549e7c..c9198af1 100644
--- a/pkg/server/fsm.go
+++ b/pkg/server/fsm.go
@@ -22,6 +22,7 @@ import (
 	"net"
 	"strconv"
 	"sync"
+	"syscall"
 	"time"
 
 	"github.com/eapache/channels"
@@ -428,28 +429,32 @@ func (fsm *fsm) connectLoop() error {
 		if err != nil {
 			log.WithFields(log.Fields{
 				"Topic": "Peer",
-				"Key":   fsm.pConf.State.NeighborAddress,
+				"Key":   addr,
 			}).Warnf("failed to resolve local address: %s", err)
 			return
 		}
-		var conn net.Conn
-		d := tcpDialer{
-			Dialer: net.Dialer{
-				LocalAddr: laddr,
-				Timeout:   time.Duration(minConnectRetry-1) * time.Second,
-			},
-			AuthPassword: fsm.pConf.Config.AuthPassword,
-		}
+		password := fsm.pConf.Config.AuthPassword
+		ttl := uint8(0)
+		ttlMin := uint8(0)
+
 		if fsm.pConf.TtlSecurity.Config.Enabled {
-			d.TTL = 255
-			d.TTLMin = fsm.pConf.TtlSecurity.Config.TtlMin
+			ttl = 255
+			ttlMin = fsm.pConf.TtlSecurity.Config.TtlMin
 		} else if fsm.pConf.Config.PeerAs != 0 && fsm.pConf.Config.PeerType == config.PEER_TYPE_EXTERNAL {
-			d.TTL = 1
+			ttl = 1
 			if fsm.pConf.EbgpMultihop.Config.Enabled {
-				d.TTL = fsm.pConf.EbgpMultihop.Config.MultihopTtl
+				ttl = fsm.pConf.EbgpMultihop.Config.MultihopTtl
 			}
 		}
-		conn, err = d.DialTCP(addr, port)
+
+		d := net.Dialer{
+			LocalAddr: laddr,
+			Timeout:   time.Duration(minConnectRetry-1) * time.Second,
+			Control: func(network, address string, c syscall.RawConn) error {
+				return dialerControl(network, address, c, ttl, ttlMin, password)
+			},
+		}
+		conn, err := d.Dial("tcp", net.JoinHostPort(addr, fmt.Sprintf("%d", port)))
 		if err == nil {
 			select {
 			case fsm.connCh <- conn:
@@ -458,13 +463,13 @@ func (fsm *fsm) connectLoop() error {
 				conn.Close()
 				log.WithFields(log.Fields{
 					"Topic": "Peer",
-					"Key":   fsm.pConf.State.NeighborAddress,
+					"Key":   addr,
 				}).Warn("active conn is closed to avoid being blocked")
 			}
 		} else {
 			log.WithFields(log.Fields{
 				"Topic": "Peer",
-				"Key":   fsm.pConf.State.NeighborAddress,
+				"Key":   addr,
 			}).Debugf("failed to connect: %s", err)
 		}
 
diff --git a/pkg/server/server.go b/pkg/server/server.go
index 09ff114e..f7a09daf 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -1589,8 +1589,8 @@ func (s *BgpServer) AddBmp(ctx context.Context, r *api.AddBmpRequest) error {
 			return fmt.Errorf("invalid bmp route monitoring policy: %v", r.Type)
 		}
 		return s.bmpManager.addServer(&config.BmpServerConfig{
-			Address: r.Address,
-			Port:    r.Port,
+			Address:               r.Address,
+			Port:                  r.Port,
 			RouteMonitoringPolicy: config.IntToBmpRouteMonitoringPolicyTypeMap[int(r.Type)],
 			StatisticsTimeout:     uint16(r.StatisticsTimeout),
 		})
@@ -2520,7 +2520,7 @@ func (s *BgpServer) ListPeer(ctx context.Context, r *api.ListPeerRequest, fn fun
 			p := config.NewPeerFromConfigStruct(s.toConfig(peer, getAdvertised))
 			for _, family := range peer.configuredRFlist() {
 				for i, afisafi := range p.AfiSafis {
-					if afisafi.Config.Enabled != true {
+					if !afisafi.Config.Enabled {
 						continue
 					}
 					afi, safi := bgp.RouteFamilyToAfiSafi(family)
@@ -2530,7 +2530,7 @@ func (s *BgpServer) ListPeer(ctx context.Context, r *api.ListPeerRequest, fn fun
 						received := uint32(peer.adjRibIn.Count(flist))
 						accepted := uint32(peer.adjRibIn.Accepted(flist))
 						advertised := uint32(0)
-						if getAdvertised == true {
+						if getAdvertised {
 							pathList, _ := s.getBestFromLocal(peer, flist)
 							advertised = uint32(len(pathList))
 						}
diff --git a/pkg/server/sockopt.go b/pkg/server/sockopt.go
index 8061f41a..e2b1c086 100644
--- a/pkg/server/sockopt.go
+++ b/pkg/server/sockopt.go
@@ -17,8 +17,8 @@
 package server
 
 import (
-	"fmt"
 	"net"
+	"syscall"
 
 	log "github.com/sirupsen/logrus"
 )
@@ -39,52 +39,24 @@ func setTCPMinTTLSockopt(conn *net.TCPConn, ttl int) error {
 	return setTcpMinTTLSockopt(conn, ttl)
 }
 
-type tcpDialer struct {
-	net.Dialer
-
-	// MD5 authentication password.
-	AuthPassword string
-
-	// The TTL value to set outgoing connection.
-	TTL uint8
-
-	// The minimum TTL value for incoming packets.
-	TTLMin uint8
-}
-
-func (d *tcpDialer) DialTCP(addr string, port int) (*net.TCPConn, error) {
-	if d.AuthPassword != "" {
+func dialerControl(network, address string, c syscall.RawConn, ttl, ttlMin uint8, password string) error {
+	if password != "" {
 		log.WithFields(log.Fields{
 			"Topic": "Peer",
-			"Key":   addr,
+			"Key":   address,
 		}).Warn("setting md5 for active connection is not supported")
 	}
-	if d.TTL != 0 {
+	if ttl != 0 {
 		log.WithFields(log.Fields{
 			"Topic": "Peer",
-			"Key":   addr,
+			"Key":   address,
 		}).Warn("setting ttl for active connection is not supported")
 	}
-	if d.TTLMin != 0 {
+	if ttlMin != 0 {
 		log.WithFields(log.Fields{
 			"Topic": "Peer",
-			"Key":   addr,
+			"Key":   address,
 		}).Warn("setting min ttl for active connection is not supported")
 	}
-
-	raddr, err := net.ResolveTCPAddr("tcp", net.JoinHostPort(addr, fmt.Sprintf("%d", port)))
-	if err != nil {
-		return nil, fmt.Errorf("invalid remote address: %s", err)
-	}
-	laddr, err := net.ResolveTCPAddr("tcp", d.LocalAddr.String())
-	if err != nil {
-		return nil, fmt.Errorf("invalid local address: %s", err)
-	}
-
-	dialer := net.Dialer{LocalAddr: laddr, Timeout: d.Timeout}
-	conn, err := dialer.Dial("tcp", raddr.String())
-	if err != nil {
-		return nil, err
-	}
-	return conn.(*net.TCPConn), nil
+	return nil
 }
diff --git a/pkg/server/sockopt_linux.go b/pkg/server/sockopt_linux.go
index ac2ecdb2..31df8397 100644
--- a/pkg/server/sockopt_linux.go
+++ b/pkg/server/sockopt_linux.go
@@ -17,7 +17,6 @@
 package server
 
 import (
-	"fmt"
 	"net"
 	"os"
 	"syscall"
@@ -40,7 +39,7 @@ type tcpmd5sig struct {
 	key [80]byte
 }
 
-func buildTcpMD5Sig(address string, key string) (tcpmd5sig, error) {
+func buildTcpMD5Sig(address, key string) (tcpmd5sig, error) {
 	t := tcpmd5sig{}
 	addr := net.ParseIP(address)
 	if addr.To4() != nil {
@@ -104,186 +103,63 @@ func setTCPMinTTLSockopt(conn *net.TCPConn, ttl int) error {
 	return setsockOptInt(sc, level, name, ttl)
 }
 
-func setsockoptTcpMD5Sig(fd int, address string, key string) error {
-	t, err := buildTcpMD5Sig(address, key)
-	if err != nil {
-		return err
-	}
-	b := *(*[unsafe.Sizeof(t)]byte)(unsafe.Pointer(&t))
-	return os.NewSyscallError("setsockopt", syscall.SetsockoptString(fd, syscall.IPPROTO_TCP, tcpMD5SIG, string(b[:])))
-}
-
-func setsockoptIpTtl2(fd int, family int, value int) error {
-	level := syscall.IPPROTO_IP
-	name := syscall.IP_TTL
-	if family == syscall.AF_INET6 {
-		level = syscall.IPPROTO_IPV6
-		name = syscall.IPV6_UNICAST_HOPS
-	}
-	return os.NewSyscallError("setsockopt", syscall.SetsockoptInt(fd, level, name, value))
-}
-
-func setsockoptIpMinTtl(fd int, family int, value int) error {
-	level := syscall.IPPROTO_IP
-	name := syscall.IP_MINTTL
-	if family == syscall.AF_INET6 {
-		level = syscall.IPPROTO_IPV6
-		name = ipv6MinHopCount
-	}
-	return os.NewSyscallError("setsockopt", syscall.SetsockoptInt(fd, level, name, value))
-}
-
-type tcpDialer struct {
-	net.Dialer
-
-	// MD5 authentication password.
-	AuthPassword string
-
-	// The TTL value to set outgoing connection.
-	TTL uint8
-
-	// The minimum TTL value for incoming packets.
-	TTLMin uint8
-}
-
-func (d *tcpDialer) DialTCP(addr string, port int) (*net.TCPConn, error) {
-	var family int
-	var ra, la syscall.Sockaddr
-
-	raddr, err := net.ResolveTCPAddr("tcp", net.JoinHostPort(addr, fmt.Sprintf("%d", port)))
-	if err != nil {
-		return nil, fmt.Errorf("invalid remote address: %s", err)
-	}
-	laddr, err := net.ResolveTCPAddr("tcp", d.LocalAddr.String())
-	if err != nil {
-		return nil, fmt.Errorf("invalid local address: %s", err)
-	}
-	if raddr.IP.To4() != nil {
-		family = syscall.AF_INET
-		rsockaddr := &syscall.SockaddrInet4{Port: port}
-		copy(rsockaddr.Addr[:], raddr.IP.To4())
-		ra = rsockaddr
-		lsockaddr := &syscall.SockaddrInet4{}
-		copy(lsockaddr.Addr[:], laddr.IP.To4())
-		la = lsockaddr
-	} else {
+func dialerControl(network, address string, c syscall.RawConn, ttl, minTtl uint8, password string) error {
+	family := syscall.AF_INET
+	raddr, _ := net.ResolveTCPAddr("tcp", address)
+	if raddr.IP.To4() == nil {
 		family = syscall.AF_INET6
-		rsockaddr := &syscall.SockaddrInet6{Port: port}
-		copy(rsockaddr.Addr[:], raddr.IP.To16())
-		ra = rsockaddr
-		var zone uint32
-		if laddr.Zone != "" {
-			if intf, err := net.InterfaceByName(laddr.Zone); err != nil {
-				return nil, err
-			} else {
-				zone = uint32(intf.Index)
-			}
-		}
-		lsockaddr := &syscall.SockaddrInet6{ZoneId: zone}
-		copy(lsockaddr.Addr[:], laddr.IP.To16())
-		la = lsockaddr
 	}
 
-	sockType := syscall.SOCK_STREAM | syscall.SOCK_CLOEXEC | syscall.SOCK_NONBLOCK
-	proto := 0
-	fd, err := syscall.Socket(family, sockType, proto)
-	if err != nil {
-		return nil, err
-	}
-	fi := os.NewFile(uintptr(fd), "")
-	defer fi.Close()
-	// A new socket was created so we must close it before this
-	// function returns either on failure or success. On success,
-	// net.FileConn() in newTCPConn() increases the refcount of
-	// the socket so this fi.Close() doesn't destroy the socket.
-	// The caller must call Close() with the file later.
-	// Note that the above os.NewFile() doesn't play with the
-	// refcount.
-
-	if err = syscall.SetsockoptInt(fd, syscall.SOL_SOCKET, syscall.SO_BROADCAST, 1); err != nil {
-		return nil, os.NewSyscallError("setsockopt", err)
-	}
-
-	if err = syscall.SetsockoptInt(fd, syscall.IPPROTO_TCP, syscall.TCP_NODELAY, 1); err != nil {
-		return nil, os.NewSyscallError("setsockopt", err)
-	}
-
-	if d.AuthPassword != "" {
-		if err = setsockoptTcpMD5Sig(fd, addr, d.AuthPassword); err != nil {
-			return nil, err
+	var sockerr error
+	if password != "" {
+		addr, _, _ := net.SplitHostPort(address)
+		t, err := buildTcpMD5Sig(addr, password)
+		if err != nil {
+			return err
 		}
-	}
-
-	if d.TTL != 0 {
-		if err = setsockoptIpTtl2(fd, family, int(d.TTL)); err != nil {
-			return nil, err
+		b := *(*[unsafe.Sizeof(t)]byte)(unsafe.Pointer(&t))
+		if err := c.Control(func(fd uintptr) {
+			sockerr = os.NewSyscallError("setsockopt", syscall.SetsockoptString(int(fd), syscall.IPPROTO_TCP, tcpMD5SIG, string(b[:])))
+		}); err != nil {
+			return err
 		}
-	}
-
-	if d.TTLMin != 0 {
-		if err = setsockoptIpMinTtl(fd, family, int(d.TTL)); err != nil {
-			return nil, err
+		if sockerr != nil {
+			return sockerr
 		}
 	}
 
-	if err = syscall.Bind(fd, la); err != nil {
-		return nil, os.NewSyscallError("bind", err)
-	}
-
-	newTCPConn := func(fi *os.File) (*net.TCPConn, error) {
-		if conn, err := net.FileConn(fi); err != nil {
-			return nil, err
-		} else {
-			return conn.(*net.TCPConn), err
+	if ttl != 0 {
+		if err := c.Control(func(fd uintptr) {
+			level := syscall.IPPROTO_IP
+			name := syscall.IP_TTL
+			if family == syscall.AF_INET6 {
+				level = syscall.IPPROTO_IPV6
+				name = syscall.IPV6_UNICAST_HOPS
+			}
+			sockerr = os.NewSyscallError("setsockopt", syscall.SetsockoptInt(int(fd), level, name, int(ttl)))
+		}); err != nil {
+			return err
+		}
+		if sockerr != nil {
+			return sockerr
 		}
 	}
 
-	err = syscall.Connect(fd, ra)
-	switch err {
-	case syscall.EINPROGRESS, syscall.EALREADY, syscall.EINTR:
-		// do timeout handling
-	case nil, syscall.EISCONN:
-		return newTCPConn(fi)
-	default:
-		return nil, os.NewSyscallError("connect", err)
-	}
-
-	epfd, e := syscall.EpollCreate1(syscall.EPOLL_CLOEXEC)
-	if e != nil {
-		return nil, e
-	}
-	defer syscall.Close(epfd)
-
-	var event syscall.EpollEvent
-	events := make([]syscall.EpollEvent, 1)
-
-	event.Events = syscall.EPOLLIN | syscall.EPOLLOUT | syscall.EPOLLPRI
-	event.Fd = int32(fd)
-	if e = syscall.EpollCtl(epfd, syscall.EPOLL_CTL_ADD, fd, &event); e != nil {
-		return nil, e
-	}
-
-	for {
-		nevents, e := syscall.EpollWait(epfd, events, int(d.Timeout/1000000) /*msec*/)
-		if e != nil {
-			return nil, e
-		}
-		if nevents == 0 {
-			return nil, fmt.Errorf("timeout")
-		} else if nevents == 1 && events[0].Fd == int32(fd) {
-			nerr, err := syscall.GetsockoptInt(fd, syscall.SOL_SOCKET, syscall.SO_ERROR)
-			if err != nil {
-				return nil, os.NewSyscallError("getsockopt", err)
-			}
-			switch err := syscall.Errno(nerr); err {
-			case syscall.EINPROGRESS, syscall.EALREADY, syscall.EINTR:
-			case syscall.Errno(0), syscall.EISCONN:
-				return newTCPConn(fi)
-			default:
-				return nil, os.NewSyscallError("getsockopt", err)
+	if minTtl != 0 {
+		if err := c.Control(func(fd uintptr) {
+			level := syscall.IPPROTO_IP
+			name := syscall.IP_MINTTL
+			if family == syscall.AF_INET6 {
+				level = syscall.IPPROTO_IPV6
+				name = ipv6MinHopCount
 			}
-		} else {
-			return nil, fmt.Errorf("unexpected epoll behavior")
+			sockerr = os.NewSyscallError("setsockopt", syscall.SetsockoptInt(int(fd), level, name, int(minTtl)))
+		}); err != nil {
+			return err
+		}
+		if sockerr != nil {
+			return sockerr
 		}
 	}
+	return nil
 }
diff --git a/pkg/server/sockopt_linux_test.go b/pkg/server/sockopt_linux_test.go
index ca72a98f..d9ca3d5e 100644
--- a/pkg/server/sockopt_linux_test.go
+++ b/pkg/server/sockopt_linux_test.go
@@ -18,12 +18,8 @@ package server
 
 import (
 	"bytes"
-	"fmt"
-	"net"
-	"os"
 	"syscall"
 	"testing"
-	"time"
 	"unsafe"
 )
 
@@ -68,39 +64,3 @@ func Test_buildTcpMD5Sigv6(t *testing.T) {
 		t.Error("Something wrong v6")
 	}
 }
-
-func Test_DialTCP_FDleak(t *testing.T) {
-	openFds := func() int {
-		pid := os.Getpid()
-		f, err := os.OpenFile(fmt.Sprintf("/proc/%d/fdinfo", pid), os.O_RDONLY, 0)
-		if err != nil {
-			t.Fatal(err)
-		}
-		defer f.Close()
-		names, err := f.Readdirnames(0)
-		if err != nil {
-			t.Fatal(err)
-		}
-		return len(names)
-	}
-
-	before := openFds()
-
-	for i := 0; i < 10; i++ {
-		laddr, _ := net.ResolveTCPAddr("tcp", net.JoinHostPort("127.0.0.1", "0"))
-		d := tcpDialer{
-			Dialer: net.Dialer{
-				LocalAddr: laddr,
-				Timeout:   1 * time.Second,
-			},
-		}
-		if _, err := d.DialTCP("127.0.0.1", 1); err == nil {
-			t.Fatalf("should not succeed")
-		}
-
-	}
-
-	if after := openFds(); before != after {
-		t.Fatalf("could be fd leak, %d %d", before, after)
-	}
-}
diff --git a/pkg/server/sockopt_openbsd.go b/pkg/server/sockopt_openbsd.go
index dcee33a2..e65aabe0 100644
--- a/pkg/server/sockopt_openbsd.go
+++ b/pkg/server/sockopt_openbsd.go
@@ -60,7 +60,7 @@ type sadbMsg struct {
 
 func (s *sadbMsg) DecodeFromBytes(data []byte) error {
 	if len(data) < SADB_MSG_SIZE {
-		fmt.Errorf("too short for sadbMsg %d", len(data))
+		return fmt.Errorf("too short for sadbMsg %d", len(data))
 	}
 	s.sadbMsgVersion = data[0]
 	s.sadbMsgType = data[1]
@@ -403,52 +403,24 @@ func setTCPMinTTLSockopt(conn *net.TCPConn, ttl int) error {
 	return setsockOptInt(sc, level, name, ttl)
 }
 
-type tcpDialer struct {
-	net.Dialer
-
-	// MD5 authentication password.
-	AuthPassword string
-
-	// The TTL value to set outgoing connection.
-	TTL uint8
-
-	// The minimum TTL value for incoming packets.
-	TTLMin uint8
-}
-
-func (d *tcpDialer) DialTCP(addr string, port int) (*net.TCPConn, error) {
-	if d.AuthPassword != "" {
+func dialerControl(network, address string, c syscall.RawConn, ttl, minTtl uint8, password string) error {
+	if password != "" {
 		log.WithFields(log.Fields{
 			"Topic": "Peer",
-			"Key":   addr,
+			"Key":   address,
 		}).Warn("setting md5 for active connection is not supported")
 	}
-	if d.TTL != 0 {
+	if ttl != 0 {
 		log.WithFields(log.Fields{
 			"Topic": "Peer",
-			"Key":   addr,
+			"Key":   address,
 		}).Warn("setting ttl for active connection is not supported")
 	}
-	if d.TTLMin != 0 {
+	if minTtl != 0 {
 		log.WithFields(log.Fields{
 			"Topic": "Peer",
-			"Key":   addr,
+			"Key":   address,
 		}).Warn("setting min ttl for active connection is not supported")
 	}
-
-	raddr, err := net.ResolveTCPAddr("tcp", net.JoinHostPort(addr, fmt.Sprintf("%d", port)))
-	if err != nil {
-		return nil, fmt.Errorf("invalid remote address: %s", err)
-	}
-	laddr, err := net.ResolveTCPAddr("tcp", d.LocalAddr.String())
-	if err != nil {
-		return nil, fmt.Errorf("invalid local address: %s", err)
-	}
-
-	dialer := net.Dialer{LocalAddr: laddr, Timeout: d.Timeout}
-	conn, err := dialer.Dial("tcp", raddr.String())
-	if err != nil {
-		return nil, err
-	}
-	return conn.(*net.TCPConn), nil
+	return nil
 }
diff --git a/pkg/server/sockopt_stub.go b/pkg/server/sockopt_stub.go
index 9e888dc5..fd600e3a 100644
--- a/pkg/server/sockopt_stub.go
+++ b/pkg/server/sockopt_stub.go
@@ -25,10 +25,6 @@ func setTcpMD5SigSockopt(l *net.TCPListener, address string, key string) error {
 	return fmt.Errorf("setting md5 is not supported")
 }
 
-func setListenTcpTTLSockopt(l *net.TCPListener, ttl int) error {
-	return fmt.Errorf("setting ttl is not supported")
-}
-
 func setTcpTTLSockopt(conn *net.TCPConn, ttl int) error {
 	return fmt.Errorf("setting ttl is not supported")
 }