Skip to content

server: expose raw NOTIFICATION error code/subcode on PeerState #3543

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Sreedhar-cp wants to merge 2 commits into
base: master
Choose a base branch
from
+179 −18
Open

server: expose raw NOTIFICATION error code/subcode on PeerState #3543

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7dfd2c3
server: fix wrong message wired into fsmStateReason on bad-peer-AS/in…
Sreedhar-cp Aug 20, 2026
29212f3
api, server: expose raw NOTIFICATION error code/subcode on PeerState
Sreedhar-cp Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions api/gobgp.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions pkg/apiutil/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ type PeerState struct {
LocalCap []bgp.ParameterCapabilityInterface
DisconnectReason api.PeerState_DisconnectReason
DisconnectMessage string
// NotificationCode/NotificationSubcode carry the raw RFC 4271 §6
// NOTIFICATION error code/subcode whenever one is present -- not limited
// to a NOTIFICATION-specific DisconnectReason, since several other FSM
// transitions can carry a real notification too. 0/0 when none was
// involved.
NotificationCode uint32
NotificationSubcode uint32
}
type Transport struct {
LocalAddress netip.Addr
Expand Down
4 changes: 2 additions & 2 deletions pkg/server/fsm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1465,9 +1465,9 @@ func (fsm *fsm) handleOpen(fmsg *fsmMsg) (bgp.FSMState, *fsmStateReason, *bgp.BG
err := err.(*bgp.MessageError)
notif := bgp.NewBGPNotificationMessage(err.TypeCode, err.SubTypeCode, err.Data)
if err.TypeCode == bgp.BGP_ERROR_OPEN_MESSAGE_ERROR && err.SubTypeCode == bgp.BGP_ERROR_SUB_BAD_PEER_AS {
return bgp.BGP_FSM_IDLE, newfsmStateReason(fsmBadPeerAS, m, nil), notif
return bgp.BGP_FSM_IDLE, newfsmStateReason(fsmBadPeerAS, notif, nil), notif
}
return bgp.BGP_FSM_IDLE, newfsmStateReason(fsmInvalidMsg, m, nil), notif
return bgp.BGP_FSM_IDLE, newfsmStateReason(fsmInvalidMsg, notif, nil), notif
}
return bgp.BGP_FSM_OPENCONFIRM, newfsmStateReason(fsmOpenMsgReceived, nil, nil), nil
}
Expand Down
46 changes: 35 additions & 11 deletions pkg/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4631,6 +4631,27 @@ func convertFSMStateReasonToAPI(stateReason *fsmStateReason) (api.PeerState_Disc
return disconnectReason, stateReason.String()
}

// extractNotificationCodeSubcode returns the raw RFC 4271 §6 NOTIFICATION
// error code/subcode whenever stateReason carries a real *bgp.BGPNotification
// -- intentionally not gated on stateReason.Type (e.g. fsmNotificationSent/
// fsmNotificationRecv only): fsmBadPeerAS, fsmInvalidMsg, fsmHoldTimerExpired,
// and fsmAdminDown can all carry a real notification too (see their
// newfsmStateReason call sites in fsm.go), and any of them should surface a
// numeric code/subcode when one is actually present. Returns 0, 0 when no
// notification was involved.
func extractNotificationCodeSubcode(stateReason *fsmStateReason) (uint32, uint32) {
if stateReason == nil || stateReason.BGPNotification == nil {
return 0, 0
}

body, ok := stateReason.BGPNotification.Body.(*bgp.BGPNotification)
if !ok {
return 0, 0
}

return uint32(body.ErrorCode), uint32(body.ErrorSubcode)
}

