From 7b45f2589c8a42543da3a8a0c307a7fb6f6669e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mattias=20Walstr=C3=B6m?= Date: Fri, 24 Apr 2026 10:10:59 +0200 Subject: [PATCH] zapi: Fix deletion of routes --- src/yangerd/internal/zapi/zapi.go | 209 ++++++------ src/yangerd/internal/zapi/zapi_test.go | 112 +++++++ .../internal/zapiwatcher/zapiwatcher.go | 115 +++++-- .../internal/zapiwatcher/zapiwatcher_test.go | 299 +++++++++++++++++- 4 files changed, 619 insertions(+), 116 deletions(-) diff --git a/src/yangerd/internal/zapi/zapi.go b/src/yangerd/internal/zapi/zapi.go index 21a5c4c0..3a4f12ea 100644 --- a/src/yangerd/internal/zapi/zapi.go +++ b/src/yangerd/internal/zapi/zapi.go @@ -7,8 +7,10 @@ package zapi import ( "encoding/binary" + "encoding/hex" "fmt" "io" + "log/slog" "net" "syscall" ) @@ -78,6 +80,11 @@ const ( AFIIPv6 uint8 = 2 ) +// Route flags from FRR (lib/zebra.h: ZEBRA_FLAG_*). +const ( + FlagSelected uint32 = 0x04 +) + // Message flags (from struct zapi_route.message). type MsgFlag uint32 @@ -109,11 +116,13 @@ const ( // Nexthop flags (from lib/zclient.h: ZAPI_NEXTHOP_FLAG_*). const ( + nhFlagOnlink uint8 = 0x01 nhFlagLabel uint8 = 0x02 nhFlagWeight uint8 = 0x04 nhFlagHasBackup uint8 = 0x08 nhFlagSeg6 uint8 = 0x10 nhFlagSeg6Local uint8 = 0x20 + nhFlagEVPN uint8 = 0x40 ) // Header is a ZAPI v6 message header. @@ -157,7 +166,7 @@ func EncodeHeader(length uint16, vrfID uint32, cmd Command) []byte { binary.BigEndian.PutUint16(buf[0:2], length) buf[2] = HeaderMarker buf[3] = HeaderVersion - binary.LittleEndian.PutUint32(buf[4:8], vrfID) + binary.BigEndian.PutUint32(buf[4:8], vrfID) binary.BigEndian.PutUint16(buf[8:10], uint16(cmd)) return buf } @@ -171,7 +180,7 @@ func DecodeHeader(data []byte) (Header, error) { Length: binary.BigEndian.Uint16(data[0:2]), Marker: data[2], Version: data[3], - VrfID: binary.LittleEndian.Uint32(data[4:8]), + VrfID: binary.BigEndian.Uint32(data[4:8]), Command: Command(binary.BigEndian.Uint16(data[8:10])), } if h.Marker != HeaderMarker { @@ -184,11 +193,10 @@ func DecodeHeader(data []byte) (Header, error) { } // EncodeHello builds a Hello message body. -// Fields: redistDefault(1), instance(2), sessionID(4), receiveNotify(1), synchronous(1), flags(4), ... -// We send zeros for everything (redistDefault=0 means "no default route type"). +// Fields: redistDefault(1), instance(2), sessionID(4), synchronous(1) = 8 bytes. +// We send zeros for everything (redistDefault=0 means ZEBRA_ROUTE_SYSTEM). func EncodeHello() []byte { - body := make([]byte, 12) // redistDefault + instance + sessionID + receiveNotify + synchronous + flags - return body + return make([]byte, 8) } // EncodeRouterIDAdd builds a RouterIDAdd message body. @@ -248,32 +256,41 @@ func ReadMessage(r io.Reader) (Header, []byte, error) { // // type(1) instance(2) flags(4) message(4) safi(1) family(1) prefixlen(1) prefix(var) ... func DecodeRoute(body []byte) (*Route, error) { + return DecodeRouteLog(body, nil) +} + +// DecodeRouteLog is like DecodeRoute but logs decode positions if log is non-nil. +func DecodeRouteLog(body []byte, log *slog.Logger) (*Route, error) { if len(body) < 10 { return nil, fmt.Errorf("route body too short: %d bytes", len(body)) } + if log != nil { + log.Debug("zapi decode: raw body", "len", len(body), "hex", hex.EncodeToString(body)) + } + r := &Route{} pos := 0 - // type(1) r.Type = RouteType(body[pos]) pos++ // instance(2) pos += 2 - // flags(4) r.Flags = binary.BigEndian.Uint32(body[pos : pos+4]) pos += 4 - // message(4) — FRR >= 7.5 uses 4-byte message field r.Message = MsgFlag(binary.BigEndian.Uint32(body[pos : pos+4])) pos += 4 + if log != nil { + log.Debug("zapi decode: header", "type", r.Type, "flags", r.Flags, "message", fmt.Sprintf("0x%x", r.Message), "pos", pos) + } + // safi(1) pos++ - // family(1) if pos >= len(body) { return nil, fmt.Errorf("truncated at family") } @@ -285,14 +302,12 @@ func DecodeRoute(body []byte) (*Route, error) { return nil, err } - // prefixlen(1) if pos >= len(body) { return nil, fmt.Errorf("truncated at prefixlen") } prefixLen := body[pos] pos++ - // prefix (ceil(prefixLen/8) bytes) byteLen := int((prefixLen + 7) / 8) if pos+byteLen > len(body) { return nil, fmt.Errorf("truncated at prefix data") @@ -310,7 +325,10 @@ func DecodeRoute(body []byte) (*Route, error) { mask := net.CIDRMask(int(prefixLen), addrLen*8) r.Prefix = net.IPNet{IP: ip, Mask: mask} - // source prefix (if MsgSrcPfx set) + if log != nil { + log.Debug("zapi decode: prefix", "prefix", r.Prefix.String(), "pos", pos) + } + if r.Message&MsgSrcPfx != 0 { if pos >= len(body) { return nil, fmt.Errorf("truncated at src prefix") @@ -324,34 +342,38 @@ func DecodeRoute(body []byte) (*Route, error) { pos += srcByteLen } - // NHG (if MsgNHG set, frr >= 8) if r.Message&MsgNHG != 0 { if pos+4 > len(body) { return nil, fmt.Errorf("truncated at nhg") } - pos += 4 // skip nhgid + pos += 4 } - // Nexthops if r.Message&MsgNexthop != 0 { if pos+2 > len(body) { return nil, fmt.Errorf("truncated at nexthop count") } numNH := binary.BigEndian.Uint16(body[pos : pos+2]) pos += 2 - r.Nexthops = make([]Nexthop, 0, numNH) + if log != nil { + log.Debug("zapi decode: nexthops", "count", numNH, "pos", pos) + } + + r.Nexthops = make([]Nexthop, 0, numNH) for i := uint16(0); i < numNH; i++ { - nh, n, err := decodeNexthop(body[pos:], family) + nh, n, err := decodeNexthop(body[pos:], r.Message) if err != nil { return nil, fmt.Errorf("nexthop %d: %w", i, err) } + if log != nil { + log.Debug("zapi decode: nexthop", "i", i, "type", nh.Type, "gate", nh.Gate, "ifindex", nh.Ifindex, "consumed", n, "nextPos", pos+n) + } r.Nexthops = append(r.Nexthops, nh) pos += n } } - // Backup nexthops (skip) if r.Message&MsgBackupNH != 0 { if pos+2 > len(body) { return nil, fmt.Errorf("truncated at backup nexthop count") @@ -359,7 +381,7 @@ func DecodeRoute(body []byte) (*Route, error) { numBackup := binary.BigEndian.Uint16(body[pos : pos+2]) pos += 2 for i := uint16(0); i < numBackup; i++ { - _, n, err := decodeNexthop(body[pos:], family) + _, n, err := decodeNexthop(body[pos:], r.Message) if err != nil { return nil, fmt.Errorf("backup nexthop %d: %w", i, err) } @@ -367,25 +389,28 @@ func DecodeRoute(body []byte) (*Route, error) { } } - // Distance if r.Message&MsgDistance != 0 { if pos >= len(body) { return nil, fmt.Errorf("truncated at distance") } r.Distance = body[pos] + if log != nil { + log.Debug("zapi decode: distance", "distance", r.Distance, "pos", pos, "byte", fmt.Sprintf("0x%02x", body[pos])) + } pos++ } - // Metric if r.Message&MsgMetric != 0 { if pos+4 > len(body) { return nil, fmt.Errorf("truncated at metric") } r.Metric = binary.BigEndian.Uint32(body[pos : pos+4]) + if log != nil { + log.Debug("zapi decode: metric", "metric", r.Metric, "pos", pos) + } pos += 4 } - // Tag if r.Message&MsgTag != 0 { if pos+4 > len(body) { return nil, fmt.Errorf("truncated at tag") @@ -394,7 +419,6 @@ func DecodeRoute(body []byte) (*Route, error) { pos += 4 } - // MTU if r.Message&MsgMTU != 0 { if pos+4 > len(body) { return nil, fmt.Errorf("truncated at mtu") @@ -403,127 +427,120 @@ func DecodeRoute(body []byte) (*Route, error) { pos += 4 } - // Remaining fields (tableID, SRTE, opaque) are not needed. return r, nil } -// decodeNexthop decodes one nexthop from the body. -// FRR >= 7.3 / ZAPI v6 format: vrfID(4) type(1) flags(1) [gate] [ifindex] ... -func decodeNexthop(data []byte, family uint8) (Nexthop, int, error) { +// decodeNexthop decodes one nexthop matching FRR 10.5 zapi_nexthop_decode. +// Wire format: vrfID(4) type(1) flags(1) [gate/ifindex] [labels] [weight] +// [rmac] [srte_color] [backup] [seg6local] [seg6] +func decodeNexthop(data []byte, routeMsg MsgFlag) (Nexthop, int, error) { nh := Nexthop{} pos := 0 - // vrfID(4) - if pos+4 > len(data) { - return nh, 0, fmt.Errorf("truncated at vrf_id") - } - pos += 4 // skip vrfID - - // type(1) - if pos >= len(data) { - return nh, 0, fmt.Errorf("truncated at nh type") + if pos+6 > len(data) { + return nh, 0, fmt.Errorf("truncated at nh header") } + pos += 4 // vrfID nh.Type = NHType(data[pos]) pos++ - - // flags(1) — FRR >= 7.3 - if pos >= len(data) { - return nh, 0, fmt.Errorf("truncated at nh flags") - } flags := data[pos] pos++ - // For FRR >= 7.3, IPv4 and IPv6 are treated as IPv4IFIndex and IPv6IFIndex - // respectively (nexthopProcessIPToIPIFindex). We do the same mapping. - nhType := nh.Type - switch nhType { - case NHIPv4: - nhType = NHIPv4IFIndex - case NHIPv6: - nhType = NHIPv6IFIndex - } - - // Decode gate address - switch nhType { - case NHIPv4IFIndex: - if pos+4 > len(data) { - return nh, 0, fmt.Errorf("truncated at ipv4 gate") + switch nh.Type { + case NHBlackhole: + if pos >= len(data) { + return nh, 0, fmt.Errorf("truncated at blackhole type") + } + pos++ + case NHIPv4, NHIPv4IFIndex: + if pos+8 > len(data) { + return nh, 0, fmt.Errorf("truncated at ipv4 gate+ifindex") } nh.Gate = net.IP(data[pos : pos+4]).To4() pos += 4 - case NHIPv6IFIndex: - if pos+16 > len(data) { - return nh, 0, fmt.Errorf("truncated at ipv6 gate") - } - nh.Gate = net.IP(data[pos : pos+16]).To16() - pos += 16 - } - - // Decode ifindex - switch nhType { - case NHIFIndex, NHIPv4IFIndex, NHIPv6IFIndex: + nh.Ifindex = binary.BigEndian.Uint32(data[pos : pos+4]) + pos += 4 + case NHIFIndex: if pos+4 > len(data) { return nh, 0, fmt.Errorf("truncated at ifindex") } nh.Ifindex = binary.BigEndian.Uint32(data[pos : pos+4]) pos += 4 - } - - // Blackhole type - if nhType == NHBlackhole { - if pos >= len(data) { - return nh, 0, fmt.Errorf("truncated at blackhole type") + case NHIPv6, NHIPv6IFIndex: + if pos+20 > len(data) { + return nh, 0, fmt.Errorf("truncated at ipv6 gate+ifindex") } - pos++ // skip blackhole type byte + nh.Gate = net.IP(data[pos : pos+16]).To16() + pos += 16 + nh.Ifindex = binary.BigEndian.Uint32(data[pos : pos+4]) + pos += 4 } - // Labels (if flag set) if flags&nhFlagLabel != 0 { - if pos >= len(data) { - return nh, 0, fmt.Errorf("truncated at label count") + if pos+2 > len(data) { + return nh, 0, fmt.Errorf("truncated at labels") } labelNum := int(data[pos]) pos++ + pos++ // label_type (1 byte) if labelNum > 16 { labelNum = 16 } - pos += labelNum * 4 // skip label data + pos += labelNum * 4 } - // Weight (if flag set) if flags&nhFlagWeight != 0 { - if pos+4 > len(data) { + if pos+8 > len(data) { return nh, 0, fmt.Errorf("truncated at weight") } + pos += 8 // uint64 + } + + if flags&nhFlagEVPN != 0 { + if pos+6 > len(data) { + return nh, 0, fmt.Errorf("truncated at evpn rmac") + } + pos += 6 // struct ethaddr + } + + if routeMsg&MsgSRTE != 0 { + if pos+4 > len(data) { + return nh, 0, fmt.Errorf("truncated at srte color") + } pos += 4 } - // SRTE color — present when message has MsgSRTE flag. - // We can't check message flags here; instead rely on the frr >=7.5 - // behavior: SRTE color is only present if the SRTE message flag was - // set on the route. The caller handles this by not passing SRTE routes - // to us (we only subscribe to simple route types). For safety, we skip - // nothing here — SRTE is handled at the route level if needed. - - // Backup nexthop indices (if flag set) if flags&nhFlagHasBackup != 0 { if pos >= len(data) { return nh, 0, fmt.Errorf("truncated at backup count") } backupNum := int(data[pos]) pos++ - pos += backupNum // skip backup indices + pos += backupNum } - // SEG6 (if flag set) - if flags&nhFlagSeg6 != 0 { - // seg6local_action(4) + seg6local_context(4+16+4 = 24) + // SEG6LOCAL comes before SEG6 in FRR's decode order + if flags&nhFlagSeg6Local != 0 { + // seg6local_action(4) + seg6local_context(sizeof(struct seg6local_context)) + // struct seg6local_context is 24 bytes in FRR (nh_seg.h) + if pos+28 > len(data) { + return nh, 0, fmt.Errorf("truncated at seg6local") + } pos += 28 } - // SEG6 local (if flag set) - if flags&nhFlagSeg6Local != 0 { - pos += 16 // struct in6_addr + if flags&nhFlagSeg6 != 0 { + if pos >= len(data) { + return nh, 0, fmt.Errorf("truncated at seg6 count") + } + segNum := int(data[pos]) + pos++ + // segs(segNum*16) + behavior(4) + skip := segNum*16 + 4 + if pos+skip > len(data) { + return nh, 0, fmt.Errorf("truncated at seg6 data") + } + pos += skip } return nh, pos, nil diff --git a/src/yangerd/internal/zapi/zapi_test.go b/src/yangerd/internal/zapi/zapi_test.go index 165636b8..92521e00 100644 --- a/src/yangerd/internal/zapi/zapi_test.go +++ b/src/yangerd/internal/zapi/zapi_test.go @@ -256,6 +256,118 @@ type testNexthop struct { ifindex uint32 } +// TestDecodeRouteFRRRedistribute tests the exact wire format that FRR's +// zsend_redistribute_route produces: message flags 0x17 = nexthop|distance|metric|mtu. +func TestDecodeRouteFRRRedistribute(t *testing.T) { + msgFlags := uint32(MsgNexthop | MsgDistance | MsgMetric | MsgMTU) + + var body []byte + body = append(body, uint8(RouteRIP)) // type + body = append(body, 0, 0) // instance + tmp := make([]byte, 4) + binary.BigEndian.PutUint32(tmp, 0) // flags + body = append(body, tmp...) + binary.BigEndian.PutUint32(tmp, msgFlags) + body = append(body, tmp...) + body = append(body, 1) // safi=unicast + body = append(body, syscall.AF_INET) // family + body = append(body, 24) // prefixlen + body = append(body, 192, 168, 10) // prefix (3 bytes for /24) + + // 1 nexthop: type=IFINDEX, flags=0, ifindex=5 + binary.BigEndian.PutUint16(tmp[:2], 1) // nexthop count + body = append(body, tmp[:2]...) + body = append(body, 0, 0, 0, 0) // vrfID + body = append(body, uint8(NHIFIndex)) // nh type + body = append(body, 0) // nh flags + binary.BigEndian.PutUint32(tmp, 5) // ifindex + body = append(body, tmp...) + + body = append(body, 120) // distance (RIP=120) + binary.BigEndian.PutUint32(tmp, 3) // metric + body = append(body, tmp...) + binary.BigEndian.PutUint32(tmp, 1500) // mtu + body = append(body, tmp...) + + route, err := DecodeRoute(body) + if err != nil { + t.Fatal(err) + } + if route.Type != RouteRIP { + t.Errorf("Type = %d, want %d", route.Type, RouteRIP) + } + if route.Prefix.String() != "192.168.10.0/24" { + t.Errorf("Prefix = %s, want 192.168.10.0/24", route.Prefix.String()) + } + if route.Distance != 120 { + t.Errorf("Distance = %d, want 120", route.Distance) + } + if route.Metric != 3 { + t.Errorf("Metric = %d, want 3", route.Metric) + } + if route.MTU != 1500 { + t.Errorf("MTU = %d, want 1500", route.MTU) + } + if len(route.Nexthops) != 1 { + t.Fatalf("Nexthops = %d, want 1", len(route.Nexthops)) + } + if route.Nexthops[0].Ifindex != 5 { + t.Errorf("Ifindex = %d, want 5", route.Nexthops[0].Ifindex) + } +} + +// TestDecodeRouteWithTag tests message flags 0x1f = nexthop|distance|metric|tag|mtu. +func TestDecodeRouteWithTag(t *testing.T) { + msgFlags := uint32(MsgNexthop | MsgDistance | MsgMetric | MsgTag | MsgMTU) + + var body []byte + body = append(body, uint8(RouteOSPF)) // type + body = append(body, 0, 0) // instance + tmp := make([]byte, 4) + binary.BigEndian.PutUint32(tmp, 0) + body = append(body, tmp...) // flags + binary.BigEndian.PutUint32(tmp, msgFlags) + body = append(body, tmp...) // message + body = append(body, 1) // safi + body = append(body, syscall.AF_INET) // family + body = append(body, 32) // prefixlen + body = append(body, 10, 1, 1, 1) // prefix + + binary.BigEndian.PutUint16(tmp[:2], 1) + body = append(body, tmp[:2]...) // nexthop count + body = append(body, 0, 0, 0, 0) // vrfID + body = append(body, uint8(NHIPv4IFIndex)) + body = append(body, 0) // nh flags + body = append(body, 10, 0, 0, 1) // gate + binary.BigEndian.PutUint32(tmp, 3) + body = append(body, tmp...) // ifindex + + body = append(body, 110) // distance (OSPF=110) + binary.BigEndian.PutUint32(tmp, 20) + body = append(body, tmp...) // metric + binary.BigEndian.PutUint32(tmp, 42) + body = append(body, tmp...) // tag + binary.BigEndian.PutUint32(tmp, 9000) + body = append(body, tmp...) // mtu + + route, err := DecodeRoute(body) + if err != nil { + t.Fatal(err) + } + if route.Distance != 110 { + t.Errorf("Distance = %d, want 110", route.Distance) + } + if route.Metric != 20 { + t.Errorf("Metric = %d, want 20", route.Metric) + } + if route.Tag != 42 { + t.Errorf("Tag = %d, want 42", route.Tag) + } + if route.MTU != 9000 { + t.Errorf("MTU = %d, want 9000", route.MTU) + } +} + func buildRouteBody(t *testing.T, family uint8, prefix net.IP, prefixLen uint8, msgFlags uint32, routeType RouteType, flags uint32, nexthops []testNexthop, distance uint8, metric uint32) []byte { t.Helper() var buf []byte diff --git a/src/yangerd/internal/zapiwatcher/zapiwatcher.go b/src/yangerd/internal/zapiwatcher/zapiwatcher.go index 540c860c..fc36bd0b 100644 --- a/src/yangerd/internal/zapiwatcher/zapiwatcher.go +++ b/src/yangerd/internal/zapiwatcher/zapiwatcher.go @@ -7,6 +7,7 @@ import ( "log/slog" "math" "net" + "sort" "strconv" "strings" "sync" @@ -42,11 +43,18 @@ var routeTypeToProtocol = map[zapi.RouteType]string{ zapi.RouteRIP: "ietf-rip:rip", } +// routeEntry pairs a ZAPI route with the wall-clock time it was first +// received. The timestamp is used for the YANG last-updated leaf. +type routeEntry struct { + route *zapi.Route + receivedAt time.Time +} + type ZAPIWatcher struct { tree *tree.Tree log *slog.Logger mu sync.Mutex - routes map[string]json.RawMessage + routes map[string]*routeEntry } const routingTreeKey = "ietf-routing:routing" @@ -55,7 +63,7 @@ func New(t *tree.Tree, log *slog.Logger) *ZAPIWatcher { if log == nil { log = slog.Default() } - return &ZAPIWatcher{tree: t, log: log, routes: make(map[string]json.RawMessage)} + return &ZAPIWatcher{tree: t, log: log, routes: make(map[string]*routeEntry)} } func (w *ZAPIWatcher) Run(ctx context.Context) error { @@ -149,7 +157,7 @@ func (w *ZAPIWatcher) handleMessage(hdr zapi.Header, body []byte) { switch hdr.Command { case zapi.CmdRedistRouteAdd: - route, err := zapi.DecodeRoute(body) + route, err := zapi.DecodeRouteLog(body, w.log) if err != nil { w.log.Warn("zapi watcher: decode route add", "err", err, "bodyLen", len(body)) return @@ -177,12 +185,34 @@ func (w *ZAPIWatcher) handleMessage(hdr zapi.Header, body []byte) { } } -func (w *ZAPIWatcher) addRoute(route *zapi.Route) { +func routeKey(route *zapi.Route) string { rib := ribName(route.Prefix) - key := rib + ":" + route.Prefix.String() + ":" + routeProtocol(route.Type) + proto := routeProtocol(route.Type) + + gates := make([]string, 0, len(route.Nexthops)) + for _, nh := range route.Nexthops { + if len(nh.Gate) > 0 && !nh.Gate.IsUnspecified() { + gates = append(gates, nh.Gate.String()) + } + } + sort.Strings(gates) + + return rib + ":" + route.Prefix.String() + ":" + proto + ":" + strings.Join(gates, ",") +} + +func routeKeyPrefix(route *zapi.Route) string { + return ribName(route.Prefix) + ":" + route.Prefix.String() + ":" + routeProtocol(route.Type) + ":" +} + +func (w *ZAPIWatcher) addRoute(route *zapi.Route) { + key := routeKey(route) + now := time.Now() w.mu.Lock() - w.routes[key] = transformRoute(route) + if existing, ok := w.routes[key]; ok { + now = existing.receivedAt + } + w.routes[key] = &routeEntry{route: route, receivedAt: now} routeCount := len(w.routes) w.mu.Unlock() @@ -191,11 +221,19 @@ func (w *ZAPIWatcher) addRoute(route *zapi.Route) { } func (w *ZAPIWatcher) deleteRoute(route *zapi.Route) { - rib := ribName(route.Prefix) - key := rib + ":" + route.Prefix.String() + ":" + routeProtocol(route.Type) + key := routeKey(route) w.mu.Lock() - delete(w.routes, key) + if _, ok := w.routes[key]; ok { + delete(w.routes, key) + } else { + prefix := routeKeyPrefix(route) + for k := range w.routes { + if strings.HasPrefix(k, prefix) { + delete(w.routes, k) + } + } + } w.mu.Unlock() w.writeRibs() @@ -203,20 +241,56 @@ func (w *ZAPIWatcher) deleteRoute(route *zapi.Route) { func (w *ZAPIWatcher) clearAllRoutes() { w.mu.Lock() - w.routes = make(map[string]json.RawMessage) + w.routes = make(map[string]*routeEntry) w.mu.Unlock() w.writeRibs() } +// destKey returns the grouping key for active route computation: +// "rib:prefix" — all routes sharing the same destination compete. +func destKey(route *zapi.Route) string { + return ribName(route.Prefix) + ":" + route.Prefix.String() +} + func (w *ZAPIWatcher) writeRibs() { w.mu.Lock() - ipv4Routes := make([]json.RawMessage, 0) - ipv6Routes := make([]json.RawMessage, 0) + bestDist := make(map[string]uint8) + for _, entry := range w.routes { + dk := destKey(entry.route) + if d, ok := bestDist[dk]; !ok || entry.route.Distance < d { + bestDist[dk] = entry.route.Distance + } + } - for key, routeData := range w.routes { - if strings.HasPrefix(key, "ipv4:") { + // Collect entries into a slice for deterministic ordering. + allEntries := make([]*routeEntry, 0, len(w.routes)) + for _, entry := range w.routes { + allEntries = append(allEntries, entry) + } + + // Sort by destination prefix so routes for the same destination are + // grouped together. Within the same prefix, lowest distance first + // (active route on top). + sort.Slice(allEntries, func(i, j int) bool { + a, b := allEntries[i].route, allEntries[j].route + ap, bp := a.Prefix.String(), b.Prefix.String() + if ap != bp { + return ap < bp + } + return a.Distance < b.Distance + }) + + ipv4Routes := make([]json.RawMessage, 0, len(allEntries)) + ipv6Routes := make([]json.RawMessage, 0, len(allEntries)) + + for _, entry := range allEntries { + dk := destKey(entry.route) + active := entry.route.Distance == bestDist[dk] + routeData := transformRoute(entry.route, active, entry.receivedAt) + + if entry.route.Prefix.IP.To4() != nil { ipv4Routes = append(ipv4Routes, routeData) } else { ipv6Routes = append(ipv6Routes, routeData) @@ -259,14 +333,14 @@ func ribName(prefix net.IPNet) string { return "ipv6" } -func transformRoute(route *zapi.Route) json.RawMessage { +func transformRoute(route *zapi.Route, active bool, receivedAt time.Time) json.RawMessage { isIPv4 := route.Prefix.IP.To4() != nil addrKey := "ietf-ipv6-unicast-routing:address" - destKey := "ietf-ipv6-unicast-routing:destination-prefix" + dpKey := "ietf-ipv6-unicast-routing:destination-prefix" if isIPv4 { addrKey = "ietf-ipv4-unicast-routing:address" - destKey = "ietf-ipv4-unicast-routing:destination-prefix" + dpKey = "ietf-ipv4-unicast-routing:destination-prefix" } nextHops := make([]map[string]any, 0, len(route.Nexthops)) @@ -292,9 +366,10 @@ func transformRoute(route *zapi.Route) json.RawMessage { } routeNode := map[string]any{ - destKey: route.Prefix.String(), + dpKey: route.Prefix.String(), "source-protocol": routeProtocol(route.Type), "route-preference": route.Distance, + "last-updated": receivedAt.Format(time.RFC3339), "next-hop": map[string]any{ "next-hop-list": map[string]any{ "next-hop": nextHops, @@ -302,6 +377,10 @@ func transformRoute(route *zapi.Route) json.RawMessage { }, } + if active { + routeNode["active"] = []any{nil} + } + encoded, err := json.Marshal(routeNode) if err != nil { return json.RawMessage(`{}`) diff --git a/src/yangerd/internal/zapiwatcher/zapiwatcher_test.go b/src/yangerd/internal/zapiwatcher/zapiwatcher_test.go index 39904ad8..d44c4619 100644 --- a/src/yangerd/internal/zapiwatcher/zapiwatcher_test.go +++ b/src/yangerd/internal/zapiwatcher/zapiwatcher_test.go @@ -4,7 +4,9 @@ import ( "encoding/json" "net" "testing" + "time" + "github.com/kernelkit/infix/src/yangerd/internal/tree" "github.com/kernelkit/infix/src/yangerd/internal/zapi" ) @@ -77,7 +79,8 @@ func TestTransformRoute(t *testing.T) { }, } - result := transformRoute(route) + received := time.Date(2025, 6, 15, 10, 30, 0, 0, time.UTC) + result := transformRoute(route, false, received) var parsed map[string]any if err := json.Unmarshal(result, &parsed); err != nil { @@ -111,6 +114,201 @@ func TestTransformRoute(t *testing.T) { if len(hops) != 0 { t.Errorf("expected 0 next-hops, got %d", len(hops)) } + + if _, ok := parsed["active"]; ok { + t.Error("non-selected route should not have 'active' leaf") + } + + lastUpdated, ok := parsed["last-updated"].(string) + if !ok { + t.Fatal("last-updated missing or not a string") + } + if lastUpdated != "2025-06-15T10:30:00Z" { + t.Errorf("last-updated = %q, want %q", lastUpdated, "2025-06-15T10:30:00Z") + } +} + +func TestTransformRouteActiveParam(t *testing.T) { + route := &zapi.Route{ + Type: zapi.RouteStatic, + Distance: 5, + Prefix: net.IPNet{ + IP: net.ParseIP("0.0.0.0").To4(), + Mask: net.CIDRMask(0, 32), + }, + } + + result := transformRoute(route, true, time.Now()) + + var parsed map[string]any + if err := json.Unmarshal(result, &parsed); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + active, ok := parsed["active"].([]any) + if !ok { + t.Fatalf("active leaf missing or wrong type: %v", parsed["active"]) + } + if len(active) != 1 || active[0] != nil { + t.Errorf("active = %v, want [null]", active) + } +} + +func TestRouteKeyDifferentGateways(t *testing.T) { + dhcp := &zapi.Route{ + Type: zapi.RouteStatic, + Distance: 5, + Prefix: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Nexthops: []zapi.Nexthop{{ + Gate: net.ParseIP("192.168.10.3").To4(), + }}, + } + static := &zapi.Route{ + Type: zapi.RouteStatic, + Distance: 120, + Prefix: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Nexthops: []zapi.Nexthop{{ + Gate: net.ParseIP("192.168.50.2").To4(), + }}, + } + + keyDHCP := routeKey(dhcp) + keyStatic := routeKey(static) + + if keyDHCP == keyStatic { + t.Errorf("routes with different gateways must have different keys: %q == %q", keyDHCP, keyStatic) + } +} + +func TestRouteKeySameGatewayDifferentDistance(t *testing.T) { + old := &zapi.Route{ + Type: zapi.RouteStatic, + Distance: 120, + Prefix: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Nexthops: []zapi.Nexthop{{ + Gate: net.ParseIP("192.168.50.2").To4(), + }}, + } + updated := &zapi.Route{ + Type: zapi.RouteStatic, + Distance: 1, + Prefix: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Nexthops: []zapi.Nexthop{{ + Gate: net.ParseIP("192.168.50.2").To4(), + }}, + } + + if routeKey(old) != routeKey(updated) { + t.Errorf("same gateway routes must share a key regardless of distance: %q != %q", routeKey(old), routeKey(updated)) + } +} + +func TestAddRouteSamePrefixDifferentDistance(t *testing.T) { + tr := tree.New() + w := New(tr, nil) + + dhcpRoute := &zapi.Route{ + Type: zapi.RouteStatic, + Distance: 5, + Prefix: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Nexthops: []zapi.Nexthop{{ + Type: zapi.NHIPv4IFIndex, + Gate: net.ParseIP("192.168.10.3").To4(), + }}, + } + staticRoute := &zapi.Route{ + Type: zapi.RouteStatic, + Distance: 120, + Prefix: net.IPNet{ + IP: net.IPv4(0, 0, 0, 0), + Mask: net.CIDRMask(0, 32), + }, + Nexthops: []zapi.Nexthop{{ + Type: zapi.NHIPv4IFIndex, + Gate: net.ParseIP("192.168.50.2").To4(), + }}, + } + + w.addRoute(dhcpRoute) + w.addRoute(staticRoute) + + w.mu.Lock() + count := len(w.routes) + w.mu.Unlock() + + if count != 2 { + t.Errorf("expected 2 routes, got %d", count) + } + + data := tr.Get(routingTreeKey) + if data == nil { + t.Fatal("routing tree key not set") + } + + var routing map[string]any + if err := json.Unmarshal(data, &routing); err != nil { + t.Fatalf("unmarshal routing: %v", err) + } + + ribs, ok := routing["ribs"].(map[string]any) + if !ok { + t.Fatal("ribs not found") + } + ribList, ok := ribs["rib"].([]any) + if !ok { + t.Fatal("rib list not found") + } + + var ipv4Routes []any + for _, rib := range ribList { + ribMap := rib.(map[string]any) + if ribMap["name"] == "ipv4" { + routes := ribMap["routes"].(map[string]any) + ipv4Routes = routes["route"].([]any) + break + } + } + + if len(ipv4Routes) != 2 { + t.Fatalf("expected 2 IPv4 routes, got %d", len(ipv4Routes)) + } + + var activeCount int + for _, r := range ipv4Routes { + rm := r.(map[string]any) + pref := int(rm["route-preference"].(float64)) + _, hasActive := rm["active"] + if hasActive { + activeCount++ + if pref != 5 { + t.Errorf("active route has preference %d, want 5", pref) + } + } + if pref == 120 && hasActive { + t.Error("route with preference 120 should not be active") + } + if _, ok := rm["last-updated"].(string); !ok { + t.Errorf("route with preference %d missing last-updated", pref) + } + } + if activeCount != 1 { + t.Errorf("expected exactly 1 active route, got %d", activeCount) + } } func TestTransformRouteIPv6(t *testing.T) { @@ -124,7 +322,7 @@ func TestTransformRouteIPv6(t *testing.T) { }, } - result := transformRoute(route) + result := transformRoute(route, false, time.Now()) var parsed map[string]any if err := json.Unmarshal(result, &parsed); err != nil { @@ -138,3 +336,100 @@ func TestTransformRouteIPv6(t *testing.T) { t.Errorf("source-protocol = %v, want %q", got, "ietf-ospf:ospfv2") } } + +func TestAddRouteDistanceChangeOverwrites(t *testing.T) { + tr := tree.New() + w := New(tr, nil) + + gateway := net.ParseIP("192.168.50.2").To4() + prefix := net.IPNet{IP: net.IPv4(0, 0, 0, 0), Mask: net.CIDRMask(0, 32)} + + w.addRoute(&zapi.Route{ + Type: zapi.RouteStatic, + Distance: 120, + Prefix: prefix, + Nexthops: []zapi.Nexthop{{Type: zapi.NHIPv4IFIndex, Gate: gateway}}, + }) + + w.addRoute(&zapi.Route{ + Type: zapi.RouteStatic, + Distance: 1, + Prefix: prefix, + Nexthops: []zapi.Nexthop{{Type: zapi.NHIPv4IFIndex, Gate: gateway}}, + }) + + w.mu.Lock() + count := len(w.routes) + w.mu.Unlock() + + if count != 1 { + t.Errorf("same-gateway route should overwrite on distance change: got %d routes, want 1", count) + } + + data := tr.Get(routingTreeKey) + if data == nil { + t.Fatal("routing tree key not set") + } + + var routing map[string]any + if err := json.Unmarshal(data, &routing); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + ribs := routing["ribs"].(map[string]any) + ribList := ribs["rib"].([]any) + var ipv4Routes []any + for _, rib := range ribList { + ribMap := rib.(map[string]any) + if ribMap["name"] == "ipv4" { + ipv4Routes = ribMap["routes"].(map[string]any)["route"].([]any) + break + } + } + + if len(ipv4Routes) != 1 { + t.Fatalf("expected 1 route, got %d", len(ipv4Routes)) + } + + rm := ipv4Routes[0].(map[string]any) + if pref := int(rm["route-preference"].(float64)); pref != 1 { + t.Errorf("route-preference = %d, want 1 (updated distance)", pref) + } + if _, hasActive := rm["active"]; !hasActive { + t.Error("sole route should be active") + } +} + +func TestDeleteRouteWithoutNexthops(t *testing.T) { + tr := tree.New() + w := New(tr, nil) + + gateway := net.ParseIP("192.168.10.3").To4() + prefix := net.IPNet{IP: net.IPv4(0, 0, 0, 0), Mask: net.CIDRMask(0, 32)} + + w.addRoute(&zapi.Route{ + Type: zapi.RouteStatic, + Distance: 5, + Prefix: prefix, + Nexthops: []zapi.Nexthop{{Type: zapi.NHIPv4IFIndex, Gate: gateway}}, + }) + + w.mu.Lock() + count := len(w.routes) + w.mu.Unlock() + if count != 1 { + t.Fatalf("expected 1 route after add, got %d", count) + } + + w.deleteRoute(&zapi.Route{ + Type: zapi.RouteStatic, + Prefix: prefix, + }) + + w.mu.Lock() + count = len(w.routes) + w.mu.Unlock() + if count != 0 { + t.Errorf("expected 0 routes after delete without nexthops, got %d", count) + } +}