feat(model): add a rwnd trace trait and models#26
Merged
Merged
Conversation
Contributor
Author
|
@Centaurus99 Please have a look of this PR. |
Centaurus99
requested changes
Jul 15, 2026
There was a problem hiding this comment.
Pull request overview
Adds receive-window (rwnd) trace support to netem-trace by introducing new public rwnd trace types/traits, a new rwnd model module with static + repeated-pattern implementations, and wiring the new model behind a rwnd-model feature included in the model umbrella.
Changes:
- Introduces public rwnd trace surface (
Rwnd,RwndAction,RwndDecision,RwndTrace) insrc/lib.rs. - Adds
src/model/rwnd.rsimplementingStaticRwndandRepeatedRwndPatternplus serde support and unit tests. - Wires
rwnd-modelinto feature flags and the model module exports.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/model/rwnd.rs | New rwnd trace model implementations, serde (de)serialization, and unit tests |
| src/model/mod.rs | Exposes the new rwnd model module/types behind rwnd-model |
| src/lib.rs | Adds public rwnd trace types/traits to the crate API |
| Cargo.toml | Adds rwnd-model feature and includes it in the model feature umbrella |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+485
to
+496
| #[test] | ||
| fn test_static_rwnd_set_rcv_buf_only() { | ||
| let mut model = StaticRwndConfig::new() | ||
| .set_rcv_buf(131072) | ||
| .duration(Duration::from_secs(1)) | ||
| .build(); | ||
| let (decision, duration) = model.next_rwnd().unwrap(); | ||
| assert_eq!(decision.set_rcv_buf, Some(131072)); | ||
| assert_eq!(decision.action, None); | ||
| assert_eq!(duration, Duration::from_secs(1)); | ||
| assert_eq!(model.next_rwnd(), None); | ||
| } |
Comment on lines
+500
to
+505
| fn test_serde_roundtrip_set_rcv_buf_only() { | ||
| let cfg = Box::new( | ||
| StaticRwndConfig::new() | ||
| .set_rcv_buf(131072) | ||
| .duration(Duration::from_secs(1)), | ||
| ) as Box<dyn RwndTraceConfig>; |
Comment on lines
+523
to
+533
| #[test] | ||
| #[cfg(feature = "serde")] | ||
| fn test_serde_action_none_when_neither_set() { | ||
| // A step with only set_rcv_buf and no action fields should deserialize to action: None. | ||
| let json = "{\"StaticRwndConfig\":{\"set_rcv_buf\":65536}}"; | ||
| let des: Box<dyn RwndTraceConfig> = serde_json::from_str(json).unwrap(); | ||
| let mut model = des.into_model(); | ||
| let (decision, _) = model.next_rwnd().unwrap(); | ||
| assert_eq!(decision.set_rcv_buf, Some(65536)); | ||
| assert_eq!(decision.action, None); | ||
| } |
Comment on lines
+241
to
+259
| /// The action a rwnd trace instructs the receiver to take at a single step. | ||
| /// | ||
| /// At most one action is present per step; a step that only reconfigures the | ||
| /// receive buffer (`set_rcv_buf`) without any read or observed-remaining update | ||
| /// leaves [`RwndDecision::action`] as `None`. | ||
| /// | ||
| /// - `AppRead` drives the receiver model by simulating the application reading | ||
| /// `bytes` from the receive buffer; the resulting rwnd is computed from the | ||
| /// buffer state. | ||
| /// - `Remaining` skips the simulation and directly enforces an observed rwnd | ||
| /// of `rwnd` bytes — useful for replaying captured traces where only the | ||
| /// advertised window is known. | ||
| #[derive(Debug, Clone, PartialEq)] | ||
| pub enum RwndAction { | ||
| /// The simulated application reads this many bytes from the receive buffer at this step. | ||
| AppRead { bytes: u64 }, | ||
| /// The remaining rwnd value observed immediately after the app consumes data at this step. | ||
| Remaining { rwnd: u64 }, | ||
| } |
Comment on lines
+269
to
+272
| /// If `Some`, reconfigure the socket's receive buffer to this size at this step. | ||
| pub set_rcv_buf: Option<u64>, | ||
| /// If `Some`, the app-read or observed-remaining action for this step. | ||
| pub action: Option<RwndAction>, |
| "rwnd step cannot set both `app_read_bytes` and `rwnd_remaining`", | ||
| )); | ||
| } | ||
| (None, None) => None, |
Comment on lines
+308
to
+311
| decision: RwndDecision { | ||
| set_rcv_buf: self.set_rcv_buf, | ||
| action: self.action, | ||
| }, |
Centaurus99
approved these changes
Jul 15, 2026
BobAnkh
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR solves issue #25
Description
This PR adds receive window (rwnd) trace support to netem-trace, enabling simulation and replay of TCP receiver-side behaviour over time.
Each step in a rwnd trace is described by four fields:
duration— how long this configuration lasts before the next step appliesset_rcv_buf(optional) — resize the socket's receive buffer to this value at this stepapp_read_bytes(optional) — simulate the application reading this many bytes from the receive buffer (drives the receiver model)rwnd_remaining(optional) — directly assert the observed remaining receive window after consumption (useful for replaying captured traces)At most one of
app_read_bytes/rwnd_remainingmay be set per step — never both. A step may also set neither, e.g. a step that only reconfigures the receive buffer; this is a valid, first-class case (not a workaround), represented asaction: None. This invariant is enforced at the type/serde level: deserialization rejects only the both-set case with a clear error message.New public surface in
src/lib.rs:RwndAction— enum encoding the action per step (AppRead { bytes: u64 }/Remaining { rwnd: u64 })RwndDecision— a single step's payload:set_rcv_buf: Option<u64>+action: Option<RwndAction>, both independently optionalRwndTrace— trait mirroringBwTrace/DelayTrace/LossTrace, returningOption<(RwndDecision, Duration)>(No separate
Rwndtype alias — byte-count fields useu64directly, sinceset_rcv_buf/app_read_bytes/rwnd_remainingdescribe unrelated quantities and a shared alias was misleading.)New model module
src/model/rwnd.rs(enabled viarwnd-model/modelfeature):StaticRwnd/StaticRwndConfig— single-step model with builder API (set_rcv_buf(),app_read(),remaining(),duration())RepeatedRwndPattern/RepeatedRwndPatternConfig— repeating pattern over a sequence of steps, with infinite-loop (count = 0) support. Iteration uses an explicit bounded loop rather than recursion, so a pattern made entirely of immediately-exhausted steps (e.g.duration == 0) terminates instead of overflowing the stack, regardless ofcount.RwndTraceConfig— config-to-model conversion trait,typetag-registered for serde polymorphismSerialize/DeserializeforStaticRwndConfigthat flattensapp_read_bytes/rwnd_remainingto the top level of the JSON object (consistent with the flat style of other models), and round-trips correctly whether or not an action is setThe
rwnd-modelfeature is added to themodelumbrella feature and follows the same dependency pattern asloss-modelandduplicate-model(dep:dyn-clone).Dependency bump:
typetagminimum version raised to0.2.22(from0.2.5). Oldertypetag-implreleases generateimplblocks that trigger thenon_local_definitionslint on every#[cfg_attr(feature = "serde", typetag::serde)]-annotated trait in this crate;0.2.22suppresses this internally.How Has This Been Tested
Unit tests (
src/model/rwnd.rs#[cfg(test)]block):test_static_rwnd_model_app_read/test_static_rwnd_model_remaining— verifies each action variant's fields and single-step exhaustiontest_static_rwnd_set_rcv_buf_only— verifies a step with onlyset_rcv_bufset producesaction: Nonetest_repeated_rwnd_pattern— verifies a 2-step pattern repeated twice, thenNonetest_repeated_rwnd_pattern_all_zero_duration_terminates— regression test: a pattern of all-duration == 0steps withcount = 0(infinite) must returnNonepromptly instead of recursing forevertest_serde_roundtrip_app_read/test_serde_roundtrip_remaining/test_serde_roundtrip_set_rcv_buf_only— serialize → deserialize → assert field equality, including theaction: Nonecase (#[cfg(feature = "serde")])test_serde_rejects_both— confirms deserialization error when bothapp_read_bytesandrwnd_remainingare presenttest_serde_action_none_when_neither_set— confirms a step with neither field deserializes toaction: Nonerather than erroringDoc tests on all public types and the module-level examples.
Tests run against
--features "model,serde"(withouthuman) and--features "model,serde,human"to confirm both duration-format paths work correctly. (Serde tests that check error messages omitdurationto avoid the human/non-human format ambiguity, since that's not what those tests are checking.)cargo test --features "model,serde" # 24 unit + 47 doc — all pass
cargo test --features "model,serde,human" # 25 unit + 47 doc — all pass
Types of changes
Checklist
Other information
The
RwndAction/RwndDecisiontypes are placed inlib.rsalongsideBwTrace,LossTrace, etc., matching the existing convention. Therwnd-modelfeature follows the same opt-in pattern as other model features and is included in themodelumbrella andfullfeature set.This design went through a review pass that removed two premature abstractions (a
Rwndtype alias and aRwndActionConfigconfig-layer enum duplicatingRwndAction) and closed three correctness gaps:RwndDecision.actionis now genuinely optional instead of requiring a dummyAppRead { bytes: 0 }for buffer-only steps,RepeatedRwndPatternno longer risks stack overflow on all-empty patterns, andStaticRwndConfigserialization no longer errors on the now-validaction: Nonecase.