func toPathApiUtil(path *table.Path) *apiutil.Path {
// Best and SendMaxFiltered are set in ListPath API
p := &apiutil.Path{
Expand Down Expand Up @@ -4715,6 +4736,7 @@ func (s *BgpServer) WatchEvent(ctx context.Context, callbacks WatchEventMessageC
}

disconnectReason, disconnectMessage := convertFSMStateReasonToAPI(msg.StateReason)
notificationCode, notificationSubcode := extractNotificationCodeSubcode(msg.StateReason)

callbacks.OnPeerUpdate(&apiutil.WatchEventMessage_PeerEvent{
Type: msg.Type,
Expand All @@ -4727,17 +4749,19 @@ func (s *BgpServer) WatchEvent(ctx context.Context, callbacks WatchEventMessageC
PeerGroup: msg.PeerGroup,
},
State: apiutil.PeerState{
PeerASN: msg.PeerAS,
LocalASN: msg.LocalAS,
NeighborAddress: msg.PeerAddress,
SessionState: msg.State,
AdminState: admin_state,
RouterID: msg.PeerID,
PeerGroup: msg.PeerGroup,
RemoteCap: msg.RemoteCap,
LocalCap: msg.LocalCap,
DisconnectReason: disconnectReason,
DisconnectMessage: disconnectMessage,
PeerASN: msg.PeerAS,
LocalASN: msg.LocalAS,
NeighborAddress: msg.PeerAddress,
SessionState: msg.State,
AdminState: admin_state,
RouterID: msg.PeerID,
PeerGroup: msg.PeerGroup,
RemoteCap: msg.RemoteCap,
LocalCap: msg.LocalCap,
DisconnectReason: disconnectReason,
DisconnectMessage: disconnectMessage,
NotificationCode: notificationCode,
NotificationSubcode: notificationSubcode,
},
Transport: apiutil.Transport{
LocalAddress: msg.LocalAddress,
Expand Down
96 changes: 96 additions & 0 deletions pkg/server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4444,3 +4444,99 @@ func TestRTCShouldNotAdvertiseVPNRouteWhenRTCIsNotPassImportPolicies(t *testing.
require.Never(t, vpnPresentAtS2AdjIn, 10*time.Second, 100*time.Millisecond,
"VPN route should not appear at s2 adj-in from s1 after second VPN prefix is added")
}

// TestWatchEventPeerNotificationCodeSubcode verifies PeerState.NotificationCode
// and PeerState.NotificationSubcode -- the raw RFC 4271 §6 NOTIFICATION error
// code/subcode, extracted directly from the FSM's underlying notification.
// DisconnectReason/DisconnectMessage alone cannot expose these as typed
// integers without fragile string-parsing.
func TestWatchEventPeerNotificationCodeSubcode(t *testing.T) {
assert := assert.New(t)

// s expects its peer to be ASN 99, but the peer that actually connects
// (peerServer, below) is ASN 2 -- a genuine peer-AS mismatch, driving
// fsm.go's opensent() into the fsmBadPeerAS branch, with a real
// NOTIFICATION (OPEN_MESSAGE_ERROR / BAD_PEER_AS) sent to the peer.
s := runNewServer(t, 1, "1.1.1.1", 10182)
defer s.StopBgp(context.Background(), &api.StopBgpRequest{})

err := s.AddPeer(context.Background(), &api.AddPeerRequest{
Peer: &api.Peer{
Conf: &api.PeerConf{
NeighborAddress: "127.0.0.1",
PeerAsn: 99,
},
Transport: &api.Transport{
PassiveMode: true,
},
},
})
assert.NoError(err)

peerServer := runNewServer(t, 2, "2.2.2.2", -1)
defer peerServer.StopBgp(context.Background(), &api.StopBgpRequest{})

var (
badPeerASReason api.PeerState_DisconnectReason
badPeerASMessage string
notificationCode, notifSubcode uint32
once sync.Once
)
badPeerASCh := make(chan struct{})
watchCtx, watchCancel := context.WithCancel(context.Background())
defer watchCancel()

err = s.WatchEvent(watchCtx, WatchEventMessageCallbacks{
OnPeerUpdate: func(peer *apiutil.WatchEventMessage_PeerEvent, _ time.Time) {
if peer == nil || peer.Type != apiutil.PEER_EVENT_STATE {
return
}
if api.PeerState_SessionState(int(peer.Peer.State.SessionState)+1) != api.PeerState_SESSION_STATE_IDLE {
return
}
once.Do(func() {
badPeerASReason = peer.Peer.State.DisconnectReason
badPeerASMessage = peer.Peer.State.DisconnectMessage
notificationCode = peer.Peer.State.NotificationCode
notifSubcode = peer.Peer.State.NotificationSubcode
close(badPeerASCh)
})
},
}, WatchPeer())
assert.NoError(err)

err = peerServer.AddPeer(context.Background(), &api.AddPeerRequest{
Peer: &api.Peer{
Conf: &api.PeerConf{
NeighborAddress: "127.0.0.1",
PeerAsn: 1,
},
Transport: &api.Transport{
RemotePort: 10182,
},
Timers: &api.Timers{
Config: &api.TimersConfig{
ConnectRetry: 1,
IdleHoldTimeAfterReset: 1,
},
},
},
})
assert.NoError(err)

select {
case <-badPeerASCh:
case <-time.After(10 * time.Second):
t.Fatal("timeout waiting for bad-peer-AS IDLE event")
}

assert.Equal(api.PeerState_DISCONNECT_REASON_BAD_PEER_AS, badPeerASReason)
// fsmBadPeerAS's String() case returns the literal "bad-peer-as" --
// unlike fsmNotificationSent/fsmNotificationRecv, it does not format the
// raw notification code/subcode into the message string. That's exactly
// why NotificationCode/NotificationSubcode exist as separate fields.
assert.Equal("bad-peer-as", badPeerASMessage)
// RFC 4271 §6: OPEN Message Error (2) / Bad Peer AS (2).
assert.Equal(uint32(bgp.BGP_ERROR_OPEN_MESSAGE_ERROR), notificationCode)
assert.Equal(uint32(bgp.BGP_ERROR_SUB_BAD_PEER_AS), notifSubcode)
}
9 changes: 9 additions & 0 deletions proto/api/gobgp.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,15 @@ message PeerState {
DisconnectReason disconnect_reason = 21;
string disconnect_message = 22;
BfdPeerState bfd_state = 23;
// Raw RFC 4271 §6 NOTIFICATION error code/subcode, extracted from the FSM's
// underlying BGP NOTIFICATION whenever one is present. This is not limited
// to DISCONNECT_REASON_NOTIFICATION_SENT/_RECEIVED -- fsmBadPeerAS,
// fsmInvalidMsg, fsmHoldTimerExpired, and fsmAdminDown can all carry a real
// notification too. 0/0 when no notification was involved. Needed because
// disconnect_reason/disconnect_message alone cannot expose these as typed
// integers without fragile string-parsing.
uint32 notification_code = 24;
uint32 notification_subcode = 25;

@fujita fujita Aug 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

// A BGP NOTIFICATION message, as defined in RFC 4271 section 4.5.
message BgpNotification {
  uint32 error_code = 1;
  uint32 error_subcode = 2;
  bytes data = 3;
}

Let's create a new type for notification because we might need data later.

}

message Messages {
Expand Down
Loading