Skip to content

Commit 304bc04

Browse files
fujitafujita
committed
server: remove Notification in fsmMsg
Send a notification immediately as RFC says. Curretnly a notification will be sent after all the pending messages in tx channel are sent. Signed-off-by: FUJITA Tomonori <[email protected]>
1 parent f3d7715 commit 304bc04

File tree

3 files changed

+49
-40
lines changed

3 files changed

+49
-40
lines changed

‎pkg/server/fsm.go‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,7 @@ type fsmMsg struct {
136136
}
137137

138138
type fsmOutgoingMsg struct {
139-
Paths []*table.Path
140-
Notification *bgp.BGPMessage
139+
Paths []*table.Path
141140
}
142141

143142
const (
@@ -192,6 +191,7 @@ type fsm struct {
192191
gracefulRestartTimer *time.Timer
193192
twoByteAsTrans bool
194193
marshallingOptions *bgp.MarshallingOption
194+
notification chan *bgp.BGPMessage
195195
deconfiguredNotification chan *bgp.BGPMessage
196196
logger log.Logger
197197
}
@@ -280,6 +280,7 @@ func newFSM(gConf *oc.Global, pConf *oc.Neighbor, logger log.Logger) *fsm {
280280
rfMap: make(map[bgp.Family]bgp.BGPAddPathMode),
281281
capMap: make(map[bgp.BGPCapabilityCode][]bgp.ParameterCapabilityInterface),
282282
gracefulRestartTimer: time.NewTimer(time.Hour),
283+
notification: make(chan *bgp.BGPMessage, 1),
283284
deconfiguredNotification: make(chan *bgp.BGPMessage, 1),
284285
logger: logger,
285286
}
@@ -1814,11 +1815,6 @@ func (h *fsmHandler) sendMessageloop(ctx context.Context, wg *sync.WaitGroup) er
18141815
return nil
18151816
}
18161817
}
1817-
if m.Notification != nil {
1818-
if err := send(m.Notification); err != nil {
1819-
return nil
1820-
}
1821-
}
18221818
default:
18231819
return nil
18241820
}
@@ -1875,33 +1871,40 @@ func (h *fsmHandler) established(ctx context.Context) (bgp.FSMState, *fsmStateRe
18751871

18761872
fsm.gracefulRestartTimer.Stop()
18771873

1874+
convertNotification := func(m *bgp.BGPMessage) *bgp.BGPMessage {
1875+
// RFC8538 defines a Hard Reset notification subcode which
1876+
// indicates that the BGP speaker wants to reset the session
1877+
// without triggering graceful restart procedures. Here we map
1878+
// notification subcodes to the Hard Reset subcode following
1879+
// the RFC8538 suggestion.
1880+
//
1881+
// We check Status instead of Config because RFC8538 states
1882+
// that A BGP speaker SHOULD NOT send a Hard Reset to a peer
1883+
// from which it has not received the "N" bit.
1884+
if fsm.pConf.GracefulRestart.State.NotificationEnabled {
1885+
if m.Body.(*bgp.BGPNotification).ErrorCode == bgp.BGP_ERROR_CEASE && bgp.ShouldHardReset(m.Body.(*bgp.BGPNotification).ErrorSubcode, false) {
1886+
return bgp.NewBGPNotificationMessage(m.Body.(*bgp.BGPNotification).ErrorCode, bgp.BGP_ERROR_SUB_HARD_RESET, m.Body.(*bgp.BGPNotification).Data)
1887+
}
1888+
}
1889+
return m
1890+
}
1891+
18781892
for {
18791893
select {
18801894
case <-ctx.Done():
18811895
select {
18821896
case m := <-fsm.deconfiguredNotification:
1883-
// RFC8538 defines a Hard Reset notification subcode which
1884-
// indicates that the BGP speaker wants to reset the session
1885-
// without triggering graceful restart procedures. Here we map
1886-
// notification subcodes to the Hard Reset subcode following
1887-
// the RFC8538 suggestion.
1888-
//
1889-
// We check Status instead of Config because RFC8538 states
1890-
// that A BGP speaker SHOULD NOT send a Hard Reset to a peer
1891-
// from which it has not received the "N" bit.
1892-
if fsm.pConf.GracefulRestart.State.NotificationEnabled {
1893-
if body := m.Body.(*bgp.BGPNotification); body.ErrorCode == bgp.BGP_ERROR_CEASE && bgp.ShouldHardReset(body.ErrorSubcode, false) {
1894-
body.ErrorSubcode = bgp.BGP_ERROR_SUB_HARD_RESET
1895-
}
1896-
}
1897-
b, _ := m.Serialize(h.fsm.marshallingOptions)
1898-
h.conn.SetWriteDeadline(time.Now().Add(time.Second))
1899-
h.conn.Write(b)
1897+
m = convertNotification(m)
1898+
_ = fsm.sendNotification(m)
19001899
default:
19011900
// nothing to do
19021901
}
19031902
h.conn.Close()
19041903
return -1, newfsmStateReason(fsmDying, nil, nil)
1904+
case m := <-fsm.notification:
1905+
m = convertNotification(m)
1906+
_ = fsm.sendNotification(m)
1907+
return bgp.BGP_FSM_IDLE, newfsmStateReason(fsmNotificationSent, m, nil)
19051908
case conn, ok := <-fsm.connCh:
19061909
if !ok {
19071910
break

‎pkg/server/peer.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,13 @@ func (peer *peer) filterPathFromSourcePeer(path, old *table.Path) *table.Path {
559559
return nil
560560
}
561561

562+
func (peer *peer) sendNotification(msg *bgp.BGPMessage) {
563+
select {
564+
case peer.fsm.notification <- msg:
565+
default:
566+
}
567+
}
568+
562569
func (peer *peer) isPrefixLimit(k bgp.Family, c *oc.PrefixLimitConfig) bool {
563570
if maxPrefixes := int(c.MaxPrefixes); maxPrefixes > 0 {
564571
count := peer.adjRibIn.Count([]bgp.Family{k})

‎pkg/server/server.go‎

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -430,10 +430,9 @@ func (s *BgpServer) matchLongestDynamicNeighborPrefix(a string) *peerGroup {
430430
return longestPG
431431
}
432432

433-
func sendfsmOutgoingMsg(peer *peer, paths []*table.Path, notification *bgp.BGPMessage) {
433+
func sendfsmOutgoingMsg(peer *peer, paths []*table.Path) {
434434
peer.fsm.outgoingCh.In() <- &fsmOutgoingMsg{
435-
Paths: paths,
436-
Notification: notification,
435+
Paths: paths,
437436
}
438437
}
439438

@@ -1257,10 +1256,10 @@ func (s *BgpServer) propagateUpdate(peer *peer, pathList []*table.Path) {
12571256
if path.IsWithdraw {
12581257
// Skips filtering because the paths are already filtered
12591258
// and the withdrawal does not need the path attributes.
1260-
sendfsmOutgoingMsg(peer, paths, nil)
1259+
sendfsmOutgoingMsg(peer, paths)
12611260
} else if !peer.getRtcEORWait() {
12621261
paths = s.processOutgoingPaths(peer, paths, nil)
1263-
sendfsmOutgoingMsg(peer, paths, nil)
1262+
sendfsmOutgoingMsg(peer, paths)
12641263
} else {
12651264
s.logger.Debug("Nothing sent in response to RT received. Waiting for RTC EOR.",
12661265
log.Fields{
@@ -1413,13 +1412,13 @@ func (s *BgpServer) propagateUpdateToNeighbors(rib *table.TableManager, source *
14131412
}
14141413
}
14151414
if needToAdvertise(targetPeer) && len(bestList) > 0 {
1416-
sendfsmOutgoingMsg(targetPeer, bestList, nil)
1415+
sendfsmOutgoingMsg(targetPeer, bestList)
14171416
}
14181417
} else {
14191418
if targetPeer.isRouteServerClient() {
14201419
if targetPeer.isSecondaryRouteEnabled() {
14211420
if paths := s.sendSecondaryRoutes(targetPeer, newPath, dsts); len(paths) > 0 {
1422-
sendfsmOutgoingMsg(targetPeer, paths, nil)
1421+
sendfsmOutgoingMsg(targetPeer, paths)
14231422
}
14241423
continue
14251424
}
@@ -1432,7 +1431,7 @@ func (s *BgpServer) propagateUpdateToNeighbors(rib *table.TableManager, source *
14321431
oldList = nil
14331432
}
14341433
if paths := s.processOutgoingPaths(targetPeer, bestList, oldList); len(paths) > 0 {
1435-
sendfsmOutgoingMsg(targetPeer, paths, nil)
1434+
sendfsmOutgoingMsg(targetPeer, paths)
14361435
}
14371436
}
14381437
}
@@ -1674,7 +1673,7 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
16741673
}
16751674

16761675
if len(pathList) > 0 {
1677-
sendfsmOutgoingMsg(peer, pathList, nil)
1676+
sendfsmOutgoingMsg(peer, pathList)
16781677
}
16791678
} else {
16801679
// RFC 4724 4.1
@@ -1704,7 +1703,7 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
17041703
}
17051704
paths, _ := s.getBestFromLocal(p, p.configuredRFlist())
17061705
if len(paths) > 0 {
1707-
sendfsmOutgoingMsg(p, paths, nil)
1706+
sendfsmOutgoingMsg(p, paths)
17081707
}
17091708
}
17101709
s.logger.Info("sync finished",
@@ -1752,13 +1751,13 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
17521751
return
17531752
}
17541753
if paths := s.handleRouteRefresh(peer, e); len(paths) > 0 {
1755-
sendfsmOutgoingMsg(peer, paths, nil)
1754+
sendfsmOutgoingMsg(peer, paths)
17561755
return
17571756
}
17581757
case fsmMsgBGPMessage:
17591758
switch m := e.MsgData.(type) {
17601759
case *bgp.MessageError:
1761-
sendfsmOutgoingMsg(peer, nil, bgp.NewBGPNotificationMessage(m.TypeCode, m.SubTypeCode, m.Data))
1760+
peer.sendNotification(bgp.NewBGPNotificationMessage(m.TypeCode, m.SubTypeCode, m.Data))
17621761
return
17631762
case *bgp.BGPMessage:
17641763
s.notifyRecvMessageWatcher(peer, e.timestamp, m)
@@ -1837,7 +1836,7 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
18371836
}
18381837
paths, _ := s.getBestFromLocal(p, p.negotiatedRFList())
18391838
if len(paths) > 0 {
1840-
sendfsmOutgoingMsg(p, paths, nil)
1839+
sendfsmOutgoingMsg(p, paths)
18411840
}
18421841
}
18431842
s.logger.Info("sync finished",
@@ -1886,7 +1885,7 @@ func (s *BgpServer) handleFSMMessage(peer *peer, e *fsmMsg) {
18861885
}
18871886
}
18881887
if paths, _ := s.getBestFromLocal(peer, families); len(paths) > 0 {
1889-
sendfsmOutgoingMsg(peer, paths, nil)
1888+
sendfsmOutgoingMsg(peer, paths)
18901889
}
18911890
}
18921891
}
@@ -2698,7 +2697,7 @@ func (s *BgpServer) softResetOut(addr string, family bgp.Family, deferral bool)
26982697
return l
26992698
}()
27002699
}
2701-
sendfsmOutgoingMsg(peer, pathList, nil)
2700+
sendfsmOutgoingMsg(peer, pathList)
27022701
}
27032702
}
27042703
return nil
@@ -3751,7 +3750,7 @@ func (s *BgpServer) sendNotification(op, addr string, subcode uint8, data []byte
37513750
if err == nil {
37523751
m := bgp.NewBGPNotificationMessage(bgp.BGP_ERROR_CEASE, subcode, data)
37533752
for _, peer := range peers {
3754-
sendfsmOutgoingMsg(peer, nil, m)
3753+
peer.sendNotification(m)
37553754
}
37563755
}
37573756
return err

0 commit comments

Comments
 (0)