Jasmeet Bagga

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

Reject invalid link-local nexthops during RIB updates Summary: Validate newly added next-hop-id sets before returning a RIB-derived switch state so routes or MySIDs with link-local next hops that reference nonexistent interfaces fail as `FbossHwUpdateError`. Validate nhops here and not in `fibUpdater_`: with nhop ID assignment, the `fibInfoMaps` do not have the updated `nhopIds` during FIB construction, so we cannot look up nhop contents there; invalid nhops may also get associated with MySIDs. Also seed the RIB LPM and rollback test states with interface objects, and add RouteTests coverage that valid routes plus an invalid link-local route leave switch state and RIB-visible routes unchanged after the failed update. Reviewed By: zechengh09 Differential Revision: D109664986 fbshipit-source-id: 3d5c0544fcb1572e3c77eaddd1e0553f97ee7ed0

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

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

Program backup next-hop routes using protection groups Summary: Select `SAI_NEXT_HOP_GROUP_TYPE_PROTECTION` for route next-hop sets containing backups. Use `SaiNextHopGroupKey::groupType` when creating the parent, create its backup child as `SAI_NEXT_HOP_GROUP_TYPE_HW_PROTECTION`, and choose members based on the explicit group type. Keep protection-specific APIs behind SAI 1.16 guards and extend next-hop manager coverage for backup-only groups. landed-with-radar-review Reviewed By: msomasundaran Differential Revision: D115496130 fbshipit-source-id: 94a3790ccd2a458258c772b6e0f31cc7e7501fbc

Support NextHopRole in RouteNextHop Summary: Preserve the NextHopRole field through RouteNextHop conversion, comparison, equality, and hashing. Reviewed By: tanmayaudupa, shri-khare Differential Revision: D114245002 fbshipit-source-id: bdabe30d76f59e9d247fcdda72a36056bf732e5a