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

added support for parsing esp packets #114

Closed
wants to merge 1 commit into from
Closed

added support for parsing esp packets #114

wants to merge 1 commit into from

Conversation

danwgun
Copy link

@danwgun danwgun commented May 29, 2019

No description provided.

Copy link
Contributor

@bricknerb bricknerb left a comment

Choose a reason for hiding this comment

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

  1. Could you add tests? See GreTests and WiresharkCompareTests.
  2. Could you add EspLayer?

I have different nits to make the style conform better with the rest of the code, let me know if you'd like me to add comments or I can just fix it after this is merged.

/// </summary>
public Datagram Payload
{
get { return new Datagram(Buffer, StartOffset + HeaderLength, Length - HeaderLength); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't that include the pading, pad length, next header and ICV?

Copy link
Author

Choose a reason for hiding this comment

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

  1. Could you add tests? See GreTests and WiresharkCompareTests.
  2. Could you add EspLayer?

I have different nits to make the style conform better with the rest of the code, let me know if you'd like me to add comments or I can just fix it after this is merged.

Thanks for checking out my PR. I'll continue working on this.

Copy link
Author

Choose a reason for hiding this comment

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

Wouldn't that include the pading, pad length, next header and ICV?

I tried to replicate the way Wireshark parses these packets. It includes all of this data in the payload.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, let me know when you want me to take another look.

This pull request was closed.
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.

None yet

2 participants