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

For ICMP, only use RawConn for the Don't Fagment case #329

Merged

Conversation

avonwyss
Copy link
Contributor

@avonwyss avonwyss commented Jun 1, 2018

Changed code to unify the IPv6 and IPv4 ICMP usage, and also how the source IP is used (listen on the source IP instead of defining it on the packet). Tested on Windows for both IPv4 and IPv6, with and without explicit source IP.

Not tested DF flag and *nix in general.

Signed-off-by: Arsène von Wyss <avw@gmx.ch>
@SuperQ
Copy link
Member

SuperQ commented Jun 1, 2018

Nice, we should probably mention in the docs that DF won't work on Windows due to RawConn.

We may possibly want to split this out into separate package files like we do in the node_exporter, to avoid building RawConn at all into Windows. What do you think?

@avonwyss avonwyss force-pushed the avonwyss/windows-ipv4-icmp-fix branch 2 times, most recently from f5b32e9 to d4841e1 Compare June 1, 2018 15:57
@avonwyss
Copy link
Contributor Author

avonwyss commented Jun 1, 2018

The docs already specify that: https://github.com/prometheus/blackbox_exporter/blob/master/CONFIGURATION.md
# Set the DF-bit in the IP-header. Only works with ip4 and on *nix systems.

Not sure about a separate package, users will just get non-successful probes on Windows if they try to use the DF, that's all. Doesn't seem to be too much of an issue IMHO.

@SuperQ
Copy link
Member

SuperQ commented Jun 1, 2018

Thanks, that seems like a reasonable enough mitigation.

@brian-brazil
Copy link
Contributor

Agreed. The long weekend is just starting here, so I'll review this next week.

@brian-brazil brian-brazil merged commit 91e917e into prometheus:master Jun 6, 2018
@brian-brazil
Copy link
Contributor

Thanks!

@avonwyss avonwyss deleted the avonwyss/windows-ipv4-icmp-fix branch June 6, 2018 13:03
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.

3 participants