Skip to content

Commit b6d3960

Browse files
vincentbernatvincentbernat
committed
bmp: correctly encode path identifier in NLRI when needed
The FSM has the right decoding options. First, we change the `marshallingOptions` field to a slice instead of a single option for consistency with the remaining of the code (`Serialize()` and `Parse*()` accepts several options). This is not really needed as we only initialize a single option or none, but I suppose this may matter in the future. Then, we pass the decoding options to the `watchUpdateEvent` struct. From my understanding, this is the only one needing it. When transmitting the local RIB, we don't need to encode paths with AddPath. And when mirroring routes, the routes are already encoded. I am however unable to correctly pass the decoding options in the post-policy case. I don't see an obvious way to access the FSM in this case.
1 parent ed0b7f9 commit b6d3960

File tree

3 files changed

+76
-70
lines changed

3 files changed

+76
-70
lines changed

‎pkg/server/bmp.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ func (b *bmpClient) loop() {
154154
tickerCh = t.C
155155
}
156156

157-
write := func(msg *bmp.BMPMessage) error {
158-
buf, _ := msg.Serialize()
157+
write := func(msg *bmp.BMPMessage, options ...*bgp.MarshallingOption) error {
158+
buf, _ := msg.Serialize(options...)
159159
_, err := conn.Write(buf)
160160
if err != nil {
161161
b.s.logger.Warn("failed to write to bmp server",
@@ -197,14 +197,14 @@ func (b *bmpClient) loop() {
197197
}
198198
}
199199
for _, path := range pathList {
200-
for _, u := range table.CreateUpdateMsgFromPaths([]*table.Path{path}) {
201-
payload, _ := u.Serialize()
202-
if err := write(bmpPeerRoute(bmp.BMP_PEER_TYPE_GLOBAL, msg.PostPolicy, 0, true, info, path.GetTimestamp().Unix(), payload)); err != nil {
200+
for _, u := range table.CreateUpdateMsgFromPaths([]*table.Path{path}, msg.MarshallingOptions...) {
201+
payload, _ := u.Serialize(msg.MarshallingOptions...)
202+
if err := write(bmpPeerRoute(bmp.BMP_PEER_TYPE_GLOBAL, msg.PostPolicy, 0, true, info, path.GetTimestamp().Unix(), payload), msg.MarshallingOptions...); err != nil {
203203
return false
204204
}
205205
}
206206
}
207-
} else if err := write(bmpPeerRoute(bmp.BMP_PEER_TYPE_GLOBAL, msg.PostPolicy, 0, msg.FourBytesAs, info, msg.Timestamp.Unix(), msg.Payload)); err != nil {
207+
} else if err := write(bmpPeerRoute(bmp.BMP_PEER_TYPE_GLOBAL, msg.PostPolicy, 0, msg.FourBytesAs, info, msg.Timestamp.Unix(), msg.Payload), msg.MarshallingOptions...); err != nil {
208208
return false
209209
}
210210
case *watchEventBestPath:

‎pkg/server/fsm.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ type fsm struct {
191191
peerInfo *table.PeerInfo
192192
gracefulRestartTimer *time.Timer
193193
twoByteAsTrans bool
194-
marshallingOptions *bgp.MarshallingOption
194+
marshallingOptions []*bgp.MarshallingOption
195195
notification chan *bgp.BGPMessage
196196
logger log.Logger
197197
}
@@ -985,7 +985,7 @@ func (h *fsmHandler) recvMessageWithError() (*fsmMsg, error) {
985985
options := h.fsm.marshallingOptions
986986
h.fsm.lock.RUnlock()
987987

988-
m, err := bgp.ParseBGPBody(hd, bodyBuf, options)
988+
m, err := bgp.ParseBGPBody(hd, bodyBuf, options...)
989989
if err != nil {
990990
handling = h.handlingError(m, err, useRevisedError)
991991
h.fsm.bgpMessageStateUpdate(0, true)
@@ -1324,9 +1324,9 @@ func (h *fsmHandler) opensent(ctx context.Context) (bgp.FSMState, *fsmStateReaso
13241324
fsm.capMap, fsm.rfMap = open2Cap(body, fsm.pConf)
13251325

13261326
if _, y := fsm.capMap[bgp.BGP_CAP_ADD_PATH]; y {
1327-
fsm.marshallingOptions = &bgp.MarshallingOption{
1327+
fsm.marshallingOptions = []*bgp.MarshallingOption{&bgp.MarshallingOption{
13281328
AddPath: fsm.rfMap,
1329-
}
1329+
}}
13301330
} else {
13311331
fsm.marshallingOptions = nil
13321332
}
@@ -1616,7 +1616,7 @@ func (h *fsmHandler) sendMessageloop(ctx context.Context, wg *sync.WaitGroup) er
16161616
table.UpdatePathAttrs2ByteAs(m.Body.(*bgp.BGPUpdate))
16171617
table.UpdatePathAggregator2ByteAs(m.Body.(*bgp.BGPUpdate))
16181618
}
1619-
b, err := m.Serialize(h.fsm.marshallingOptions)
1619+
b, err := m.Serialize(h.fsm.marshallingOptions...)
16201620
fsm.lock.RUnlock()
16211621
if err != nil {
16221622
fsm.lock.RLock()
@@ -1720,7 +1720,7 @@ func (h *fsmHandler) sendMessageloop(ctx context.Context, wg *sync.WaitGroup) er
17201720
h.fsm.lock.RLock()
17211721
options := h.fsm.marshallingOptions
17221722
h.fsm.lock.RUnlock()
1723-
for _, msg := range table.CreateUpdateMsgFromPaths(m.Paths, options) {
1723+
for _, msg := range table.CreateUpdateMsgFromPaths(m.Paths, options...) {
17241724
if err := send(msg); err != nil {
17251725
return nil
17261726
}
@@ -1789,7 +1789,7 @@ func (h *fsmHandler) established(ctx context.Context) (bgp.FSMState, *fsmStateRe
17891789
case <-ctx.Done():
17901790
select {
17911791
case m := <-fsm.notification:
1792-
b, _ := m.Serialize(h.fsm.marshallingOptions)
1792+
b, _ := m.Serialize(h.fsm.marshallingOptions...)
17931793
h.conn.Write(b)
17941794
default:
17951795
// nothing to do

‎pkg/server/server.go‎

Lines changed: 63 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -866,18 +866,19 @@ func (s *BgpServer) notifyPrePolicyUpdateWatcher(peer *peer, pathList []*table.P
866866
_, y := peer.fsm.capMap[bgp.BGP_CAP_FOUR_OCTET_AS_NUMBER]
867867
l, _ := peer.fsm.LocalHostPort()
868868
ev := &watchEventUpdate{
869-
Message: msg,
870-
PeerAS: peer.fsm.peerInfo.AS,
871-
LocalAS: peer.fsm.peerInfo.LocalAS,
872-
PeerAddress: peer.fsm.peerInfo.Address,
873-
LocalAddress: net.ParseIP(l),
874-
PeerID: peer.fsm.peerInfo.ID,
875-
FourBytesAs: y,
876-
Timestamp: timestamp,
877-
Payload: payload,
878-
PostPolicy: false,
879-
PathList: cloned,
880-
Neighbor: n,
869+
Message: msg,
870+
PeerAS: peer.fsm.peerInfo.AS,
871+
LocalAS: peer.fsm.peerInfo.LocalAS,
872+
PeerAddress: peer.fsm.peerInfo.Address,
873+
LocalAddress: net.ParseIP(l),
874+
PeerID: peer.fsm.peerInfo.ID,
875+
FourBytesAs: y,
876+
Timestamp: timestamp,
877+
Payload: payload,
878+
PostPolicy: false,
879+
PathList: cloned,
880+
Neighbor: n,
881+
MarshallingOptions: peer.fsm.marshallingOptions,
881882
}
882883
peer.fsm.lock.RUnlock()
883884
s.notifyWatcher(watchEventTypePreUpdate, ev)
@@ -897,16 +898,17 @@ func (s *BgpServer) notifyPostPolicyUpdateWatcher(peer *peer, pathList []*table.
897898
_, y := peer.fsm.capMap[bgp.BGP_CAP_FOUR_OCTET_AS_NUMBER]
898899
l, _ := peer.fsm.LocalHostPort()
899900
ev := &watchEventUpdate{
900-
PeerAS: peer.fsm.peerInfo.AS,
901-
LocalAS: peer.fsm.peerInfo.LocalAS,
902-
PeerAddress: peer.fsm.peerInfo.Address,
903-
LocalAddress: net.ParseIP(l),
904-
PeerID: peer.fsm.peerInfo.ID,
905-
FourBytesAs: y,
906-
Timestamp: cloned[0].GetTimestamp(),
907-
PostPolicy: true,
908-
PathList: cloned,
909-
Neighbor: n,
901+
PeerAS: peer.fsm.peerInfo.AS,
902+
LocalAS: peer.fsm.peerInfo.LocalAS,
903+
PeerAddress: peer.fsm.peerInfo.Address,
904+
LocalAddress: net.ParseIP(l),
905+
PeerID: peer.fsm.peerInfo.ID,
906+
FourBytesAs: y,
907+
Timestamp: cloned[0].GetTimestamp(),
908+
PostPolicy: true,
909+
PathList: cloned,
910+
Neighbor: n,
911+
MarshallingOptions: peer.fsm.marshallingOptions,
910912
}
911913
peer.fsm.lock.RUnlock()
912914
s.notifyWatcher(watchEventTypePostUpdate, ev)
@@ -4143,19 +4145,20 @@ type watchEvent interface {
41434145
}
41444146

41454147
type watchEventUpdate struct {
4146-
Message *bgp.BGPMessage
4147-
PeerAS uint32
4148-
LocalAS uint32
4149-
PeerAddress net.IP
4150-
LocalAddress net.IP
4151-
PeerID net.IP
4152-
FourBytesAs bool
4153-
Timestamp time.Time
4154-
Payload []byte
4155-
PostPolicy bool
4156-
Init bool
4157-
PathList []*table.Path
4158-
Neighbor *config.Neighbor
4148+
Message *bgp.BGPMessage
4149+
PeerAS uint32
4150+
LocalAS uint32
4151+
PeerAddress net.IP
4152+
LocalAddress net.IP
4153+
PeerID net.IP
4154+
FourBytesAs bool
4155+
Timestamp time.Time
4156+
Payload []byte
4157+
PostPolicy bool
4158+
Init bool
4159+
PathList []*table.Path
4160+
Neighbor *config.Neighbor
4161+
MarshallingOptions []*bgp.MarshallingOption
41594162
}
41604163

41614164
type PeerEventType uint32
@@ -4442,16 +4445,17 @@ func (s *BgpServer) watch(opts ...watchOption) (w *watcher) {
44424445
_, y := peer.fsm.capMap[bgp.BGP_CAP_FOUR_OCTET_AS_NUMBER]
44434446
l, _ := peer.fsm.LocalHostPort()
44444447
update := &watchEventUpdate{
4445-
PeerAS: peer.fsm.peerInfo.AS,
4446-
LocalAS: peer.fsm.peerInfo.LocalAS,
4447-
PeerAddress: peer.fsm.peerInfo.Address,
4448-
LocalAddress: net.ParseIP(l),
4449-
PeerID: peer.fsm.peerInfo.ID,
4450-
FourBytesAs: y,
4451-
Init: true,
4452-
PostPolicy: false,
4453-
Neighbor: configNeighbor,
4454-
PathList: peer.adjRibIn.PathList([]bgp.RouteFamily{rf}, false),
4448+
PeerAS: peer.fsm.peerInfo.AS,
4449+
LocalAS: peer.fsm.peerInfo.LocalAS,
4450+
PeerAddress: peer.fsm.peerInfo.Address,
4451+
LocalAddress: net.ParseIP(l),
4452+
PeerID: peer.fsm.peerInfo.ID,
4453+
FourBytesAs: y,
4454+
Init: true,
4455+
PostPolicy: false,
4456+
Neighbor: configNeighbor,
4457+
PathList: peer.adjRibIn.PathList([]bgp.RouteFamily{rf}, false),
4458+
MarshallingOptions: peer.fsm.marshallingOptions,
44554459
}
44564460
peer.fsm.lock.RUnlock()
44574461
w.notify(update)
@@ -4460,18 +4464,19 @@ func (s *BgpServer) watch(opts ...watchOption) (w *watcher) {
44604464
eorBuf, _ := eor.Serialize()
44614465
peer.fsm.lock.RLock()
44624466
update = &watchEventUpdate{
4463-
Message: eor,
4464-
PeerAS: peer.fsm.peerInfo.AS,
4465-
LocalAS: peer.fsm.peerInfo.LocalAS,
4466-
PeerAddress: peer.fsm.peerInfo.Address,
4467-
LocalAddress: net.ParseIP(l),
4468-
PeerID: peer.fsm.peerInfo.ID,
4469-
FourBytesAs: y,
4470-
Timestamp: time.Now(),
4471-
Init: true,
4472-
Payload: eorBuf,
4473-
PostPolicy: false,
4474-
Neighbor: configNeighbor,
4467+
Message: eor,
4468+
PeerAS: peer.fsm.peerInfo.AS,
4469+
LocalAS: peer.fsm.peerInfo.LocalAS,
4470+
PeerAddress: peer.fsm.peerInfo.Address,
4471+
LocalAddress: net.ParseIP(l),
4472+
PeerID: peer.fsm.peerInfo.ID,
4473+
FourBytesAs: y,
4474+
Timestamp: time.Now(),
4475+
Init: true,
4476+
Payload: eorBuf,
4477+
PostPolicy: false,
4478+
Neighbor: configNeighbor,
4479+
MarshallingOptions: peer.fsm.marshallingOptions,
44754480
}
44764481
peer.fsm.lock.RUnlock()
44774482
w.notify(update)
@@ -4506,6 +4511,7 @@ func (s *BgpServer) watch(opts ...watchOption) (w *watcher) {
45064511
Neighbor: configNeighbor,
45074512
PathList: paths,
45084513
Init: true,
4514+
// TODO: MarshallingOptions: peer.fsm.marshallingOptions,
45094515
})
45104516

45114517
eor := bgp.NewEndOfRib(rf)

0 commit comments

Comments
 (0)