diff --git a/api/gobgp.pb.go b/api/gobgp.pb.go index a60b72c95..6ec1002d1 100644 --- a/api/gobgp.pb.go +++ b/api/gobgp.pb.go @@ -22,7 +22,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: // protoc-gen-go v1.36.6 -// protoc (unknown) +// protoc v7.35.1 // source: api/gobgp.proto package api @@ -7994,8 +7994,17 @@ type PeerState struct { DisconnectReason PeerState_DisconnectReason `protobuf:"varint,21,opt,name=disconnect_reason,json=disconnectReason,proto3,enum=api.PeerState_DisconnectReason" json:"disconnect_reason,omitempty"` DisconnectMessage string `protobuf:"bytes,22,opt,name=disconnect_message,json=disconnectMessage,proto3" json:"disconnect_message,omitempty"` BfdState *BfdPeerState `protobuf:"bytes,23,opt,name=bfd_state,json=bfdState,proto3" json:"bfd_state,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // 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. + NotificationCode uint32 `protobuf:"varint,24,opt,name=notification_code,json=notificationCode,proto3" json:"notification_code,omitempty"` + NotificationSubcode uint32 `protobuf:"varint,25,opt,name=notification_subcode,json=notificationSubcode,proto3" json:"notification_subcode,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *PeerState) Reset() { @@ -8182,6 +8191,20 @@ func (x *PeerState) GetBfdState() *BfdPeerState { return nil } +func (x *PeerState) GetNotificationCode() uint32 { + if x != nil { + return x.NotificationCode + } + return 0 +} + +func (x *PeerState) GetNotificationSubcode() uint32 { + if x != nil { + return x.NotificationSubcode + } + return 0 +} + type Messages struct { state protoimpl.MessageState `protogen:"open.v1"` Received *Message `protobuf:"bytes,1,opt,name=received,proto3" json:"received,omitempty"` @@ -13661,7 +13684,7 @@ const file_api_gobgp_proto_rawDesc = "" + "\fmultihop_ttl\x18\x02 \x01(\rR\vmultihopTtl\"\x83\x01\n" + "\x0eRouteReflector\x124\n" + "\x16route_reflector_client\x18\x01 \x01(\bR\x14routeReflectorClient\x12;\n" + - "\x1aroute_reflector_cluster_id\x18\x02 \x01(\tR\x17routeReflectorClusterId\"\x98\x0e\n" + + "\x1aroute_reflector_cluster_id\x18\x02 \x01(\tR\x17routeReflectorClusterId\"\xf8\x0e\n" + "\tPeerState\x12#\n" + "\rauth_password\x18\x01 \x01(\tR\fauthPassword\x12 \n" + "\vdescription\x18\x02 \x01(\tR\vdescription\x12\x1b\n" + @@ -13688,7 +13711,9 @@ const file_api_gobgp_proto_rawDesc = "" + "\trouter_id\x18\x14 \x01(\tR\brouterId\x12L\n" + "\x11disconnect_reason\x18\x15 \x01(\x0e2\x1f.api.PeerState.DisconnectReasonR\x10disconnectReason\x12-\n" + "\x12disconnect_message\x18\x16 \x01(\tR\x11disconnectMessage\x12.\n" + - "\tbfd_state\x18\x17 \x01(\v2\x11.api.BfdPeerStateR\bbfdState\"\xd4\x01\n" + + "\tbfd_state\x18\x17 \x01(\v2\x11.api.BfdPeerStateR\bbfdState\x12+\n" + + "\x11notification_code\x18\x18 \x01(\rR\x10notificationCode\x121\n" + + "\x14notification_subcode\x18\x19 \x01(\rR\x13notificationSubcode\"\xd4\x01\n" + "\fSessionState\x12\x1d\n" + "\x19SESSION_STATE_UNSPECIFIED\x10\x00\x12\x16\n" + "\x12SESSION_STATE_IDLE\x10\x01\x12\x19\n" + diff --git a/pkg/apiutil/util.go b/pkg/apiutil/util.go index 41a85e236..91da4a81b 100644 --- a/pkg/apiutil/util.go +++ b/pkg/apiutil/util.go @@ -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 diff --git a/pkg/server/fsm.go b/pkg/server/fsm.go index 811e0b469..f961250e2 100644 --- a/pkg/server/fsm.go +++ b/pkg/server/fsm.go @@ -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 } diff --git a/pkg/server/server.go b/pkg/server/server.go index dc393c786..b9c4006d3 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -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{ @@ -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, @@ -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, diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 10e2f831a..f46adb396 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -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) +} diff --git a/proto/api/gobgp.proto b/proto/api/gobgp.proto index b0354be32..68c509adc 100644 --- a/proto/api/gobgp.proto +++ b/proto/api/gobgp.proto @@ -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; } message Messages {