[WIP] Modular Router Configuration by nicolaskagami · Pull Request #386 · oxidecomputer/dendrite

nicolaskagami · 2026-10-06T21:18:44Z

Context

In order to implement RFD 662 - Modular Router Configuration, the switch needs to support separate routing contexts. For context, RFD 404 - Boundary Tunnel Routing defines how we use tunneling to achieve external reachability. We're modularizing external reachability (for tunneled traffic), not implementing VRFs.

This PR focuses on separating these routing contexts while keeping strong backwards compatibility. We'll now support multiple TEP endpoints, each redirecting to a separate router configuration (endpoint selection is done in OPTE). A "default" routing context is kept for backwards compatibility, allowing legacy APIs to remain functional. Thus, it's still used for both underlay routing and the default router configuration (which we'll likely want to remove later).

Internal Changes

Route lookup keys (route_index) are enriched with an exact match on routing_table_id, used to select the routing context (lpm -> (exact, lpm)).
This routing_table_id lives entirely inside dendrite, associated with the router configuration uuid provided by the control plane.
meta.routing_table_id (8 bits) is added to ingress metadata and defaults to 0.
The claimv6 action now takes a routing_table_id and associates the registered address (TEP endpoint or other loopback) with it, enriching the packet metadata. Non-default tunnel endpoints use ids 1-255. Default endpoints and underlay use 0.
Apparently adding 8 bits exact to the keys costs 1 entry in IPV4_LPM_SIZE. IPV6 seems to be unaffected.

API Changes

(v14, VERSION_MULTI_ROUTER)

Routers

Method	Path	Body / response
GET	`/router`	`BTreeMap<Uuid, Ipv6Addr>`
GET	`/router/{router_id}`	`Ipv6Addr`
PUT	`/router/{router_id}`	`Ipv6Addr`
DELETE	`/router/{router_id}`

Routes

Method	Path
GET	`/router/{router_id}/route/{ip_family}`
GET	`/router/{router_id}/route/{ip_family}/{cidr}`
POST	`/router/{router_id}/route/{ip_family}`
PUT	`/router/{router_id}/route/{ip_family}`
DELETE	`/router/{router_id}/route/{ip_family}/{cidr}`
DELETE	`/router/{router_id}/route/{ip_family}/{cidr}/{port_id}/{link_id}/{tgt_ip}`

Architectural decisions (in decreasing order of confidence)

routing_table_id lives in dpd, not defined by the control plane (or derived from the uuid): This has many benefits, such as not introducing needless coupling, allowing switches to have different IDs and capacities in the long run and not limiting the total number of routing configurations to the minimum number any switch can support. For example, we could easily reassign the routing tables for different purposes in the future without breaking compatibility with the current API.
Endpoints vs other loopbacks: It seems that we only use loopback addresses for endpoints at the moment, though the switch seems to support them. This PR is a first step in allowing us to separate our boundary services loopbacks from whatever else we might want to use loopbacks for. Endpoints must be unique to select the routing context.
Default Router: This is the trick that allows backwards compatibility. The legacy API will implicitly use this routing context, both for the underlay routes as well as for the default router configuration. Every other router configuration goes elsewhere. We'll likely want to remove the default router in the future so we know underlay traffic routing is segregated. We can't do this now without re-routing every loopback address registration to it. /route/* returns both underlay and default router routes.
Names: Naming things is really hard. I accept suggestions here.
- The router_id name clashes with the Router ID concept. While it's identifying a (logical) router's context, it invites the wrong inference regarding its lifetime and purpose.
- The default router could be called the system routing table or something else.
- Depending on how we separate things, we could just talk about TunnelEndpoints instead of "routers".
Locking and new structs: I put the new Routers inside RouteData so they share a lock because otherwise I'd have to hold an async lock through await points because, for some reason, RouteData is locked with an async lock. Loopbacks now also have a slight dependence on this data, which just means it needs to be locked in order. There are many improvements one can do to the overall structure depending on how we want to deal with endpoints x loopbacks. The current approach is trying to impose as few new architectural decisions as possible.

companion sidecar-lite branch: nsk/662-modular-router-config

cfzimmerman

router.rs comment is most important imo. Lmk if you'd like to hop on a call to discuss. Nice work!

cfzimmerman · 2026-10-23T17:42:47Z

+    #[endpoint {
+        method = GET,
+        path = "/route/ipv6/{cidr}",
+        versions = ..,


Why this approach for API migration? Looks like this is the first instance of versions = .. instead of deprecation with versions = ..NEW_VERSION. Is this a pattern other dropshot servers use?

cfzimmerman · 2026-10-23T17:49:01Z


    /**
-     * Get the configured route for the given IPv4 subnet.
+     * Get the configured route for the given IPv4 subnet on the given router.


Reflecting on naming: Do we have a consistent definition of what a "router" is? In the context of Modular Router Configuration, this comment makes sense. But from a high level view, the word "router" feels overloaded. At least in these API docs, does it make sense to be more specific? For example, "in the given routing config."?

cfzimmerman · 2026-10-23T18:01:10Z

+    async fn route_ipv4_set(
+        rqctx: RequestContext<Self::Context>,
+        path: Path<latest::route::RouterPath>,
+        update: TypedBody<latest::route::Ipv4RouteUpdate>,
+    ) -> Result<HttpResponseUpdatedNoContent, HttpError>;


This is a higher level discussion, but I think it's relevant to this set of APIs:

DPD's api bifurcates IPv4 and IPv6 most places outside of port_settings_* functions. Presumably this is a familiar shape from systems that implemented IPv6 well after IPv4. It also guarantees a valid state when retrieving data.

However, it costs a lot of code duplication, and I would prefer to a single IpAddr API rather than distinct Ipv4Addr / Ipv6Addr APIs.

What do you think about a single route_set(IpRouteUpdate) that handles the differences internally between route_ipv4_set, route_ipv4_set_v6, route_ipv6_set, etc. Same comment for update and delete.

There are tradeoffs, but I'm at least curious to hear your thoughts

cfzimmerman · 2026-10-23T18:46:14Z

+    /**
+     * List the routers with their endpoints.
+     */
+    #[endpoint {


I'm reading this PR in file order, so perhaps my question will be answered later. But I really wonder why the existing loopback API isn't sufficient for this. Maintaining both this and loopback over the same domain of addresses feels uncomfortable.

Update: I think the discussion in the router.rs comment will answer this. Viewing router as a new "owner" over the address domain shared by link/loopback/router makes a distinct set of APIs feel reasonable.

cfzimmerman · 2026-10-23T19:34:26Z

+    /// The router being addressed.
+    pub router_id: Uuid,
+    /// The subnet being routed
+    pub cidr: Ipv4Net,


Defer to my comment in dpd-api about duplication. If we stick with separate V4 and V6 APIs, thoughts on making this generic? So define RouteTargetPath<T> and then use RouteTargetPath<Ipv4Net> / RouteTargetPath<Ipv6Net>?

cfzimmerman · 2026-10-23T19:35:50Z

+		key             = {
+			meta.routing_table_id: exact;
+			hdr.ipv6.dst_addr: lpm;
+		}


cfzimmerman · 2026-10-23T19:55:56Z

A situation we never want is for a spurious asic error to put DPD in an unrecoverable state. Or for asic state and DPD state to permanently diverge. A common cause of this is when we execute asic ops in sequence and reach and don't properly handle when one of them fails. More info

Are there any partial failure cases in your additions that might be worth testing? I can take a closer look as well, but you probably have a better sense. If so, the chaos test harness makes it easy to exercise failure cases in CI. Here's how we test port settings failures: https://github.com/oxidecomputer/dendrite/blob/main/dpd-client/tests/chaos_tests/port_settings.rs

cfzimmerman · 2026-10-23T20:01:16Z

Can we add swadm tests? Zeeshan has some good examples in his multicast PR: #366

cfzimmerman · 2026-10-23T20:23:20Z

Address management is my greatest piece of feedback in this PR. I've been thinking about the same problem from a different angle my own PR, which I've paused to focus on another bug but would still like to land in R24: #374

Here's the background:
DPD tracks addresses according to loopback or a particular link, but these addresses all share the same domain in the actual switch. Conflicts are currently managed by trying to register an address and then retreating if the asic yields AsicError::Exists. However, in certain partial failures, an AsicError::Exists "this address is owned by another link/loopback" is indistinguishable from an AsicError::Exists "you already registered this address". And retreating on the latter causes perpetual failures for reconcilers. The former also places undue trust in the SDE for synchronization that I argue should live explicitly in DPD.

In the linked PR, I propose moving addresses out of separate link/loopback containers and into a global address table. Keying on an IpAddr guarantees uniqueness across the switch, and it escapes subtle bugs like we have in Link right now, where we may drop a Link struct without successfully freeing all of its registered addresses.

Had I already merged that PR, I believe you could fit Router into that design by just adding RoutingTableId to this enum:

dendrite/dpd/src/addr.rs

Lines 449 to 453 in 286b18e

#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]

enum AddrIface {

Loopback,

Link { port_id: PortId, link_id: LinkId },

}

Doing so would automatically inherit the same uniqueness and failure recovery semantics we need in link and loopback addresses.

How do you feel about this? Idk your merge timeline, but I would love to agree on a design for both our PRs that works for Modular Routing Config and fixes/escapes the bugs in #354

nicolaskagami force-pushed the nsk/662-modular-router-config branch 2 times, most recently from d1af7ab to d89bf67 Compare October 9, 2026 19:37

big refactor

21a3967

nicolaskagami force-pushed the nsk/662-modular-router-config branch from d89bf67 to 21a3967 Compare October 16, 2026 17:19

nicolaskagami self-assigned this Oct 16, 2026

nicolaskagami requested review from cfzimmerman and zeeshanlakhani October 16, 2026 18:05

nicolaskagami added 4 commits October 17, 2026 11:21

collapse back to one action

06bb86e

fix delete_route_locked order and improve testing

2bc426b

improve router endpoint resilience when delete fails

8bb7137

return /route/ for underlay and default router routes

551068a

cfzimmerman reviewed Oct 23, 2026

View reviewed changes

Navigation Menu

Repository navigation

[WIP] Modular Router Configuration - #386

[WIP] Modular Router Configuration#386
nicolaskagami wants to merge 5 commits into
mainfrom
nsk/662-modular-router-config

nicolaskagami commented Oct 6, 2026 •

edited

Loading

Uh oh!

cfzimmerman left a comment

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

cfzimmerman Oct 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)]
	enum AddrIface {
	Loopback,
	Link { port_id: PortId, link_id: LinkId },
	}

Conversation

nicolaskagami commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Internal Changes

API Changes

Routers

Routes

Architectural decisions (in decreasing order of confidence)

Uh oh!

cfzimmerman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nicolaskagami commented Oct 6, 2026 •

edited

Loading