Do not suppress initial non-established peer state in MonitorPeer

The issue with the initial update was that sometimes a connection had
not yet been established. Trying to get remote and local connection
data caused a segmentation violation because `fsm.conn` was nil.

Also adds a flag to MonitorPeerRequest in the GRPC API to enable the
new behavior to avoid backward incompatibility.

fixes #2047

2 files changed, 26 insertions, 10 deletions

diff --git a/pkg/server/bmp.go b/pkg/server/bmp.go
index 839dd786..67eebd0c 100644
--- a/pkg/server/bmp.go
+++ b/pkg/server/bmp.go
@@ -115,7 +115,7 @@ func (b *bmpClient) loop() {
 		}
 
 		if func() bool {
-			ops := []watchOption{watchPeerState(true)}
+			ops := []watchOption{watchPeerState(true, false)}
 			if b.c.RouteMonitoringPolicy == config.BMP_ROUTE_MONITORING_POLICY_TYPE_BOTH {
 				log.WithFields(
 					log.Fields{"Topic": "bmp"},
diff --git a/pkg/server/server.go b/pkg/server/server.go
index 84a1e2da..3cdcd6e4 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -875,8 +875,12 @@ func (s *BgpServer) notifyPostPolicyUpdateWatcher(peer *peer, pathList []*table.
 }
 
 func newWatchEventPeerState(peer *peer, m *fsmMsg) *watchEventPeerState {
-	_, rport := peer.fsm.RemoteHostPort()
-	laddr, lport := peer.fsm.LocalHostPort()
+	var laddr string
+	var rport, lport uint16
+	if peer.fsm.conn != nil {
+		_, rport = peer.fsm.RemoteHostPort()
+		laddr, lport = peer.fsm.LocalHostPort()
+	}
 	sentOpen := buildopen(peer.fsm.gConf, peer.fsm.pConf)
 	peer.fsm.lock.RLock()
 	recvOpen := peer.fsm.recvOpen
@@ -3841,7 +3845,13 @@ func (s *BgpServer) MonitorPeer(ctx context.Context, r *api.MonitorPeerRequest,
 	}
 
 	go func() {
-		w := s.watch(watchPeerState(r.Current))
+		// So that both flags are not required, assume that if the
+		// initial_state flag is true, then the caller desires that the initial
+		// state be returned whether or not it is established and regardless of
+		// the value of `current`.
+		current := r.Current || r.InitialState
+		nonEstablished := r.InitialState
+		w := s.watch(watchPeerState(current, nonEstablished))
 		defer func() {
 			w.Stop()
 		}()
@@ -3982,6 +3992,7 @@ type watchOptions struct {
 	initUpdate     bool
 	initPostUpdate bool
 	initPeerState  bool
+	nonEstablished bool
 	tableName      string
 	recvMessage    bool
 	peerAddress    string
@@ -4018,11 +4029,14 @@ func watchPostUpdate(current bool, peerAddress string) watchOption {
 	}
 }
 
-func watchPeerState(current bool) watchOption {
+func watchPeerState(current, includeNonEstablished bool) watchOption {
 	return func(o *watchOptions) {
 		o.peerState = true
 		if current {
 			o.initPeerState = true
+			if includeNonEstablished {
+				o.nonEstablished = true
+			}
 		}
 	}
 }
@@ -4177,11 +4191,13 @@ func (s *BgpServer) watch(opts ...watchOption) (w *watcher) {
 		}
 		if w.opts.initPeerState {
 			for _, peer := range s.neighborMap {
-				peer.fsm.lock.RLock()
-				notEstablished := peer.fsm.state != bgp.BGP_FSM_ESTABLISHED
-				peer.fsm.lock.RUnlock()
-				if notEstablished {
-					continue
+				if !w.opts.nonEstablished {
+					peer.fsm.lock.RLock()
+					notEstablished := peer.fsm.state != bgp.BGP_FSM_ESTABLISHED
+					peer.fsm.lock.RUnlock()
+					if notEstablished {
+						continue
+					}
 				}
 				w.notify(newWatchEventPeerState(peer, nil))
 			}