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

ICMP Extensions #33

Open
7 of 11 tasks
fujiapple852 opened this issue Apr 25, 2022 · 10 comments
Open
7 of 11 tasks

ICMP Extensions #33

fujiapple852 opened this issue Apr 25, 2022 · 10 comments
Labels
enhancement New feature or request icmp tracking

Comments

@fujiapple852
Copy link
Owner

fujiapple852 commented Apr 25, 2022

Tracking issue for ICMP Extensions.

Add support for generic Ipv4 & Ipv6 ICMP extensions, with specific parsing for:

@fujiapple852 fujiapple852 added the enhancement New feature or request label Apr 25, 2022
@fujiapple852 fujiapple852 changed the title Support MPLS extension MPLS extension Apr 25, 2022
@ytti
Copy link

ytti commented Sep 7, 2023

Or perhaps more generic EO support.

https://www.rfc-editor.org/rfc/rfc5837.html
https://github.com/8enet/traceroute/blob/master/traceroute/extension.c#L101
https://lore.kernel.org/all/6a7f33a5-13ca-e009-24ac-fde59fb1c080@gmail.com/T/
https://www.juniper.net/documentation/us/en/software/junos/transport-ip/topics/topic-map/icmp.html

This would allow you to for example report latency and loss per LAG member, as well as discover which keys are hashed to which path.

@fujiapple852
Copy link
Owner Author

fujiapple852 commented Sep 20, 2023

Thanks for the suggestion and links @ytti.

I agree that Trippy should target supporting a larger set of ICMP extension objects and not focus solely on MPLS.

I must confess I haven't spent much time looking at these extension, mainly as I've observed that they are seldom used on public networks, perhaps they are used more within private networks? What has been your experience of how these extensions are used in practise and what useful insights they can provide?

Once added, how would you imagine this data would be used within Trippy?

The easiest thing to do would be to support them generically and simply render the data somehow (perhaps in the expanded hop detail navigation view?). This appears to be the approach taken in the example traceroute code you linked.

Beyond that, Trippy could be aware of specific Extension Objects, such as MPLS and the "Interface Information Object" defined in rfc 5837, such that it could do more semantically useful things with the data, such as "report latency and loss per LAG member" as you suggest. I think this would be more useful, but only if the majority of hops in the path provide the extended data.

As a start, the Trippy net code needs to be able to parse ICMP Extension Objects (Multi-Part Messages) as defined in rfc4884. This is something I can do, unless you were keen to work on this yourself?

@fujiapple852
Copy link
Owner Author

fujiapple852 commented Sep 20, 2023

Also do you know of any public hosts which include "Interface Information Object" EO in ICMP TimeExceeded messages? This is very useful for development.

I did manage to find an example host which provides MPLS EO: 129.250.2.50

@ytti
Copy link

ytti commented Sep 20, 2023

I do not, I hope it'll become more common but currently as far as I know, only Juniper QFX5k does it. I have some reasons to expect that wider availability is coming.

@fujiapple852 fujiapple852 changed the title MPLS extension Support ICMP extensions Oct 2, 2023
@fujiapple852 fujiapple852 self-assigned this Oct 2, 2023
@fujiapple852 fujiapple852 added this to the 0.9.0 milestone Oct 2, 2023
@fujiapple852
Copy link
Owner Author

@ytti I have some (very rough!) WIP code up in #696 that parses ICMP extensions (v4 only for now) generically. For now it just prints out any extension object(s) found for each ICMP TimeExceeded or DestinationUnreachable response.

It supports both compliant and non-compliant ICMP extensions as per section 5 of rfc4884.

Example (the only one I can find in the wild, a non-compliant MPLS extension):

extension header: version=2, checksum=39226
extension object: length=8, class_num=MultiProtocolLabelSwitchingLabelStack, class_subtype=ClassSubType(1), payload=04 bb 41 01

Next up, I'm going to add parsing for the four classes with assigned numbers.

After that is icmp V6 extensions and then some thought about how to display the information in the TUI. This will also need some new command line config to enable/disable extension parsing and rendering.

