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

RSVP improvements to tests as well as P2MP support #1267

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mstigge
Copy link

@mstigge mstigge commented Dec 22, 2024

This PR makes as few improvements in the RSVP dissector.

  1. Add a test pcap for the most common protocol messages, for P2P as well as P2MP.
  2. Adjust rendering of the P2MP ID.
  3. P2MP Secondary ERO and RRO support was missing, add that.

All three are in separate commits and I suggest merging them separately to the master branch. Steps 2 and 3 are modifying the expected test output added in step 1, illustrating the effect of their changes.

Initially I planned to make 3 separate PRs, but GitHub's stacking support seems to be lacking somewhat, so doing a single PR instead with separate commits. Please indicate if a different commit structure is preferred in your project.

Included are PATH, RESV, PATH ERROR, PATH TEAR and RESV TEAR message types.
The pcap contains two sessions, a P2P one and a P2MP one.
The P2MP ID is typically denoted as a single value, either in decimal or in
hexadecimal format. The 4-octet notation is quite uncommon and I believe this
was just a mistake in the code since the IPv6 version 11 lines above uses a
hexadecimal notation instead.

I'm also using this opportunity to change the name from "P2MP LSP ID" to "P2MP
ID" since that's more correct. The term "LSP ID" has a well-defined meaning in
the RSVP protocol and is something else than the P2MP ID. The BGP dissector
uses the term P2MP ID as well for the same value (and shows it in hexadecimal
format).
The code can just re-use the printer of the P2P ERO and RRO. However, for P2MP
the C-Type is 2, which make the check a little cumbersome: We can only do the
printing of the sub-objects if either we are dealing with a P2P ERO/RRO having
C-Type 1 (indicating IPv4), or if we are dealing with a P2MP ERO/RRO having
C-Type 2 (indicating P2MP).
@mstigge
Copy link
Author

mstigge commented Dec 23, 2024

The build failure on openbsd-mips64 was a timeout in an ipv6 test which looks to me like a case of flaky test infra instead of being related to my changes.

Re-running that build will probably pass, but admin rights are required to initiate that, which I don't have.

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

Successfully merging this pull request may close these issues.

2 participants