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

feat(icmp) support DoNotFragment + Size #633

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
7 changes: 5 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ Have any feedback or questions? [Create a discussion](https://github.com/TwiN/ga
- [Storage](#storage)
- [Client configuration](#client-configuration)
- [Alerting](#alerting)
- [Configuring AWS SES alerts](#configuring-aws-ses-alerts)
- [Configuring Discord alerts](#configuring-discord-alerts)
- [Configuring Email alerts](#configuring-email-alerts)
- [Configuring GitHub alerts](#configuring-github-alerts)
Expand All @@ -66,6 +65,7 @@ Have any feedback or questions? [Create a discussion](https://github.com/TwiN/ga
- [Configuring Teams alerts](#configuring-teams-alerts)
- [Configuring Telegram alerts](#configuring-telegram-alerts)
- [Configuring Twilio alerts](#configuring-twilio-alerts)
- [Configuring AWS SES alerts](#configuring-aws-ses-alerts)
- [Configuring custom alerts](#configuring-custom-alerts)
- [Setting a default alert](#setting-a-default-alert)
- [Maintenance](#maintenance)
Expand Down Expand Up @@ -109,7 +109,6 @@ Have any feedback or questions? [Create a discussion](https://github.com/TwiN/ga
- [API](#api)
- [Installing as binary](#installing-as-binary)
- [High level design overview](#high-level-design-overview)
- [Sponsors](#sponsors)


## Why Gatus?
Expand Down Expand Up @@ -352,6 +351,8 @@ the client used to send the request.
| `client.ignore-redirect` | Whether to ignore redirects (true) or follow them (false, default). | `false` |
| `client.timeout` | Duration before timing out. | `10s` |
| `client.dns-resolver` | Override the DNS resolver using the format `{proto}://{host}:{port}`. | `""` |
| `client.icmp.df` | Option to set the DoNotFragement bit for ICMP packet | `false` |
| `client.icmp.size` | Size of the ICMP packet | `25` |
| `client.oauth2` | OAuth2 client configuration. | `{}` |
| `client.oauth2.token-url` | The token endpoint URL | required `""` |
| `client.oauth2.client-id` | The client id which should be used for the `Client credentials flow` | required `""` |
Expand All @@ -363,6 +364,8 @@ the client used to send the request.
> 📝 Some of these parameters are ignored based on the type of endpoint. For instance, there's no certificate involved
in ICMP requests (ping), therefore, setting `client.insecure` to `true` for an endpoint of that type will not do anything.

> 📝 The ICMP parameters, including size and the 'Don't Fragment' (DF) flag, are utilized exclusively for ICMP pings. [These parameters can be used to validate MTU](https://stackoverflow.com/questions/45953716/when-to-set-dont-fragment-flag-in-ip-header)

This default configuration is as follows:
```yaml
client:
Expand Down
4 changes: 4 additions & 0 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,16 @@ func Ping(address string, config *Config) (bool, time.Duration) {
}
pinger.Count = 1
pinger.Timeout = config.Timeout
pinger.Size = config.Icmp.Size
// Set the pinger's privileged mode to true for every GOOS except darwin
// See https://github.com/TwiN/gatus/issues/132
//
// Note that for this to work on Linux, Gatus must run with sudo privileges.
// See https://github.com/prometheus-community/pro-bing#linux
pinger.SetPrivileged(runtime.GOOS != "darwin")
if config.Icmp.Df {
pinger.SetDoNotFragment(true)
}
err = pinger.Run()
if err != nil {
return false, 0
Expand Down
31 changes: 31 additions & 0 deletions client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (

const (
defaultTimeout = 10 * time.Second
// By default, an ICMP packet can have a size of 0
defaultSize = 0
)

var (
Expand All @@ -30,6 +32,10 @@ var (
Insecure: false,
IgnoreRedirect: false,
Timeout: defaultTimeout,
Icmp: &Icmp{
Df: false,
Size: defaultSize,
},
}
)

Expand All @@ -54,6 +60,9 @@ type Config struct {
// Expected format is {protocol}://{host}:{port}, e.g. tcp://8.8.8.8:53
DNSResolver string `yaml:"dns-resolver,omitempty"`

// See ICMP for more details.
Icmp *Icmp `yaml:"icmp,omitempty"`

// OAuth2Config is the OAuth2 configuration used for the client.
//
// If non-nil, the http.Client returned by getHTTPClient will automatically retrieve a token if necessary.
Expand All @@ -73,6 +82,14 @@ type DNSResolverConfig struct {
Port string
}

// ICMP is the configuration for the ICMP client specific config
type Icmp struct {
// Don't Fragment flag (DF) for the client
Df bool `yaml:"df"`
// Size of the packet
Size int `yaml:"size"`
}
antoinekh marked this conversation as resolved.
Show resolved Hide resolved

// OAuth2Config is the configuration for the OAuth2 client credentials flow
type OAuth2Config struct {
TokenURL string `yaml:"token-url"` // e.g. https://dev-12345678.okta.com/token
Expand All @@ -91,6 +108,20 @@ func (c *Config) ValidateAndSetDefaults() error {
if c.Timeout < time.Millisecond {
c.Timeout = 10 * time.Second
}

// Only validate or set defaults for Icmp if it's not nil
if c.Icmp != nil {
// limit for pro-ping, below 25 it's not working
if c.Icmp.Size < 25 {
c.Icmp.Size = 25
}
} else {
// If Icmp is nil, let's initialize it with default values
c.Icmp = &Icmp{
Df: false,
Size: 25,
Copy link
Owner

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 🤣)

image

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.

Copy link
Author

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:

  • A classic linux ping can have a data of 0.
  • If I don't specify a Size or defaultSize by default the Size is 24.
    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.

        // If Icmp is nil, let's initialize it with default values
        c.ICMPConfig = &ICMPConfig{
            DF:   false,
            Size: 24,
        }

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

    if c.ICMPConfig != nil {
        // limit for pro-ping, below 24 it's not working
        if c.ICMPConfig.Size < 24 {
            c.ICMPConfig.Size = 24
        }

And my mistake: the limit is 24, not 25.

}
}
if c.HasCustomDNSResolver() {
// Validate the DNS resolver now to make sure it will not return an error later.
if _, err := c.parseDNSResolver(); err != nil {
Expand Down