It'll also need to be thoroughly tested on all platforms due to weirdnesses of the way some platforms treat some Ipv4 fields (i.e. total_length) in raw socket mode that are important here. Without a source of extensions to test against it may be impossible to test the Interface Information Object, Interface Identification Object & Extended Information objects, beyond syntactic test data.

@ytti
Copy link

ytti commented Oct 2, 2023

Sounds very good indeed. And I believe this is right approach, in the quoted traceroute URL we can see similar approach. Where as mtr takes opposite strategy, and implements MPLS labels as its own specific thing, making it much harder for next person to add pretty printing of new extensions.

The MPLS example while not pretty printed, is very useful example how unrecognised data can be printed, and pretty printing can be added once someone explains what the data is and how it could be used to enrich UX.

Many thanks!

fujiapple852 added a commit that referenced this issue Oct 4, 2023
fujiapple852 added a commit that referenced this issue Oct 4, 2023
fujiapple852 added a commit that referenced this issue Oct 4, 2023
fujiapple852 added a commit that referenced this issue Oct 5, 2023
fujiapple852 added a commit that referenced this issue Oct 5, 2023
fujiapple852 added a commit that referenced this issue Oct 5, 2023
@fujiapple852
Copy link
Owner Author

fujiapple852 commented Oct 5, 2023

@ytti progress...

Screenshot 2023-10-05 at 11 17 54 PM

Only rendering a single MPLS label/ttl for now, but the plumbing is there to add any/all extensions.

We'll need to think how best to render these in the Tui and textual report modes given:

  • Each hop (ttl) can have multiple hosts (so do we show the union of extension objects per hop or per host?)
  • Each host receives multiple probe responses (i.e. one per round), each of which may or may not have extensions (so would these extensions accumulate over the rounds or would they be removed if they do not appear in a round?)
  • Each probe response can contain multiple extensions, of various types (mpls, Interface Information Object, unknown, etc)
  • Specifically for MPLS, each stack can contain multiple members (i.e. labels), so we'd need to show them all. Each member has a label, a ttl, an exp which we can show.

It feels like we need to render a tree structure here.

Edit: moved the Tui discussion to #752

fujiapple852 added a commit that referenced this issue Oct 6, 2023
fujiapple852 added a commit that referenced this issue Oct 6, 2023
fujiapple852 added a commit that referenced this issue Oct 6, 2023
fujiapple852 added a commit that referenced this issue Oct 6, 2023
fujiapple852 added a commit that referenced this issue Oct 15, 2023
fujiapple852 added a commit that referenced this issue Oct 15, 2023
fujiapple852 added a commit that referenced this issue Oct 15, 2023
@fujiapple852
Copy link
Owner Author

Need to test if the label extraction is accurate (matches Wireshark on MacOS but seems to differ from tcpdump on Linux, maybe an endianness thing?)

@fujiapple852 fujiapple852 changed the title Support ICMP extensions ICMP Extensions Nov 2, 2023
fujiapple852 added a commit that referenced this issue Nov 2, 2023
@fujiapple852 fujiapple852 removed this from the 0.9.0 milestone Nov 2, 2023
@fujiapple852
Copy link
Owner Author

fujiapple852 commented Nov 20, 2023

@ytti do you know anyone who would be able to help diagnose the issue I describe in #804? I'm unsure which of tcpdump or Wireshark is interpreting it correctly (the differ!). My reading of rfc4884 is that Wireshark is interpreting it correctly, as I apply the same logic in Trippy and get the same result. However there is clearly an MPLS extension present and tcpdump does manage to parse it out, somehow.

@fujiapple852
Copy link
Owner Author

Code was merged and can be found here along with all test cases: https://github.com/fujiapple852/trippy/blob/master/src/tracing/packet/icmp_extension.rs#L804

@fujiapple852 fujiapple852 removed their assignment Nov 22, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Dec 1, 2023
[0.9.0] - 2023-11-30

Added

