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.

Sign up for GitHub

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

Jump to bottom

Handle parsing errors #5

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Handle parsing errors #5

merged 1 commit into from
Nov 7, 2022

Conversation

mem
Copy link

@mem mem commented Oct 21, 2022

This implementation has the problem that it never times out.

Signed-off-by: Marcelo E. Magallon marcelo.magallon@grafana.com

@mem mem force-pushed the handle_parsing_errors branch from 534a109 to 11ddf57 Compare October 21, 2022 23:29
adriansr
adriansr approved these changes Oct 24, 2022
View reviewed changes
Copy link

@adriansr adriansr left a comment

Choose a reason for hiding this comment

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

LGTM.

I had been thinking about why is this failing, in our case, very rarely.

Maybe it receives a truncated packet?

I suspect the problem could also be related the bytes.Index call, maybe the sent[:4] bytes are not as random as we would like them to be and occasionally it finds an earlier match in the IP header?

Just wondering if it's worth to continue scanning the packet for those bytes.

@mem
Rework the logic that handles "replies"
d4e77dd 
Modify the logic so that it's more robust against errors and malformed
packets. Also try to make sure that we are matching against the correct
bytes in the response.

Signed-off-by: Marcelo E. Magallon <marcelo.magallon@grafana.com>
@mem mem force-pushed the handle_parsing_errors branch from 11ddf57 to d4e77dd Compare October 24, 2022 22:36
@mem
Copy link
Author

mem commented Oct 24, 2022

LGTM.

I had been thinking about why is this failing, in our case, very rarely.

Maybe it receives a truncated packet?

I suspect the problem could also be related the bytes.Index call, maybe the sent[:4] bytes are not as random as we would like them to be and occasionally it finds an earlier match in the IP header?

Just wondering if it's worth to continue scanning the packet for those bytes.

Thanks for going over that.

I reworked more of the logic in those functions. I think it should handle error conditions better in this way, and it should avoid spurious matches.

@mem mem merged commit a9806fd into master Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriansr adriansr adriansr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mem @adriansr