vendor: Update GoBGP to fix fd leak (#124)

glide.lock

@@ -1,5 +1,5 @@
-hash: 0b35e7130df3f81011b44875ba84fc444742e3ea71b5d5b8d6787938d2f1402b
+hash: dca664c88bf9e7943a14d9afc93b05acd235f7260e717e559dfbafc69c79177c
-updated: 2017-08-04T01:06:37.90272016-05:00
+updated: 2017-08-13T11:15:41.1230094-05:00
 imports:
 - name: github.com/armon/go-radix
   version: 1fca145dffbcaa8fe914309b1ec0cfc67500fe61
@@ -167,7 +167,7 @@ imports:
 - name: github.com/opencontainers/go-digest
   version: 279bed98673dd5bef374d3b6e4b09e2af76183bf
 - name: github.com/osrg/gobgp
-  version: 97063ee76decb4105ad3991dcf34432f5e009f9d
+  version: f615fa49a768bb358e6318e64dc42e9a6af10681
   subpackages:
   - api
   - config

glide.yaml

@@ -19,7 +19,7 @@ import:
   subpackages:
   - ipvs
 - package: github.com/osrg/gobgp
-  version: master
+  version: f615fa49a768bb358e6318e64dc42e9a6af10681
   subpackages:
   - api
   - config

vendor/github.com/osrg/gobgp/README.md

@@ -18,7 +18,9 @@ Try [a binary release](https://github.com/osrg/gobgp/releases/latest).
 You need a working [Go environment](https://golang.org/doc/install).
 
 ```bash
-$ go get github.com/osrg/gobgp/...
+$ go get -u github.com/golang/dep/cmd/dep
+$ go get github.com/osrg/gobgp
+$ cd $GOPATH/src/github.com/osrg/gobgp && dep ensure
 ```
 
 ## Documentation

vendor/github.com/osrg/gobgp/config/default.go

@@ -106,12 +106,18 @@ func SetDefaultNeighborConfigValues(n *Neighbor, asn uint32) error {
 
 func setDefaultNeighborConfigValuesWithViper(v *viper.Viper, n *Neighbor, asn uint32) error {
 	if v == nil {
+		// Determines this function is called against the same Neighbor struct,
+		// and if already called, returns immediately.
+		if n.State.LocalAs != 0 {
+			return nil
+		}
 		v = viper.New()
 	}
 
 	if n.Config.LocalAs == 0 {
 		n.Config.LocalAs = asn
 	}
+	n.State.LocalAs = n.Config.LocalAs
 
 	if n.Config.PeerAs != n.Config.LocalAs {
 		n.Config.PeerType = PEER_TYPE_EXTERNAL
@@ -199,17 +205,19 @@ func setDefaultNeighborConfigValuesWithViper(v *viper.Viper, n *Neighbor, asn ui
 		if err != nil {
 			return err
 		}
-		for i, af := range n.AfiSafis {
+		for i := range n.AfiSafis {
 			vv := viper.New()
 			if len(afs) > i {
 				vv.Set("afi-safi", afs[i])
 			}
-			af.State.AfiSafiName = af.Config.AfiSafiName
+			if _, err := bgp.GetRouteFamily(string(n.AfiSafis[i].Config.AfiSafiName)); err != nil {
-			if !vv.IsSet("afi-safi.config") {
+				return err
-				af.Config.Enabled = true
 			}
-			af.MpGracefulRestart.State.Enabled = af.MpGracefulRestart.Config.Enabled
+			n.AfiSafis[i].State.AfiSafiName = n.AfiSafis[i].Config.AfiSafiName
-			n.AfiSafis[i] = af
+			if !vv.IsSet("afi-safi.config.enabled") {
+				n.AfiSafis[i].Config.Enabled = true
+			}
+			n.AfiSafis[i].MpGracefulRestart.State.Enabled = n.AfiSafis[i].MpGracefulRestart.Config.Enabled
 		}
 	}
 

vendor/github.com/osrg/gobgp/gobgpd/main.go

@@ -277,9 +277,9 @@ func main() {
 				}
 				updatePolicy = updatePolicy || u
 			}
-			for _, dn := range newConfig.DynamicNeighbors {
+			for i, dn := range newConfig.DynamicNeighbors {
 				log.Infof("Dynamic Neighbor %s is added to PeerGroup %s", dn.Config.Prefix, dn.Config.PeerGroup)
-				if err := bgpServer.AddDynamicNeighbor(&dn); err != nil {
+				if err := bgpServer.AddDynamicNeighbor(&newConfig.DynamicNeighbors[i]); err != nil {
 					log.Warn(err)
 				}
 			}

vendor/github.com/osrg/gobgp/server/sockopt_linux.go

@@ -177,6 +177,15 @@ func (d *TCPDialer) DialTCP(addr string, port int) (*net.TCPConn, error) {
 	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)
@@ -208,10 +217,7 @@ func (d *TCPDialer) DialTCP(addr string, port int) (*net.TCPConn, error) {
 		return nil, os.NewSyscallError("bind", err)
 	}
 
-	newTCPConn := func(fd int) (*net.TCPConn, error) {
+	newTCPConn := func(fi *os.File) (*net.TCPConn, error) {
-		fi := os.NewFile(uintptr(fd), "")
-		defer fi.Close()
-
 		if conn, err := net.FileConn(fi); err != nil {
 			return nil, err
 		} else {
@@ -224,7 +230,7 @@ func (d *TCPDialer) DialTCP(addr string, port int) (*net.TCPConn, error) {
 	case syscall.EINPROGRESS, syscall.EALREADY, syscall.EINTR:
 		// do timeout handling
 	case nil, syscall.EISCONN:
-		return newTCPConn(fd)
+		return newTCPConn(fi)
 	default:
 		return nil, os.NewSyscallError("connect", err)
 	}
@@ -259,7 +265,7 @@ func (d *TCPDialer) DialTCP(addr string, port int) (*net.TCPConn, error) {
 			switch err := syscall.Errno(nerr); err {
 			case syscall.EINPROGRESS, syscall.EALREADY, syscall.EINTR:
 			case syscall.Errno(0), syscall.EISCONN:
-				return newTCPConn(fd)
+				return newTCPConn(fi)
 			default:
 				return nil, os.NewSyscallError("getsockopt", err)
 			}

vendor/github.com/osrg/gobgp/server/sockopt_linux_test.go

@@ -18,8 +18,12 @@ package server
 
 import (
 	"bytes"
+	"fmt"
+	"net"
+	"os"
 	"syscall"
 	"testing"
+	"time"
 	"unsafe"
 )
 
@@ -64,3 +68,39 @@ 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)
+	}
+}

vendor/github.com/osrg/gobgp/table/message.go

@@ -278,8 +278,16 @@ func createUpdateMsgFromPath(path *Path, msg *bgp.BGPMessage) *bgp.BGPMessage {
 				u := msg.Body.(*bgp.BGPUpdate)
 				u.NLRI = append(u.NLRI, nlri)
 			} else {
-				pathAttrs := path.GetPathAttrs()
+				attrs := make([]bgp.PathAttributeInterface, 0, 8)
-				return bgp.NewBGPUpdateMessage(nil, pathAttrs, []*bgp.IPAddrPrefix{nlri})
+				for _, p := range path.GetPathAttrs() {
+					switch p.GetType() {
+					case bgp.BGP_ATTR_TYPE_MP_REACH_NLRI:
+					case bgp.BGP_ATTR_TYPE_MP_UNREACH_NLRI:
+					default:
+						attrs = append(attrs, p)
+					}
+				}
+				return bgp.NewBGPUpdateMessage(nil, attrs, []*bgp.IPAddrPrefix{nlri})
 			}
 		}
 	} else {
@@ -315,11 +323,12 @@ func createUpdateMsgFromPath(path *Path, msg *bgp.BGPMessage) *bgp.BGPMessage {
 				}
 			} else {
 				attrs := make([]bgp.PathAttributeInterface, 0, 8)
-
 				for _, p := range path.GetPathAttrs() {
-					if p.GetType() == bgp.BGP_ATTR_TYPE_MP_REACH_NLRI {
+					switch p.GetType() {
+					case bgp.BGP_ATTR_TYPE_MP_REACH_NLRI:
 						attrs = append(attrs, bgp.NewPathAttributeMpReachNLRI(path.GetNexthop().String(), []bgp.AddrPrefixInterface{path.GetNlri()}))
-					} else {
+					case bgp.BGP_ATTR_TYPE_MP_UNREACH_NLRI:
+					default:
 						attrs = append(attrs, p)
 					}
 				}

vendor/github.com/osrg/gobgp/table/message_test.go

@@ -432,3 +432,56 @@ func TestBMP(t *testing.T) {
 	pList := ProcessMessage(msg, peerR1(), time.Now())
 	CreateUpdateMsgFromPaths(pList)
 }
+
+func TestMixedMPReachMPUnreach(t *testing.T) {
+	aspath1 := []bgp.AsPathParamInterface{
+		bgp.NewAs4PathParam(2, []uint32{100}),
+	}
+	nlri1 := []bgp.AddrPrefixInterface{bgp.NewIPv6AddrPrefix(32, "2222::")}
+	nlri2 := []bgp.AddrPrefixInterface{bgp.NewIPv6AddrPrefix(32, "1111::")}
+
+	p := []bgp.PathAttributeInterface{
+		bgp.NewPathAttributeOrigin(0),
+		bgp.NewPathAttributeAsPath(aspath1),
+		bgp.NewPathAttributeMpReachNLRI("1::1", nlri1),
+		bgp.NewPathAttributeMpUnreachNLRI(nlri2),
+	}
+	msg := bgp.NewBGPUpdateMessage(nil, p, nil)
+	pList := ProcessMessage(msg, peerR1(), time.Now())
+	assert.Equal(t, len(pList), 2)
+	assert.Equal(t, pList[0].IsWithdraw, false)
+	assert.Equal(t, pList[1].IsWithdraw, true)
+	msgs := CreateUpdateMsgFromPaths(pList)
+	assert.Equal(t, len(msgs), 2)
+	attrs := msgs[0].Body.(*bgp.BGPUpdate).PathAttributes
+	assert.Equal(t, len(attrs), 3)
+	attrs = msgs[1].Body.(*bgp.BGPUpdate).PathAttributes
+	assert.Equal(t, len(attrs), 1)
+}
+
+func TestMixedNLRIAndMPUnreach(t *testing.T) {
+	aspath1 := []bgp.AsPathParamInterface{
+		bgp.NewAs4PathParam(2, []uint32{100}),
+	}
+	nlri1 := []*bgp.IPAddrPrefix{bgp.NewIPAddrPrefix(24, "10.0.0.0")}
+	nlri2 := []bgp.AddrPrefixInterface{bgp.NewIPv6AddrPrefix(32, "1111::")}
+
+	p := []bgp.PathAttributeInterface{
+		bgp.NewPathAttributeOrigin(0),
+		bgp.NewPathAttributeAsPath(aspath1),
+		bgp.NewPathAttributeNextHop("1.1.1.1"),
+		bgp.NewPathAttributeMpUnreachNLRI(nlri2),
+	}
+	msg := bgp.NewBGPUpdateMessage(nil, p, nlri1)
+	pList := ProcessMessage(msg, peerR1(), time.Now())
+
+	assert.Equal(t, len(pList), 2)
+	assert.Equal(t, pList[0].IsWithdraw, false)
+	assert.Equal(t, pList[1].IsWithdraw, true)
+	msgs := CreateUpdateMsgFromPaths(pList)
+	assert.Equal(t, len(msgs), 2)
+	attrs := msgs[0].Body.(*bgp.BGPUpdate).PathAttributes
+	assert.Equal(t, len(attrs), 1)
+	attrs = msgs[1].Body.(*bgp.BGPUpdate).PathAttributes
+	assert.Equal(t, len(attrs), 3)
+}