Repository: fujiapple852/trippy (language: rust)

Issue:
Restrict flows to Paris and Dublin strategies
The calculation and display of flows (including `dot` and `flows` reports) should be restricted to Paris and Dublin strategies only.

Each probe sent in each round is an "independent trail" and may follow a different network path and therefore produce flows which  do not exist.  

Using the Paris and Dublin tracing strategy all probes within a round are more likely (though not guaranteed) to follow the same path and therefore produce accurate flows.  When operating with `FixedSrc` port (default for UDP) or `FixedDest` port (default or TCP) for the Paris or Dublin strategy, the variable port is constant for all probes sent within a single round.  

For example, for UDP (Paris or Dublin) with a fixed src port (5000) and a variable dest port and initial sequence of 33000, the ports used for each probe will be as follows:

| Round | Hop (ttl) | SrcPort | DestPort | Seq |
|--------|--------|--------|--------|--------|
| 0 | 1 | 5000 | 33000 | 33000 |
| 0 | 2 | 5000 | 33000 | 33001 |
| 0 | 3 | 5000 | 33000 | 33002 |
| 1 | 1 | 5000 | 33001 | 33003 |
| 1 | 2 | 5000 | 33001 | 33004 |
| 1 | 3 | 5000 | 33001 | 33005 |
| 2 | 1 | 5000 | 33002 | 33006 |
| 2 | 2 | 5000 | 33002 | 33007 |
| 2 | 3 | 5000 | 33002 | 33008 |

