Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(icmp) support DoNotFragment + Size #633
base: master
Are you sure you want to change the base?
feat(icmp) support DoNotFragment + Size #633
Changes from 11 commits
4a2b642
65f24e9
4515572
4f1cd93
39952da
e71d178
2e3c5bb
104ec1d
30aea72
2340f65
4c0a523
73c0ff2
e9703ec
ccafe4b
a2ebfd1
dbe1815
88c94e2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on what I just looked at, the current default value is 25 (on my machine, at least 🤣)
Side note, I also see that there's a comment on L112 that may no longer be valid - it says
// limit for pro-ping, below 25 it's not working
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've set this configuration because it's the only way I've found to make it work.
But maybe I'm missing something in Go.
I did the tests by validating with network captures:
So you are right I can remove defaultSize = 24
But then, If I don't specifiy anything in icmp in my yaml file, the ping is not working.
That's why I put this code with the Size in it.
But maybe this is not the good way to initialise it.
If I test with a Size <24 it doesn't work either.
I notice that this limitation exists in other tools that use pro-bing
For example
https://github.com/SuperQ/smokeping_prober
Packetdata size in bytes. Default 56 (Range: 24 - 65535)
That's why I put
And my mistake: the limit is 24, not 25.