- Added support for tracing flows
  ([#776](fujiapple852/trippy#776))
- Added support for `icmp` extensions
  ([#33](fujiapple852/trippy#33))
- Added support for `MPLS` label stack class `icmp` extension
  objects ([#753](fujiapple852/trippy#753))
- Added support for [paris]
  (https://github.com/libparistraceroute/libparistraceroute) ECMP routing
  for `IPv6/udp` ([#749](fujiapple852/trippy#749))
- Added `--unprivileged` (`-u`) flag to allow tracing without elevated
  privileges (macOS
  only) ([#101](fujiapple852/trippy#101))
- Added `--tui-privacy-max-ttl` flag to hide host and IP details for low ttl
  hops ([#766](fujiapple852/trippy#766))
- Added `toggle-privacy` (default: `p`) key binding to show or hide private
  hops ([#823](fujiapple852/trippy#823))
- Added `toggle-flows` (default: `f`) key binding to show or hide tracing
  flows ([#777](fujiapple852/trippy#777))
- Added `--dns-resolve-all` (`-y`) flag to allow tracing to all IPs resolved
  from DNS lookup
  entry ([#743](fujiapple852/trippy#743))
- Added `dot` report mode (`-m dot`) to output hop graph in Graphviz `DOT`
  format ([#582](fujiapple852/trippy#582))
- Added `flows` report mode (`-m flows`) to output a list of all unique tracing
  flows ([#770](fujiapple852/trippy#770))
- Added `--icmp-extensions` (`-e`) flag for parsing `IPv4`/`IPv6` `icmp`
  extensions ([#751](fujiapple852/trippy#751))
- Added `--tui-icmp-extension-mode` flag to control how `icmp` extensions are
  rendered ([#752](fujiapple852/trippy#752))
- Added `--print-config-template` flag to output a template config
  file ([#792](fujiapple852/trippy#792))
- Added `--icmp` flag as a shortcut for `--protocol icmp`
  ([#649](fujiapple852/trippy#649))
- Added `toggle-help-alt` (default: `?`) key binding to show or hide
  help ([#694](fujiapple852/trippy#694))
- Added panic handing to Tui
  ([#784](fujiapple852/trippy#784))
- Added official Windows `scoop` package
  ([#462](fujiapple852/trippy#462))
- Added official Windows `winget` package
  ([#460](fujiapple852/trippy#460))
- Release `musl` Debian `deb` binary asset
  ([#568](fujiapple852/trippy#568))
- Release `armv7` Linux binary assets
  ([#712](fujiapple852/trippy#712))
- Release `aarch64-apple-darwin` (aka macOS Apple Silicon) binary
  assets ([#801](fujiapple852/trippy#801))
- Added additional Rust Tier 1 and Tier 2 binary assets
  ([#811](fujiapple852/trippy#811))

Changed

- [BREAKING CHANGE] `icmp` extension object data added to `json` and `stream`
  reports ([#806](fujiapple852/trippy#806))
- [BREAKING CHANGE] IPs field added to `csv` and all tabular
  reports ([#597](fujiapple852/trippy#597))
- [BREAKING CHANGE] Command line flags `--dns-lookup-as-info` and
  `--tui-preserve-screen` no longer require a boolean
  argument ([#708](fujiapple852/trippy#708))
- [BREAKING CHANGE] Default key binding for `ToggleFreeze` changed from `f`
  to `ctrl+f` ([#785](fujiapple852/trippy#785))
- Always render AS lines in hop details mode
  ([#825](fujiapple852/trippy#825))
- Expose DNS resolver module as part of `trippy` library
  ([#754](fujiapple852/trippy#754))
- Replaced unmaintained `tui-rs` crate with `ratatui` crate
  ([#569](fujiapple852/trippy#569))

Fixed

- Reverse DNS lookup not working in reports
  ([#509](fujiapple852/trippy#509))
- Crash on NetBSD during window resizing
  ([#276](fujiapple852/trippy#276))
- Protocol mismatch causes tracer panic
  ([#745](fujiapple852/trippy#745))
- Incorrect row height in Tui hop detail navigation view for hops with no
  responses ([#765](fujiapple852/trippy#765))
- Unnecessary socket creation in certain tracing modes
  ([#647](fujiapple852/trippy#647))
- Incorrect byte order in `IPv4` packet length calculation
  ([#686](fujiapple852/trippy#686))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request icmp tracking
Projects
None yet
Development

No branches or pull requests

2 participants