diff options
Diffstat (limited to 'pkg/server')
| -rw-r--r-- | pkg/server/fsm.go | 33 | ||||
| -rw-r--r-- | pkg/server/fsm_test.go | 46 | ||||
| -rw-r--r-- | pkg/server/server.go | 6 |
3 files changed, 70 insertions, 15 deletions
diff --git a/pkg/server/fsm.go b/pkg/server/fsm.go index 39618aa5..b037c0f5 100644 --- a/pkg/server/fsm.go +++ b/pkg/server/fsm.go @@ -1788,17 +1788,20 @@ func (h *fsmHandler) established(ctx context.Context) (bgp.FSMState, *fsmStateRe // it now. h.outgoing.In() <- err fsm.lock.RLock() - if s := fsm.pConf.GracefulRestart.State; s.Enabled && - (s.NotificationEnabled && err.Type == fsmNotificationRecv || + if s := fsm.pConf.GracefulRestart.State; s.Enabled { + if (s.NotificationEnabled && err.Type == fsmNotificationRecv) || + (err.Type == fsmNotificationSent && + err.BGPNotification.Body.(*bgp.BGPNotification).ErrorCode == bgp.BGP_ERROR_HOLD_TIMER_EXPIRED) || err.Type == fsmReadFailed || - err.Type == fsmWriteFailed) { - err = *newfsmStateReason(fsmGracefulRestart, nil, nil) - log.WithFields(log.Fields{ - "Topic": "Peer", - "Key": fsm.pConf.State.NeighborAddress, - "State": fsm.state.String(), - }).Info("peer graceful restart") - fsm.gracefulRestartTimer.Reset(time.Duration(fsm.pConf.GracefulRestart.State.PeerRestartTime) * time.Second) + err.Type == fsmWriteFailed { + err = *newfsmStateReason(fsmGracefulRestart, nil, nil) + log.WithFields(log.Fields{ + "Topic": "Peer", + "Key": fsm.pConf.State.NeighborAddress, + "State": fsm.state.String(), + }).Info("peer graceful restart") + fsm.gracefulRestartTimer.Reset(time.Duration(fsm.pConf.GracefulRestart.State.PeerRestartTime) * time.Second) + } } fsm.lock.RUnlock() return bgp.BGP_FSM_IDLE, &err @@ -1812,7 +1815,15 @@ func (h *fsmHandler) established(ctx context.Context) (bgp.FSMState, *fsmStateRe fsm.lock.RUnlock() m := bgp.NewBGPNotificationMessage(bgp.BGP_ERROR_HOLD_TIMER_EXPIRED, 0, nil) h.outgoing.In() <- &fsmOutgoingMsg{Notification: m} - return bgp.BGP_FSM_IDLE, newfsmStateReason(fsmHoldTimerExpired, m, nil) + fsm.lock.RLock() + s := fsm.pConf.GracefulRestart.State + fsm.lock.RUnlock() + // Do not return hold timer expired to server if graceful restart is enabled + // Let it fallback to read/write error or fsmNotificationSent handled above + // Reference: https://github.com/osrg/gobgp/issues/2174 + if !s.Enabled { + return bgp.BGP_FSM_IDLE, newfsmStateReason(fsmHoldTimerExpired, m, nil) + } case <-h.holdTimerResetCh: fsm.lock.RLock() if fsm.pConf.Timers.State.NegotiatedHoldTime != 0 { diff --git a/pkg/server/fsm_test.go b/pkg/server/fsm_test.go index 9351c71d..240f5185 100644 --- a/pkg/server/fsm_test.go +++ b/pkg/server/fsm_test.go @@ -244,9 +244,53 @@ func TestFSMHandlerEstablish_HoldTimerExpired(t *testing.T) { p.fsm.pConf.Timers.State.NegotiatedHoldTime = 2 go pushPackets() - state, _ := h.established(context.Background()) + state, fsmStateReason := h.established(context.Background()) time.Sleep(time.Second * 1) assert.Equal(bgp.BGP_FSM_IDLE, state) + assert.Equal(fsmHoldTimerExpired, fsmStateReason.Type) + m.mtx.Lock() + lastMsg := m.sendBuf[len(m.sendBuf)-1] + m.mtx.Unlock() + sent, _ := bgp.ParseBGPMessage(lastMsg) + assert.Equal(uint8(bgp.BGP_MSG_NOTIFICATION), sent.Header.Type) + assert.Equal(uint8(bgp.BGP_ERROR_HOLD_TIMER_EXPIRED), sent.Body.(*bgp.BGPNotification).ErrorCode) +} + +func TestFSMHandlerEstablish_HoldTimerExpired_GR_Enabled(t *testing.T) { + assert := assert.New(t) + m := NewMockConnection(t) + + p, h := makePeerAndHandler() + + // push mock connection + p.fsm.conn = m + p.fsm.h = h + + // set keepalive ticker + p.fsm.pConf.Timers.State.NegotiatedHoldTime = 3 + + msg := keepalive() + header, _ := msg.Header.Serialize() + body, _ := msg.Body.Serialize() + + pushPackets := func() { + // first keepalive from peer + m.setData(header) + m.setData(body) + } + + // set holdtime + p.fsm.pConf.Timers.Config.HoldTime = 2 + p.fsm.pConf.Timers.State.NegotiatedHoldTime = 2 + + // Enable graceful restart + p.fsm.pConf.GracefulRestart.State.Enabled = true + + go pushPackets() + state, fsmStateReason := h.established(context.Background()) + time.Sleep(time.Second * 1) + assert.Equal(bgp.BGP_FSM_IDLE, state) + assert.Equal(fsmGracefulRestart, fsmStateReason.Type) m.mtx.Lock() lastMsg := m.sendBuf[len(m.sendBuf)-1] m.mtx.Unlock() diff --git a/pkg/server/server.go b/pkg/server/server.go index 4fbb76ff..fa0f5225 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -1380,7 +1380,7 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) { "Topic": "Peer", "Key": peer.ID(), "Family": family, - }).Debugf("start LLGR restart timer (%d sec) for %s", t, family) + }).Infof("start LLGR restart timer (%d sec) for %s", t, family) select { case <-timer.C: @@ -1389,7 +1389,7 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) { "Topic": "Peer", "Key": peer.ID(), "Family": family, - }).Debugf("LLGR restart timer (%d sec) for %s expired", t, family) + }).Infof("LLGR restart timer (%d sec) for %s expired", t, family) s.propagateUpdate(peer, peer.DropAll([]bgp.RouteFamily{family})) // when all llgr restart timer expired, stop PeerRestarting @@ -1403,7 +1403,7 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) { "Topic": "Peer", "Key": peer.ID(), "Family": family, - }).Debugf("stop LLGR restart timer (%d sec) for %s", t, family) + }).Infof("stop LLGR restart timer (%d sec) for %s", t, family) } }(f, endCh) } |