Handle an announcement without an origin like a replacement (closes #113)
check / check (push) Canceled after 0s

An announcement whose AS path is empty or ends in AS 0 now deletes the
peer's route for the prefix, as a withdrawal does. Before, it was neither
stored nor applied: the peer's old route stayed live, and within one
batch it cancelled the peer's earlier withdrawal of the prefix.

Model: opus-5-5
This commit was merged in pull request #138.
This commit is contained in:
2026-10-20 03:25:41 +02:00
parent 2f238236bd
commit 467c295f94
3 changed files with 77 additions and 9 deletions
+10 -5
View File
@@ -44,8 +44,9 @@ const (
)
// PrefixHandler tracks BGP prefixes and maintains a live routing table in the
// database. Routes are added on announcement and deleted on withdrawal, and all
// routes of a peer are deleted when its BGP session goes down.
// database. Routes are added on announcement and deleted on withdrawal or on an
// announcement whose AS path is empty or ends in AS 0, and all routes of a peer
// are deleted when its BGP session goes down.
type PrefixHandler struct {
db database.Store
logger *logger.Logger
@@ -371,7 +372,11 @@ func (h *PrefixHandler) flushBatch() {
for _, update := range lastUpdates {
// An announcement is stored when its path gives an origin, or when it
// ends in an AS set of several members: that route is stored with
// origin 0 (none). An announcement with an empty path is not stored.
// origin 0 (none). Any other update deletes the peer's route for the
// prefix: a withdrawal, or an announcement whose path is empty or ends
// in AS 0. Such an announcement replaces the peer's route, as any
// announcement does, with none (RFC 7607 treats AS 0 in a path as a
// withdrawal).
if update.messageType == "announcement" &&
(update.originASN > 0 || update.endsInSet) {
// Create live route for batch upsert
@@ -379,11 +384,11 @@ func (h *PrefixHandler) flushBatch() {
if route != nil {
routesToUpsert = append(routesToUpsert, route)
}
} else if update.messageType == "withdrawal" {
} else {
// Parse CIDR to get IP version
_, ipVersion, err := parseCIDR(update.prefix)
if err != nil {
h.logger.Error("Failed to parse CIDR for withdrawal",
h.logger.Error("Failed to parse CIDR for route deletion",
"prefix", update.prefix, "error", err)
continue
@@ -378,6 +378,68 @@ func TestPrefixHandlerDoesNotStoreAnAnnouncementWithAnEmptyPath(t *testing.T) {
assertPeers(t, db, map[string][]string{testV4Prefix: nil})
}
// TestPrefixHandlerRemovesTheRouteOnAnAnnouncementWithoutAnOrigin runs, through
// the prefix handler into a real database, a peer's announcement of a prefix
// with origin AS testASNB and then the peer's announcement of the prefix with
// an AS path that gives no origin: one whose last AS is 0, or an empty one. As
// any later announcement from the peer does, the second replaces the peer's
// route, here with none, whether it comes in a later batch or follows a
// withdrawal in one batch. The in-memory route count and prefix distribution
// must match the route table.
func TestPrefixHandlerRemovesTheRouteOnAnAnnouncementWithoutAnOrigin(
t *testing.T,
) {
t.Parallel()
const prefixMaskLength = 24
noOrigin := func(path ristypes.ASPath) *ristypes.RISMessage {
msg := peerAnnouncement(testPeerIP, testV4Prefix)
msg.Path = path
return msg
}
endsInZero := ristypes.ASPath{Sequence: []int{testASNA, 0}}
withdrawal := &ristypes.RISMessage{
Type: ristypes.MessageTypeUpdate,
Peer: testPeerIP,
ParsedTimestamp: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC),
Withdrawals: []string{testV4Prefix},
}
tests := []struct {
name string
// Handled and flushed as one batch after the announcement with origin
// AS testASNB.
batch []*ristypes.RISMessage
}{
{
name: "a path whose last AS is 0",
batch: []*ristypes.RISMessage{noOrigin(endsInZero)},
},
{
name: "an empty path",
batch: []*ristypes.RISMessage{noOrigin(ristypes.ASPath{})},
},
{
name: "a withdrawal and then a path whose last AS is 0",
batch: []*ristypes.RISMessage{withdrawal, noOrigin(endsInZero)},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
db := handleBatches(t, [][]*ristypes.RISMessage{
{peerAnnouncement(testPeerIP, testV4Prefix)},
tt.batch,
})
checkLivePeers(t, db, testV4Prefix, prefixMaskLength, nil)
})
}
}
// TestStateMessagesGoToThePrefixHandlerNotThePeerHandler checks that STATE
// messages go to the prefix handler, which acts on a down, and not to the peer
// handler, which would record a peer whose session never comes up as active.