Jasmeet Bagga

Treat defaulted weight (0) as 1 when resolving UCMP weights Summary: If we got a mix of weighted and unweighted next hops all next hops were silently being made unweighted. So a route with next hops of weights, say R1 -> nh1 (5), nh2 (ECMP_WEIGHT - default) would become - R1 -> nh1 (ECMP_WEIGHT - default), nh2 (ECMP_WEIGHT - default). This is very counter intuitive. Instead treat this default weight as a weight of 1 when doing weight calculations for UCMP. Routes with no weighted next hop are unaffected: they took the flattening path before and still do. This diff deliberately does not change the case where such a route resolves onto next hops that *are* weighted — that set still flattens, because it never reaches `combineWeights()`. Reviewed By: tanmayaudupa Differential Revision: D122067708 fbshipit-source-id: 9ef80ee845bedf4ce9c47e58a26cf5b7426a8b92

Prune backup next hops that duplicate a primary during resolution Summary: Wires `removeBackupNextHopsWithMatchingPrimary` into `RibRouteUpdater::resolveOne` so a route never ends up programming the same egress twice, once as `PRIMARY` and once as `BACKUP`. A backup next hop that recursively resolves onto the same `(intfID, addr)` as one of the route's primaries offers no additional protection, since the two share a failure domain. The dedup runs on the merged forwarding set, before the resolution cache insert and before capacity pruning normalization, so the cached and normalized sets both see the trimmed result. Adds a `PrimaryBackupDedupTest` fixture covering recursive resolution. Its `SetUp` programs `10::/64`, `20::/64`, `30::/64` and `40::/64` as interface routes and `100::/64` as a route resolving to all four, so a BGP route using `100::1` as a backup picks up `10::1`, `20::2`, `30::3` and `40::4` as backups, any of which may duplicate a primary. Coverage: - backups with no primary at all are left alone - primaries with no backups are left alone (the no-backup fast path) - one primary matching the first recursive next hop - one primary matching a later recursive next hop - two primaries, both matching backups pruned - transition out of a `DROP` route Every case with both roles present is driven through three arrival orders - primaries first, backups first, and both together - on separate prefixes, and all three must converge on the same resolved set. The intermediate states are asserted too, so a first update that silently did nothing cannot make an ordering collapse into the both-together case and still pass. Reviewed By: zechengh09, ravi861 Differential Revision: D117622457 fbshipit-source-id: d696967a9b9ccd131e490e0a41e87ebd1859c11f

Sum weights for duplicate next hops in named next hop groups Summary: The BB team has a use case where they want to send the same next hop multiple times in a named NHG and have it become a weighted next hop. Today that silently does not work. `ThriftHandler::addNamedNextHopGroupsImpl` converts the thrift next hops with `util::toRouteNextHopSet`, which inserts into a `RouteNextHopSet` -- a `boost::container::flat_set<NextHop>` ordered by `NextHop::operator<`. That comparator includes `weight()`, so the current behaviour splits two ways and neither is what BB wants. Next hops identical in every field collapse to one and the repeat is dropped, losing its weight. Next hops naming the same destination with different weights are not recognised as the same next hop at all and survive as two separate set members. This adds an opt-in `combineDuplicateWeights` parameter to `util::toRouteNextHopSet`, defaulted to `false` so every existing caller -- unicast route add/sync, MPLS, mySid, `RouteNextHopEntry::from`, `getOverrideNextHops` -- is unaffected. `addNamedNextHopGroupsImpl` passes `true`, which covers both `addNamedNextHopGroups` and `addOrUpdateNamedNextHopGroups`. When enabled, next hops are collapsed on their identity minus weight, and the collapsed weight is the sum of `max(w, 1)` over the duplicates. The `max(w, 1)` floor matches what `RouteNextHopEntry::normalizeNextHops` already applies to `ECMP_WEIGHT`. Rather than hand-enumerating the attributes that make up that identity, which would drift as `NextHopThrift` gains fields, `weightlessKey` zeroes the weight on a copy and converts via `util::fromThrift`. The key is then compared with the same `NextHop::operator<` that the destination `flat_set` uses, so it stays in step automatically. A next hop that appears exactly once keeps its weight verbatim, including `ECMP_WEIGHT`. Without that carve-out an all-distinct ECMP group would be silently promoted to UCMP with every weight 1, and the `NextHopThrift.weight` comment in `common.thrift` explicitly warns that route resolution relies on the arithmetic properties of 0. Weights accumulate in `int64_t` because the thrift `weight` field is `i32`. A combined weight that would overflow throws `FbossError` rather than being silently clamped. The in-repo precedent for this is `NextHopCombinedWeightsKey` in `fboss/agent/rib/RouteUpdater.cpp`, which does the same weightless-key accumulation for recursive route resolution. It cannot be reused here because its constructor calls `nhop.intf()`, which throws for an `UnresolvedNextHop`, and the named NHG path can produce those. Note this leaves the same latent behaviour on the normal route programming path, where duplicates are still dropped. Enabling it there is a one-argument change per call site if we decide we want it. Reviewed By: tanmayaudupa Differential Revision: D118481342 fbshipit-source-id: 612c9e236cb9334e8c410acf794e989f3298e51b

