Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Full MDA implementation #4

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

teo-lohrer-su
Copy link

Summary of changes

  • change the stopping point formula
  • implement full mda in the diamond-miner algorithm
  • use ICMP Echo Reply in the results for the destination
  • change the confidence level to a float to allow better precision than 99%
  • revert pycaracal to 14.5.0 (I never managed to get it working with 14.7.0 on macOS ; I may be missing something)

Context

After multiple experiments comparing performance of fmda vs scamper, some issues appeared relative to the number of discovered IPs and the number of probes sent, among others.
I was tasked by @timur-friedman to investigate and modify fmda if necessary. Since I'm no network expert, I would be extremely grateful for a careful review of the full mda part. The suggested changes were quantitatively evaluated on thousands of paths vs. scamper.

In the near future however, fmda will be entirely rewritten in Rust (port has already started). The Python version should help network measurements actors to evaluate the precision and gains of fmda, before switching to a full Rust version that benefits from easier packaging, thanks to work on caracat.

As a result, I did not edit the RATIONALE.md file, nor did I polish additional options or tests for now.

Do not hesitate to ping me for any clarification.

@teo-lohrer-su teo-lohrer-su marked this pull request as ready for review March 13, 2024 15:40
@teo-lohrer-su teo-lohrer-su marked this pull request as draft March 13, 2024 15:40
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 86.60714% with 15 lines in your changes missing coverage. Please review.

Project coverage is 88.19%. Comparing base (506d81a) to head (9f6f9e5).
Report is 1 commits behind head on main.

Current head 9f6f9e5 differs from pull request most recent head 1c3f595

Please upload reports for the commit 1c3f595 to get more accurate results.

Files Patch % Lines
fast_mda_traceroute/links.py 23.07% 10 Missing ⚠️
..._mda_traceroute/algorithms/utils/stopping_point.py 77.77% 4 Missing ⚠️
fast_mda_traceroute/algorithms/diamond_miner.py 98.41% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main       #4       +/-   ##
===========================================
- Coverage   98.98%   88.19%   -10.80%     
===========================================
  Files          17       18        +1     
  Lines         297      398      +101     
===========================================
+ Hits          294      351       +57     
- Misses          3       47       +44     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@teo-lohrer-su teo-lohrer-su marked this pull request as ready for review March 21, 2024 15:43
@teo-lohrer-su
Copy link
Author

@maxmouchet @timur-friedman
I added the FakeNet class, a fake-fakeroute tool to emulate probe routing in arbitrary networks.
It provides a Python-only layer that can create arbitrary networks from files or various specifications.
I wrote some documentation in the files that should help understand the internals.

Consequently, a lot of tests were added (100 at this time), using real-life topologies and artificial ones, like meshed networks.
Tests are taking much longer now, around 8 minutes, but verify the correctness of the topology discovered by fmda against a known topology.

diamond-miner is parametrised with a 95% confidence rate, and I check that topologies are discovered more than 75% of the time. The difference is due to the confidence rate being a local feature of next-hop discovery and topology recovery rate is a global feature.

Since the link between these two measure is not immediately clear to me, I settled on the 95/75 metrics for now, though this will need to be revisited for validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant