server: handle open message only in fsm

we don't need to spread this handling to peer.go and server.go

Signed-off-by: ISHIDA Wataru <ishida.wataru@lab.ntt.co.jp>

4 files changed, 56 insertions, 71 deletions

diff --git a/server/fsm.go b/server/fsm.go
index 16267c17..1a5943d2 100644
--- a/server/fsm.go
+++ b/server/fsm.go
@@ -85,6 +85,7 @@ type FSM struct {
 	getActiveCh        chan struct{}
 	h                  *FSMHandler
 	rfMap              map[bgp.RouteFamily]bool
+	capMap             map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface
 	confedCheck        bool
 	peerInfo           *table.PeerInfo
 	policy             *table.RoutingPolicy
@@ -154,6 +155,7 @@ func NewFSM(gConf *config.Global, pConf *config.Neighbor, policy *table.RoutingP
 		adminStateCh:     make(chan AdminState, 1),
 		getActiveCh:      make(chan struct{}),
 		rfMap:            make(map[bgp.RouteFamily]bool),
+		capMap:           make(map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface),
 		confedCheck:      !config.IsConfederationMember(gConf, pConf) && config.IsEBGPPeer(gConf, pConf),
 		peerInfo:         table.NewPeerInfo(gConf, pConf),
 		policy:           policy,
@@ -551,6 +553,35 @@ func (h *FSMHandler) recvMessage() error {
 	return nil
 }
 
+func open2Cap(open *bgp.BGPOpen, n *config.Neighbor) (map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface, map[bgp.RouteFamily]bool) {
+	capMap := make(map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface)
+	rfMap := config.CreateRfMap(n)
+	r := make(map[bgp.RouteFamily]bool)
+	for _, p := range open.OptParams {
+		if paramCap, y := p.(*bgp.OptionParameterCapability); y {
+			for _, c := range paramCap.Capability {
+				m, ok := capMap[c.Code()]
+				if !ok {
+					m = make([]bgp.ParameterCapabilityInterface, 0, 1)
+				}
+				capMap[c.Code()] = append(m, c)
+
+				if c.Code() == bgp.BGP_CAP_MULTIPROTOCOL {
+					m := c.(*bgp.CapMultiProtocol)
+					r[m.CapValue] = true
+				}
+			}
+		}
+	}
+
+	for rf, _ := range rfMap {
+		if _, y := r[rf]; !y {
+			delete(rfMap, rf)
+		}
+	}
+	return capMap, rfMap
+}
+
 func (h *FSMHandler) opensent() bgp.FSMState {
 	fsm := h.fsm
 	m := buildopen(fsm.gConf, fsm.pConf)
@@ -596,15 +627,21 @@ func (h *FSMHandler) opensent() bgp.FSMState {
 						return bgp.BGP_FSM_IDLE
 					}
 					fsm.peerInfo.ID = body.ID
-					_, fsm.rfMap = open2Cap(body, fsm.pConf)
-
-					e := &FsmMsg{
-						MsgType: FSM_MSG_BGP_MESSAGE,
-						MsgSrc:  fsm.pConf.Config.NeighborAddress,
-						MsgDst:  fsm.pConf.Transport.Config.LocalAddress,
-						MsgData: m,
+					fsm.capMap, fsm.rfMap = open2Cap(body, fsm.pConf)
+
+					// calculate HoldTime
+					// RFC 4271 P.13
+					// a BGP speaker MUST calculate the value of the Hold Timer
+					// by using the smaller of its configured Hold Time and the Hold Time
+					// received in the OPEN message.
+					holdTime := float64(body.HoldTime)
+					myHoldTime := fsm.pConf.Timers.Config.HoldTime
+					if holdTime > myHoldTime {
+						fsm.negotiatedHoldTime = myHoldTime
+					} else {
+						fsm.negotiatedHoldTime = holdTime
 					}
-					h.incoming <- e
+
 					msg := bgp.NewBGPKeepAliveMessage()
 					b, _ := msg.Serialize()
 					fsm.conn.Write(b)
diff --git a/server/fsm_test.go b/server/fsm_test.go
index b5618edd..4eb907f3 100644
--- a/server/fsm_test.go
+++ b/server/fsm_test.go
@@ -289,9 +289,8 @@ func makePeerAndHandler() (*Peer, *FSMHandler) {
 	pConf := config.Neighbor{}
 
 	p := &Peer{
-		gConf:  gConf,
-		conf:   pConf,
-		capMap: make(map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface),
+		gConf: gConf,
+		conf:  pConf,
 	}
 
 	p.fsm = NewFSM(&gConf, &pConf, table.NewRoutingPolicy())
diff --git a/server/peer.go b/server/peer.go
index f083baf4..9b7e4a81 100644
--- a/server/peer.go
+++ b/server/peer.go
@@ -36,8 +36,6 @@ type Peer struct {
 	gConf     config.Global
 	conf      config.Neighbor
 	fsm       *FSM
-	rfMap     map[bgp.RouteFamily]bool
-	capMap    map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface
 	adjRibIn  *table.AdjRib
 	adjRibOut *table.AdjRib
 	outgoing  chan *bgp.BGPMessage
@@ -50,8 +48,6 @@ func NewPeer(g config.Global, conf config.Neighbor, loc *table.TableManager, pol
 	peer := &Peer{
 		gConf:    g,
 		conf:     conf,
-		rfMap:    make(map[bgp.RouteFamily]bool),
-		capMap:   make(map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface),
 		outgoing: make(chan *bgp.BGPMessage, 128),
 		localRib: loc,
 		policy:   policy,
@@ -124,35 +120,6 @@ func (peer *Peer) getBestFromLocal(rfList []bgp.RouteFamily) ([]*table.Path, []*
 	return pathList, filtered
 }
 
-func open2Cap(open *bgp.BGPOpen, n *config.Neighbor) (map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface, map[bgp.RouteFamily]bool) {
-	capMap := make(map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface)
-	rfMap := config.CreateRfMap(n)
-	r := make(map[bgp.RouteFamily]bool)
-	for _, p := range open.OptParams {
-		if paramCap, y := p.(*bgp.OptionParameterCapability); y {
-			for _, c := range paramCap.Capability {
-				m, ok := capMap[c.Code()]
-				if !ok {
-					m = make([]bgp.ParameterCapabilityInterface, 0, 1)
-				}
-				capMap[c.Code()] = append(m, c)
-
-				if c.Code() == bgp.BGP_CAP_MULTIPROTOCOL {
-					m := c.(*bgp.CapMultiProtocol)
-					r[m.CapValue] = true
-				}
-			}
-		}
-	}
-
-	for rf, _ := range rfMap {
-		if _, y := r[rf]; !y {
-			delete(rfMap, rf)
-		}
-	}
-	return capMap, rfMap
-}
-
 func (peer *Peer) handleBGPmessage(e *FsmMsg) ([]*table.Path, []*bgp.BGPMessage) {
 	m := e.MsgData.(*bgp.BGPMessage)
 	log.WithFields(log.Fields{
@@ -162,28 +129,10 @@ func (peer *Peer) handleBGPmessage(e *FsmMsg) ([]*table.Path, []*bgp.BGPMessage)
 	}).Debug("received")
 
 	switch m.Header.Type {
-	case bgp.BGP_MSG_OPEN:
-		peer.recvOpen = m
-		body := m.Body.(*bgp.BGPOpen)
-		peer.capMap, peer.rfMap = open2Cap(body, &peer.conf)
-
-		// calculate HoldTime
-		// RFC 4271 P.13
-		// a BGP speaker MUST calculate the value of the Hold Timer
-		// by using the smaller of its configured Hold Time and the Hold Time
-		// received in the OPEN message.
-		holdTime := float64(body.HoldTime)
-		myHoldTime := peer.conf.Timers.Config.HoldTime
-		if holdTime > myHoldTime {
-			peer.fsm.negotiatedHoldTime = myHoldTime
-		} else {
-			peer.fsm.negotiatedHoldTime = holdTime
-		}
-
 	case bgp.BGP_MSG_ROUTE_REFRESH:
 		rr := m.Body.(*bgp.BGPRouteRefresh)
 		rf := bgp.AfiSafiToRouteFamily(rr.AFI, rr.SAFI)
-		if _, ok := peer.rfMap[rf]; !ok {
+		if _, ok := peer.fsm.rfMap[rf]; !ok {
 			log.WithFields(log.Fields{
 				"Topic": "Peer",
 				"Key":   peer.conf.Config.NeighborAddress,
@@ -191,7 +140,7 @@ func (peer *Peer) handleBGPmessage(e *FsmMsg) ([]*table.Path, []*bgp.BGPMessage)
 			}).Warn("Route family isn't supported")
 			break
 		}
-		if _, ok := peer.capMap[bgp.BGP_CAP_ROUTE_REFRESH]; ok {
+		if _, ok := peer.fsm.capMap[bgp.BGP_CAP_ROUTE_REFRESH]; ok {
 			rfList := []bgp.RouteFamily{rf}
 			peer.adjRibOut.Drop(rfList)
 			accepted, filtered := peer.getBestFromLocal(rfList)
@@ -258,8 +207,8 @@ func (peer *Peer) ToApiStruct() *api.Peer {
 	f := peer.fsm
 	c := f.pConf
 
-	remoteCap := make([][]byte, 0, len(peer.capMap))
-	for _, c := range peer.capMap {
+	remoteCap := make([][]byte, 0, len(peer.fsm.capMap))
+	for _, c := range peer.fsm.capMap {
 		for _, m := range c {
 			buf, _ := m.Serialize()
 			remoteCap = append(remoteCap, buf)
diff --git a/server/server.go b/server/server.go
index 6d0fc060..afb4550a 100644
--- a/server/server.go
+++ b/server/server.go
@@ -444,7 +444,7 @@ func (server *BgpServer) Serve() {
 }
 
 func newSenderMsg(peer *Peer, messages []*bgp.BGPMessage) *SenderMsg {
-	_, y := peer.capMap[bgp.BGP_CAP_FOUR_OCTET_AS_NUMBER]
+	_, y := peer.fsm.capMap[bgp.BGP_CAP_FOUR_OCTET_AS_NUMBER]
 	return &SenderMsg{
 		messages:    messages,
 		sendCh:      peer.outgoing,
@@ -466,7 +466,7 @@ func filterpath(peer *Peer, path *table.Path) *table.Path {
 	if path == nil {
 		return nil
 	}
-	if _, ok := peer.rfMap[path.GetRouteFamily()]; !ok {
+	if _, ok := peer.fsm.rfMap[path.GetRouteFamily()]; !ok {
 		return nil
 	}
 
@@ -550,7 +550,7 @@ func (server *BgpServer) dropPeerAllRoutes(peer *Peer) []*SenderMsg {
 				if !targetPeer.isRouteServerClient() || targetPeer == peer || targetPeer.fsm.state != bgp.BGP_FSM_ESTABLISHED {
 					continue
 				}
-				if _, ok := targetPeer.rfMap[rf]; !ok {
+				if _, ok := targetPeer.fsm.rfMap[rf]; !ok {
 					continue
 				}
 
@@ -583,7 +583,7 @@ func (server *BgpServer) dropPeerAllRoutes(peer *Peer) []*SenderMsg {
 				if targetPeer.isRouteServerClient() || targetPeer.fsm.state != bgp.BGP_FSM_ESTABLISHED {
 					continue
 				}
-				if _, ok := targetPeer.rfMap[rf]; !ok {
+				if _, ok := targetPeer.fsm.rfMap[rf]; !ok {
 					continue
 				}
 				targetPeer.adjRibOut.Update(pathList)
@@ -847,7 +847,7 @@ func (server *BgpServer) handleFSMMessage(peer *Peer, e *FsmMsg, incoming chan *
 					}
 				}
 				if len(listener) > 0 {
-					_, y := peer.capMap[bgp.BGP_CAP_FOUR_OCTET_AS_NUMBER]
+					_, y := peer.fsm.capMap[bgp.BGP_CAP_FOUR_OCTET_AS_NUMBER]
 					l, _ := peer.fsm.LocalHostPort()
 					ev := &watcherEventUpdateMsg{
 						message:      m,