Add per port `access_policy_state` counter Summary: Add `PortStats::setAccessPolicyState`, a per port counter mirroring `PortStats::setPortStatus`, that exports the port's `AclLookupClassPort` (`Port::userMetaData`) as `<portName>.access_policy_state`. The counter carries the raw enum value for `CLASS_PORT_UNCONSTRAINED` (0), `CLASS_PORT_RESTRICTED` (1) and `CLASS_PORT_BLOCKED` (2). Anything else -- an unset `userMetaData` or a lookup class this agent build does not know about -- is reported as `CLASS_PORT_UNCONSTRAINED`. `PortUpdateHandler` drives the counter the same way it drives `<portName>.up`: on port add, on `userMetaData` change, and re published under the new name on port rename. It is cleared on port removal and from `PortStats::clearCounters()`. Reviewed By: shri-khare Differential Revision: D118167607 fbshipit-source-id: c51c64b81ab4e1acdfc285f13233de80b21044e1

Skip ECMP switching mode for protection groups Summary: Extract `getDesiredEcmpSwitchingMode` to centralize override-versus-primary mode selection. Return `std::nullopt` for `SAI_NEXT_HOP_GROUP_TYPE_PROTECTION` parents while preserving the existing computation for ECMP and `HW_PROTECTION` backup groups. Guard adapter host key mode conversion for the empty result and add focused coverage for each case. Reviewed By: ravi861 Differential Revision: D114803640 fbshipit-source-id: 1a19aa8a94f596569a7df4aa837777d14108aa05

Add CLI command to show route counters Summary: Add `fboss2 show route counters` to retrieve route counters through `FbossCtrl.getRouteCounters()` and display counter IDs with byte and packet counts. Add a normalized CLI model, command registration, mock support, and unit tests. Reviewed By: shri-khare Differential Revision: D116113715 fbshipit-source-id: 0d9adcbc0e3d94999bb5e3642075dc42a4560e7c

Inherit nexthop role from root during route resolution Summary: Next hop role is a policy decision. That is when a route is specified as DST -> NHA (Primary), NHB (Backup) This information needs to carried through resolution even as the next hops may themselves be resolved via other next hops. E.g. NHA -> NHC NHB -> NHD The final resolution here would be DST -> NHC (Primary), NHD (Backup). Reviewed By: ravi861, tanmayaudupa Differential Revision: D114260482 fbshipit-source-id: 062cb5420adbe553cb7f160a8829ee08b4177430

Test MySID FRR backup next hops across route updates Summary: The existing adjacency FRR tests only change backup next hops via `updateMySidFrrProtection`; none of them lets a *route* update drive re-resolution. That leaves the production path untested: a backup gateway's covering route reconverging or being withdrawn must flow through `updateRib`'s automatic `RibMySidUpdater::resolve()` into `backupResolvedNextHopsId`, SwitchState and FibInfo. Add three `RibMySidFibInfoTest` cases: - `mySidFrrBackupNextHopsFollowRouteLifecycle` programs the backup before any covering route exists and asserts it reaches SwitchState unresolved, then resolves when a route appears, re-resolves when that route is re-pointed, and goes empty again when the route is withdrawn -- all without a further FRR API call. The requested (unresolved) set is asserted unchanged throughout, so only resolution moves. - `mySidFrrBackupNextHopsResolvedByPreexistingRoute` covers the inline resolve in `updateMySidFrrProtection` when the covering route is already present. - `mySidFrrBackupNextHopsTrackPartialResolution` uses two backup gateways and asserts the resolved set holds exactly those with a covering route as one appears and another is withdrawn. Each step checks the RIB `MySidTable`, `NextHopIDManager` refcounts, SwitchState and the FibInfo `IdToNextHopIdSetMap`, and that every resolved backup next hop carries `NextHopRole::BACKUP`. Supporting test helpers: `routeToSwitchStateUpdateViaRibUpdater` (route-update counterpart of `mySidToSwitchStateUpdateViaRibUpdater`, so a route update's MySid re-resolution is visible in SwitchState) and `makeGatewayRoute`, which builds routes with plain gateway next hops. `makeOpenRRoute` cannot be used here: its link-local next hops are rejected by `SwitchStateNextHopIdUpdater::isInvalidLinkLocalNextHop` in a SwitchState with no Interfaces. Reviewed By: vasantspatil Differential Revision: D120486382 fbshipit-source-id: f87394f763d8c54111e21061ea7472a9eb52dd3b

Let clients replicate weighted next hops when reading next hop groups Summary: Exposes the replication added in D118815767 over thrift, as the read-side counterpart to the `combineDuplicatedNextHops` flag D118494426 added on the write side. A client that wrote `[A, A, B]` with combining on can now ask for `[A, A, B]` back instead of `A(w=2), B`. `getNextHopGroups` and `getNamedNextHopGroups` each take `replicateWeightedNexthops`, defaulted to false, so existing callers see no change. As with the add path, the handler overrides repeat the default in C++ because a thrift default only applies on the wire. That wire-only default is also why this diff has to touch the fboss2 CLI and the EBB SRv6 driver in the same commit. Generated cpp2 clients and `ServiceHandler` virtuals take the new field positionally with no C++ default, so every call site and every gmock `override` breaks the moment the field is added. All of those hunks pass `false` and preserve today's behavior. Reviewed By: shri-khare Differential Revision: D118819371 fbshipit-source-id: e527169ad03de489aa134855c54c2d00a7f1116f

Add first adj SID FRR agent hw test Summary: Test where both primary and backup nhops are resolved ___ Differential Revision: D120610213 fbshipit-source-id: 648ee57fb743a26df452b5f208ee39fd9b4d86c8