oxidecomputer / oxidecomputer/dendrite

Improve types for channels and lanes

Open
#348 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
20
Forks
3
Avg merge
3d 11h
Merged PRs (30d)
4

Description

Explanation

Dendrite refers to channels in connector code and lanes in serdes code. I believe the types used for representing these could be improved. Here are some concerns in the current design:

https://github.com/oxidecomputer/dendrite/blob/f6d41dd821f88f09ce7539ec981642efd32797d5/asic/src/tofino_common/ports.rs#L19-L25
https://github.com/oxidecomputer/dendrite/blob/f6d41dd821f88f09ce7539ec981642efd32797d5/asic/src/tofino_asic/serdes.rs#L50-L58

  • The channels used by a port are stored in a Vec<u8>. But afaict the SDE uses base + offset during port creation, so we'd never have a port using non-contiguous lanes.

https://github.com/oxidecomputer/dendrite/blob/f6d41dd821f88f09ce7539ec981642efd32797d5/asic/src/tofino_common/ports.rs#L68-L72

Some non-contiguous tracking is required, but we also have a BTreeSet for available_channels.

https://github.com/oxidecomputer/dendrite/blob/f6d41dd821f88f09ce7539ec981642efd32797d5/asic/src/tofino_common/ports.rs#L221-L227

A final design will require further investigation, but I feel like we can use a more descriptive type than Vec<u8> for this. And if not, a doc comment is warranted.

Proposal
  • Decide whether we should continue differentiating lanes and channels in this code. Changes if warranted.
  • Explore if a newtype wrapper makes sense for lanes/channels. If keeping lanes and channels distinct, define and document how exactly they're different and why we're differentiating. Move boundary and range checking into the type(s).
  • Determine whether there's any more appropriate way to track channels than a Vec<u8>. If not, document why Vec is needed here.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with asic/src/tofino_common/ports.rs, asic/src/tofino_asic/ports.rs, and asic/src/tofino_asic/serdes.rs, including the referenced comments and channel-tracking code. Investigate whether lanes and channels should remain distinct, whether a newtype can enforce their ranges, and whether Vec is appropriate. Done means the chosen representation is implemented and its distinctions, bounds, and storage rationale are documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
networking
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.