As the src and dest port are constant _within_ each round, the path through the network is more likely to be the same for each probe within that round and therefore the flow calculated for the round is more likely to be genuine.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 57bee6f03..8474ae1cd 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,6 +26,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 - Ensure `paris` and `dublin` ECMP strategy are only used with supported
   protocols ([#848](https://github.com/fujiapple852/trippy/issues/848))
+- Restrict flows to `paris` and `dublin` ECMP strategies ([#1007](https://github.com/fujiapple852/trippy/issues/1007))
 - Improved Tui table column layout logic ([#925](https://github.com/fujiapple852/trippy/issues/925))
 - Use exclusive reference `&mut` for all Socket operations ([#843](https://github.com/fujiapple852/trippy/issues/843))
 
diff --git a/src/backend.rs b/src/backend.rs
index c95005b22..73b3e3858 100644
--- a/src/backend.rs
+++ b/src/backend.rs
@@ -19,11 +19,16 @@ pub struct Backend {
 
 impl Backend {
     /// Create a tracing `Backend`.
-    pub fn new(tracer_config: Config, channel_config: ChannelConfig, max_samples: usize) -> Self {
+    pub fn new(
+        tracer_config: Config,
+        channel_config: ChannelConfig,
+        max_samples: usize,
+        max_flows: usize,
+    ) -> Self {
         Self {
             tracer_config,
             channel_config,
-            trace: Arc::new(RwLock::new(Trace::new(max_samples))),
+            trace: Arc::new(RwLock::new(Trace::new(max_samples, max_flows))),
         }
     }
 
diff --git a/src/backend/trace.rs b/src/backend/trace.rs
index 3915f75b4..9f166189d 100644
--- a/src/backend/trace.rs
+++ b/src/backend/trace.rs
@@ -10,7 +10,16 @@ use trippy::tracing::{Extensions, ProbeState, Round, TimeToLive, TracerRound};
 /// The state of all hops in a trace.
 #[derive(Debug, Clone)]
 pub struct Trace {
+    /// The maximum number of samples to record per hop.
+    ///
+    /// Once the maximum number of samples has been reached the oldest sample
+    /// is discarded (FIFO).
     max_samples: usize,
+    /// The maximum number of flows to record.
+    ///
+    /// Once the maximum number of flows has been reached no new flows will be
+    /// created, existing flows are updated and are never removed.
+    max_flows: usize,
     /// The flow id for the current round.
     round_flow_id: FlowId,
     /// Tracing data per registered flow id.
@@ -23,12 +32,13 @@ pub struct Trace {
 
 impl Trace {
     /// Create a new `Trace`.
-    pub fn new(max_samples: usize) -> Self {
+    pub fn new(max_samples: usize, max_flows: usize) -> Self {
         Self {
             trace_data: once((Self::default_flow_id(), TraceData::new(max_samples)))
                 .collect::<HashMap<FlowId, TraceData>>(),
             round_flow_id: Self::default_flow_id(),
             max_samples,
+            max_flows,
             registry: FlowRegistry::new(),
             error: None,
         }
@@ -105,10 +115,12 @@ impl Trace {
                 })
                 .take(usize::from(round.largest_ttl.0)),
         );
-        let flow_id = self.registry.register(flow);
-        self.round_flow_id = flow_id;
         self.update_trace_flow(Self::default_flow_id(), round);
-        self.update_trace_flow(flow_id, round);
+        if self.registry.flows().len() < self.max_flows {
+            let flow_id = self.registry.register(flow);
+            self.round_flow_id = flow_id;
+            self.update_trace_flow(flow_id, round);
+        }
     }
 
     fn update_trace_flow(&mut self, flow_id: FlowId, round: &TracerRound<'_>) {
diff --git a/src/config.rs b/src/config.rs
index 419f9aa36..277321bba 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -36,9 +36,9 @@ pub enum Mode {
     Tui,
     /// Display a continuous stream of tracing data
     Stream,
-    /// Generate an pretty text table report for N cycles.
+    /// Generate a pretty text table report for N cycles.
     Pretty,
-    /// Generate a markdown text table report for N cycles.
+    /// Generate a Markdown text table report for N cycles.
     Markdown,
     /// Generate a CSV report for N cycles.
     Csv,
@@ -46,7 +46,7 @@ pub enum Mode {
     Json,
     /// Generate a Graphviz DOT file for N cycles.
     Dot,
-    /// Display all flows.
+    /// Display all flows for N cycles.
     Flows,
     /// Do not generate any tracing output for N cycles.
     Silent,
@@ -334,6 +334,16 @@ impl TrippyConfig {
         Self::build_config(args, cfg_file, platform)
     }
 
+    /// The maximum number of flows allowed.
+    ///
+    /// This is restricted to 1 for the classic strategy.
+    pub fn max_flows(&self) -> usize {
+        match self.multipath_strategy {
+            MultipathStrategy::Classic => 1,
+            _ => self.tui_max_flows,
+        }
+    }
+
     #[allow(clippy::too_many_lines)]
     fn build_config(args: Args, cfg_file: ConfigFile, platform: &Platform) -> anyhow::Result<Self> {
         let &Platform {
@@ -641,6 +651,7 @@ impl TrippyConfig {
         validate_strategy(multipath_strategy, unprivileged)?;
         validate_protocol_strategy(protocol, multipath_strategy)?;
         validate_multi(mode, protocol, &args.targets, dns_resolve_all)?;
+        validate_flows(mode, multipath_strategy)?;
         validate_ttl(first_ttl, max_ttl)?;
         validate_max_inflight(max_inflight)?;
         validate_read_timeout(read_timeout)?;
@@ -918,6 +929,17 @@ fn validate_multi(
     }
 }
 
+/// Validate that flows and dot mode are only used with paris or dublin
+/// multipath strategy.
+fn validate_flows(mode: Mode, strategy: MultipathStrategy) -> anyhow::Result<()> {
+    match (mode, strategy) {
+        (Mode::Flows | Mode::Dot, MultipathStrategy::Classic) => Err(anyhow!(
+            "this mode requires the paris or dublin multipath strategy"
+        )),
+        _ => Ok(()),
+    }
+}
+
 /// Validate `first_ttl` and `max_ttl`.
 fn validate_ttl(first_ttl: u8, max_ttl: u8) -> anyhow::Result<()> {
     if (first_ttl as usize) < 1 || (first_ttl as usize) > MAX_HOPS {
@@ -1150,11 +1172,13 @@ mod tests {
     #[test_case("trip example.com --mode markdown", Ok(cfg().mode(Mode::Markdown).max_rounds(Some(10)).build()); "markdown mode")]
     #[test_case("trip example.com --mode csv", Ok(cfg().mode(Mode::Csv).max_rounds(Some(10)).build()); "csv mode")]
     #[test_case("trip example.com --mode json", Ok(cfg().mode(Mode::Json).max_rounds(Some(10)).build()); "json mode")]
-    #[test_case("trip example.com --mode dot", Ok(cfg().mode(Mode::Dot).max_rounds(Some(10)).build()); "dot mode")]
-    #[test_case("trip example.com --mode flows", Ok(cfg().mode(Mode::Flows).max_rounds(Some(10)).build()); "flows mode")]
+    #[test_case("trip example.com --mode dot --udp -R paris", Ok(cfg().mode(Mode::Dot).max_rounds(Some(10)).multipath_strategy(MultipathStrategy::Paris).protocol(Protocol::Udp).port_direction(PortDirection::FixedSrc(Port(1024))).build()); "dot mode")]
+    #[test_case("trip example.com --mode flows --udp -R paris", Ok(cfg().mode(Mode::Flows).max_rounds(Some(10)).multipath_strategy(MultipathStrategy::Paris).protocol(Protocol::Udp).port_direction(PortDirection::FixedSrc(Port(1024))).build()); "flows mode")]
     #[test_case("trip example.com --mode silent", Ok(cfg().mode(Mode::Silent).max_rounds(Some(10)).build()); "silent mode")]
     #[test_case("trip example.com -m tui", Ok(cfg().mode(Mode::Tui).build()); "tui mode short")]
     #[test_case("trip example.com --mode foo", Err(anyhow!(format!("error: one of the values isn't valid for an argument"))); "invalid mode")]
+    #[test_case("trip example.com --mode dot", Err(anyhow!(format!("this mode requires the paris or dublin multipath strategy"))); "invalid dot mode")]
+    #[test_case("trip example.com --mode flows", Err(anyhow!(format!("this mode requires the paris or dublin multipath strategy"))); "invalid flows mode")]
     fn test_mode(cmd: &str, expected: anyhow::Result<TrippyConfig>) {
         compare(parse_config(cmd), expected);
     }
diff --git a/src/frontend/render/header.rs b/src/frontend/render/header.rs
index 0ca075c90..edb9f9af8 100644
--- a/src/frontend/render/header.rs
+++ b/src/frontend/render/header.rs
@@ -85,10 +85,23 @@ pub fn render(f: &mut Frame<'_>, app: &TuiApp, rect: Rect) {
     let source = render_source(app);
     let dest = render_destination(app);
     let target = format!("{source} -> {dest}");
-    let plural_flows = if app.tracer_data().flows().len() > 1 {
-        "flows"
+    let discovered = if app.tui_config.max_flows > 1 {
+        let plural_flows = if app.tracer_data().flows().len() > 1 {
+            "flows"
+        } else {
+            "flow"
+        };
+        format!(
+            ", discovered {} hops and {} unique {}",
+            app.tracer_data().hops(app.selected_flow).len(),
+            app.tracer_data().flows().len(),
+            plural_flows
+        )
     } else {
-        "flow"
+        format!(
+            ", discovered {} hops",
+            app.tracer_data().hops(app.selected_flow).len()
+        )
     };
     let left_line = vec![
         Line::from(vec![
@@ -104,12 +117,7 @@ pub fn render(f: &mut Frame<'_>, app: &TuiApp, rect: Rect) {
         Line::from(vec![
             Span::styled("Status: ", Style::default().add_modifier(Modifier::BOLD)),
             Span::raw(render_status(app)),
-            Span::raw(format!(
-                ", discovered {} hops and {} unique {}",
-                app.tracer_data().hops(app.selected_flow).len(),
-                app.tracer_data().flows().len(),
-                plural_flows
-            )),
+            Span::raw(discovered),
         ]),
     ];
 
diff --git a/src/frontend/tui_app.rs b/src/frontend/tui_app.rs
index 7275ce867..2d6c93828 100644
--- a/src/frontend/tui_app.rs
+++ b/src/frontend/tui_app.rs
@@ -54,7 +54,7 @@ impl TuiApp {
         trace_info: Vec<TraceInfo>,
     ) -> Self {
         Self {
-            selected_tracer_data: Trace::new(tui_config.max_samples),
+            selected_tracer_data: Trace::new(tui_config.max_samples, tui_config.max_flows),
             trace_info,
             tui_config,
             table_state: TableState::default(),
@@ -88,7 +88,7 @@ impl TuiApp {
 
     pub fn clear_trace_data(&mut self) {
         *self.trace_info[self.trace_selected].data.write() =
-            Trace::new(self.tui_config.max_samples);
+            Trace::new(self.tui_config.max_samples, self.tui_config.max_flows);
     }
 
     pub fn selected_hop_or_target(&self) -> &Hop {
@@ -323,7 +323,7 @@ impl TuiApp {
     }
 
     pub fn toggle_flows(&mut self) {
-        if self.trace_info.len() == 1 {
+        if self.trace_info.len() == 1 && self.tui_config.max_flows > 1 {
             if self.show_flows {
                 self.selected_flow = FlowId(0);
                 self.show_flows = false;
diff --git a/src/main.rs b/src/main.rs
index df63cc767..5c065629a 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -214,7 +214,12 @@ fn start_tracer(
     };
     let channel_config = make_channel_config(cfg, source_addr, target_addr);
     let tracer_config = make_tracer_config(cfg, target_addr, trace_identifier)?;
-    let backend = Backend::new(tracer_config, channel_config, cfg.tui_max_samples);
+    let backend = Backend::new(
+        tracer_config,
+        channel_config,
+        cfg.tui_max_samples,
+        cfg.max_flows(),
+    );
     let trace_data = backend.trace();
     thread::Builder::new()
         .name(format!("tracer-{}", tracer_config.trace_identifier.0))
@@ -339,7 +344,7 @@ fn make_tui_config(args: &TrippyConfig) -> TuiConfig {
         args.tui_geoip_mode,
         args.tui_max_addrs,
         args.tui_max_samples,
-        args.tui_max_flows,
+        args.max_flows(),
         args.tui_theme,
         &args.tui_bindings,
         &args.tui_custom_columns,
diff --git a/trippy-config-sample.toml b/trippy-config-sample.toml
index e72fda7d7..f970ed4e6 100644
--- a/trippy-config-sample.toml
+++ b/trippy-config-sample.toml
@@ -25,13 +25,16 @@
 # Allowed values are:
 #   tui         - Display interactive Tui [default]
 #   stream      - Display a continuous stream of tracing data
-#   pretty      - Generate an pretty text table report for N cycles
-#   markdown    - Generate a markdown text table report for N cycles
+#   pretty      - Generate a pretty text table report for N cycles
+#   markdown    - Generate a Markdown text table report for N cycles
 #   csv         - Generate a CSV report for N cycles
 #   json        - Generate a JSON report for N cycles
 #   dot         - Generate a Graphviz DOT report for N cycles
-#   flows       - Display all flows
+#   flows       - Display all flows for N cycles
 #   silent      - Do not generate any output for N cycles
+#
+# Note: the dot and flows modes are only allowed with paris or dublin
+# multipath strategy.
 mode = "tui"
 
 # Trace without requiring elevated privileges [default: false]
@@ -306,7 +309,7 @@ tui-max-addrs = 0
 # The maximum number of samples to record per hop [default: 256]
 tui-max-samples = 256
 
-# The maximum number of flows to show [default: 40]
+# The maximum number of flows to show [default: 64]
 tui-max-flows = 64
 
 # Whether to preserve the screen on exit [default: false]


Test patch:
diff --git a/test_resources/completions_fish.txt b/test_resources/completions_fish.txt
index 37b276a4d..b291578c3 100644
--- a/test_resources/completions_fish.txt
+++ b/test_resources/completions_fish.txt
@@ -1,5 +1,5 @@
 complete -c trip -s c -l config-file -d 'Config file' -r -F
-complete -c trip -s m -l mode -d 'Output mode [default: tui]' -r -f -a "{tui	'Display interactive TUI',stream	'Display a continuous stream of tracing data',pretty	'Generate an pretty text table report for N cycles',markdown	'Generate a markdown text table report for N cycles',csv	'Generate a CSV report for N cycles',json	'Generate a JSON report for N cycles',dot	'Generate a Graphviz DOT file for N cycles',flows	'Display all flows',silent	'Do not generate any tracing output for N cycles'}"
+complete -c trip -s m -l mode -d 'Output mode [default: tui]' -r -f -a "{tui	'Display interactive TUI',stream	'Display a continuous stream of tracing data',pretty	'Generate a pretty text table report for N cycles',markdown	'Generate a Markdown text table report for N cycles',csv	'Generate a CSV report for N cycles',json	'Generate a JSON report for N cycles',dot	'Generate a Graphviz DOT file for N cycles',flows	'Display all flows for N cycles',silent	'Do not generate any tracing output for N cycles'}"
 complete -c trip -s p -l protocol -d 'Tracing protocol [default: icmp]' -r -f -a "{icmp	'Internet Control Message Protocol',udp	'User Datagram Protocol',tcp	'Transmission Control Protocol'}"
 complete -c trip -s F -l addr-family -d 'The address family [default: Ipv4thenIpv6]' -r -f -a "{ipv4	'Ipv4 only',ipv6	'Ipv6 only',ipv6-then-ipv4	'Ipv6 with a fallback to Ipv4',ipv4-then-ipv6	'Ipv4 with a fallback to Ipv6'}"
 complete -c trip -s P -l target-port -d 'The target port (TCP & UDP only) [default: 80]' -r
diff --git a/test_resources/completions_zsh.txt b/test_resources/completions_zsh.txt
index 2c0186a2e..b0bb2648a 100644
--- a/test_resources/completions_zsh.txt
+++ b/test_resources/completions_zsh.txt
@@ -19,21 +19,21 @@ _trip() {
 '--config-file=[Config file]:CONFIG_FILE:_files' \
 '-m+[Output mode \[default\: tui\]]:MODE:((tui\:"Display interactive TUI"
 stream\:"Display a continuous stream of tracing data"
-pretty\:"Generate an pretty text table report for N cycles"
-markdown\:"Generate a markdown text table report for N cycles"
+pretty\:"Generate a pretty text table report for N cycles"
+markdown\:"Generate a Markdown text table report for N cycles"
 csv\:"Generate a CSV report for N cycles"
 json\:"Generate a JSON report for N cycles"
 dot\:"Generate a Graphviz DOT file for N cycles"
-flows\:"Display all flows"
+flows\:"Display all flows for N cycles"
 silent\:"Do not generate any tracing output for N cycles"))' \
 '--mode=[Output mode \[default\: tui\]]:MODE:((tui\:"Display interactive TUI"
 stream\:"Display a continuous stream of tracing data"
-pretty\:"Generate an pretty text table report for N cycles"
-markdown\:"Generate a markdown text table report for N cycles"
+pretty\:"Generate a pretty text table report for N cycles"
+markdown\:"Generate a Markdown text table report for N cycles"
 csv\:"Generate a CSV report for N cycles"
 json\:"Generate a JSON report for N cycles"
 dot\:"Generate a Graphviz DOT file for N cycles"
-flows\:"Display all flows"
+flows\:"Display all flows for N cycles"
 silent\:"Do not generate any tracing output for N cycles"))' \
 '-p+[Tracing protocol \[default\: icmp\]]:PROTOCOL:((icmp\:"Internet Control Message Protocol"
 udp\:"User Datagram Protocol"
diff --git a/test_resources/usage_long.txt b/test_resources/usage_long.txt
index 1e1225396..ebfa241af 100644
--- a/test_resources/usage_long.txt
+++ b/test_resources/usage_long.txt
@@ -16,12 +16,12 @@ Options:
           Possible values:
           - tui:      Display interactive TUI
           - stream:   Display a continuous stream of tracing data
-          - pretty:   Generate an pretty text table report for N cycles
-          - markdown: Generate a markdown text table report for N cycles
+          - pretty:   Generate a pretty text table report for N cycles
+          - markdown: Generate a Markdown text table report for N cycles
           - csv:      Generate a CSV report for N cycles
           - json:     Generate a JSON report for N cycles
           - dot:      Generate a Graphviz DOT file for N cycles
-          - flows:    Display all flows
+          - flows:    Display all flows for N cycles
           - silent:   Do not generate any tracing output for N cycles
 
   -u, --unprivileged