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

producer: fix sFlow parsing when sampled header is truncated #139

Open
wants to merge 1 commit into
base: v1
Choose a base branch
from

Conversation

vincentbernat
Copy link
Contributor

@vincentbernat vincentbernat commented Jan 16, 2023

We return early if we don't even have an Ethernet header. We also need to increment the offset even if data is truncated to not confuse the remaining of the packet with something else (eg extract TCP/UDP ports when TCP/UDP header is too short).

Use IHL to correctly skip the IPv4 header when there are IP options. Also fix the offset for TCP payload.

This also removes the encap/iteration loop. As encap is set to false at the beginning of the loop, it was always executed one. I suppose this was the remaining of some other code. The iterateMpls flag is removed by moving some code around. One should look the diff with blanks ignored.

Unfortunately, not everything is covered by tests, notably MPLS parsing. Please double check this part.

Also, unrelated, but I wonder why you use (*flowMessage).SrcAddr instead of just flowMessage.SrcAddr.

We return early if we don't even have an Ethernet header. We also need
to increment the offset even if data is truncated to not confuse the
remaining of the packet with something else (eg extract TCP/UDP ports
when TCP/UDP header is too short).

Use IHL to correctly skip the IPv4 header when there are IP options.
Also fix the offset for TCP payload.

This also removes the encap/iteration loop. As encap is set to false at
the beginning of the loop, it was always executed one. I suppose this
was the remaining of some other code. The iterateMpls is removed by
moving some code around. One should look the diff with blanks ignored.
@lspgn lspgn added the producer Conversion from flow to protobuf label Mar 7, 2023
@lspgn lspgn changed the base branch from main to v1 September 1, 2023 05:00
@vincentbernat
Copy link
Contributor Author

This is fixed in #168. We can close this one.

@vincentbernat
Copy link
Contributor Author

Sorry, wrong PR!

@vincentbernat vincentbernat reopened this Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
producer Conversion from flow to protobuf
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants