Skip to content

Conversation

povsister
Copy link
Contributor

This is a bugfix.
Since the origin upstream maintained by google is stall for a long time, I just submit the fix here.
There might be more contributes to OSPFv2 because I am currently working on it.

According to RFC 2328 A.4.1
The LSType field should be located at 4th byte of LSAheader.
and the missing LSOptions field should be at 3th.

add missing LSOptions field and fix LSType field offset
@mosajjal
Copy link
Contributor

Hi. thanks for this. please add a test packet that'll fail with master and will pass with your commit as well.

@povsister
Copy link
Contributor Author

Hi. thanks for this. please add a test packet that'll fail with master and will pass with your commit as well.

Added. PTAL

Copy link
Contributor

@mosajjal mosajjal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RFC 5340's LSA header differs from RFC 2328's LSA header. One has the Options byte, one doesn't. We need a few tests specifically for IPv6 and OSPFv2 if it doesn't exist already.

@povsister
Copy link
Contributor Author

povsister commented May 28, 2024

One has the Options byte, one doesn't.

Yes, it is. However, OSPFv2 and v3 are not compatible within a network configuration, in other words, they do not directly communicate with each other: v2 instances can only communicate with v2, and v3 can only communicate with v3.

We need a few tests specifically for IPv6 and OSPFv2 if it doesn't exist already.

Please refer to my explanation above. OSPFv2 with IPv6 are not impossible in a real OSPF configuration. So I don't think there is any necessary to add such test cases.

@mosajjal mosajjal merged commit fc98b62 into gopacket:master Nov 9, 2024
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.

2 participants