oxidecomputer / oxidecomputer/dendrite
Improve types for channels and lanes
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:
-
Should we actually have a distinction between channel and lane? This comment makes me question if we should. https://github.com/oxidecomputer/dendrite/blob/f6d41dd821f88f09ce7539ec981642efd32797d5/asic/src/tofino_asic/ports.rs#L23-L25
-
There's a matrix of numbers involved. There's the maximum quantity of lanes/channels on a port and the lane/channel indexes. I believe the range of acceptable lane counts is
1..=8, and the range of acceptable channel counts is1..=4. The indexes are then0..8and0..4. Lane count currently usesu32.
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 usesbase + offsetduring port creation, so we'd never have a port using non-contiguous lanes.
Some non-contiguous tracking is required, but we also have a BTreeSet for available_channels.
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 whyVecis needed here.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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